Date: Wed, 22 Sep 2004 15:14:37 -0400 From: John Baldwin <jhb@FreeBSD.org> To: Roman Kurakin <rik@cronyx.ru> Cc: Nate Lawson <nate@root.org> Subject: Re: mp_machdep.c (was Re: [Fwd: Re: Bug reports requested - acpi]) Message-ID: <200409221514.37187.jhb@FreeBSD.org> In-Reply-To: <415130B3.5040205@cronyx.ru> References: <41421D6A.8070805@cronyx.ru> <200409211445.52372.jhb@FreeBSD.org> <415130B3.5040205@cronyx.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 22 September 2004 03:58 am, Roman Kurakin wrote:
> John Baldwin:
> >On Tuesday 21 September 2004 12:27 pm, Roman Kurakin wrote:
> >>My solution works for current so I am going to commit it and MFC after
> >>a while. To be sure that I am not on the wrong way I need some
> >>reviewed/approved signs ;-) I also hope to get one (or more) tested
> >> signs.
> >>
> >>Patch I plan to commit following patch:
> >>
> >>Index: mp_machdep.c
> >>===================================================================
> >>RCS file: /home/ncvs/src/sys/i386/i386/mp_machdep.c,v
> >>retrieving revision 1.238
> >>diff -u -r1.238 mp_machdep.c
> >>--- mp_machdep.c 1 Sep 2004 06:42:01 -0000 1.238
> >>+++ mp_machdep.c 21 Sep 2004 15:54:41 -0000
> >>@@ -743,10 +743,11 @@
> >> u_int8_t *dst8;
> >> u_int16_t *dst16;
> >> u_int32_t *dst32;
> >>+ vm_offset_t va = (vm_offset_t) dst;
> >>
> >> POSTCODE(INSTALL_AP_TRAMP_POST);
> >>
> >>- pmap_kenter(boot_address + KERNBASE, boot_address);
> >>+ pmap_map(&va, boot_address, boot_address + size, 0);
> >> for (x = 0; x < size; ++x)
> >> *dst++ = *src++;
> >>
> >>Any signs for(or against)?
> >>
> >>Thanks!
> >>
> >>PS. John: I am against of pmap_kenter/pmap_invalidate_XXX since we could
> >>get
> >>the same problem if we would use atomic functions instead of composite
> >>functions,
> >>which, I hope, will track all changes in the future.
> >
> >pmap_foo() doesn't change much. :) One reason I would prefer the
> >kenter/invalidate is that we explicitly assume a single page for the boot
> >code when we go to allocate an address for it, so I'd kind of like to keep
> > it as an explicit assumption, but I'd be ok with just adding a
> > KASSERT(size <=
>
> Are you sure that some one who will add new features wouldn't forget
> about this
> place? If you consider that we can ignore this I'll commit
> kenter/invalidate pair with
> KASSERT().
Umm, the MP boot code pretty much hasn't changed since it was added in 3.0 and
probably won't ever change. I don't expect pmap_kenter() or
pmap_invalidate_page() to go away anytime soon either. If someone does break
those interfaces it is up to them to fix all callers. But you can use
pmap_map(), just KASSERT() the size, and maybe do 'dst = pmap_map()'.
> >PAGE_SIZE, ("bewm")); Also, I think your end va needs to be boot_address
> > + size -1 so that if size == PAGE_SIZE you don't bogusly try to map the
> > first page of Video RAM as read/write memory.
>
> Tell me if I am wrong, but as I understand this code "end" is not really
> last, but next to
> last. Hm, may be this is other (potential) bug, probably we should
> rename 'end' to smth
> else? (va + psz < va + psz)
The code has end as the last address. If end starts on a new page then that
entire page is mapped.
while (start < end) {
pmap_kenter(va, start);
va += PAGE_SIZE;
start += PAGE_SIZE;
}
Thus, end needs to be the last virtual address, which is start + size - 1.
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200409221514.37187.jhb>