Re: [PATCH 1/4] HVM vcpu add/remove: qemu logic for vcpu add/revmoe

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Fri, 31 May 2013, Anthony PERARD wrote:
> From: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> -- at qemu side, get vcpu_avail which used for original cpu avail map;
> -- setup gpe ioread/iowrite at qmeu;
> -- setup vcpu add/remove user interface through monitor;
> -- setup SCI logic;
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> 
> [ PATCH 4/4 ] HVM vcpu add/remove: qemu logic for vcpu add/revmoe
> 
> Port from qemu-xen-traditionnal to qemu-xen.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  hmp-commands.hx |  9 +++++++
>  hw/acpi_piix4.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  monitor.c       | 18 ++++++++++++++
>  qemu-options.hx |  3 +++
>  sysemu.h        |  3 +++
>  vl.c            |  6 +++++
>  6 files changed, 113 insertions(+), 1 deletion(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 010b8c9..e92f173 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1581,3 +1581,12 @@ ETEXI
>  STEXI
>  @end table
>  ETEXI
> +
> +HXCOMM TODO doc
> +    {
> +        .name       = "cpu_set",
> +        .args_type  = "cpu_index:i,status:s",
> +        .params     = "cpu [online|offline]",
> +        .help       = "change cpu state",
> +        .mhandler.cmd = do_cpu_set_nr,
> +    },
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 519269a..c73dc7c 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -45,8 +45,11 @@
>  #define PCI_DOWN_BASE 0xae04
>  #define PCI_EJ_BASE 0xae08
>  #define PCI_RMV_BASE 0xae0c
> +/* ioport to monitor cpu add/remove status */
> +#define PROC_BASE 0xaf00
>  
>  #define PIIX4_PCI_HOTPLUG_STATUS 2
> +#define PIIX4_CPU_HOTPLUG_STATUS 4
>  
>  struct pci_status {
>      uint32_t up; /* deprecated, maintained for migration compatibility */
> @@ -77,6 +80,9 @@ typedef struct PIIX4PMState {
>      uint8_t disable_s3;
>      uint8_t disable_s4;
>      uint8_t s4_val;
> +
> +    /* CPU bitmap */
> +    uint8_t cpus_sts[32];
>  } PIIX4PMState;
>  
>  static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
> @@ -95,7 +101,7 @@ static void pm_update_sci(PIIX4PMState *s)
>                     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);
> +          & (PIIX4_PCI_HOTPLUG_STATUS|PIIX4_CPU_HOTPLUG_STATUS)) != 0);
>  
>      qemu_set_irq(s->irq, sci_level);
>      /* schedule a timer interruption if needed */
> @@ -602,11 +608,48 @@ static uint32_t pcirmv_read(void *opaque, uint32_t addr)
>      return s->pci0_hotplug_enable;
>  }
>  
> +static uint32_t gpe_cpus_readb(void *opaque, uint32_t addr)
> +{
> +    uint32_t val = 0;
> +    PIIX4PMState *g = opaque;
> +
> +    switch (addr) {
> +        case PROC_BASE ... PROC_BASE+31:
> +            val = g->cpus_sts[addr - PROC_BASE];
> +        default:
> +            break;
> +    }
> +
> +    return val;
> +}
> +
> +static void gpe_cpus_writeb(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    switch (addr) {
> +        case PROC_BASE ... PROC_BASE + 31:
> +            /* don't allow to change cpus_sts from inside a guest */
> +            break;
> +        default:
> +            break;
> +    }
> +}
> +
>  static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
>                                  PCIHotplugState state);
>  
> +extern uint64_t vcpu_avail;
> +static PIIX4PMState *acpi_state;
>  static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
>  {
> +    int i = 0, cpus = max_cpus;
> +    char *vcpumap = (char *)&vcpu_avail;
> +
> +    while (cpus > 0) {
> +        s->cpus_sts[i] = vcpumap[i];
> +        i++;
> +        cpus -= 8;
> +    }
> +    acpi_state = s;
>  
>      register_ioport_write(GPE_BASE, GPE_LEN, 1, gpe_writeb, s);
>      register_ioport_read(GPE_BASE, GPE_LEN, 1,  gpe_readb, s);
> @@ -620,6 +663,9 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
>  
>      register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
>  
> +    register_ioport_read(PROC_BASE, 32, 1,  gpe_cpus_readb, s);
> +    register_ioport_write(PROC_BASE, 32, 1, gpe_cpus_writeb, s);
> +
>      pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
>  }
>  
> @@ -660,3 +706,30 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
>  
>      return 0;
>  }
> +
> +static void enable_processor(PIIX4PMState *g, int cpu)
> +{
> +    g->ar.gpe.sts[0] |= 4;
> +    g->cpus_sts[cpu/8] |= (1 << (cpu%8));
> +}
> +
> +static void disable_processor(PIIX4PMState *g, int cpu)
> +{
> +    g->ar.gpe.sts[0] |= 4;
> +    g->cpus_sts[cpu/8] &= ~(1 << (cpu%8));
> +}
> +
> +void qemu_cpu_add_remove(int cpu, int state)
> +{
> +    if ((cpu <=0) || (cpu >= max_cpus)) {

the original code has cpu < 0

> +        fprintf(stderr, "vcpu out of range, should be [1~%d]\n", max_cpus - 1);
> +        return;
> +    }
> +
> +    if (state)
> +        enable_processor(acpi_state, cpu);
> +    else
> +        disable_processor(acpi_state, cpu);
> +
> +    pm_update_sci(acpi_state);
> +}

