[SeaBIOS] [PATCH] vcpu hotplug: Setup vcpu add/remove infrastructure, including madt bios_info and dsdt
Gleb Natapov
gleb at redhat.com
Sun Jan 24 11:17:50 CET 2010
On Sat, Jan 23, 2010 at 06:46:18PM -0500, Kevin O'Connor wrote:
> On Fri, Jan 22, 2010 at 09:35:55AM +0800, Liu, Jinsong wrote:
> > From cb997030cba02e7e74a29b3d942aeba9808ed293 Mon Sep 17 00:00:00 2001
> > From: Liu, Jinsong <jinsong.liu at 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 at intel.com>
>
> Thanks.
>
> I'd like to see Gleb's comments on the ACPI changes.
>
I already commented on KVM list. Here is the link
http://www.mail-archive.com/kvm@vger.kernel.org/msg27562.html
> [...]
> > diff --git a/src/acpi.c b/src/acpi.c
> > index f613b03..037da46 100644
> > --- a/src/acpi.c
> > +++ b/src/acpi.c
> > @@ -344,6 +344,9 @@ build_fadt(int bdf)
> > return fadt;
> > }
> >
> > +u32 madt_csum_addr, madt_lapic0_addr;
> > +u32 max_cpus_byte, max_cpus_bit;
> > +
> > static void*
> > build_madt(void)
> > {
> > @@ -360,6 +363,10 @@ build_madt(void)
> > madt->local_apic_address = cpu_to_le32(BUILD_APIC_ADDR);
> > madt->flags = cpu_to_le32(1);
> > struct madt_processor_apic *apic = (void*)&madt[1];
> > + madt_csum_addr = (u32)&madt->checksum;
> > + madt_lapic0_addr = (u32)apic;
> > + max_cpus_byte = MaxCountCPUs / 8;
> > + max_cpus_bit = MaxCountCPUs % 8;
>
> I'm confused at why global variables are being created, exported, and
> set, just to be copied to another location later on.
>
> > +/* *****************************************************************
> > + * use bios_info to enable dsdt get run-time madt and cpu info,
> > + * and to avoid more hardcode magic number.
> > + *******************************************************************/
> > +#define BIOS_INFO_PHYSICAL_ADDRESS 0x000EA000
>
> That wont work - nothing stops 0xea000 from being used by an option
> rom or the bios itself. If a physical address is needed, the
> VAR16FIXED() macro needs to be used. However, I'm very reluctant to
> add new hardcoded addresses - there has to be a very compelling reason
> to do that.
>
> -Kevin
>
> _______________________________________________
> SeaBIOS mailing list
> SeaBIOS at seabios.org
> http://www.seabios.org/mailman/listinfo/seabios
--
Gleb.
More information about the SeaBIOS
mailing list