On 2012-01-17 14:17, Igor Mammedov wrote:
> Rebase of the missing bits from qemu-kvm for vcpu hotplug
Description, please. Please try to split up, at least into PIIX4
preparations and "the rest". Maybe also a patch for a generic CPU
hotplug infrastructure.
>
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> ---
> Makefile.objs | 2 +-
> Makefile.target | 2 +-
> hw/acpi_piix4.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> hw/pc.c | 21 +++++++++++++-
> hw/pc_piix.c | 4 ++
> sysemu.h | 2 +
> target-i386/cpu.h | 1 +
> 7 files changed, 108 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 45df666..2a8ccc1 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -212,7 +212,7 @@ hw-obj-$(CONFIG_USB_UHCI) += usb-uhci.o
> hw-obj-$(CONFIG_USB_OHCI) += usb-ohci.o
> hw-obj-$(CONFIG_USB_EHCI) += usb-ehci.o
> hw-obj-$(CONFIG_FDC) += fdc.o
> -hw-obj-$(CONFIG_ACPI) += acpi.o acpi_piix4.o icc_bus.o
> +hw-obj-$(CONFIG_ACPI) += acpi.o icc_bus.o
> hw-obj-$(CONFIG_APM) += pm_smbus.o apm.o
> hw-obj-$(CONFIG_DMA) += dma.o
> hw-obj-$(CONFIG_HPET) += hpet.o
> diff --git a/Makefile.target b/Makefile.target
> index 06d79b8..4865dde 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -226,7 +226,7 @@ obj-y += device-hotplug.o
> # Hardware support
> obj-i386-y += vga.o
> obj-i386-y += mc146818rtc.o pc.o
> -obj-i386-y += cirrus_vga.o sga.o apic.o ioapic.o piix_pci.o
> +obj-i386-y += cirrus_vga.o sga.o apic.o ioapic.o piix_pci.o acpi_piix4.o
That's a qemu-kvm hack, see below for the discussion. No-go for upstream
- likely.
> obj-i386-y += vmport.o
> obj-i386-y += pci-hotplug.o smbios.o wdt_ib700.o
> obj-i386-y += debugcon.o multiboot.o
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index bdc55a1..f1cdcc1 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -39,13 +39,19 @@
> #define ACPI_DBG_IO_ADDR 0xb044
>
> #define GPE_BASE 0xafe0
> +#define PROC_BASE 0xaf00
> #define GPE_LEN 4
> #define PCI_BASE 0xae00
> #define PCI_EJ_BASE 0xae08
> #define PCI_RMV_BASE 0xae0c
>
> +#define PIIX4_CPU_HOTPLUG_STATUS 4
> #define PIIX4_PCI_HOTPLUG_STATUS 2
>
> +struct gpe_regs {
> + uint8_t cpus_sts[32];
> +};
> +
> struct pci_status {
> uint32_t up;
> uint32_t down;
> @@ -71,6 +77,7 @@ typedef struct PIIX4PMState {
>
> /* for pci hotplug */
> ACPIGPE gpe;
> + struct gpe_regs gpe_cpu;
> struct pci_status pci0_status;
> uint32_t pci0_hotplug_enable;
> } PIIX4PMState;
> @@ -90,9 +97,11 @@ static void pm_update_sci(PIIX4PMState *s)
> ACPI_BITMASK_POWER_BUTTON_ENABLE |
> ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
> ACPI_BITMASK_TIMER_ENABLE)) != 0) ||
> - (((s->gpe.sts[0] & s->gpe.en[0]) & PIIX4_PCI_HOTPLUG_STATUS) != 0);
> + (((s->gpe.sts[0] & s->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 */
> acpi_pm_tmr_update(&s->tmr, (s->pm1a.en & ACPI_BITMASK_TIMER_ENABLE) &&
> !(pmsts & ACPI_BITMASK_TIMER_STATUS));
> @@ -219,10 +228,9 @@ static int vmstate_acpi_post_load(void *opaque, int
> version_id)
> { \
> .name = (stringify(_field)), \
> .version_id = 0, \
> - .num = GPE_LEN, \
> .info = &vmstate_info_uint16, \
> .size = sizeof(uint16_t), \
> - .flags = VMS_ARRAY | VMS_POINTER, \
> + .flags = VMS_SINGLE | VMS_POINTER, \
Does this make the vmstate incompatible to the current version?
> .offset = vmstate_offset_pointer(_state, _field, uint8_t), \
> }
>
> @@ -329,11 +337,16 @@ static void piix4_pm_machine_ready(Notifier *n, void
> *opaque)
>
> }
>
> +static PIIX4PMState *global_piix4_pm_state; /* cpu hotadd */
> +
> static int piix4_pm_initfn(PCIDevice *dev)
> {
> PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);
> uint8_t *pci_conf;
>
> + /* for cpu hotadd */
> + global_piix4_pm_state = s;
> +
> pci_conf = s->dev.config;
> pci_conf[0x06] = 0x80;
> pci_conf[0x07] = 0x02;
> @@ -425,7 +438,16 @@ device_init(piix4_pm_register);
> static uint32_t gpe_readb(void *opaque, uint32_t addr)
> {
> PIIX4PMState *s = opaque;
> - uint32_t val = acpi_gpe_ioport_readb(&s->gpe, addr);
> + uint32_t val = 0;
> + struct gpe_regs *g = &s->gpe_cpu;
> +
> + switch (addr) {
> + case PROC_BASE ... PROC_BASE+31:
> + val = g->cpus_sts[addr - PROC_BASE];
> + break;
> + default:
> + val = acpi_gpe_ioport_readb(&s->gpe, addr);
> + }
>
> PIIX4_DPRINTF("gpe read %x == %x\n", addr, val);
> return val;
> @@ -519,11 +541,20 @@ static int piix4_device_hotplug(DeviceState *qdev,
> PCIDevice *dev,
> static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
> {
> struct pci_status *pci0_status = &s->pci0_status;
> + int i = 0, cpus = smp_cpus;
> +
> + while (cpus > 0) {
> + s->gpe_cpu.cpus_sts[i++] = (cpus < 8) ? (1 << cpus) - 1 : 0xff;
> + cpus -= 8;
> + }
>
> register_ioport_write(GPE_BASE, GPE_LEN, 1, gpe_writeb, s);
> register_ioport_read(GPE_BASE, GPE_LEN, 1, gpe_readb, s);
> acpi_gpe_blk(&s->gpe, GPE_BASE);
>
> + register_ioport_write(PROC_BASE, 32, 1, gpe_writeb, s);
> + register_ioport_read(PROC_BASE, 32, 1, gpe_readb, s);
> +
> register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, pci0_status);
> register_ioport_read(PCI_BASE, 8, 4, pcihotplug_read, pci0_status);
>
> @@ -536,6 +567,50 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus,
> PIIX4PMState *s)
> pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
> }
>
> +extern const char *global_cpu_model;
> +
> +#ifdef TARGET_I386
Why only TARGET_I386? If the PIIX4 is used on other targets, the
infrastructure should be prepared to enable hotplugging for them as well
(at a later point). pc_new_cpu is a bad name then. And APIC fiddling
should be pushed into the arch-specific new-cpu handler.
Also note that target dependencies are a no-go for hwlib compilations
which is clearly preferrable today.
> +static void enable_processor(PIIX4PMState *s, int cpu)
> +{
> + struct gpe_regs *g = &s->gpe_cpu;
> + ACPIGPE *gpe = &s->gpe;
> +
> + *gpe->sts = *gpe->sts | PIIX4_CPU_HOTPLUG_STATUS;
> + g->cpus_sts[cpu/8] |= (1 << (cpu%8));
"cpu / 8", "cpu % 8", please. Here and below. checkpatch doesn't complain?
> +}
> +
> +static void disable_processor(PIIX4PMState *s, int cpu)
> +{
> + struct gpe_regs *g = &s->gpe_cpu;
> + ACPIGPE *gpe = &s->gpe;
> +
> + *gpe->sts = *gpe->sts | PIIX4_CPU_HOTPLUG_STATUS;
> + g->cpus_sts[cpu/8] &= ~(1 << (cpu%8));
> +}
> +
> +void qemu_system_cpu_hot_add(int cpu, int state)
> +{
> + CPUState *env;
> + PIIX4PMState *s = global_piix4_pm_state;
> +
> + if (state && !qemu_get_cpu(cpu)) {
> + env = pc_new_cpu(global_cpu_model);
> + if (!env) {
> + fprintf(stderr, "cpu %d creation failed\n", cpu);
> + return;
> + }
> + env->cpuid_apic_id = cpu;
See comment above about proper target abstraction.
> + }
> +
> + if (state) {
> + enable_processor(s, cpu);
> + } else {
> + disable_processor(s, cpu);
> + }
> + pm_update_sci(s);
> +}
> +#endif
> +
> static void enable_device(PIIX4PMState *s, int slot)
> {
> s->gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
> diff --git a/hw/pc.c b/hw/pc.c
> index 33d8090..c7342d8 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -44,6 +44,8 @@
> #include "ui/qemu-spice.h"
> #include "memory.h"
> #include "exec-memory.h"
> +#include "cpus.h"
> +#include "kvm.h"
kvm? Likely not what you want.
>
> /* output Bochs bios info messages */
> //#define DEBUG_BIOS
> @@ -930,10 +932,22 @@ static void pc_cpu_reset(void *opaque)
> env->halted = !cpu_is_bsp(env);
> }
>
> -static CPUState *pc_new_cpu(const char *cpu_model)
> +CPUState *pc_new_cpu(const char *cpu_model)
> {
> CPUState *env;
>
> + if (cpu_model == NULL) {
> +#ifdef TARGET_X86_64
> + cpu_model = "qemu64";
> +#else
> + cpu_model = "qemu32";
> +#endif
Just always initialize global_cpu_model to a non-NULL value.
> + }
> +
> + if (runstate_is_running()) {
> + pause_all_vcpus();
> + }
> +
> env = cpu_init(cpu_model);
> if (!env) {
> fprintf(stderr, "Unable to find x86 CPU definition\n");
> @@ -944,6 +958,11 @@ static CPUState *pc_new_cpu(const char *cpu_model)
> }
> qemu_register_reset(pc_cpu_reset, env);
> pc_cpu_reset(env);
> +
> + cpu_synchronize_post_init(env);
> + if (runstate_is_running()) {
> + resume_all_vcpus();
> + }
> return env;
> }
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index b70431f..1c77ea1 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -53,6 +53,8 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
> static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
> static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>
> +const char *global_cpu_model; /* cpu hotadd */
> +
> static void ioapic_init(GSIState *gsi_state)
> {
> DeviceState *dev;
> @@ -102,6 +104,8 @@ static void pc_init1(MemoryRegion *system_memory,
> MemoryRegion *rom_memory;
> DeviceState *dev;
>
> + global_cpu_model = cpu_model;
> +
Move this into pc_cpus_init, and you automatically resolve the cpu_model
== NULL issue.
> pc_cpus_init(cpu_model);
>
> if (kvmclock_enabled) {
> diff --git a/sysemu.h b/sysemu.h
> index ddef2bb..678b478 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -155,6 +155,8 @@ void pcie_aer_inject_error_print(Monitor *mon, const
> QObject *data);
> int do_pcie_aer_inject_error(Monitor *mon,
> const QDict *qdict, QObject **ret_data);
>
> +void qemu_system_cpu_hot_add(int cpu, int state);
> +
> /* serial ports */
>
> #define MAX_SERIAL_PORTS 4
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 5dd1627..a9a6136 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -931,6 +931,7 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t
> new_cr4);
> /* hw/pc.c */
> void cpu_smm_update(CPUX86State *env);
> uint64_t cpu_get_tsc(CPUX86State *env);
> +CPUState *pc_new_cpu(const char *cpu_model);
"cpu_new" or so would be better. Every target supporting hotplug would
have to implement it.
>
> /* used to debug */
> #define X86_DUMP_FPU 0x0001 /* dump FPU state too */
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
