Re: [PATCH] Setup vcpu add/remove infrastructure, including madt bios_info and dsdt.

[Gleb Natapov <gleb@redhat.com>]

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@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.liu@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.

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.

--
			Gleb.