Re: Alt SeaBIOS SSDT cpu hotplug

[Gleb Natapov <gleb@redhat.com>]

On Thu, Jul 08, 2010 at 08:45:06PM -0400, Kevin O'Connor wrote:
> On Thu, Jul 08, 2010 at 03:54:10PM +0300, Gleb Natapov wrote:
> > On Wed, Jul 07, 2010 at 07:26:07PM -0400, Kevin O'Connor wrote:
> > > On Wed, Jul 07, 2010 at 01:22:49PM +0300, Gleb Natapov wrote:
> > > > On Wed, Jul 07, 2010 at 12:57:05AM -0400, Kevin O'Connor wrote:
> > > > > The "CPUS" package stores references to the Processor objects, and the
> > > > > "CPON" package stores the state of which cpus are active.  With this
> > > > > info, hopefully there is no need to update the MADT tables.
> > > > > 
> > > > The way you wrote it acpi_id is always equal to lapic_id which is not
> > > > alway true. By using MADT from memory we remove this dependency from
> > > > DSDT code.
> > > 
> > > The current code always sets the apic->processor_id equal to
> > > apic->local_apic_id in the madt apic table.  If this changes, we could
> > > add a dynamically created mapping table to the ssdt.  Something like:
> > > 
> > > Name(CPLA, Package() { 0x00, 0x01, 0x02, 0x03, 0x04 })
> > > 
> > Yeah, but if we can avoid it in the first place why not doing so.
> 
> There is a cost to reading/writing the MADT tables.  The hardcoding of
> 0x514 has a risk - it's not clear to me that an optionrom wont clobber
> that space.  It also recently occurred to me that there may be a
> problem if a guest expects the MADT address to remain constant across
> a hibernate/restore cycle - the malloc_high() function doesn't
> guarentee stable addresses across reboots.
> 
That's definitely an issue. HW shouldn't change between hibernate and
resume, so boot process should be the same and address should end up be
the same too, but I wouldn't want to rely on this blindly.

> >BTW how
> > do you pass to DSDT what cpus are initially on? Previously this info was
> > taken from memory copy of MADT too.
> 
> The CPON array is dynamically generated with the status of the
> processors.  (It is then kept up to date by the DSDT code.)  The code
> for generating CPON is:
> 
>     // build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })"
>     *(ssdt_ptr++) = 0x08; // NameOp
>     *(ssdt_ptr++) = 'C';
>     *(ssdt_ptr++) = 'P';
>     *(ssdt_ptr++) = 'O';
>     *(ssdt_ptr++) = 'N';
>     *(ssdt_ptr++) = 0x12; // PackageOp
>     ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2);
>     *(ssdt_ptr++) = acpi_cpus;
>     for (i=0; i<acpi_cpus; i++)
>         *(ssdt_ptr++) = (i < CountCPUs) ? 0x01 : 0x00;
> 
See it now. Thanks.

--
			Gleb.