Re: [Qemu-devel] [PATCH 2/2] q35: add cpu hotplug support

[Igor Mammedov]

On Wed, 21 Aug 2013 17:04:28 +0800
Hu Tao <address@hidden> wrote:

> 
> Signed-off-by: Hu Tao <address@hidden>
> ---
>  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?