[SeaBIOS] [PATCH 3/3] Take in account hot(un)plugged cpus on reboot

[Igor Mammedov]

----- Original Message -----
> From: "Gleb Natapov" <gleb at redhat.com>
> To: "Igor Mammedov" <imammedo at redhat.com>
> Cc: "Kevin O'Connor" <kevin at koconnor.net>, "jan kiszka" <jan.kiszka at siemens.com>, seabios at seabios.org
> Sent: Tuesday, March 13, 2012 11:21:28 AM
> Subject: Re: [SeaBIOS] [PATCH 3/3] Take in account hot(un)plugged cpus on reboot
> 
> On Tue, Mar 13, 2012 at 11:16:51AM +0100, Igor Mammedov wrote:
> > On 03/13/2012 10:51 AM, Gleb Natapov wrote:
> > >On Tue, Mar 13, 2012 at 10:37:54AM +0100, Igor Mammedov wrote:
> > >>On 03/13/2012 01:09 AM, Kevin O'Connor wrote:
> > >>>On Sat, Mar 10, 2012 at 12:47:28PM +0100, Igor Mammedov wrote:
> > >>>>Initial count of active cpus is communicated to bios from qemu
> > >>>>via
> > >>>>CMOS_BIOS_SMP_COUNT io port. However if cpus are hotplugged
> > >>>>after
> > >>>>boot and then guest is rebooted without taking down qemu then
> > >>>>bios might be stuck at smp_probe
> > >>>[...]
> > >>>>--- a/src/smp.c
> > >>>>+++ b/src/smp.c
> > >>>>@@ -17,6 +17,9 @@
> > >>>>
> > >>>>  #define APIC_ENABLED 0x0100
> > >>>>
> > >>>>+#define ACPI_CPU_STATUS_MAP 0xaf00
> > >>>>+#define ACPI_CPU_STATUS_MAP_SZ 32
> > >>>>+
> > >>>>  struct { u32 ecx, eax, edx; } smp_mtrr[32] VAR16VISIBLE;
> > >>>>  u32 smp_mtrr_count VAR16VISIBLE;
> > >>>>
> > >>>>@@ -115,6 +118,26 @@ smp_probe(void)
> > >>>>          msleep(10);
> > >>>>      } else {
> > >>>>          u8 cmos_smp_count = inb_cmos(CMOS_BIOS_SMP_COUNT);
> > >>>>+        dprintf(1, "Powered-on with %d cpu(s)\n",
> > >>>>cmos_smp_count + 1);
> > >>>>+
> > >>>>+        if (qemu_cfg_have_acpi_cpus_map()) {
> > >>>[...]
> > >>>
> > >>>Please don't clutter up smp.c with this.  The code should look
> > >>>something like:
> > >>>
> > >>>           int total_cpus = getCPUcount();
> > >>>           while (readl(&CountCPUs)<   total_cpus)
> > >>>               yield();
> > >>>
> > >>>The getCPUcount() can be buried in paravirt.c, acpi.c, or where
> > >>>ever
> > >>>it makes sense.
> > >>
> > >>Thanks Kevin,
> > >>I'll amend and repost patch-set taking in account your
> > >>suggestions.
> > >>
> > >>>BTW, why do we have to call qemu_cfg_have_acpi_cpus_map?  Can't
> > >>>this
> > >>>just be inferred if the data at inb(0xaf00) is all zeros?
> > >We can't rely on the fact that unused port returns all zeros.
> > >Actually
> > >on real HW I think it return all ones.
> > >
> > >>What if qemu doesn't have 0xaf00 (like current upstream)?
> > >>cfg_have_acpi_cpus_map is just a flag that allows to detect if
> > >>qemu
> > >>provides 0xaf00. This way it would be possible for seabios to
> > >>work with
> > >>old and new versions of qemu without breaking anything.
> > >>
> > >If QEMU will update cmos cpu count on cpu (un)plug we will not
> > >have to
> > >introduce new PV interface to telll SeaBIOS about 0xaf00.
> > 
> > But we will have to introduce it anyway in future for correct
> > building of
> > mp/acpi/smbios tables. So why not do it now and be ready later for
> > patches
> > to mp/acpi/smbios tables.
> > 
> Good point. You mean we need a list of apic ids of available cpus
> right?
> Can SeaBIOS build this list by reading apic id in smp_ap_boot_code?

I've looked at how tables are build. It seems that it's not hard and sufficient
to build an equivalent temporary byte array of (un)available apic ids that
corresponds to 0xaf00 bitmap at boot time, under following restriction:
  - apic id shall be in range [0..max_cpus)
     * this will allow to allocate array where apic id will serve as index,
     thus making it equvivalent to 0xaf00 bitmap.
     * and easily fill this array in smp_ap_boot_code without inventing
     and using mutexes/spinlocks in smp_ap_boot_code.


This will require cooperation from qemu side. apic id could be checked
somewhere in device_add command handler call chain or at cpu object creation
time.

Jan,
  What do you think about setting such restriction on apic id of cpus in
  qemu?

> --
> 			Gleb.
>