diff mbox

[2/2] q35: add cpu hotplug support

Message ID 0577863ea4bc4e0d91675e4fd250c1065ba4ebd9.1377075625.git.hutao@cn.fujitsu.com
State New
Headers show

Commit Message

Hu Tao Aug. 21, 2013, 9:04 a.m. UTC
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/acpi/ich9.c         | 91 ++++++++++++++++++++++++++++++++++++++++++++++++--
 include/hw/acpi/ich9.h | 11 ++++++
 2 files changed, 100 insertions(+), 2 deletions(-)

Comments

Hu Tao Aug. 21, 2013, 9:10 a.m. UTC | #1
Added: Igor Mammedov <imammedo@redhat.com>

On Wed, Aug 21, 2013 at 05:04:28PM +0800, Hu Tao wrote:
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/acpi/ich9.c         | 91 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  include/hw/acpi/ich9.h | 11 ++++++
>  2 files changed, 100 insertions(+), 2 deletions(-)

Hi Igor,

Sorry I forgot to add you.
Gerd Hoffmann Aug. 21, 2013, 11:01 a.m. UTC | #2
Hi,

> +#define ICH9_PROC_BASE 0xaf00
> +#define ICH9_PROC_LEN 32

No, please don't.  It makes it impossible to assign the 0xa000 -> 0xafff
I/O port window to a PCI bridge.  Please lets stop occupy random io
ports above 0x1000 and burn I/O address space that way.

I'd suggest to place it at 0x0a00 instead.  Also the dsdt should get a
device with the address in _CRS so the guest knows those ports are used.

cheers,
  Gerd
Paolo Bonzini Aug. 21, 2013, 11:27 a.m. UTC | #3
Il 21/08/2013 13:01, Gerd Hoffmann ha scritto:
>   Hi,
> 
>> +#define ICH9_PROC_BASE 0xaf00
>> +#define ICH9_PROC_LEN 32
> 
> No, please don't.  It makes it impossible to assign the 0xa000 -> 0xafff
> I/O port window to a PCI bridge.  Please lets stop occupy random io
> ports above 0x1000 and burn I/O address space that way.
> 
> I'd suggest to place it at 0x0a00 instead.  Also the dsdt should get a
> device with the address in _CRS so the guest knows those ports are used.

