Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 23 Sep 2004 18:35:35 +0400
From:      Roman Kurakin <rik@cronyx.ru>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        Nate Lawson <nate@root.org>
Subject:   Re: mp_machdep.c (was Re: [Fwd: Re: Bug reports requested - acpi])
Message-ID:  <4152DF37.2000604@cronyx.ru>
In-Reply-To: <200409221514.37187.jhb@FreeBSD.org>
References:  <41421D6A.8070805@cronyx.ru> <200409211445.52372.jhb@FreeBSD.org> <415130B3.5040205@cronyx.ru> <200409221514.37187.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
John Baldwin wrote:

>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()'.
>  
>
I am talking about new features in CPU, for example. In which case we 
may need to add another function,
like now we have to add page_invalidation. pmap_map () would be fixed, 
but install_ap_trump () may stay
not fixed. But it is also possible that pmap_map () could be changed in 
the way it wouldn't suit our needs
and may broke smt again. In any case I'll use kenter + invalidate since 
map confuses a bit to close this
question and move on.

rik

>>>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.
>
>  
>





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4152DF37.2000604>