On Wed, 24 Apr 2013 18:06:51 +0200
Andreas Färber <afaer...@suse.de> wrote:
> Am 24.04.2013 17:58, schrieb Igor Mammedov:
> > * introduce processor status bitmask visible to guest at 0xaf00 addr,
> > where ACPI asl code expects it
> > * set bit corresponding to APIC ID in processor status bitmask on
> > receiving CPU hot-plug notification
> > * trigger CPU hot-plug SCI, to notify guest about CPU hot-plug event
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > Note:
> > gpe_cpu.sts isn't need to be migrated, since CPU hotpluging during
> > migration just doesn't work, since destination QEMU has to be started
> > with all present in guest CPUs (including hotplugged).
> > i.e. src-qemu -smp 2,max-cpus=4; cpu-add id=2; dst-qemu -smp 3,max-cpus=4
> > Destination QEMU will recreate the same gpe_cpu.sts=t'111' bitmap as
> > on source by calling qemu_for_each_cpu(piix4_init_cpu_status,
> > &s->gpe_cpu);
> > since it has been started with 3 CPUs on command line.
> >
> > v6:
> > * drop gpe_cpu.sts migration hunks
> > v5:
> > * add optional vmstate subsection if there was CPU hotplug event
> > * remove unused Error*
> > * use qemu_for_each_cpu() instead of recursion over QOM tree
> > v4:
> > * added spec for QEMU-Seabios interface
> > * added PIIX4_ prefix to PROC_ defines
> > v3:
> > * s/get_firmware_id()/get_arch_id()/ due rebase
> > * s/cpu_add_notifier/cpu_added_notifier/
> > v2:
> > * use CPUClass.get_firmware_id() to make code target independent
> > * bump up vmstate_acpi version
> > ---
> > docs/specs/acpi_cpu_hotplug.txt | 22 +++++++++
> > hw/acpi/piix4.c | 90
> > ++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 110 insertions(+), 2 deletions(-)
> > create mode 100644 docs/specs/acpi_cpu_hotplug.txt
> >
> [...]
> > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > index 88386d7..b18202c 100644
> > --- a/hw/acpi/piix4.c
> > +++ b/hw/acpi/piix4.c
> > @@ -48,19 +48,28 @@
> > #define PCI_EJ_BASE 0xae08
> > #define PCI_RMV_BASE 0xae0c
> >
> > +#define PIIX4_PROC_BASE 0xaf00
> > +#define PIIX4_PROC_LEN 32
> > +
> > #define PIIX4_PCI_HOTPLUG_STATUS 2
> > +#define PIIX4_CPU_HOTPLUG_STATUS 4
> >
> > struct pci_status {
> > uint32_t up; /* deprecated, maintained for migration compatibility */
> > uint32_t down;
> > };
> >
> > +struct cpu_status {
>
> I see that you're copying pci_status above, but Coding Style asks for
> CamelCase and typedef, so could we change this to CPUStatus or better
> PIIX4CPUStatus? (I counted 5 occurrences)
sure, I'll resend it with CPUStatus variant.
> Andreas
>
> > + uint8_t sts[PIIX4_PROC_LEN];
> > +};
> > +
> > typedef struct PIIX4PMState {
> > PCIDevice dev;
> >
> > MemoryRegion io;
> > MemoryRegion io_gpe;
> > MemoryRegion io_pci;
> > + MemoryRegion io_cpu;
> > ACPIREGS ar;
> >
> > APMState apm;
> > @@ -82,6 +91,9 @@ typedef struct PIIX4PMState {
> > uint8_t disable_s3;
> > uint8_t disable_s4;
> > uint8_t s4_val;
> > +
> > + struct cpu_status gpe_cpu;
> > + Notifier cpu_added_notifier;
> > } PIIX4PMState;
> >
> > static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
> > @@ -100,8 +112,8 @@ static void pm_update_sci(PIIX4PMState *s)
> > ACPI_BITMASK_POWER_BUTTON_ENABLE |
> > ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
> > ACPI_BITMASK_TIMER_ENABLE)) != 0) ||
> > - (((s->ar.gpe.sts[0] & s->ar.gpe.en[0])
> > - & PIIX4_PCI_HOTPLUG_STATUS) != 0);
> > + (((s->ar.gpe.sts[0] & s->ar.gpe.en[0]) &
> > + (PIIX4_PCI_HOTPLUG_STATUS | PIIX4_CPU_HOTPLUG_STATUS)) != 0);
> >
> > qemu_set_irq(s->irq, sci_level);
> > /* schedule a timer interruption if needed */
> > @@ -585,6 +597,73 @@ static const MemoryRegionOps piix4_pci_ops = {
> > },
> > };
> >
> > +static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned width)
> > +{
> > + PIIX4PMState *s = opaque;
> > + struct cpu_status *cpus = &s->gpe_cpu;
> > + uint64_t val = cpus->sts[addr];
> > +
> > + return val;
> > +}
> > +
> > +static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
> > + unsigned int size)
> > +{
> > + /* TODO: implement VCPU removal on guest signal that CPU can be
> > removed */
> > +}
> > +
> > +static const MemoryRegionOps cpu_hotplug_ops = {
> > + .read = cpu_status_read,
> > + .write = cpu_status_write,
> > + .endianness = DEVICE_LITTLE_ENDIAN,
> > + .valid = {
> > + .min_access_size = 1,
> > + .max_access_size = 1,
> > + },
> > +};
> > +
> > +typedef enum {
> > + PLUG,
> > + UNPLUG,
> > +} HotplugEventType;
> > +
> > +static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
> > + HotplugEventType action)
> > +{
> > + struct cpu_status *g = &s->gpe_cpu;
> > + ACPIGPE *gpe = &s->ar.gpe;
> > + CPUClass *k = CPU_GET_CLASS(cpu);
> > + int64_t cpu_id;
> > +
> > + assert(s != NULL);
> > +
> > + *gpe->sts = *gpe->sts | PIIX4_CPU_HOTPLUG_STATUS;
> > + cpu_id = k->get_arch_id(CPU(cpu));
> > + if (action == PLUG) {
> > + g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
> > + } else {
> > + g->sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
> > + }
> > + pm_update_sci(s);
> > +}
> > +
> > +static void piix4_cpu_added_req(Notifier *n, void *opaque)
> > +{
> > + PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier);
> > +
> > + piix4_cpu_hotplug_req(s, CPU(opaque), PLUG);
> > +}
> > +
> > +static void piix4_init_cpu_status(CPUState *cpu, void *data)
> > +{
> > + struct cpu_status *g = (struct cpu_status *)data;
> > + CPUClass *k = CPU_GET_CLASS(cpu);
> > + int64_t id = k->get_arch_id(cpu);
> > +
> > + g_assert((id / 8) < PIIX4_PROC_LEN);
> > + g->sts[id / 8] |= (1 << (id % 8));
> > +}
> > +
> > static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
> > PCIHotplugState state);
> >
> > @@ -600,6 +679,13 @@ static void
> > piix4_acpi_system_hot_add_init(MemoryRegion *parent,
> > memory_region_add_subregion(parent, PCI_HOTPLUG_ADDR,
> > &s->io_pci);
> > pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
> > +
> > + qemu_for_each_cpu(piix4_init_cpu_status, &s->gpe_cpu);
> > + memory_region_init_io(&s->io_cpu, &cpu_hotplug_ops, s,
> > "apci-cpu-hotplug",
> > + PIIX4_PROC_LEN);
> > + memory_region_add_subregion(parent, PIIX4_PROC_BASE, &s->io_cpu);
> > + s->cpu_added_notifier.notify = piix4_cpu_added_req;
> > + qemu_register_cpu_added_notifier(&s->cpu_added_notifier);
> > }
> >
> > static void enable_device(PIIX4PMState *s, int slot)
> >
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
--
Regards,
Igor
