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.liu@xxxxxxxxx>
>>>>>>>> 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.liu@xxxxxxxxx>
>>>>>>> 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.
However, I think it's not key issue. Both are OK.
Just different implement way.
Thanks,
Jinsong
>>> BTW seabios creates very simple ssdt with \_PR scope right now and I
>>> don't see yous patch removes this so you end up with two conflicting
>>> \_PR scopes.
>>
>> Yes, I just notice \_PR at acpi.c.
>> we can move \_PR to ssdt after ssdt infrastructure was full setup.
>>
>> Thanks,
>> Jinsong
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