Would this use 0x0A ("not present, device functional, decoding
resources") for _STA to "indicate a valid device for which no device
driver should be loaded" (quoting from the ACPI spec)?  Hopefully this
prevents Windows from showing the UI.

Paolo
Gerd Hoffmann Aug. 21, 2013, 11:36 a.m. UTC | #4
On Mi, 2013-08-21 at 13:27 +0200, Paolo Bonzini wrote:
> Il 21/08/2013 13:01, Gerd Hoffmann ha scritto:
> >   Hi,
> > 
> >> +#define ICH9_PROC_BASE 0xaf00
> >> +#define ICH9_PROC_LEN 32
> > 
> > No, please don't.  It makes it impossible to assign the 0xa000 -> 0xafff
> > I/O port window to a PCI bridge.  Please lets stop occupy random io
> > ports above 0x1000 and burn I/O address space that way.
> > 
> > I'd suggest to place it at 0x0a00 instead.  Also the dsdt should get a
> > device with the address in _CRS so the guest knows those ports are used.
> 
> Would this use 0x0A ("not present, device functional, decoding
> resources") for _STA to "indicate a valid device for which no device
> driver should be loaded" (quoting from the ACPI spec)?  Hopefully this
> prevents Windows from showing the UI.

Sounds reasonable.  Needs careful testing ...

cheers,
  Gerd
Andreas Färber Aug. 21, 2013, 11:40 a.m. UTC | #5
Am 21.08.2013 11:04, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/acpi/ich9.c         | 91 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  include/hw/acpi/ich9.h | 11 ++++++
>  2 files changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 8717c15..146216a 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -43,17 +43,22 @@ do { printf("%s "fmt, __func__, ## __VA_ARGS__); } while (0)
>  #define ICH9_DEBUG(fmt, ...)    do { } while (0)
>  #endif
>  
> +
> +#define ICH9_CPU_HOTPLUG_STATUS 4
> +
>  static void pm_update_sci(ICH9LPCPMRegs *pm)
>  {
>      int sci_level, pm1a_sts;
>  
>      pm1a_sts = acpi_pm1_evt_get_sts(&pm->acpi_regs);
>  
> -    sci_level = (((pm1a_sts & pm->acpi_regs.pm1.evt.en) &
> +    sci_level = ((((pm1a_sts & pm->acpi_regs.pm1.evt.en) &
>                    (ACPI_BITMASK_RT_CLOCK_ENABLE |
>                     ACPI_BITMASK_POWER_BUTTON_ENABLE |
>                     ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
> -                   ACPI_BITMASK_TIMER_ENABLE)) != 0);
> +                   ACPI_BITMASK_TIMER_ENABLE)) != 0) ||
> +                 (((pm->acpi_regs.gpe.sts[0] & pm->acpi_regs.gpe.en[0]) &
> +          ICH9_CPU_HOTPLUG_STATUS) != 0));
>      qemu_set_irq(pm->irq, sci_level);
>  
>      /* schedule a timer interruption if needed */
> @@ -93,6 +98,80 @@ static const MemoryRegionOps ich9_gpe_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
> +static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    ICH9LPCPMRegs *pm = opaque;
> +    CPUStatus *cpus = &pm->gpe_cpu;
> +    uint64_t val = cpus->sts[addr];
> +
> +    ICH9_DEBUG("addr: %" HWADDR_PRIx ", val: %" PRIx64 "\n", addr, val);
> +
> +    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 ich9_cpu_hotplug_req(ICH9LPCPMRegs *pm, CPUState *cpu,
> +                                 HotplugEventType action)
> +{
> +    CPUStatus *g = &pm->gpe_cpu;
> +    ACPIGPE *gpe = &pm->acpi_regs.gpe;
> +    CPUClass *k = CPU_GET_CLASS(cpu);

cc please. (c is not a reserved symbol, and other classes such as
X86CPUClass could be used in the future.)

> +    int64_t cpu_id;
> +
> +    assert(pm != NULL);
> +
> +    *gpe->sts = *gpe->sts | ICH9_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));
> +    }
> +
> +    ICH9_DEBUG("cpu_id: %"PRIx64", action: %s\n", cpu_id,
> +               action == PLUG ? "PLUG" : "UNPLUG");
> +
> +    pm_update_sci(pm);
> +}
> +
> +static void ich9_cpu_added_req(Notifier *n, void *opaque)
> +{
> +    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier);
> +
> +    ich9_cpu_hotplug_req(pm, CPU(opaque), PLUG);
> +}
> +
> +static void ich9_init_cpu_status(CPUState *cpu, void *data)
> +{
> +    CPUStatus *g = (CPUStatus *)data;
> +    CPUClass *k = CPU_GET_CLASS(cpu);

cc

> +    int64_t id = k->get_arch_id(cpu);
> +
> +    g_assert((id / 8) < ICH9_PROC_LEN);
> +    g->sts[id / 8] |= (1 << (id % 8));
> +}
> +
> +
>  static uint64_t ich9_smi_readl(void *opaque, hwaddr addr, unsigned width)
>  {
>      ICH9LPCPMRegs *pm = opaque;
> @@ -221,6 +300,12 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>                            "apci-gpe0", ICH9_PMIO_GPE0_LEN);
>      memory_region_add_subregion(&pm->io, ICH9_PMIO_GPE0_STS, &pm->io_gpe);
>  
> +    qemu_for_each_cpu(ich9_init_cpu_status, &pm->gpe_cpu);

Please don't copy this from i440fx, there are patches on the list
dropping qemu_for_each_cpu(). Just use a for loop until they hit master
- still waiting for reviews.

Regards,
Andreas

