[Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Jens Freimann]

Hi all,

this is what our approach to CPU hotplug looks like.
With respect to Igor's CPU hotplug series, how should we proceed? 
Should we change the interface to 
qemu_system_cpu_add_notifier/qemu_system_cpu_hotplug_request/cpu-add etc?

Feedback regarding the non-API part is also highly welcome!

regards
Jens

Thang Pham (1):
  s390: implement CPU hotplug

 hmp-commands.hx           |  14 +++++
 hw/s390x/Makefile.objs    |   2 +-
 hw/s390x/event-facility.c |   9 ++++
 hw/s390x/event-facility.h |   3 ++
 hw/s390x/s390-virtio.c    |  34 +++++++++---
 hw/s390x/sclp.c           | 134 +++++++++++++++++++++++++++++++++++++++++++++-
 hw/s390x/sclp.h           |  47 +++++++++++++++-
 include/sysemu/sysemu.h   |   1 +
 monitor.c                 |  31 +++++++++++
 target-s390x/cpu.c        |  59 ++++++++++++++++++++
 target-s390x/cpu.h        |   7 +++
 target-s390x/helper.c     |  43 +++++++++++++++
 vl.c                      |   6 +++
 13 files changed, 379 insertions(+), 11 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [RFC/PATCH 1/1] s390: implement CPU hotplug

[Jens Freimann]

From: Thang Pham <thang.pham@us.ibm.com>

This patch allows CPUs to be hotplugged to a running guest. The QEMU
option (maxcpus) can be used to define the total number of standby
and configured CPUs  available at boot time, although it is not
necessary because you can hotplug standby CPUs after boot time. For
example, the QEMU command-line uses: -smp 3,maxcpus=5. This
brings up 3 configured CPUs and 2 standby CPUs. More standby CPUs can
be attached using the cpu_set command from the monitor. The maximum
number of CPUs that can be attached to an s390x guest is 64; this is
because the Linux kernel restricts it.

Note that for s390x, a standby CPU is one that has an underlying
thread that is halted. When a standby CPU is attached to a KVM guest,
the CPU sysfs for that CPU is created as well as the underlying
thread. Only the guest can bring a CPU from standby to configured
state, and vice versa. Also, only the guest can bring a CPU online
and offline. It is done this way to be architecture compliant.

Signed-off-by: Thang Pham <thang.pham@us.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 hmp-commands.hx           |  14 +++++
 hw/s390x/Makefile.objs    |   2 +-
 hw/s390x/event-facility.c |   9 ++++
 hw/s390x/event-facility.h |   3 ++
 hw/s390x/s390-virtio.c    |  34 +++++++++---
 hw/s390x/sclp.c           | 134 +++++++++++++++++++++++++++++++++++++++++++++-
 hw/s390x/sclp.h           |  47 +++++++++++++++-
 include/sysemu/sysemu.h   |   1 +
 monitor.c                 |  31 +++++++++++
 target-s390x/cpu.c        |  59 ++++++++++++++++++++
 target-s390x/cpu.h        |   7 +++
 target-s390x/helper.c     |  43 +++++++++++++++
 vl.c                      |   6 +++
 13 files changed, 379 insertions(+), 11 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 3d98604..c2b9943 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1555,6 +1555,20 @@ Removes the chardev @var{id}.
 ETEXI
 
     {
+        .name       = "cpu_set",
+        .args_type  = "cpu:i,state:s",
+        .params     = "cpu [online|offline]",
+        .help       = "change cpu state",
+        .mhandler.cmd  = do_cpu_set_nr,
+    },
+
+STEXI
+@item cpu_set @var{cpu} [online|offline]
+Set CPU @var{cpu} online or offline.
+
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 9f2f419..24a170e 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -2,7 +2,7 @@ obj-y = s390-virtio-bus.o s390-virtio.o
 obj-y += s390-virtio-hcall.o
 obj-y += sclp.o
 obj-y += event-facility.o
-obj-y += sclpquiesce.o sclpconsole.o
+obj-y += sclpquiesce.o sclpconsole.o sclpcpu.o
 obj-y += ipl.o
 obj-y += css.o
 obj-y += s390-virtio-ccw.o
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 0faade0..4714bc2 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -317,6 +317,7 @@ static int init_event_facility(S390SCLPDevice *sdev)
 {
     SCLPEventFacility *event_facility;
     DeviceState *quiesce;
+    DeviceState *cpu_hotplug;
 
     event_facility = g_malloc0(sizeof(SCLPEventFacility));
     sdev->ef = event_facility;
@@ -335,6 +336,14 @@ static int init_event_facility(S390SCLPDevice *sdev)
     }
     qdev_init_nofail(quiesce);
 
+    /* Register for CPU hotplug */
+    cpu_hotplug = qdev_create(&event_facility->sbus.qbus,
+            "sclpcpuhotplug");
+    if (!cpu_hotplug) {
+        return -1;
+    }
+    qdev_init_nofail(cpu_hotplug);
+
     return 0;
 }
 
diff --git a/hw/s390x/event-facility.h b/hw/s390x/event-facility.h
index 791ab2a..588b211 100644
--- a/hw/s390x/event-facility.h
+++ b/hw/s390x/event-facility.h
@@ -17,14 +17,17 @@
 
 #include <hw/qdev.h>
 #include "qemu/thread.h"
+#include "hw/s390x/sclp.h"
 
 /* SCLP event types */
 #define SCLP_EVENT_ASCII_CONSOLE_DATA           0x1a
 #define SCLP_EVENT_SIGNAL_QUIESCE               0x1d
+#define SCLP_EVENT_CONFIG_MGT_DATA              0x04
 
 /* SCLP event masks */
 #define SCLP_EVENT_MASK_SIGNAL_QUIESCE          0x00000008
 #define SCLP_EVENT_MASK_MSG_ASCII               0x00000040
+#define SCLP_EVENT_MASK_CONFIG_MGT_DATA         0x10000000
 
 #define SCLP_UNCONDITIONAL_READ                 0x00
 #define SCLP_SELECTIVE_READ                     0x01
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index ca275bd..6a7c613 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -55,13 +55,15 @@ static S390CPU **ipi_states;
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
-    if (cpu_addr >= smp_cpus) {
-        return NULL;
-    }
-
     return ipi_states[cpu_addr];
 }
 