This is also different, the code we have on qemu-xen-unstable is:

if (gpe_state.gpe0_en[0] & 4) {
        qemu_set_irq(sci_irq, 1);
        qemu_set_irq(sci_irq, 0);
}

what's the reason for the change?


If possible, it would be better to port a QMP command for vcpu hotplug
to avoid compatibility problems, so I would get rid of the code below


> diff --git a/monitor.c b/monitor.c
> index c0e32d6..564373c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2421,6 +2421,24 @@ int monitor_handle_fd_param(Monitor *mon, const char *fdname)
>      return fd;
>  }
>  
> +static void do_cpu_set_nr(Monitor *mon, const QDict *qdict)
> +{
> +    int cpu_index = qdict_get_int(qdict, "cpu_index");
> +    const char *status = qdict_get_str(qdict, "status");
> +    int state;
> +
> +    if (!strcmp(status, "online")) {
> +        state = 1;
> +    } else if (!strcmp(status, "offline")) {
> +        state = 0;
> +    } else {
> +        monitor_printf(mon, "invalid status: %s\n", status);
> +        return;
> +    }
> +
> +    qemu_cpu_add_remove(cpu_index, state);
> +}
> +
>  /* mon_cmds and info_cmds would be sorted at runtime */
>  static mon_cmd_t mon_cmds[] = {
>  #include "hmp-commands.h"
> diff --git a/qemu-options.hx b/qemu-options.hx
> index de43b1b..0ba668f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -94,6 +94,9 @@ given, the total number of CPUs @var{n} can be omitted. @var{maxcpus}
>  specifies the maximum number of hotpluggable CPUs.
>  ETEXI
>  
> +DEF("vcpu_avail", HAS_ARG, QEMU_OPTION_vcpu_avail,
> +    "-vcpu_avail bitmap\n", QEMU_ARCH_ALL)
> +
>  DEF("numa", HAS_ARG, QEMU_OPTION_numa,
>      "-numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
>  STEXI
> diff --git a/sysemu.h b/sysemu.h
> index f5ac664..ed7b264 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -149,6 +149,9 @@ int pci_drive_hot_add(Monitor *mon, const QDict *qdict,
>                        DriveInfo *dinfo, int type);
>  void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
>  
> +/* cpu hotplug */
> +void qemu_cpu_add_remove(int cpu, int state);
> +
>  /* generic hotplug */
>  void drive_hot_add(Monitor *mon, const QDict *qdict);
>  
> diff --git a/vl.c b/vl.c
> index a3ab384..27ae6c1 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -207,6 +207,7 @@ int win2k_install_hack = 0;
>  int singlestep = 0;
>  int smp_cpus = 1;
>  int max_cpus = 0;
> +uint64_t vcpu_avail = 1;
>  int smp_cores = 1;
>  int smp_threads = 1;
>  #ifdef CONFIG_VNC
> @@ -3302,6 +3303,11 @@ int main(int argc, char **argv, char **envp)
>                      exit(1);
>                  }
>                  break;
> +            case QEMU_OPTION_vcpu_avail:
> +                vcpu_avail = atol(optarg);
> +                fprintf(stderr, "qemu: the avail cpu bitmap is %lx\n",
> +                        vcpu_avail);
> +                break;
>  	    case QEMU_OPTION_vnc:
>  #ifdef CONFIG_VNC
>                  display_remote++;