Gleb Natapov wrote:
> On Fri, Jan 22, 2010 at 10:52:29PM +0800, Liu, Jinsong wrote:
>> Gleb Natapov wrote:
>>> On Fri, Jan 22, 2010 at 06:54:42PM +0800, Liu, Jinsong wrote:
>>>> Gleb Natapov wrote:
>>>>> On Fri, Jan 22, 2010 at 05:08:32PM +0800, Liu, Jinsong wrote:
>>>>>> Gleb Natapov wrote:
>>>>>>> On Fri, Jan 22, 2010 at 10:15:44AM +0800, Liu, Jinsong wrote:
>>>>>>>> Gleb Natapov wrote:
>>>>>>>>> On Thu, Jan 21, 2010 at 07:48:23PM +0800, Liu, Jinsong wrote:
>>>>>>>>>>> From cb997030cba02e7e74a29b3d942aeba9808ed293 Mon Sep 17
>>>>>>>>>>> 00:00:00 2001
>>>>>>>>>> From: Liu, Jinsong <jinsong....@intel.com>
>>>>>>>>>> Date: Fri, 22 Jan 2010 03:18:46 +0800
>>>>>>>>>> Subject: [PATCH] Setup vcpu add/remove
>>>>>>>>>> infrastructure, including madt bios_info and dsdt.
>>>>>>>>>>
>>>>>>>>>> 1. setup madt bios_info structure, so that static
>>>>>>>>>> dsdt get run-time madt info like checksum
>>>>>>>>>> address, lapic address, max cpu numbers, with
>>>>>>>>>> least hardcode magic number (realmode address of
>>>>>>>>>> bios_info).
>>>>>>>>>> 2. setup vcpu add/remove dsdt infrastructure,
>>>>>>>>>> including processor related acpi objects and
>>>>>>>>>> control methods. vcpu add/remove will trigger SCI
>>>>>>>>>> and then control method _L02. By matching madt,
>>>>>>>>>> vcpu number and add/remove action were found,
>>>>>>>>>> then by notify control
>>>>>>>>>> method, it will notify OS acpi driver.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Liu, Jinsong <jinsong....@intel.com>
>>>>>>>>> It looks like AML code is a port of what we had in BOCHS bios
>>>>>>>>> with minor changes. Can you detail what is changed and why for
>>>>>>>>> easy review please? And this still doesn't work with Windows
>>>>>>>>> I assume.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Yes, my work is based on BOCHS infrastructure, thanks BOCHS :)
>>>>>>>> I just change some minor points:
>>>>>>>> 1. explicitly define returen value of '_MAT' as 'buffer',
>>>>>>>> otherwise some linux acpi driver (i.e. linux 2.6.30) would
>>>>>>>> parse error which will handle it as 'integer' not 'buffer';
>>>>>>>> 2. keep correct 'checksum' of madt when vcpu add/remove,
>>>>>>>> otherwise it will report 'checksum error' when using acpi tools
>>>>>>>> to get madt info if we add/remove vcpu;
>>>>>>>> 3. add '_EJ0' so that linux has acpi obj under
>>>>>>>> /sys/devices/LNXSYSTM:00, which is need for vcpu remove;
>>>>>>>> 4. on Method(PRSC, 0), just scan 'xxx' vcpus that qemu get from
>>>>>>>> cmdline para 'maxcpus=xxx', not all 256 vcpus, otherwise under
>>>>>>>> some dsdt processor define, it will result error;
>>>>>>> What kind of errors? Qemu should never set bit over maxcpus in
>>>>>>> PRS.
>>>>>>>
>>>>>> suppose cmdline define vcpus=4, maxvcpus=8
>>>>>> in original BOCHS, will scan 15 lapic items start from lapic0 of
>>>>>> madt, where it only has maxvcpus lapic items in madt table, hence
>>>>>> there is risk to scan over boundary, scan to other acpi table,
>>>>>> and result in wrong vcpu online/offline status (in my test, I
>>>>>> meet this situation). Because of this reason, this patch limit
>>>>>> scan maxvcpus lapic of madt.
>>>>>>
>>>>> But what if cmdline will use vcpu=64? The idea was that \_PR scope
>>>>> will reside in its own ssdt and for each maxvcpus value there will
>>>>> be ssdt with exactly this number of processors. Ideally ssdt will
>>>>> be created dynamically like it is done now, but another solution
>>>>> is to create them at bios compilation time and load correct one
>>>>> at runtime.
>>>>>
>>>>
>>>> It's OK for vcpu=64. vcpu<maxvcpus.
>>>> if maxvcpus > processor defined in dsdt, it's OK since no risk scan
>>>> (bios only support 15 processor is another story); if processor
>>>> defined in dsdt > maxvcpus, it has risk to scan over boundary.
>>>>
>>>>
>>> Yes, correct. So why not export maxcpus to DSDT and at the beginning
>>> of NTFY check that Arg0 < maxcpus then?
>>>
>>
>> Yes, your solution also work.
>> In fact, we implicitly export maxcpus to DSDT by
>> struct bios_info {
>> ...
>> u32 max_cpus_byte; /* max cpus bitmap bytes */
>> u32 max_cpus_bit; /* max cpus bitmap bits of last byte */ };
>> this indicate maxvcpus.
>> In this way it can reduce scan loop.
>>
> Actually your scan loop is twice as big as it was in BOCHS because of
> this tricks with bytes and bits.
>
No,
original BOCHS solution: PRSC has 256 loop, and NTFY has 15 scan;
this patch solution: PRSC has maxvcpus loop, and NTFY has min(15,
maxvcpus) scan.
>> However, I think it's not key issue. Both are OK.
>> Just different implement way.
>>
> The AML code is hard to read as it is, so making it simpler is
> important. But the way I want to see this solved in seabios is to
> create exactly maxvcpu processors in _PR scope. This will solve MS
> SVVP problem too (BOCHS doesn't pass SVVP).
Yes, to create exactly maxvcpu processor is key points.
Thanks,
Jinsong--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
