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

Liu, Jinsong jinsong.liu at intel.com
Fri Jan 29 20:22:59 CET 2010


Gleb Natapov wrote:
> On Sat, Jan 30, 2010 at 02:16:11AM +0800, Liu, Jinsong wrote:
>> 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?
>> I can't parse that.  
> 
>> 
>> 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.
>> 
> If you can't get this conclusion you'll have to believe me.
> 
>> 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. 
> Have you tested checked build installation of win7 like bug report
> says? 
> 
>> You can update vcpu_hotplug_1.patch per your understanding.
>> 
> I am not going to. Just not including it into seabios is my advice
> if submitter doesn't want to address issues and read specs.  I want
> proper CPU hotplug support in seabios that works for Windows too,
> doesn't break MS SVVP test and support arbitrary number of CPUs. The
> code you ported from BOCHS fails on all accounts. I am OK with
> including it as temporary solution if you address at least easy to
> address issues, but you refuse to do even that.

Seems you don't like to update my patch, it doesn't matter.
Currently issue is, through discussion, I didn't quite understand what the 'ACPI NVS issue' is and what for.
You may meet this issue before, but you didn't speak clear to make me understand.

I will go through ACPI sepc 4.0 to see if there are some clue. 
If I find what I think 'ACPI NVS issue' is, I will confirm with you to prevent any misunderstanding. 
Based on I correctly understand what you say 'ACPI NVS issue', I will then update my patch.
This is my solution to 'ACPI NVS issue'.

Thanks,
Jinsong


More information about the SeaBIOS mailing list