> +    memory_region_init_io(&pm->io_cpu, OBJECT(lpc_pci), &cpu_hotplug_ops, pm,
> +                          "acpi-cpu-hotplug", ICH9_PROC_LEN);
> +    memory_region_add_subregion(pci_address_space_io(lpc_pci), ICH9_PROC_BASE,
> +                                &pm->io_cpu);
> +
>      memory_region_init_io(&pm->io_smi, OBJECT(lpc_pci), &ich9_smi_ops, pm,
>                            "apci-smi", 8);
>      memory_region_add_subregion(&pm->io, ICH9_PMIO_SMI_EN, &pm->io_smi);
> @@ -229,4 +314,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>      qemu_register_reset(pm_reset, pm);
>      pm->powerdown_notifier.notify = pm_powerdown_req;
>      qemu_register_powerdown_notifier(&pm->powerdown_notifier);
> +    pm->cpu_added_notifier.notify = ich9_cpu_added_req;
> +    qemu_register_cpu_added_notifier(&pm->cpu_added_notifier);
>  }
> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index b1fe71f..bac70c6 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -23,6 +23,13 @@
>  
>  #include "hw/acpi/acpi.h"
>  
> +#define ICH9_PROC_BASE 0xaf00
> +#define ICH9_PROC_LEN 32
> +
> +typedef struct CPUStatus {
> +    uint8_t sts[ICH9_PROC_LEN];
> +} CPUStatus;
> +
>  typedef struct ICH9LPCPMRegs {
>      /*
>       * In ich9 spec says that pm1_cnt register is 32bit width and
> @@ -31,8 +38,11 @@ typedef struct ICH9LPCPMRegs {
>       */
>      ACPIREGS acpi_regs;
>  
> +    CPUStatus gpe_cpu;
> +
>      MemoryRegion io;
>      MemoryRegion io_gpe;
> +    MemoryRegion io_cpu;
>      MemoryRegion io_smi;
>  
>      uint32_t smi_en;
> @@ -42,6 +52,7 @@ typedef struct ICH9LPCPMRegs {
>  
>      uint32_t pm_io_base;
>      Notifier powerdown_notifier;
> +    Notifier cpu_added_notifier;
>  } ICH9LPCPMRegs;
>  
>  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>
Igor Mammedov Sept. 12, 2013, 3:29 p.m. UTC | #6
On Wed, 21 Aug 2013 17:04:28 +0800
Hu Tao <hutao@cn.fujitsu.com> wrote:

> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/acpi/ich9.c         | 91 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  include/hw/acpi/ich9.h | 11 ++++++
>  2 files changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 8717c15..146216a 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -43,17 +43,22 @@ do { printf("%s "fmt, __func__, ## __VA_ARGS__); } while (0)
>  #define ICH9_DEBUG(fmt, ...)    do { } while (0)
>  #endif
>  
> +
> +#define ICH9_CPU_HOTPLUG_STATUS 4
> +
>  static void pm_update_sci(ICH9LPCPMRegs *pm)
>  {
>      int sci_level, pm1a_sts;
>  
>      pm1a_sts = acpi_pm1_evt_get_sts(&pm->acpi_regs);
>  
> -    sci_level = (((pm1a_sts & pm->acpi_regs.pm1.evt.en) &
> +    sci_level = ((((pm1a_sts & pm->acpi_regs.pm1.evt.en) &
>                    (ACPI_BITMASK_RT_CLOCK_ENABLE |
>                     ACPI_BITMASK_POWER_BUTTON_ENABLE |
>                     ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
> -                   ACPI_BITMASK_TIMER_ENABLE)) != 0);
> +                   ACPI_BITMASK_TIMER_ENABLE)) != 0) ||
> +                 (((pm->acpi_regs.gpe.sts[0] & pm->acpi_regs.gpe.en[0]) &
> +          ICH9_CPU_HOTPLUG_STATUS) != 0));
>      qemu_set_irq(pm->irq, sci_level);
>  
>      /* schedule a timer interruption if needed */
> @@ -93,6 +98,80 @@ static const MemoryRegionOps ich9_gpe_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
> +static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    ICH9LPCPMRegs *pm = opaque;
> +    CPUStatus *cpus = &pm->gpe_cpu;
> +    uint64_t val = cpus->sts[addr];
> +
> +    ICH9_DEBUG("addr: %" HWADDR_PRIx ", val: %" PRIx64 "\n", addr, val);
> +
> +    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 ich9_cpu_hotplug_req(ICH9LPCPMRegs *pm, CPUState *cpu,
> +                                 HotplugEventType action)
> +{
> +    CPUStatus *g = &pm->gpe_cpu;
> +    ACPIGPE *gpe = &pm->acpi_regs.gpe;
> +    CPUClass *k = CPU_GET_CLASS(cpu);
> +    int64_t cpu_id;
> +
> +    assert(pm != NULL);
> +
> +    *gpe->sts = *gpe->sts | ICH9_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));
> +    }
> +
> +    ICH9_DEBUG("cpu_id: %"PRIx64", action: %s\n", cpu_id,
> +               action == PLUG ? "PLUG" : "UNPLUG");
> +
> +    pm_update_sci(pm);
> +}
> +
> +static void ich9_cpu_added_req(Notifier *n, void *opaque)
> +{
> +    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier);
> +
> +    ich9_cpu_hotplug_req(pm, CPU(opaque), PLUG);
> +}
> +
> +static void ich9_init_cpu_status(CPUState *cpu, void *data)
> +{
> +    CPUStatus *g = (CPUStatus *)data;
> +    CPUClass *k = CPU_GET_CLASS(cpu);
> +    int64_t id = k->get_arch_id(cpu);
> +
> +    g_assert((id / 8) < ICH9_PROC_LEN);
> +    g->sts[id / 8] |= (1 << (id % 8));
> +}
> +
> +
>  static uint64_t ich9_smi_readl(void *opaque, hwaddr addr, unsigned width)
>  {
>      ICH9LPCPMRegs *pm = opaque;
> @@ -221,6 +300,12 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>                            "apci-gpe0", ICH9_PMIO_GPE0_LEN);
>      memory_region_add_subregion(&pm->io, ICH9_PMIO_GPE0_STS, &pm->io_gpe);
>  
> +    qemu_for_each_cpu(ich9_init_cpu_status, &pm->gpe_cpu);
> +    memory_region_init_io(&pm->io_cpu, OBJECT(lpc_pci), &cpu_hotplug_ops, pm,
> +                          "acpi-cpu-hotplug", ICH9_PROC_LEN);
> +    memory_region_add_subregion(pci_address_space_io(lpc_pci), ICH9_PROC_BASE,
> +                                &pm->io_cpu);
> +
>      memory_region_init_io(&pm->io_smi, OBJECT(lpc_pci), &ich9_smi_ops, pm,
>                            "apci-smi", 8);
>      memory_region_add_subregion(&pm->io, ICH9_PMIO_SMI_EN, &pm->io_smi);
> @@ -229,4 +314,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>      qemu_register_reset(pm_reset, pm);
>      pm->powerdown_notifier.notify = pm_powerdown_req;
>      qemu_register_powerdown_notifier(&pm->powerdown_notifier);
> +    pm->cpu_added_notifier.notify = ich9_cpu_added_req;
> +    qemu_register_cpu_added_notifier(&pm->cpu_added_notifier);
>  }
> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index b1fe71f..bac70c6 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -23,6 +23,13 @@
>  
>  #include "hw/acpi/acpi.h"
>  
> +#define ICH9_PROC_BASE 0xaf00
> +#define ICH9_PROC_LEN 32
> +
> +typedef struct CPUStatus {
> +    uint8_t sts[ICH9_PROC_LEN];
> +} CPUStatus;
> +
>  typedef struct ICH9LPCPMRegs {
>      /*
>       * In ich9 spec says that pm1_cnt register is 32bit width and
> @@ -31,8 +38,11 @@ typedef struct ICH9LPCPMRegs {
>       */
>      ACPIREGS acpi_regs;
>  
> +    CPUStatus gpe_cpu;
> +
>      MemoryRegion io;
>      MemoryRegion io_gpe;
> +    MemoryRegion io_cpu;
>      MemoryRegion io_smi;
>  
>      uint32_t smi_en;
> @@ -42,6 +52,7 @@ typedef struct ICH9LPCPMRegs {
>  
>      uint32_t pm_io_base;
>      Notifier powerdown_notifier;
> +    Notifier cpu_added_notifier;
>  } ICH9LPCPMRegs;
>  
>  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,

new code mostly duplicates piix4 parts. Could we unify it and move it into separate file or into core.c?
Igor Mammedov Oct. 4, 2013, 1:20 p.m. UTC | #7
On Wed, 21 Aug 2013 13:01:32 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > +#define ICH9_PROC_BASE 0xaf00
> > +#define ICH9_PROC_LEN 32
> 
> No, please don't.  It makes it impossible to assign the 0xa000 -> 0xafff
> I/O port window to a PCI bridge.  Please lets stop occupy random io
> ports above 0x1000 and burn I/O address space that way.
I'm curios why 0x1000 is any better than 0xa000, it still would be random
port occupation. Is there any guideline which ports could be used and which
shouldn't?

Could we safely move PIIX CPU hotplug 0xaf00-0xaf20 range below 0x1000?

> 
> I'd suggest to place it at 0x0a00 instead.  Also the dsdt should get a
> device with the address in _CRS so the guest knows those ports are used.


> 
> cheers,
>   Gerd
> 
> 
> 
>
Gerd Hoffmann Oct. 7, 2013, 7:54 a.m. UTC | #8
On Fr, 2013-10-04 at 15:20 +0200, Igor Mammedov wrote:
> On Wed, 21 Aug 2013 13:01:32 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> >   Hi,
> > 
> > > +#define ICH9_PROC_BASE 0xaf00
> > > +#define ICH9_PROC_LEN 32
> > 
> > No, please don't.  It makes it impossible to assign the 0xa000 -> 0xafff
> > I/O port window to a PCI bridge.  Please lets stop occupy random io
> > ports above 0x1000 and burn I/O address space that way.
> I'm curios why 0x1000 is any better than 0xa000, it still would be random
> port occupation. Is there any guideline which ports could be used and which
> shouldn't?

0x1000 doesn't by us anything.  *below* 0x1000 gives us one more pci io
window which we can assign to a pci bridge then.

> Could we safely move PIIX CPU hotplug 0xaf00-0xaf20 range below 0x1000?

I'd love to, but unfortunaly it isn't that easy as the address is
hard-coded in the DSDT.

Once mst acpi generation patches are in it would be a bit simpler, but
we would still break compatibility with seabios versions not fetching
the acpi tables from qemu.

cheers,
  Gerd
diff mbox

Patch

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 8717c15..146216a 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -43,17 +43,22 @@  do { printf("%s "fmt, __func__, ## __VA_ARGS__); } while (0)
 #define ICH9_DEBUG(fmt, ...)    do { } while (0)
 #endif
 
+
+#define ICH9_CPU_HOTPLUG_STATUS 4
+
 static void pm_update_sci(ICH9LPCPMRegs *pm)
 {
     int sci_level, pm1a_sts;
 
     pm1a_sts = acpi_pm1_evt_get_sts(&pm->acpi_regs);
 
-    sci_level = (((pm1a_sts & pm->acpi_regs.pm1.evt.en) &
+    sci_level = ((((pm1a_sts & pm->acpi_regs.pm1.evt.en) &
                   (ACPI_BITMASK_RT_CLOCK_ENABLE |
                    ACPI_BITMASK_POWER_BUTTON_ENABLE |
                    ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
-                   ACPI_BITMASK_TIMER_ENABLE)) != 0);
+                   ACPI_BITMASK_TIMER_ENABLE)) != 0) ||
+                 (((pm->acpi_regs.gpe.sts[0] & pm->acpi_regs.gpe.en[0]) &
+          ICH9_CPU_HOTPLUG_STATUS) != 0));
     qemu_set_irq(pm->irq, sci_level);
 
     /* schedule a timer interruption if needed */
@@ -93,6 +98,80 @@  static const MemoryRegionOps ich9_gpe_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    ICH9LPCPMRegs *pm = opaque;
+    CPUStatus *cpus = &pm->gpe_cpu;
+    uint64_t val = cpus->sts[addr];
+
+    ICH9_DEBUG("addr: %" HWADDR_PRIx ", val: %" PRIx64 "\n", addr, val);
+
+    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 ich9_cpu_hotplug_req(ICH9LPCPMRegs *pm, CPUState *cpu,
+                                 HotplugEventType action)
+{
+    CPUStatus *g = &pm->gpe_cpu;
+    ACPIGPE *gpe = &pm->acpi_regs.gpe;
+    CPUClass *k = CPU_GET_CLASS(cpu);
+    int64_t cpu_id;
+
+    assert(pm != NULL);
+
+    *gpe->sts = *gpe->sts | ICH9_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));
+    }
+
+    ICH9_DEBUG("cpu_id: %"PRIx64", action: %s\n", cpu_id,
+               action == PLUG ? "PLUG" : "UNPLUG");
+
+    pm_update_sci(pm);
+}
+
+static void ich9_cpu_added_req(Notifier *n, void *opaque)
+{
+    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier);
+
+    ich9_cpu_hotplug_req(pm, CPU(opaque), PLUG);
+}
+
+static void ich9_init_cpu_status(CPUState *cpu, void *data)
+{
+    CPUStatus *g = (CPUStatus *)data;
+    CPUClass *k = CPU_GET_CLASS(cpu);
+    int64_t id = k->get_arch_id(cpu);
+
+    g_assert((id / 8) < ICH9_PROC_LEN);
+    g->sts[id / 8] |= (1 << (id % 8));
+}
+
+
 static uint64_t ich9_smi_readl(void *opaque, hwaddr addr, unsigned width)
 {
     ICH9LPCPMRegs *pm = opaque;
@@ -221,6 +300,12 @@  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
                           "apci-gpe0", ICH9_PMIO_GPE0_LEN);
     memory_region_add_subregion(&pm->io, ICH9_PMIO_GPE0_STS, &pm->io_gpe);
 