+void s390_cpu_add_state(uint16_t cpu_addr, S390CPU *state)
+{
+    /* Add S390CPU to ipi_states */
+    ipi_states[cpu_addr] = state;
+}
+
 static int s390_virtio_hcall_notify(const uint64_t *args)
 {
     uint64_t mem = args[0];
@@ -176,14 +178,27 @@ void s390_init_ipl_dev(const char *kernel_filename,
 void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
 {
     int i;
+    int cpu_count = smp_cpus + standby_cpus;
+    int max_vcpus = smp_cpus + standby_cpus;
+#ifdef CONFIG_KVM
+    max_vcpus = kvm_check_extension(kvm_state, KVM_CAP_MAX_VCPUS);
+#endif
 
     if (cpu_model == NULL) {
         cpu_model = "host";
     }
 
-    ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
-
-    for (i = 0; i < smp_cpus; i++) {
+    /*
+     * The maximum # of configurable CPU is 64, which is limited by Linux
+     * kernel. Pre-allocate 64 S390CPU entries in ipi_states.
+     */
+    ipi_states = g_malloc(sizeof(S390CPU *) * max_vcpus);
+
+    /*
+     * Initialize S390CPU entries in ipi_states for standby CPUs.
+     * This will create an underlying thread (halted) for each standby CPU.
+     */
+    for (i = 0; i < cpu_count; i++) {
         S390CPU *cpu;
         CPUState *cs;
 
@@ -195,6 +210,11 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
         cpu->env.exception_index = EXCP_HLT;
         cpu->env.storage_keys = storage_keys;
     }
+
+    /* Initialize the rest of ipi_states */
+    for (i = cpu_count; i < max_vcpus; i++) {
+        ipi_states[i] = NULL;
+    }
 }
 
 
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 86d6ae0..303d210 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -15,6 +15,9 @@
 #include "cpu.h"
 #include "sysemu/kvm.h"
 #include "exec/memory.h"
+#include "sysemu/sysemu.h"
+#include "event-facility.h"
+#include "sclp.h"
 
 #include "hw/s390x/sclp.h"
 
@@ -31,25 +34,152 @@ static inline S390SCLPDevice *get_event_facility(void)
 static void read_SCP_info(SCCB *sccb)
 {
     ReadInfo *read_info = (ReadInfo *) sccb;
+    int cpu_count = standby_cpus + smp_cpus;
+    int i = 0;
+    int max_vcpus = standby_cpus + smp_cpus;
     int shift = 0;
-
+#ifdef CONFIG_KVM
+    max_vcpus = kvm_check_extension(kvm_state, KVM_CAP_MAX_VCPUS);
+#endif
     while ((ram_size >> (20 + shift)) > 65535) {
         shift++;
     }
     read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
     read_info->rnsize = 1 << shift;
+
+    /* CPU information */
+    read_info->entries_cpu = cpu_to_be16(standby_cpus + smp_cpus);
+    read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
+    read_info->highest_cpu = cpu_to_be16(max_vcpus);
+
+    /*
+     * Insert a valid 16-byte entry for each CPU in each the
+     * list (configured & standby)
+     */
+    for (i = 0; i < cpu_count; i++) {
+        read_info->entries[i].address = i;
+        read_info->entries[i].type = 0; /* General purpose CPU */
+    }
+
+    read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
+    sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
+}
+
+/* Provide information about the CPU */
+static void sclp_read_cpu_info(SCCB *sccb)
+{
+    ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
+    int cpu_count = standby_cpus + smp_cpus;
+    int i = 0;
+
+    cpu_info->nr_configured = cpu_to_be16(smp_cpus);
+    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
+    cpu_info->nr_standby = cpu_to_be16(standby_cpus);
+
+    /* The standby offset is 16-byte for each CPU */
+    cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
+        + cpu_info->nr_configured*sizeof(CpuEntry));
+
+    /*
+     * Insert a valid 16-byte entry for each CPU in each the
+     * list (configured & standby)
+     */
+    for (i = 0; i < cpu_count; i++) {
+        cpu_info->entries[i].address = i;
+        cpu_info->entries[i].type = 0; /* General purpose CPU */
+    }
+
     sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
 }
 
+static void sclp_configure_cpu(SCCB *sccb, uint16_t cpu_addr)
+{
+    /* Create underlying CPU thread */
+    S390CPU *target_cpu = s390_cpu_addr2state(cpu_addr);
+
+    if (!target_cpu) {
+        /* Create new vCPU thread and associate it with the CPU address */
+        target_cpu = configure_cpu(cpu_addr);
+    }
+
+    /*
+     * Bring CPU from standby to configure state. Increment configure CPU count
+     * and decrement standby CPU count.
+     */
+    smp_cpus++;
+    if (standby_cpus) {
+        standby_cpus--;
+    }
+
+    /* CPU hotplug requires SCCB response code */
+    sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+}
+
+static void sclp_deconfigure_cpu(SCCB *sccb, uint16_t cpu_addr)
+{
+    /*
+     * Bring configured CPU to standby state.
+     * Underlying CPU thread should be halted.
+     */
+    S390CPU *target_cpu = s390_cpu_addr2state(cpu_addr);
+    CPUState *target_cpu_state = CPU(target_cpu);
+    CPUS390XState *target_env;
+
+    if (target_cpu) {
+        target_env = &target_cpu->env;
+
+        /* Halt CPU */
+        if (target_cpu_state->halted != 1) {
+            fprintf(stderr, "CPU %d must be off-lined first before it can be "
+                        "de-configured\n", cpu_addr);
+            sccb->h.response_code =
+                    cpu_to_be16(SCLP_RC_IMPROPER_RESOURCE_STATE);
+            return;
+        }
+
+        /* Do not de-configure the last configured CPU */
+        if (smp_cpus == 1) {
+            fprintf(stderr, "At least one CPU must be running. CPU %d cannot "
+                        "be de-configured\n", cpu_addr);
+            sccb->h.response_code = cpu_to_be16(SCLP_RC_REQUIRED_RESOURCE);
+            return;
+        }
+
+        standby_cpus++;
+        smp_cpus--;
+
+        qemu_cpu_kick(ENV_GET_CPU(target_env));
+
+        /* CPU hotplug done on guest requires SCCB response code*/
+        sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+    } else {
+        /* CPU was not created if target_cpu is NULL */
+        fprintf(stderr, "CPU %d does not exist\n", cpu_addr);
+        sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_RESOURCE_ID);
+    }
+}
+
 static void sclp_execute(SCCB *sccb, uint64_t code)
 {
     S390SCLPDevice *sdev = get_event_facility();
 
-    switch (code) {
+    switch (code & SCLP_NO_CMD_PARM) {
     case SCLP_CMDW_READ_SCP_INFO:
     case SCLP_CMDW_READ_SCP_INFO_FORCED:
         read_SCP_info(sccb);
         break;
+    case SCLP_CMDW_READ_CPU_INFO:
+        sclp_read_cpu_info(sccb);
+        break;
+    case SCLP_CMDW_CONFIGURE_CPU:
+        sclp_configure_cpu(sccb,
+                (uint16_t) (code & SCLP_CMDW_CPU_CMD_PARM) >> 8);
+        break;
+    case SCLP_CMDW_DECONFIGURE_CPU:
+        /* Obtain CPU address from code: (uint16_t) (code & 0xff00) >> 8 */
+        sclp_deconfigure_cpu(sccb,
+                (uint16_t) (code & SCLP_CMDW_CPU_CMD_PARM) >> 8);
+        break;
     default:
         sdev->sclp_command_handler(sdev->ef, sccb, code);
         break;
diff --git a/hw/s390x/sclp.h b/hw/s390x/sclp.h
index 231a38a..1757b74 100644
--- a/hw/s390x/sclp.h
+++ b/hw/s390x/sclp.h
@@ -26,6 +26,14 @@
 #define SCLP_CMD_WRITE_EVENT_DATA               0x00760005
 #define SCLP_CMD_WRITE_EVENT_MASK               0x00780005
 
+/* CPU hotplug SCLP codes */
+#define SCLP_NO_CMD_PARM                        0xffff00ff
+#define SCLP_HAS_CPU_INFO                       0x0C00000000000000ULL
+#define SCLP_CMDW_READ_CPU_INFO                 0x00010001
+#define SCLP_CMDW_CONFIGURE_CPU                 0x00110001
+#define SCLP_CMDW_DECONFIGURE_CPU               0x00100001
+#define SCLP_CMDW_CPU_CMD_PARM                  0xff00
+
 /* SCLP response codes */
 #define SCLP_RC_NORMAL_READ_COMPLETION          0x0010
 #define SCLP_RC_NORMAL_COMPLETION               0x0020
@@ -38,6 +46,9 @@
 #define SCLP_RC_INCONSISTENT_LENGTHS            0x72f0
 #define SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR       0x73f0
 #define SCLP_RC_INVALID_MASK_LENGTH             0x74f0
+#define SCLP_RC_INVALID_RESOURCE_ID             0x03f0
+#define SCLP_RC_IMPROPER_RESOURCE_STATE         0x05f0
+#define SCLP_RC_REQUIRED_RESOURCE               0x0af0
 
 
 /* Service Call Control Block (SCCB) and its elements */
@@ -71,12 +82,44 @@ typedef struct SCCBHeader {
 
 #define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader))
 
+/* CPU information */
+typedef struct CpuEntry {
+    uint8_t address;
+    uint8_t reserved0[13];
+    uint8_t type;
+    uint8_t reserved1;
+} QEMU_PACKED CpuEntry;
+
 typedef struct ReadInfo {
     SCCBHeader h;
     uint16_t rnmax;
-    uint8_t rnsize;
+    uint8_t  rnsize;
+    uint8_t  _reserved1[16 - 11];       /* 11-15 */
+    uint16_t entries_cpu;               /* 16-17 */
+    uint16_t offset_cpu;                /* 18-19 */
+    uint8_t  _reserved2[24 - 20];       /* 20-23 */
+    uint8_t  loadparm[8];               /* 24-31 */
+    uint8_t  _reserved3[48 - 32];       /* 32-47 */
+    uint64_t facilities;                /* 48-55 */
+    uint8_t  _reserved0[100 - 56];
+    uint32_t rnsize2;
+    uint64_t rnmax2;
+    uint8_t  _reserved4[120-112];       /* 112-119 */
+    uint16_t highest_cpu;
+    uint8_t  _reserved5[128 - 122];     /* 122-127 */
+    struct CpuEntry entries[0];
 } QEMU_PACKED ReadInfo;
 
+typedef struct ReadCpuInfo {
+    SCCBHeader h;
+    uint16_t nr_configured;         /* 8-9 */
+    uint16_t offset_configured;     /* 10-11 */
+    uint16_t nr_standby;            /* 12-13 */
+    uint16_t offset_standby;        /* 14-15 */
+    uint8_t reserved0[24-16];       /* 16-23 */
+    struct CpuEntry entries[0];
+} QEMU_PACKED ReadCpuInfo;
+
 typedef struct SCCB {
     SCCBHeader h;
     char data[SCCB_DATA_LEN];
@@ -112,6 +155,8 @@ typedef struct S390SCLPDeviceClass {
     int (*init)(S390SCLPDevice *sdev);
 } S390SCLPDeviceClass;
 
+void raise_irq_cpu_hotplug(void);
+S390CPU *configure_cpu(uint16_t cpu_addr);
 void s390_sclp_init(void);
 void sclp_service_interrupt(uint32_t sccb);
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6578782..e268eb4 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -114,6 +114,7 @@ extern int alt_grab;
 extern int ctrl_grab;
 extern int smp_cpus;
 extern int max_cpus;
+extern int standby_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
diff --git a/monitor.c b/monitor.c
index 4ec1db9..7aec5a7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -69,6 +69,11 @@
 #include "hmp.h"
 #include "qemu/thread.h"
 
+/* For s390x */
+#if defined(TARGET_S390X)
+#include "hw/s390x/sclp.h"
+#endif
+
 /* for pic/irq_info */
 #if defined(TARGET_SPARC)
 #include "hw/sun4m.h"
@@ -2440,6 +2445,32 @@ int monitor_handle_fd_param(Monitor *mon, const char *fdname)
     return fd;
 }
 
+static void do_cpu_set_nr(Monitor *mon, const QDict *qdict)
+{
+    /* CPU state: online or offline */
+    const char *status = qdict_get_str(qdict, "state");
+#if defined(TARGET_S390X)
+    int state;
+    int value = qdict_get_int(qdict, "cpu"); /* CPU address */
+
+    if (!strcmp(status, "online")) {
+        state = 1;
+    } else if (!strcmp(status, "offline")) {
+        state = 0;
+    } else {
+        monitor_printf(mon, "invalid status: %s\n", status);
+        return;
+    }
+
+    qemu_system_cpu_hot_add(value, state);
+#else
+    if (strcmp(status, "online") && strcmp(status, "offline")) {
+        monitor_printf(mon, "invalid status: %s\n", status);
+        return;
+    }
+#endif
+}
+
 /* Please update hmp-commands.hx when adding or changing commands */
 static mon_cmd_t info_cmds[] = {
     {
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 23fe51f..71afe6e 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -28,12 +28,71 @@
 #include "qemu/timer.h"
 #include "hw/hw.h"
 #ifndef CONFIG_USER_ONLY
+#include "hw/s390x/sclp.h"
 #include "sysemu/arch_init.h"
+#include "sysemu/sysemu.h"
 #endif
 
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
 
+#ifndef CONFIG_USER_ONLY
+
+void qemu_system_cpu_hot_add(int cpu, int state)
+{
+    int max_vcpus = smp_cpus + standby_cpus;
+    int next_cpu = smp_cpus + standby_cpus;
+#ifdef CONFIG_KVM
+    max_vcpus = kvm_check_extension(kvm_state, KVM_CAP_MAX_VCPUS);
+#endif
+
+    /*
+     * Do not continue if CPU is greater the maximum # of configurable CPUs
+     */
+    if (cpu >= max_vcpus) {
+        fprintf(stderr, "CPU address is greater than the maximum number "
+                    "of configurable CPUs\n");
+        return;
+    }
+
+    /*
+     * Monitor cannot bring a CPU from configured to standby state. Only
+     * the guest can do this.
+     */
+    if (!state) {
+        fprintf(stderr, "CPU %d cannot be de-configured from the monitor\n",
+                    cpu);
+        return;
+    }
+
+    /*
+     * Find out next CPU address that could be attached.
+     * Only standby CPUs can be added by the monitor and it must be in
+     * sequential order
+     */
+    if (next_cpu >= max_vcpus) {
+        fprintf(stderr, "The maximum number of configurable CPUs has been "
+                    "reached\n");
+        return;
+    }
+
+    if (cpu != next_cpu) {
+        fprintf(stderr, "The next standby CPU that can be hotplugged is "
+                    "CPU %d\n", next_cpu);
+        return;
+    }
+
+    /* Configure standby CPU */
+    configure_cpu((uint16_t) cpu);
+
+    /* Configure is invoked from monitor. Increment standby CPU count. */
+    standby_cpus++;
+
+    /* Trigger SCLP interrupt */
+    raise_irq_cpu_hotplug();
+}
+#endif
+
 /* generate CPU information for cpu -? */
 void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index e351005..12bea46 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -316,6 +316,12 @@ S390CPU *cpu_s390x_init(const char *cpu_model);
 void s390x_translate_init(void);
 int cpu_s390x_exec(CPUS390XState *s);
 
+#ifndef CONFIG_USER_ONLY
+/* CPU hotplug support on s390 */
+void qemu_system_cpu_hot_add(int cpu, int state);
+S390CPU *s390x_cpu_hotplug(int cpu_addr);
+#endif
+
 /* you can call this signal handler from your SIGBUS and SIGSEGV
    signal handlers to inform the virtual CPU of exceptions. non zero
    is returned if the signal was handled by the virtual CPU.  */
@@ -373,6 +379,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU *cpu, int type,
 }
 #endif
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
+void s390_cpu_add_state(uint16_t cpu_addr, S390CPU *state);
 void s390_add_running_cpu(S390CPU *cpu);
 unsigned s390_del_running_cpu(S390CPU *cpu);
 
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index b425054..c159c78 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -51,6 +51,8 @@
 #endif
 
 #ifndef CONFIG_USER_ONLY
+static int last_hotplug_cpu; /* Track which CPU was last hotplugged */
+
 void s390x_tod_timer(void *opaque)
 {
     S390CPU *cpu = opaque;
@@ -68,6 +70,47 @@ void s390x_cpu_timer(void *opaque)
     env->pending_int |= INTERRUPT_CPUTIMER;
     cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
 }
+
+S390CPU *s390x_cpu_hotplug(int cpu_addr)
+{
+    const char *cpu_model = "host";
+    S390CPU *new_cpu;
+    CPUS390XState *new_env;
+    CPUState *new_cs;
+    S390CPU *conf_cpu;
+    CPUS390XState *conf_env;
+
+    /* Initialize new CPU */
+    new_cpu = S390_CPU(object_new(TYPE_S390_CPU));
+    new_cs = CPU(new_cpu);
+    new_env = &new_cpu->env;
+    new_env->cpu_model_str = cpu_model;
+    new_env->cpu_num = cpu_addr;
+    new_cs->cpu_index = cpu_addr;
+
+    /*
+     * Find the last CPU hotplugged and chain it to this one
+     */
+    if (!last_hotplug_cpu) {
+        /*
+         * If no CPU was hotplugged before, set it to the last configured CPU
+         * and/or standby CPU brought online
+         */
+        last_hotplug_cpu = smp_cpus + standby_cpus - 1;
+    }
+
+    /* Use the last hotplugged CPU */
+    conf_cpu = s390_cpu_addr2state((uint16_t) last_hotplug_cpu);
+    conf_env = &conf_cpu->env;
+    conf_env->next_cpu = new_env;
+    new_env->next_cpu = NULL;
+    qemu_init_vcpu(new_env);
+
+    /* Update last hotplugged CPU */
+    last_hotplug_cpu = cpu_addr;
+
+    return new_cpu;
+}
 #endif
 
 S390CPU *cpu_s390x_init(const char *cpu_model)
diff --git a/vl.c b/vl.c
index 52eacca..fa2d45f 100644
--- a/vl.c
+++ b/vl.c
@@ -211,6 +211,7 @@ int win2k_install_hack = 0;
 int singlestep = 0;
 int smp_cpus = 1;
 int max_cpus = 0;
+int standby_cpus = 0;
 int smp_cores = 1;
 int smp_threads = 1;
 #ifdef CONFIG_VNC
@@ -1423,6 +1424,11 @@ static void smp_parse(const char *optarg)
     smp_threads = threads > 0 ? threads : 1;
     if (max_cpus == 0)
         max_cpus = smp_cpus;
+
+    /* Compute standby CPUs */
+    if (max_cpus) {
+        standby_cpus = max_cpus - smp_cpus;
+    }
 }
 
 /***********************************************************/
-- 
1.7.12.4

Re: [Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Andreas Färber]

Hi Jens,

Am 03.04.2013 08:42, schrieb Jens Freimann:
> this is what our approach to CPU hotplug looks like.
> With respect to Igor's CPU hotplug series, how should we proceed? 
> Should we change the interface to 
> qemu_system_cpu_add_notifier/qemu_system_cpu_hotplug_request/cpu-add etc?

I am wondering if my recent qdev/device_add fixes would allow to
implement CPU hot-add via device_add for s390x?

Background is that for x86 we currently have a flat CPU core/thread
namespace but would need to deal with sockets, cores and threads to get
topologies right. I assume there are no such issues on s390x, so that
the vCPU to CPUState mapping could stay 1:1?

> Feedback regarding the non-API part is also highly welcome!

I did spot some QOM'ish nitpicks in the qdev_create() but I am wondering
if you and Alex see an urgent need to get this into 1.5 during the Soft
Freeze or whether we can just align it to x86 work for now and deal with
it after the release?

Regards,
Andreas

> Thang Pham (1):
>   s390: implement CPU hotplug
> 
>  hmp-commands.hx           |  14 +++++
>  hw/s390x/Makefile.objs    |   2 +-
>  hw/s390x/event-facility.c |   9 ++++
>  hw/s390x/event-facility.h |   3 ++
>  hw/s390x/s390-virtio.c    |  34 +++++++++---
>  hw/s390x/sclp.c           | 134 +++++++++++++++++++++++++++++++++++++++++++++-
>  hw/s390x/sclp.h           |  47 +++++++++++++++-
>  include/sysemu/sysemu.h   |   1 +
>  monitor.c                 |  31 +++++++++++
>  target-s390x/cpu.c        |  59 ++++++++++++++++++++
>  target-s390x/cpu.h        |   7 +++
>  target-s390x/helper.c     |  43 +++++++++++++++
>  vl.c                      |   6 +++
>  13 files changed, 379 insertions(+), 11 deletions(-)
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Eduardo Habkost]

On Wed, Apr 17, 2013 at 08:06:37PM +0200, Andreas Färber wrote:
> Hi Jens,
> 
> Am 03.04.2013 08:42, schrieb Jens Freimann:
> > this is what our approach to CPU hotplug looks like.
> > With respect to Igor's CPU hotplug series, how should we proceed? 
> > Should we change the interface to 
> > qemu_system_cpu_add_notifier/qemu_system_cpu_hotplug_request/cpu-add etc?
> 
> I am wondering if my recent qdev/device_add fixes would allow to
> implement CPU hot-add via device_add for s390x?
> 
> Background is that for x86 we currently have a flat CPU core/thread
> namespace but would need to deal with sockets, cores and threads to get
> topologies right. I assume there are no such issues on s390x, so that
> the vCPU to CPUState mapping could stay 1:1?

Also: x86 has a huge number of CPU models we want to convert to separate
classes, and a huge number of configurable CPU features we want to
expose as device properties.

If we don't need any of that on s390x, I agree that it may be reasonable
to try to make device_add work.

-- 
Eduardo

Re: [Qemu-devel] [RFC/PATCH 1/1] s390: implement CPU hotplug

[Igor Mammedov]

On Wed,  3 Apr 2013 08:42:25 +0200
Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:

> From: Thang Pham <thang.pham@us.ibm.com>
> 
[...]
>  
> @@ -31,25 +34,152 @@ static inline S390SCLPDevice *get_event_facility(void)
>  static void read_SCP_info(SCCB *sccb)
>  {
>      ReadInfo *read_info = (ReadInfo *) sccb;
> +    int cpu_count = standby_cpus + smp_cpus;
> +    int i = 0;
> +    int max_vcpus = standby_cpus + smp_cpus;
>      int shift = 0;
> -
> +#ifdef CONFIG_KVM
> +    max_vcpus = kvm_check_extension(kvm_state, KVM_CAP_MAX_VCPUS);
> +#endif
>      while ((ram_size >> (20 + shift)) > 65535) {
>          shift++;
>      }
>      read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
>      read_info->rnsize = 1 << shift;
> +
> +    /* CPU information */
> +    read_info->entries_cpu = cpu_to_be16(standby_cpus + smp_cpus);
see below comment, possibly entries_cpu value might be +off.

> +    read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
> +    read_info->highest_cpu = cpu_to_be16(max_vcpus);
> +
> +    /*
> +     * Insert a valid 16-byte entry for each CPU in each the
> +     * list (configured & standby)
> +     */
> +    for (i = 0; i < cpu_count; i++) {
> +        read_info->entries[i].address = i;
> +        read_info->entries[i].type = 0; /* General purpose CPU */
> +    }
> +
> +    read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
> +    sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
> +}
> +
> +/* Provide information about the CPU */
> +static void sclp_read_cpu_info(SCCB *sccb)
> +{
> +    ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
> +    int cpu_count = standby_cpus + smp_cpus;
in qemu_system_cpu_hot_add() standby_cpus increased but  smp_cpus isn't
touched. That might create access out of array bound in below for loop,
and.

> +    int i = 0;
> +
> +    cpu_info->nr_configured = cpu_to_be16(smp_cpus);
> +    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo,
> entries));
> +    cpu_info->nr_standby = cpu_to_be16(standby_cpus);
> +
> +    /* The standby offset is 16-byte for each CPU */
> +    cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
> +        + cpu_info->nr_configured*sizeof(CpuEntry));
> +
> +    /*
> +     * Insert a valid 16-byte entry for each CPU in each the
> +     * list (configured & standby)
> +     */
> +    for (i = 0; i < cpu_count; i++) {
> +        cpu_info->entries[i].address = i;
> +        cpu_info->entries[i].type = 0; /* General purpose CPU */
> +    }
> +
>      sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
>  }
>  
> +static void sclp_configure_cpu(SCCB *sccb, uint16_t cpu_addr)
> +{
> +    /* Create underlying CPU thread */
> +    S390CPU *target_cpu = s390_cpu_addr2state(cpu_addr);
> +
> +    if (!target_cpu) {
> +        /* Create new vCPU thread and associate it with the CPU address */
> +        target_cpu = configure_cpu(cpu_addr);
> +    }
> +
> +    /*
> +     * Bring CPU from standby to configure state. Increment configure CPU
> count
> +     * and decrement standby CPU count.
> +     */
> +    smp_cpus++;
> +    if (standby_cpus) {
> +        standby_cpus--;
> +    }
what if guest calls it several times for the same CPU?
It could increase its VCPU limit given on cmd line.

> +
> +    /* CPU hotplug requires SCCB response code */
> +    sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> +}
> +
> +static void sclp_deconfigure_cpu(SCCB *sccb, uint16_t cpu_addr)
> +{
> +    /*
> +     * Bring configured CPU to standby state.
> +     * Underlying CPU thread should be halted.
> +     */
> +    S390CPU *target_cpu = s390_cpu_addr2state(cpu_addr);
> +    CPUState *target_cpu_state = CPU(target_cpu);
> +    CPUS390XState *target_env;
> +
> +    if (target_cpu) {
> +        target_env = &target_cpu->env;
> +
> +        /* Halt CPU */
> +        if (target_cpu_state->halted != 1) {
> +            fprintf(stderr, "CPU %d must be off-lined first before it can
> be "
> +                        "de-configured\n", cpu_addr);
> +            sccb->h.response_code =
> +                    cpu_to_be16(SCLP_RC_IMPROPER_RESOURCE_STATE);
> +            return;
> +        }
> +
> +        /* Do not de-configure the last configured CPU */
> +        if (smp_cpus == 1) {
> +            fprintf(stderr, "At least one CPU must be running. CPU %d
> cannot "
> +                        "be de-configured\n", cpu_addr);
> +            sccb->h.response_code = cpu_to_be16(SCLP_RC_REQUIRED_RESOURCE);
> +            return;
> +        }
> +
> +        standby_cpus++;
> +        smp_cpus--;
> +
> +        qemu_cpu_kick(ENV_GET_CPU(target_env));
> +
> +        /* CPU hotplug done on guest requires SCCB response code*/
> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> +    } else {
> +        /* CPU was not created if target_cpu is NULL */
> +        fprintf(stderr, "CPU %d does not exist\n", cpu_addr);
> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_RESOURCE_ID);
> +    }
> +}
> +
>  static void sclp_execute(SCCB *sccb, uint64_t code)
>  {
>      S390SCLPDevice *sdev = get_event_facility();
>  
> -    switch (code) {
> +    switch (code & SCLP_NO_CMD_PARM) {
>      case SCLP_CMDW_READ_SCP_INFO:
>      case SCLP_CMDW_READ_SCP_INFO_FORCED:
>          read_SCP_info(sccb);
>          break;
> +    case SCLP_CMDW_READ_CPU_INFO:
> +        sclp_read_cpu_info(sccb);
> +        break;
> +    case SCLP_CMDW_CONFIGURE_CPU:
> +        sclp_configure_cpu(sccb,
> +                (uint16_t) (code & SCLP_CMDW_CPU_CMD_PARM) >> 8);
> +        break;
> +    case SCLP_CMDW_DECONFIGURE_CPU:
> +        /* Obtain CPU address from code: (uint16_t) (code & 0xff00) >> 8 */
> +        sclp_deconfigure_cpu(sccb,
> +                (uint16_t) (code & SCLP_CMDW_CPU_CMD_PARM) >> 8);
> +        break;
>      default:
>          sdev->sclp_command_handler(sdev->ef, sccb, code);
>          break;
[...]

> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 23fe51f..71afe6e 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -28,12 +28,71 @@
>  #include "qemu/timer.h"
>  #include "hw/hw.h"
>  #ifndef CONFIG_USER_ONLY
> +#include "hw/s390x/sclp.h"
>  #include "sysemu/arch_init.h"
> +#include "sysemu/sysemu.h"
>  #endif
>  
>  #define CR0_RESET       0xE0UL
>  #define CR14_RESET      0xC2000000UL;
>  
> +#ifndef CONFIG_USER_ONLY
> +
> +void qemu_system_cpu_hot_add(int cpu, int state)
> +{
> +    int max_vcpus = smp_cpus + standby_cpus;
> +    int next_cpu = smp_cpus + standby_cpus;
/me confused
from vl.c

+    if (max_cpus) {
+        standby_cpus = max_cpus - smp_cpus;
+    }

> +#ifdef CONFIG_KVM
> +    max_vcpus = kvm_check_extension(kvm_state, KVM_CAP_MAX_VCPUS);
> +#endif
MIN(max_vcpus, max_cpus) ? maybe machine should be aborted at startup if
kvm_check_extension(kvm_state, KVM_CAP_MAX_VCPUS) < max_cpus

> +
[...]
> +    /*
> +     * Find out next CPU address that could be attached.
> +     * Only standby CPUs can be added by the monitor and it must be in
> +     * sequential order
> +     */
> +    if (next_cpu >= max_vcpus) {
> +        fprintf(stderr, "The maximum number of configurable CPUs has been "
> +                    "reached\n");
> +        return;
> +    }
from above vl.c snippet
next_cpu might be == max_cpu ???

> +
> +    if (cpu != next_cpu) {
> +        fprintf(stderr, "The next standby CPU that can be hotplugged is "
> +                    "CPU %d\n", next_cpu);
> +        return;
> +    }
> +
> +    /* Configure standby CPU */
> +    configure_cpu((uint16_t) cpu);
> +
> +    /* Configure is invoked from monitor. Increment standby CPU count. */
> +    standby_cpus++;
Is smp_cpus-- missing here intentionally?

> +
> +    /* Trigger SCLP interrupt */
> +    raise_irq_cpu_hotplug();
> +}
> +#endif
> +
>  /* generate CPU information for cpu -? */
>  void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>  {
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index e351005..12bea46 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -316,6 +316,12 @@ S390CPU *cpu_s390x_init(const char *cpu_model);
>  void s390x_translate_init(void);
>  int cpu_s390x_exec(CPUS390XState *s);
>  
> +#ifndef CONFIG_USER_ONLY
> +/* CPU hotplug support on s390 */
> +void qemu_system_cpu_hot_add(int cpu, int state);
> +S390CPU *s390x_cpu_hotplug(int cpu_addr);
> +#endif
> +
>  /* you can call this signal handler from your SIGBUS and SIGSEGV
>     signal handlers to inform the virtual CPU of exceptions. non zero
>     is returned if the signal was handled by the virtual CPU.  */
> @@ -373,6 +379,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU
> *cpu, int type, }
>  #endif
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
> +void s390_cpu_add_state(uint16_t cpu_addr, S390CPU *state);
>  void s390_add_running_cpu(S390CPU *cpu);
>  unsigned s390_del_running_cpu(S390CPU *cpu);
>  
> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> index b425054..c159c78 100644
> --- a/target-s390x/helper.c
> +++ b/target-s390x/helper.c
> @@ -51,6 +51,8 @@
>  #endif
>  
>  #ifndef CONFIG_USER_ONLY
> +static int last_hotplug_cpu; /* Track which CPU was last hotplugged */
> +
>  void s390x_tod_timer(void *opaque)
>  {
>      S390CPU *cpu = opaque;
> @@ -68,6 +70,47 @@ void s390x_cpu_timer(void *opaque)
>      env->pending_int |= INTERRUPT_CPUTIMER;
>      cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
>  }
> +
> +S390CPU *s390x_cpu_hotplug(int cpu_addr)
> +{
> +    const char *cpu_model = "host";
> +    S390CPU *new_cpu;
> +    CPUS390XState *new_env;
> +    CPUState *new_cs;
> +    S390CPU *conf_cpu;
> +    CPUS390XState *conf_env;
> +
> +    /* Initialize new CPU */
> +    new_cpu = S390_CPU(object_new(TYPE_S390_CPU));
> +    new_cs = CPU(new_cpu);
> +    new_env = &new_cpu->env;
> +    new_env->cpu_model_str = cpu_model;
> +    new_env->cpu_num = cpu_addr;
> +    new_cs->cpu_index = cpu_addr;
usage of cpu_num and cpu_index looks like a complete mess.
just do:
 git grep cpu_num target-s390x hw/s390x

It make sense to return cpu_num from kvm_arch_vcpu_id() and drop usage of
cpu_index altogether.

moreover cpu_index is assigned in cpu_exec_init() and used there by
vmstate_*(), yes CPU marked as unmigrateable but why intentionally make
the current state worse.

> +
> +    /*
> +     * Find the last CPU hotplugged and chain it to this one
> +     */
> +    if (!last_hotplug_cpu) {
> +        /*
> +         * If no CPU was hotplugged before, set it to the last configured
> CPU
> +         * and/or standby CPU brought online
> +         */
> +        last_hotplug_cpu = smp_cpus + standby_cpus - 1;
> +    }
> +
> +    /* Use the last hotplugged CPU */
> +    conf_cpu = s390_cpu_addr2state((uint16_t) last_hotplug_cpu);
since goal is sequential hot-add then all this last_hotplug_cpu and
cpu_num/cpu_index fiddling looks unnecessary.
cpu_num provides this kind of counter and increments with every new CPU
in initfn().

> +    conf_env = &conf_cpu->env;
> +    conf_env->next_cpu = new_env;
> +    new_env->next_cpu = NULL;
cpu_exec_init() does this for you, doesn't it?
why this meddling with next_cpu needed?

> +    qemu_init_vcpu(new_env);
Something wrong here, ^^^ is part of realizefn() of CPU class and probably
shouldn't be accessed directly outside of it, ever.

> +
> +    /* Update last hotplugged CPU */
> +    last_hotplug_cpu = cpu_addr;
> +
> +    return new_cpu;
> +}
>  #endif
>  
>  S390CPU *cpu_s390x_init(const char *cpu_model)
> diff --git a/vl.c b/vl.c
> index 52eacca..fa2d45f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -211,6 +211,7 @@ int win2k_install_hack = 0;
>  int singlestep = 0;
>  int smp_cpus = 1;
>  int max_cpus = 0;
> +int standby_cpus = 0;
target specific, pls do it inside target code if possible

>  int smp_cores = 1;
>  int smp_threads = 1;
>  #ifdef CONFIG_VNC
> @@ -1423,6 +1424,11 @@ static void smp_parse(const char *optarg)
>      smp_threads = threads > 0 ? threads : 1;
>      if (max_cpus == 0)
>          max_cpus = smp_cpus;
> +
> +    /* Compute standby CPUs */
> +    if (max_cpus) {
> +        standby_cpus = max_cpus - smp_cpus;
> +    }
ditto

>  }
>  
>  /***********************************************************/

Re: [Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Jens Freimann]

On Wed, Apr 17, 2013 at 08:06:37PM +0200, Andreas Färber wrote:
> Hi Jens,
> 
> Am 03.04.2013 08:42, schrieb Jens Freimann:
> > this is what our approach to CPU hotplug looks like.
> > With respect to Igor's CPU hotplug series, how should we proceed? 
> > Should we change the interface to 
> > qemu_system_cpu_add_notifier/qemu_system_cpu_hotplug_request/cpu-add etc?
> 
> I am wondering if my recent qdev/device_add fixes would allow to
> implement CPU hot-add via device_add for s390x?

>From what I've seen so far it could be possible, but...
 
> Background is that for x86 we currently have a flat CPU core/thread
> namespace but would need to deal with sockets, cores and threads to get
> topologies right. I assume there are no such issues on s390x, so that
> the vCPU to CPUState mapping could stay 1:1?

s390 hardware today already has a topology and CPU features.  We are not 
modelling it in QEMU right now, but want to go there in the future so that 
there would be no simple 1:1 mapping anymore.
 
> > Feedback regarding the non-API part is also highly welcome!
> 
> I did spot some QOM'ish nitpicks in the qdev_create() but I am wondering
> if you and Alex see an urgent need to get this into 1.5 during the Soft
> Freeze or whether we can just align it to x86 work for now and deal with
> it after the release?

We'll try to have it ready as soon as possible but I think it's unlikely
for 1.5. Getting started with Igor's review feedback right now and looking
at qdev_create() as well. 


Regards,
Jens
 
> Regards,
> Andreas
> 
> > Thang Pham (1):
> >   s390: implement CPU hotplug
> > 
> >  hmp-commands.hx           |  14 +++++
> >  hw/s390x/Makefile.objs    |   2 +-
> >  hw/s390x/event-facility.c |   9 ++++
> >  hw/s390x/event-facility.h |   3 ++
> >  hw/s390x/s390-virtio.c    |  34 +++++++++---
> >  hw/s390x/sclp.c           | 134 +++++++++++++++++++++++++++++++++++++++++++++-
> >  hw/s390x/sclp.h           |  47 +++++++++++++++-
> >  include/sysemu/sysemu.h   |   1 +
> >  monitor.c                 |  31 +++++++++++
> >  target-s390x/cpu.c        |  59 ++++++++++++++++++++
> >  target-s390x/cpu.h        |   7 +++
> >  target-s390x/helper.c     |  43 +++++++++++++++
> >  vl.c                      |   6 +++
> >  13 files changed, 379 insertions(+), 11 deletions(-)
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 

Re: [Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Andreas Färber]

Am 19.04.2013 09:51, schrieb Jens Freimann:
> On Wed, Apr 17, 2013 at 08:06:37PM +0200, Andreas Färber wrote:
>> Hi Jens,
>>
>> Am 03.04.2013 08:42, schrieb Jens Freimann:
>>> this is what our approach to CPU hotplug looks like.
>>> With respect to Igor's CPU hotplug series, how should we proceed? 
>>> Should we change the interface to 
>>> qemu_system_cpu_add_notifier/qemu_system_cpu_hotplug_request/cpu-add etc?
>>
>> I am wondering if my recent qdev/device_add fixes would allow to
>> implement CPU hot-add via device_add for s390x?
> 
> From what I've seen so far it could be possible, but...

Hm, so probably not a good idea? Just looking for a guinea pig for
infrastructure testing...

>> Background is that for x86 we currently have a flat CPU core/thread
>> namespace but would need to deal with sockets, cores and threads to get
>> topologies right. I assume there are no such issues on s390x, so that
>> the vCPU to CPUState mapping could stay 1:1?
> 
> s390 hardware today already has a topology and CPU features.  We are not 
> modelling it in QEMU right now, but want to go there in the future so that 
> there would be no simple 1:1 mapping anymore.

In that case please enlighten us how the topology model should/could
look like in the future, so that we can align this with the x86
remodelling and APIC/ID discussion.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Igor Mammedov]

On Fri, 19 Apr 2013 15:16:36 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 19.04.2013 09:51, schrieb Jens Freimann:
> > On Wed, Apr 17, 2013 at 08:06:37PM +0200, Andreas Färber wrote:
> >> Hi Jens,
> >>
> >> Am 03.04.2013 08:42, schrieb Jens Freimann:
> >>> this is what our approach to CPU hotplug looks like.
> >>> With respect to Igor's CPU hotplug series, how should we proceed? 
> >>> Should we change the interface to 
> >>> qemu_system_cpu_add_notifier/qemu_system_cpu_hotplug_request/cpu-add
> >>> etc?
> >>
> >> I am wondering if my recent qdev/device_add fixes would allow to
> >> implement CPU hot-add via device_add for s390x?
> > 
> > From what I've seen so far it could be possible, but...
> 
> Hm, so probably not a good idea? Just looking for a guinea pig for
> infrastructure testing...
Well it looks like good candidate for this, from what I saw though my
cursory review was that so far s390:
 1. it has only one CPU model, and 
 2. cpu_model string isn't at all
 3. no features are set externally (at least explicitly during CPU creation)

Once patches 1-8/16 from v4 cpu-add are in + Andreas patch for bussless
devices in device_add, generic infrastructure for device_add will be in place.
So new respin might be device_add based.

Perhaps there are missing pieces yet, but it would be easier to spot them
once trying implement stuff this way.

> 
> >> Background is that for x86 we currently have a flat CPU core/thread
> >> namespace but would need to deal with sockets, cores and threads to get
> >> topologies right. I assume there are no such issues on s390x, so that
> >> the vCPU to CPUState mapping could stay 1:1?
> > 
> > s390 hardware today already has a topology and CPU features.  We are not 
> > modelling it in QEMU right now, but want to go there in the future so
> > that there would be no simple 1:1 mapping anymore.
> 
> In that case please enlighten us how the topology model should/could
> look like in the future, so that we can align this with the x86
> remodelling and APIC/ID discussion.

Let's discuss this not only between me and Eduardo.
Is short we propose to have generic numa topology interface that will be
accessible via QOM tree. Currently idea is to have tree that looks like:

/machine / node[0..x]/ socket[0..y] / core [0..z] / thread [0..n]
                                    |
                                    / thread [0..n]

where total amount of threads == max_cpus and the rest of nodes are calculated
using respective -smp options.

> 
> Regards,
> Andreas
> 

Re: [Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Christian Borntraeger]

On 19/04/13 15:16, Andreas Färber wrote:
[...]
>>> Background is that for x86 we currently have a flat CPU core/thread
>>> namespace but would need to deal with sockets, cores and threads to get
>>> topologies right. I assume there are no such issues on s390x, so that
>>> the vCPU to CPUState mapping could stay 1:1?
>>
>> s390 hardware today already has a topology and CPU features.  We are not 
>> modelling it in QEMU right now, but want to go there in the future so that 
>> there would be no simple 1:1 mapping anymore.
> 
> In that case please enlighten us how the topology model should/could
> look like in the future, so that we can align this with the x86
> remodelling and APIC/ID discussion.

Current models provide topology information via the STSI instruction
(http://publibfi.boulder.ibm.com/epubs/pdf/dz9zr009.pdf
see page 10-134 and following) function code 15. The system basically informs about
cpus and containers, which boils down to how the hardware is organized:

e.g. fr zec12 we have up to 4 Nodes(aka books). A book contains one MCM
with 6 processors. Each processor then has 6 cores.
(see http://publibfi.boulder.ibm.com/epubs/pdf/dz9zr009.pdf)

Future systems might look different, I dont know if and how the topology will
change. Currently there is no information about memory locality, only processor
grouping.

As Jens said, as of now we do not provide any topology information to the
guest, but we might to change that in the future.

I havent followed the x86 discussion, so please let me know if that
doesnt answer your question.


Christian

Re: [Qemu-devel] [RFC/PATCH 0/1] cpu hotplug for s390

[Eduardo Habkost]

On Fri, Apr 19, 2013 at 09:13:32PM +0200, Christian Borntraeger wrote:
> On 19/04/13 15:16, Andreas Färber wrote:
> [...]
> >>> Background is that for x86 we currently have a flat CPU core/thread
> >>> namespace but would need to deal with sockets, cores and threads to get
> >>> topologies right. I assume there are no such issues on s390x, so that
> >>> the vCPU to CPUState mapping could stay 1:1?
> >>
> >> s390 hardware today already has a topology and CPU features.  We are not 
> >> modelling it in QEMU right now, but want to go there in the future so that 
> >> there would be no simple 1:1 mapping anymore.
> > 
> > In that case please enlighten us how the topology model should/could
> > look like in the future, so that we can align this with the x86
> > remodelling and APIC/ID discussion.
> 
> Current models provide topology information via the STSI instruction
> (http://publibfi.boulder.ibm.com/epubs/pdf/dz9zr009.pdf
> see page 10-134 and following) function code 15. The system basically informs about
> cpus and containers, which boils down to how the hardware is organized:
> 
> e.g. fr zec12 we have up to 4 Nodes(aka books). A book contains one MCM
> with 6 processors. Each processor then has 6 cores.
> (see http://publibfi.boulder.ibm.com/epubs/pdf/dz9zr009.pdf)
> 
> Future systems might look different, I dont know if and how the topology will
> change. Currently there is no information about memory locality, only processor
> grouping.
> 
> As Jens said, as of now we do not provide any topology information to the
> guest, but we might to change that in the future.
> 
> I havent followed the x86 discussion, so please let me know if that
> doesnt answer your question.

The x86 solution involves adding a "cpu-add" QMP command by now because
we don't have all that's necessary to make device_add work on x86 but we
want to have CPU hotplug in QEMU 1.5.

But in the case of s390 it may be feasible to make device_add work at
the lowest level (CPU core?) (with no topology setting features, because
you still don't have it), and later you can consider adding objects to
represent books, MCM, processors, cores, etc. That's the long-term plan
for x86: having new classes and objects for CPU sockets/cores/threads.

You could even translate cpu-add to the equivalent of a device_add
command, if you want to make the cpu-add command work on s390 too.

Later we may try to agree on a common target-independent device tree
layout so we can have a target-independent topology-aware CPU hotplug
interface. Or we could simply allow each architecture to use a different
device tree layout to represent CPU topology.

-- 
Eduardo