[SeaBIOS] [PATCH] vcpu hotplug: Setup vcpu add/remove infrastructure

[Liu, Jinsong]

Gleb Natapov wrote:
> On Sat, Jan 30, 2010 at 01:42:49AM +0800, Liu, Jinsong wrote:
>> Gleb Natapov wrote:
>>> On Sat, Jan 30, 2010 at 01:05:04AM +0800, Liu, Jinsong wrote:
>>>> Gleb Natapov wrote:
>>>>> On Sat, Jan 30, 2010 at 12:52:54AM +0800, Liu, Jinsong wrote:
>>>>>> Gleb Natapov wrote:
>>>>>>> On Fri, Jan 29, 2010 at 11:34:24PM +0800, Liu, Jinsong wrote:
>>>>>>>> Gleb Natapov wrote:
>>>>>>>>> On Thu, Jan 28, 2010 at 10:54:48PM +0800, Liu, Jinsong wrote:
>>>>>>>>>> Connor and Gleb,
>>>>>>>>>> 
>>>>>>>>>> I updated my patch according to our discussion:
>>>>>>>>>> 1. simplify scan loop according to 'maxvcpus';
>>>>>>>>>> 2. remove unecessary global variables;
>>>>>>>>>> 3. change hardcode address to bios use only area '0x514';
>>>>>>>>>> 4. remove simple \_PR scope from ssdt;
>>>>>>>>>> 
>>>>>>>>> I still don't see that ACPI NVS issue is addressed.
>>>>>>>> 
>>>>>>>> Gleb,
>>>>>>>> 
>>>>>>>> I'm not quite sure Win7 BSOD issue you mentioned last email,
>>>>>>>> anyway I update my patch, define ACPI NVS per my understanding.
>>>>>>> You need checked build of Windows 7. This is not regular Win7
>>>>>>> this is debug version that you can download from MSDN. Here is
>>>>>>> the bug report about the crash:
>>>>>>> http://sourceforge.net/tracker/?func=detail&aid=2902983&group_id=180599&atid=893831
>>>>>>> 
>>>>>>> Anyway according to ACPI spec ACPI DATA area can be reused by an
>>>>>>> OS after reading ACPI tables so it is wrong to access this
>>>>>>> memory after an OS boot. 
>>>>>>> 
>>>>>> 
>>>>>> Yes, it's safer to define ACPI NVS, although it's bios use only
>>>>>> area. 
>>>>>> 
>>>>> APCI DATA is explicitly not defined by ACPI spec as "bios use only
>>>>> area".
>>>> 
>>>> Yes, it's not defined by ACPI spec, but from default x86 memory
>>>> layout, refer to Documentation/x86/boot.txt: 001000
>>>>         +------------------------+ |  Reserved for MBR/BIOS |
>>>> 000800  +------------------------+
>>>>         |  Typically used by MBR |
>>>> 000600  +------------------------+
>>>>         |  BIOS use only         |
>>>> 000000  +------------------------+
>>>> 
>>>> 
>>> Only now I actually looked at you patch at it is wrong. You don't
>>> need to mark 0x512 as NVS for reason you are stating above. You
>>> need to put MADT table into ACPI NVS and that is not what your
>>> patch is doing. 
>> 
>> Do you mean ACPI table like MADT may be destroyed by os, so it need
>> explicitly marked as ACPI NVS ? 
> Yes. That is what spec says. But we don't what all ACPI tables to be
> in ACPI NVS only those which are accessed from AML code and for now
> it's only MADT.

I hesitate what you say. AML code access MADT is normal access, so why need marked ACPI NVS? to prevent who access MADT or avoid what?

In fact, I cannot get the conclusion ACPI NVS cause win7 build BSOD from bug report 
http://sourceforge.net/tracker/?func=detail&aid=2902983&group_id=180599&atid=893831
it only change cmdline and then installation works OK.

I still not quite clear your ACPI NVS issue.
I remove vcpu_hotplug_2.patch.
leave vcpu_hotplug_1.pach, and it was tested linux guestos and win7 guestos, all work OK.
You can update vcpu_hotplug_1.patch per your understanding.

Thanks,
Jinsong
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vcpu_hotplug_1.patch
Type: application/octet-stream
Size: 44751 bytes
Desc: vcpu_hotplug_1.patch
URL: <http://www.seabios.org/pipermail/seabios/attachments/20100130/8cb1197a/attachment-0001.dmg>