+    qemu_for_each_cpu(ich9_init_cpu_status, &pm->gpe_cpu);
+    memory_region_init_io(&pm->io_cpu, OBJECT(lpc_pci), &cpu_hotplug_ops, pm,
+                          "acpi-cpu-hotplug", ICH9_PROC_LEN);
+    memory_region_add_subregion(pci_address_space_io(lpc_pci), ICH9_PROC_BASE,
+                                &pm->io_cpu);
+
     memory_region_init_io(&pm->io_smi, OBJECT(lpc_pci), &ich9_smi_ops, pm,
                           "apci-smi", 8);
     memory_region_add_subregion(&pm->io, ICH9_PMIO_SMI_EN, &pm->io_smi);
@@ -229,4 +314,6 @@  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
     qemu_register_reset(pm_reset, pm);
     pm->powerdown_notifier.notify = pm_powerdown_req;
     qemu_register_powerdown_notifier(&pm->powerdown_notifier);
+    pm->cpu_added_notifier.notify = ich9_cpu_added_req;
+    qemu_register_cpu_added_notifier(&pm->cpu_added_notifier);
 }
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index b1fe71f..bac70c6 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -23,6 +23,13 @@ 
 
 #include "hw/acpi/acpi.h"
 
+#define ICH9_PROC_BASE 0xaf00
+#define ICH9_PROC_LEN 32
+
+typedef struct CPUStatus {
+    uint8_t sts[ICH9_PROC_LEN];
+} CPUStatus;
+
 typedef struct ICH9LPCPMRegs {
     /*
      * In ich9 spec says that pm1_cnt register is 32bit width and
@@ -31,8 +38,11 @@  typedef struct ICH9LPCPMRegs {
      */
     ACPIREGS acpi_regs;
 
+    CPUStatus gpe_cpu;
+
     MemoryRegion io;
     MemoryRegion io_gpe;
+    MemoryRegion io_cpu;
     MemoryRegion io_smi;
 
     uint32_t smi_en;
@@ -42,6 +52,7 @@  typedef struct ICH9LPCPMRegs {
 
     uint32_t pm_io_base;
     Notifier powerdown_notifier;
+    Notifier cpu_added_notifier;
 } ICH9LPCPMRegs;
 
 void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,