Re: mp_machdep.c (was Re: [Fwd: Re: Bug reports requested - acpi])

Nate Lawson:

> 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
>> _at__at_ -743,10 +743,11 _at__at_
>>        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!
>
>
> A quick "no" vote from me until this is really understood.  I think 
> the real problem is an interference between the pmap for the AP 
> trampoline and the acpi wake code (sys/i386/acpica/acpi_wakeup.c).  
> The address you gave (0x9f000) is right before the base address we use 
> for the wakeup code (0xa0000).  As I woke up this morning, I was 
> wondering if this could be the issue.  An easy way to test is to 
> disable the call to acpi_install_wakeup_handler() in 
> sys/i386/acpica/acpi_machdep.c and see if this alone fixes the problem. 

Ok, I'll check it tomorrow. If it is, it really should be fixed.

But any way, we modify pte and it could be cached (we have this now).
Who could guarantee that it wouldn't/shouldn't. If it shouldn't, do we 
really
should rely on it, or cache invalidation is unsafe or have some side 
effects?
I can't unsfer to these questions for sure by my self ...

If I didn't miss smth, it seems that install_ap_tramp is the only place 
that do
not invalidate cache after modifing pte entry.

> If I'm wrong, feel free to commit your patch.
>
> P.S. Spaces instead of tabs in your diff. 

I know, it was just cat-pasted from console as an example. Real one
without spaces.

rik

> -Nate