From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:32965) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV3Io-0003v3-4N for qemu-devel@nongnu.org; Wed, 24 Apr 2013 13:16:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UV3Ih-00010W-54 for qemu-devel@nongnu.org; Wed, 24 Apr 2013 13:16:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV3Ig-000106-T2 for qemu-devel@nongnu.org; Wed, 24 Apr 2013 13:16:11 -0400 Date: Wed, 24 Apr 2013 19:15:30 +0200 From: Igor Mammedov Message-ID: <20130424191530.769c5493@thinkpad> In-Reply-To: <5178031B.30707@suse.de> References: <1366705795-24732-11-git-send-email-imammedo@redhat.com> <1366819084-28461-1-git-send-email-imammedo@redhat.com> <5178031B.30707@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 08/19 v7] acpi_piix4: add infrastructure to send CPU hot-plug GPE to guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?ISO-8859-1?B?RuRyYmVy?= Cc: kwolf@redhat.com, peter.maydell@linaro.org, aliguori@us.ibm.com, ehabkost@redhat.com, gleb@redhat.com, mst@redhat.com, jan.kiszka@siemens.com, quintela@redhat.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, aderumier@odiso.com, lcapitulino@redhat.com, blauwirbel@gmail.com, anthony.perard@citrix.com, alex.williamson@redhat.com, kraxel@redhat.com, yang.z.zhang@intel.com, pbonzini@redhat.com, stefano.stabellini@eu.citrix.com, armbru@redhat.com, rth@twiddle.net On Wed, 24 Apr 2013 18:06:51 +0200 Andreas F=E4rber 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 > >=20 > > Signed-off-by: Igor Mammedov > > --- > > 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=3D4; cpu-add id=3D2; dst-qemu -smp 3,ma= x-cpus=3D4 > > Destination QEMU will recreate the same gpe_cpu.sts=3Dt'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. > >=20 > > 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 > >=20 > [...] > > 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 > > =20 > > +#define PIIX4_PROC_BASE 0xaf00 > > +#define PIIX4_PROC_LEN 32 > > + > > #define PIIX4_PCI_HOTPLUG_STATUS 2 > > +#define PIIX4_CPU_HOTPLUG_STATUS 4 > > =20 > > struct pci_status { > > uint32_t up; /* deprecated, maintained for migration compatibility= */ > > uint32_t down; > > }; > > =20 > > +struct cpu_status { >=20 > 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 >=20 > > + uint8_t sts[PIIX4_PROC_LEN]; > > +}; > > + > > typedef struct PIIX4PMState { > > PCIDevice dev; > > =20 > > MemoryRegion io; > > MemoryRegion io_gpe; > > MemoryRegion io_pci; > > + MemoryRegion io_cpu; > > ACPIREGS ar; > > =20 > > 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; > > =20 > > 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)) !=3D 0) || > > - (((s->ar.gpe.sts[0] & s->ar.gpe.en[0]) > > - & PIIX4_PCI_HOTPLUG_STATUS) !=3D 0); > > + (((s->ar.gpe.sts[0] & s->ar.gpe.en[0]) & > > + (PIIX4_PCI_HOTPLUG_STATUS | PIIX4_CPU_HOTPLUG_STATUS)) !=3D = 0); > > =20 > > qemu_set_irq(s->irq, sci_level); > > /* schedule a timer interruption if needed */ > > @@ -585,6 +597,73 @@ static const MemoryRegionOps piix4_pci_ops =3D { > > }, > > }; > > =20 > > +static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned wi= dth) > > +{ > > + PIIX4PMState *s =3D opaque; > > + struct cpu_status *cpus =3D &s->gpe_cpu; > > + uint64_t val =3D 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 re= moved */ > > +} > > + > > +static const MemoryRegionOps cpu_hotplug_ops =3D { > > + .read =3D cpu_status_read, > > + .write =3D cpu_status_write, > > + .endianness =3D DEVICE_LITTLE_ENDIAN, > > + .valid =3D { > > + .min_access_size =3D 1, > > + .max_access_size =3D 1, > > + }, > > +}; > > + > > +typedef enum { > > + PLUG, > > + UNPLUG, > > +} HotplugEventType; > > + > > +static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu, > > + HotplugEventType action) > > +{ > > + struct cpu_status *g =3D &s->gpe_cpu; > > + ACPIGPE *gpe =3D &s->ar.gpe; > > + CPUClass *k =3D CPU_GET_CLASS(cpu); > > + int64_t cpu_id; > > + > > + assert(s !=3D NULL); > > + > > + *gpe->sts =3D *gpe->sts | PIIX4_CPU_HOTPLUG_STATUS; > > + cpu_id =3D k->get_arch_id(CPU(cpu)); > > + if (action =3D=3D PLUG) { > > + g->sts[cpu_id / 8] |=3D (1 << (cpu_id % 8)); > > + } else { > > + g->sts[cpu_id / 8] &=3D ~(1 << (cpu_id % 8)); > > + } > > + pm_update_sci(s); > > +} > > + > > +static void piix4_cpu_added_req(Notifier *n, void *opaque) > > +{ > > + PIIX4PMState *s =3D container_of(n, PIIX4PMState, cpu_added_notifi= er); > > + > > + piix4_cpu_hotplug_req(s, CPU(opaque), PLUG); > > +} > > + > > +static void piix4_init_cpu_status(CPUState *cpu, void *data) > > +{ > > + struct cpu_status *g =3D (struct cpu_status *)data; > > + CPUClass *k =3D CPU_GET_CLASS(cpu); > > + int64_t id =3D k->get_arch_id(cpu); > > + > > + g_assert((id / 8) < PIIX4_PROC_LEN); > > + g->sts[id / 8] |=3D (1 << (id % 8)); > > +} > > + > > static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, > > PCIHotplugState state); > > =20 > > @@ -600,6 +679,13 @@ static void piix4_acpi_system_hot_add_init(MemoryR= egion *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-h= otplug", > > + PIIX4_PROC_LEN); > > + memory_region_add_subregion(parent, PIIX4_PROC_BASE, &s->io_cpu); > > + s->cpu_added_notifier.notify =3D piix4_cpu_added_req; > > + qemu_register_cpu_added_notifier(&s->cpu_added_notifier); > > } > > =20 > > static void enable_device(PIIX4PMState *s, int slot) > >=20 >=20 >=20 > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg >=20 --=20 Regards, Igor