[Qemu-devel] [RFC PATCH v1 0/7] Reset and Halting modifications + Zynq SMP

[Qemu-devel] [RFC PATCH v1 0/7] Reset and Halting modifications + Zynq SMP

[Peter Crosthwaite]

Hi All. The clock controller module in the Zynq platform has the ability to halt
and reset arbitrary devices, including the CPU. We use this feature to implement
SMP Linux - the kernel halts CPU1 then rewrites the vector table to the
secondary entry point and unhalts.

The clock controller however is reasonable generic, in that in has the ability
to halt and reset any arbitrary device in the Zynq SoC. The interface for doing
this is reasonably consistent. So im looking for unified way to halt and reset
arbitrary devices CPUs included. So given we already have reset() defined on the
TYPE_DEVICE level, i've added halt as well.

This series is based on v3 of the current qom-cpu queue to pick up the move of
cpu halt out of the env. From this I attach the cpu::halted bit to the
Device::(un)halt function.

Next up, CPUs seem to have a different reset path to normal devices. So I have
trivially hooked up CPU::Reset to Device::Reset. This means that anyone who
holds a DeviceState pointer to the CPU can reset it properly without actually
knowing its a CPU.

With the halt API I played with changing an existing device over to use it in
place of the CPU halted bit (sun4m).

I then finally add SMP support to the Zynq machine, and patch the Zynq SLCR
(my clock controller) to have DeviceState pointers to the CPUs. device_reset()
and device_halt() is where the magic happens.

Future work, more devices in Zynq will have halt and reset. My agenda for
abstracting away the fact that attached device is a CPU is to allow for
consistent implementation and a single code path for the clock controlled
devices.


Peter A. G. Crosthwaite (3):
  xilinx_zynq: added smp support
  zynq_slcr: Add links to the CPUs
  zynq_slcr: Implement CPU reset and halting

Peter Crosthwaite (4):
  qdev: Define halting API
  qom/cpu.c: Encapsulate cpu halting
  qom/cpu.c: Hook CPU reset up to device reset
  sun4m: Use halting API to halt/unhalt CPUs

 hw/qdev-core.h   |   17 ++++++++++++++
 hw/qdev.c        |   18 ++++++++++++++
 hw/sun4m.c       |   24 +++++++++---------
 hw/xilinx_zynq.c |   66 ++++++++++++++++++++++++++++++++++++++++++++---------
 hw/zynq_slcr.c   |   30 ++++++++++++++++++++++++
 qom/cpu.c        |   22 ++++++++++++++++++
 6 files changed, 153 insertions(+), 24 deletions(-)

[Qemu-devel] [RFC PATCH v1 1/7] qdev: Define halting API

[Peter Crosthwaite]

Define APIs on the device level for halting and un-halting a device. The
semantic is the device freezes its state and does not respond to any form
of transaction.

The main intended use cases is to implement clock gating and power down. clock
gating is just a halt, whereas power down is a reset() and halt(). Unhalt is
then used to ungate the clock or power up the device.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/qdev-core.h |   17 +++++++++++++++++
 hw/qdev.c      |   18 ++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index 2486f36..1fee53a 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -87,6 +87,9 @@ typedef struct DeviceClass {
 
     /* callbacks */
     void (*reset)(DeviceState *dev);
+    void (*halt)(DeviceState *dev);
+    void (*unhalt)(DeviceState *dev);
+
     DeviceRealize realize;
     DeviceUnrealize unrealize;
 
@@ -280,6 +283,20 @@ void qdev_machine_init(void);
  */
 void device_reset(DeviceState *dev);
 
+/**
+ * @device_halt
+ *
+ * Halt a single device (by calling the halt method).
+ */
+void device_halt(DeviceState *dev);
+
+/**
+ * @device_unhalt
+ *
+ * Unhalt a single device (by calling the unhalt method).
+ */
+void device_unhalt(DeviceState *dev);
+
 const struct VMStateDescription *qdev_get_vmsd(DeviceState *dev);
 
 const char *qdev_fw_name(DeviceState *dev);
diff --git a/hw/qdev.c b/hw/qdev.c
index 689cd54..c338b74 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -797,6 +797,24 @@ void device_reset(DeviceState *dev)
     }
 }
 
+void device_halt(DeviceState *dev)
+{
+    DeviceClass *klass = DEVICE_GET_CLASS(dev);
+
+    if (klass->halt) {
+        klass->halt(dev);
+    }
+}
+
+void device_unhalt(DeviceState *dev)
+{
+    DeviceClass *klass = DEVICE_GET_CLASS(dev);
+
+    if (klass->halt) {
+        klass->unhalt(dev);
+    }
+}
+
 Object *qdev_get_machine(void)
 {
     static Object *dev;
-- 
1.7.0.4

[Qemu-devel] [RFC PATCH v1 2/7] qom/cpu.c: Encapsulate cpu halting

[Peter Crosthwaite]

Set up the Device level halting API to halt cpus. This allows holders of a
pointer to the CPU to halt it from the TYPE_DEVICE abstraction. (i.e. they
can halt it even if they dont know its a CPU).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 qom/cpu.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index e242dcb..00a07d9 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -43,6 +43,20 @@ static void cpu_common_reset(CPUState *cpu)
     cpu->halted = 0;
 }
 
+static void cpu_common_halt(DeviceState *dev)
+{
+    CPUState *s = CPU(dev);
+
+    s->halted = 1;
+}
+
+static void cpu_common_unhalt(DeviceState *dev)
+{
+    CPUState *s = CPU(dev);
+
+    s->halted = 0;
+}
+
 ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
 {
     CPUClass *cc = CPU_CLASS(object_class_by_name(typename));
@@ -66,6 +80,8 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 
     k->class_by_name = cpu_common_class_by_name;
     k->reset = cpu_common_reset;
+    dc->halt = cpu_common_halt;
+    dc->unhalt = cpu_common_unhalt;
     dc->realize = cpu_common_realizefn;
     dc->no_user = 1;
 }
-- 
1.7.0.4

[Qemu-devel] [RFC PATCH v1 3/7] qom/cpu.c: Hook CPU reset up to device reset

[Peter Crosthwaite]

Hookup the CPU::reset function up as the Device::reset function. This allows
devices that abstract away the fact that the device is a CPU to reset it.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 qom/cpu.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index 00a07d9..7084a67 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -43,6 +43,11 @@ static void cpu_common_reset(CPUState *cpu)
     cpu->halted = 0;
 }
 
+static void cpu_device_reset(DeviceState *dev)
+{
+    cpu_reset(CPU(dev));
+}
+
 static void cpu_common_halt(DeviceState *dev)
 {
     CPUState *s = CPU(dev);
@@ -80,6 +85,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 
     k->class_by_name = cpu_common_class_by_name;
     k->reset = cpu_common_reset;
+    dc->reset = cpu_device_reset;
     dc->halt = cpu_common_halt;
     dc->unhalt = cpu_common_unhalt;
     dc->realize = cpu_common_realizefn;
-- 
1.7.0.4

[Qemu-devel] [RFC PATCH v1 4/7] sun4m: Use halting API to halt/unhalt CPUs

[Peter Crosthwaite]

Example for a change pattern that could be applied to use the halting API rather
than directly modifying the CPU halted bit.

Untested RFC.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/sun4m.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/sun4m.c b/hw/sun4m.c
index d83d42c..b544782 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -261,8 +261,9 @@ static void cpu_kick_irq(SPARCCPU *cpu)
 {
     CPUSPARCState *env = &cpu->env;
     CPUState *cs = CPU(cpu);
+    DeviceState *d = DEVICE(cpu);
 
-    cs->halted = 0;
+    device_unhalt(d);
     cpu_check_irqs(env);
     qemu_cpu_kick(cs);
 }
@@ -289,20 +290,19 @@ static void dummy_cpu_set_irq(void *opaque, int irq, int level)
 
 static void main_cpu_reset(void *opaque)
 {
-    SPARCCPU *cpu = opaque;
-    CPUState *cs = CPU(cpu);
+    DeviceState *d = DEVICE(opaque);
 
-    cpu_reset(cs);
-    cs->halted = 0;
+    device_reset(d);
+    device_unhalt(d);
 }
 
 static void secondary_cpu_reset(void *opaque)
 {
-    SPARCCPU *cpu = opaque;
-    CPUState *cs = CPU(cpu);
+    DeviceState *d = DEVICE(opaque);
+
+    device_reset(d);
+    device_halt(d);
 
-    cpu_reset(cs);
-    cs->halted = 1;
 }
 
 static void cpu_halt_signal(void *opaque, int irq, int level)
@@ -833,7 +833,7 @@ static const TypeInfo ram_info = {
 static void cpu_devinit(const char *cpu_model, unsigned int id,
                         uint64_t prom_addr, qemu_irq **cpu_irqs)
 {
-    CPUState *cs;
+    DeviceState *d;
     SPARCCPU *cpu;
     CPUSPARCState *env;
 
@@ -849,8 +849,8 @@ static void cpu_devinit(const char *cpu_model, unsigned int id,
         qemu_register_reset(main_cpu_reset, cpu);
     } else {
         qemu_register_reset(secondary_cpu_reset, cpu);
-        cs = CPU(cpu);
-        cs->halted = 1;
+        d = DEVICE(cpu);
+        device_halt(DEIVCE(cpu));
     }
     *cpu_irqs = qemu_allocate_irqs(cpu_set_irq, cpu, MAX_PILS);
     env->prom_addr = prom_addr;
-- 
1.7.0.4

[Qemu-devel] [RFC PATCH v1 5/7] xilinx_zynq: added smp support

[Peter Crosthwaite]

From: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>

Added linux smp support for the xilinx zynq platform (2x cpus are supported)

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
Changed from v2:
macro defined the maximum number of CPUS
Changed from v1:
Addressed PMM review
Shorted secondary bootloop using MVN instruction.
Used default reset secondary instead of custom one.
Rebased against QOM cpu developments.
Few whitespace fixes.

 hw/xilinx_zynq.c |   59 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c
index 2f67d90..80a2777 100644
--- a/hw/xilinx_zynq.c
+++ b/hw/xilinx_zynq.c
@@ -26,6 +26,8 @@
 #include "loader.h"
 #include "ssi.h"
 
+#define MAX_CPUS 2
+
 #define NUM_SPI_FLASHES 4
 #define NUM_QSPI_FLASHES 2
 #define NUM_QSPI_BUSSES 2
@@ -35,6 +37,32 @@
 
 #define IRQ_OFFSET 32 /* pic interrupts start from index 32 */
 
+#define SMP_BOOT_ADDR 0x0fff0000
+#define SMP_BOOTREG_ADDR 0xfffffff0
+
+/* Entry point for secondary CPU */
+static uint32_t zynq_smpboot[] = {
+    0xe3e0000f, /* ldr r0, =0xfffffff0 (mvn r0, #15) */
+    0xe320f002, /* wfe */
+    0xe5901000, /* ldr     r1, [r0] */
+    0xe1110001, /* tst     r1, r1 */
+    0x0afffffb, /* beq     <wfe> */
+    0xe12fff11, /* bx      r1 */
+    0,
+};
+
+static void zynq_write_secondary_boot(ARMCPU *cpu,
+                                      const struct arm_boot_info *info)
+{
+    int n;
+
+    for (n = 0; n < ARRAY_SIZE(zynq_smpboot); n++) {
+        zynq_smpboot[n] = tswap32(zynq_smpboot[n]);
+    }
+    rom_add_blob_fixed("smpboot", zynq_smpboot, sizeof(zynq_smpboot),
+                       SMP_BOOT_ADDR);
+}
+
 static struct arm_boot_info zynq_binfo = {};
 
 static void gem_init(NICInfo *nd, uint32_t base, qemu_irq irq)
@@ -99,7 +127,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
     const char *kernel_filename = args->kernel_filename;
     const char *kernel_cmdline = args->kernel_cmdline;
     const char *initrd_filename = args->initrd_filename;
-    ARMCPU *cpu;
+    ARMCPU *cpus[MAX_CPUS];
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
     MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
@@ -109,19 +137,21 @@ static void zynq_init(QEMUMachineInitArgs *args)
     qemu_irq pic[64];
     NICInfo *nd;
     int n;
-    qemu_irq cpu_irq;
+    qemu_irq cpu_irq[MAX_CPUS];
 
     if (!cpu_model) {
         cpu_model = "cortex-a9";
     }
 
-    cpu = cpu_arm_init(cpu_model);
-    if (!cpu) {
-        fprintf(stderr, "Unable to find CPU definition\n");
-        exit(1);
+    for (n = 0; n < smp_cpus; n++) {
+        cpus[n] = cpu_arm_init(cpu_model);
+        if (!cpus[n]) {
+            fprintf(stderr, "Unable to find CPU definition\n");
+            exit(1);
+        }
+        irqp = arm_pic_init_cpu(cpus[n]);
+        cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
     }
-    irqp = arm_pic_init_cpu(cpu);
-    cpu_irq = irqp[ARM_PIC_CPU_IRQ];
 
     /* max 2GB ram */
     if (ram_size > 0x80000000) {
@@ -152,11 +182,13 @@ static void zynq_init(QEMUMachineInitArgs *args)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
 
     dev = qdev_create(NULL, "a9mpcore_priv");
-    qdev_prop_set_uint32(dev, "num-cpu", 1);
+    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
     qdev_init_nofail(dev);
     busdev = SYS_BUS_DEVICE(dev);
     sysbus_mmio_map(busdev, 0, 0xF8F00000);
-    sysbus_connect_irq(busdev, 0, cpu_irq);
+    for (n = 0; n < smp_cpus; n++) {
+        sysbus_connect_irq(busdev, n, cpu_irq[n]);
+    }
 
     for (n = 0; n < 64; n++) {
         pic[n] = qdev_get_gpio_in(dev, n);
@@ -200,7 +232,10 @@ static void zynq_init(QEMUMachineInitArgs *args)
     zynq_binfo.kernel_filename = kernel_filename;
     zynq_binfo.kernel_cmdline = kernel_cmdline;
     zynq_binfo.initrd_filename = initrd_filename;
-    zynq_binfo.nb_cpus = 1;
+    zynq_binfo.nb_cpus = smp_cpus;
+    zynq_binfo.write_secondary_boot = zynq_write_secondary_boot;
+    zynq_binfo.smp_loader_start = SMP_BOOT_ADDR;
+    zynq_binfo.smp_bootreg_addr = SMP_BOOTREG_ADDR;
     zynq_binfo.board_id = 0xd32;
     zynq_binfo.loader_start = 0;
     arm_load_kernel(arm_env_get_cpu(first_cpu), &zynq_binfo);
@@ -211,7 +246,7 @@ static QEMUMachine zynq_machine = {
     .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9",
     .init = zynq_init,
     .block_default_type = IF_SCSI,
-    .max_cpus = 1,
+    .max_cpus = MAX_CPUS,
     .no_sdcard = 1,
     DEFAULT_MACHINE_OPTIONS,
 };
-- 
1.7.0.4

[Qemu-devel] [RFC PATCH v1 6/7] zynq_slcr: Add links to the CPUs

[Peter Crosthwaite]

From: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>

The SLCR needs to be able to reset the CPUs, so link the CPUs to the slcr.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
Changed from v2:
Soften type of CPU to Device
Looped link creator

 hw/xilinx_zynq.c |    7 +++++++
 hw/zynq_slcr.c   |   11 +++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c
index 80a2777..9be2b3b 100644
--- a/hw/xilinx_zynq.c
+++ b/hw/xilinx_zynq.c
@@ -179,6 +179,13 @@ static void zynq_init(QEMUMachineInitArgs *args)
 
     dev = qdev_create(NULL, "xilinx,zynq_slcr");
     qdev_init_nofail(dev);
+    Error *errp = NULL;
+    object_property_set_link(OBJECT(dev), OBJECT(cpus[0]), "cpu0", &errp);
+    assert_no_error(errp);
+    if (smp_cpus > 1) {
+        object_property_set_link(OBJECT(dev), OBJECT(cpus[1]), "cpu1", NULL);
+        assert_no_error(errp);
+    }
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
 
     dev = qdev_create(NULL, "a9mpcore_priv");
diff --git a/hw/zynq_slcr.c b/hw/zynq_slcr.c
index 27b00f0..4925358 100644
--- a/hw/zynq_slcr.c
+++ b/hw/zynq_slcr.c
@@ -19,6 +19,8 @@
 #include "sysbus.h"
 #include "sysemu/sysemu.h"
 
+#define NUM_CPUS 2
+
 #ifdef ZYNQ_ARM_SLCR_ERR_DEBUG
 #define DB_PRINT(...) do { \
     fprintf(stderr,  ": %s: ", __func__); \
@@ -118,6 +120,8 @@ typedef struct {
     SysBusDevice busdev;
     MemoryRegion iomem;
 
+    DeviceState *cpus[NUM_CPUS];
+
     union {
         struct {
             uint16_t scl;
@@ -492,11 +496,18 @@ static const MemoryRegionOps slcr_ops = {
 
 static int zynq_slcr_init(SysBusDevice *dev)
 {
+    int i;
     ZynqSLCRState *s = FROM_SYSBUS(ZynqSLCRState, dev);
 
     memory_region_init_io(&s->iomem, &slcr_ops, s, "slcr", 0x1000);
     sysbus_init_mmio(dev, &s->iomem);
 
+    for (i = 0; i < NUM_CPUS; ++i) {
+        gchar *name = g_strdup_printf("cpu%d", i);
+        object_property_add_link(OBJECT(dev), name, TYPE_DEVICE,
+                                 (Object **) &s->cpus[i], NULL);
+        g_free(name);
+    }
     return 0;
 }
 
-- 
1.7.0.4

[Qemu-devel] [RFC PATCH v1 7/7] zynq_slcr: Implement CPU reset and halting

[Peter Crosthwaite]

From: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>

Implement the CPU reset and halt functions of the A9_CPU_RST_CTRL register
(offset 0x244).

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
changed from v2:
used device halting API instead of talking to the cpu.

 hw/zynq_slcr.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/hw/zynq_slcr.c b/hw/zynq_slcr.c
index 4925358..945355e 100644
--- a/hw/zynq_slcr.c
+++ b/hw/zynq_slcr.c
@@ -116,6 +116,9 @@ typedef enum {
   RESET_MAX
 } ResetValues;
 
+#define A9_CPU_RST_CTRL_RST_SHIFT 0
+#define A9_CPU_RST_CTRL_CLKSTOP_SHIFT 4
+
 typedef struct {
     SysBusDevice busdev;
     MemoryRegion iomem;
@@ -346,6 +349,7 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
                           uint64_t val, unsigned size)
 {
     ZynqSLCRState *s = (ZynqSLCRState *)opaque;
+    int i;
 
     DB_PRINT("offset: %08x data: %08x\n", (unsigned)offset, (unsigned)val);
 
@@ -400,6 +404,21 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
                 goto bad_reg;
             }
             s->reset[(offset - 0x200) / 4] = val;
+            if (offset - 0x200 == A9_CPU * 4) { /* CPU Reset */
+                for (i = 0; i < NUM_CPUS && s->cpus[i]; ++i) {
+                    bool is_rst = val & (1 << (A9_CPU_RST_CTRL_RST_SHIFT + i));
+                    bool is_clkstop = val &
+                                    (1 << (A9_CPU_RST_CTRL_CLKSTOP_SHIFT + i));
+                    if (is_rst) {
+                        DB_PRINT("resetting cpu %d\n", i);
+                        device_reset(s->cpus[i]);
+                    }
+                    DB_PRINT("%shalting cpu %d\n", is_rst || is_clkstop ?
+                             "" : "un", i);
+                    (is_rst || is_clkstop ?
+                                device_halt : device_unhalt)(s->cpus[i]);
+                }
+            }
             break;
         case 0x300:
             s->apu_ctrl = val;
-- 
1.7.0.4

Re: [Qemu-devel] [RFC PATCH v1 0/7] Reset and Halting modifications + Zynq SMP

[Andreas Färber]

Hi Peter,

Am 04.03.2013 10:01, schrieb Peter Crosthwaite:
> Hi All. The clock controller module in the Zynq platform has the ability to halt
> and reset arbitrary devices, including the CPU. We use this feature to implement
> SMP Linux - the kernel halts CPU1 then rewrites the vector table to the
> secondary entry point and unhalts.
> 
> The clock controller however is reasonable generic, in that in has the ability
> to halt and reset any arbitrary device in the Zynq SoC. The interface for doing
> this is reasonably consistent. So im looking for unified way to halt and reset
> arbitrary devices CPUs included. So given we already have reset() defined on the
> TYPE_DEVICE level, i've added halt as well.
> 
> This series is based on v3 of the current qom-cpu queue to pick up the move of
> cpu halt out of the env. From this I attach the cpu::halted bit to the
> Device::(un)halt function.

I have doubts whether that is a suitable API... My initial thought was:
Isn't that pretty much realize/unrealize according to Anthony's
definition via Vcc? The technical difference I see is that in your case
devices are still migrated, not sure if that makes a difference,
assuming unhalt/realize puts them into a defined state (i.e., reset).

Reminds me that some time ago we had a discussion about
enabling/disabling/reconfiguring ISA devices. realize/unrealize might be
a solution there too, but it may still be tied to the concept of
hot-plug, which is disabled for ISA and SysBus.

> Next up, CPUs seem to have a different reset path to normal devices. So I have
> trivially hooked up CPU::Reset to Device::Reset. This means that anyone who
> holds a DeviceState pointer to the CPU can reset it properly without actually
> knowing its a CPU.

If we do that, it needs a better explanation of *why* this is okay.

Regards,
Andreas

> 
> With the halt API I played with changing an existing device over to use it in
> place of the CPU halted bit (sun4m).
> 
> I then finally add SMP support to the Zynq machine, and patch the Zynq SLCR
> (my clock controller) to have DeviceState pointers to the CPUs. device_reset()
> and device_halt() is where the magic happens.
> 
> Future work, more devices in Zynq will have halt and reset. My agenda for
> abstracting away the fact that attached device is a CPU is to allow for
> consistent implementation and a single code path for the clock controlled
> devices.
> 
> 
> Peter A. G. Crosthwaite (3):
>   xilinx_zynq: added smp support
>   zynq_slcr: Add links to the CPUs
>   zynq_slcr: Implement CPU reset and halting
> 
> Peter Crosthwaite (4):
>   qdev: Define halting API
>   qom/cpu.c: Encapsulate cpu halting
>   qom/cpu.c: Hook CPU reset up to device reset
>   sun4m: Use halting API to halt/unhalt CPUs
> 
>  hw/qdev-core.h   |   17 ++++++++++++++
>  hw/qdev.c        |   18 ++++++++++++++
>  hw/sun4m.c       |   24 +++++++++---------
>  hw/xilinx_zynq.c |   66 ++++++++++++++++++++++++++++++++++++++++++++---------
>  hw/zynq_slcr.c   |   30 ++++++++++++++++++++++++
>  qom/cpu.c        |   22 ++++++++++++++++++
>  6 files changed, 153 insertions(+), 24 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 v1 0/7] Reset and Halting modifications + Zynq SMP

[Peter Crosthwaite]

Hi Andreas,

On Mon, Mar 4, 2013 at 10:03 PM, Andreas Färber <afaerber@suse.de> wrote:
> Hi Peter,
>
> Am 04.03.2013 10:01, schrieb Peter Crosthwaite:
>> Hi All. The clock controller module in the Zynq platform has the ability to halt
>> and reset arbitrary devices, including the CPU. We use this feature to implement
>> SMP Linux - the kernel halts CPU1 then rewrites the vector table to the
>> secondary entry point and unhalts.
>>
>> The clock controller however is reasonable generic, in that in has the ability
>> to halt and reset any arbitrary device in the Zynq SoC. The interface for doing
>> this is reasonably consistent. So im looking for unified way to halt and reset
>> arbitrary devices CPUs included. So given we already have reset() defined on the
>> TYPE_DEVICE level, i've added halt as well.
>>
>> This series is based on v3 of the current qom-cpu queue to pick up the move of
>> cpu halt out of the env. From this I attach the cpu::halted bit to the
>> Device::(un)halt function.
>
> I have doubts whether that is a suitable API... My initial thought was:
> Isn't that pretty much realize/unrealize according to Anthony's
> definition via Vcc?

Loss of Vcc generally implies loss of state (unless your device state
is non volatile storage) which is not what I am going for. I'm trying
to suspend for later resumption. Could we hook up the cpu::halted bit
to unrealize/realize and it would work?

 The technical difference I see is that in your case
> devices are still migrated, not sure if that makes a difference,
> assuming unhalt/realize puts them into a defined state (i.e., reset).
>

The defined state for unhalt is exactly what it was when you halted.
Not sure of the migration issues here.

> Reminds me that some time ago we had a discussion about
> enabling/disabling/reconfiguring ISA devices. realize/unrealize might be
> a solution there too, but it may still be tied to the concept of
> hot-plug, which is disabled for ISA and SysBus.
>
Does hotplug support preservation of state? Or does hot-unplug imply
loss or power implying loss of state?

>> Next up, CPUs seem to have a different reset path to normal devices. So I have
>> trivially hooked up CPU::Reset to Device::Reset. This means that anyone who
>> holds a DeviceState pointer to the CPU can reset it properly without actually
>> knowing its a CPU.
>
> If we do that, it needs a better explanation of *why* this is okay.
>

Shouldn't CPUs implement device::reset anyway? Its mandated for all
regular devices that Device::reset function is implemented as a
power-on-reset. I'm not sure why CPUs should be any different
considering they are implementers of TYPE_DEVICE.

Regards,
Peter

> Regards,
> Andreas
>
>>
>> With the halt API I played with changing an existing device over to use it in
>> place of the CPU halted bit (sun4m).
>>
>> I then finally add SMP support to the Zynq machine, and patch the Zynq SLCR
>> (my clock controller) to have DeviceState pointers to the CPUs. device_reset()
>> and device_halt() is where the magic happens.
>>
>> Future work, more devices in Zynq will have halt and reset. My agenda for
>> abstracting away the fact that attached device is a CPU is to allow for
>> consistent implementation and a single code path for the clock controlled
>> devices.
>>
>>
>> Peter A. G. Crosthwaite (3):
>>   xilinx_zynq: added smp support
>>   zynq_slcr: Add links to the CPUs
>>   zynq_slcr: Implement CPU reset and halting
>>
>> Peter Crosthwaite (4):
>>   qdev: Define halting API
>>   qom/cpu.c: Encapsulate cpu halting
>>   qom/cpu.c: Hook CPU reset up to device reset
>>   sun4m: Use halting API to halt/unhalt CPUs
>>
>>  hw/qdev-core.h   |   17 ++++++++++++++
>>  hw/qdev.c        |   18 ++++++++++++++
>>  hw/sun4m.c       |   24 +++++++++---------
>>  hw/xilinx_zynq.c |   66 ++++++++++++++++++++++++++++++++++++++++++++---------
>>  hw/zynq_slcr.c   |   30 ++++++++++++++++++++++++
>>  qom/cpu.c        |   22 ++++++++++++++++++
>>  6 files changed, 153 insertions(+), 24 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 v1 0/7] Reset and Halting modifications + Zynq SMP

[Edgar E. Iglesias]

Hi

On Mon, Mar 04, 2013 at 10:57:49PM +1000, Peter Crosthwaite wrote:
> Hi Andreas,
> 
> On Mon, Mar 4, 2013 at 10:03 PM, Andreas Färber <afaerber@suse.de> wrote:
> > Hi Peter,
> >
> > Am 04.03.2013 10:01, schrieb Peter Crosthwaite:
> >> Hi All. The clock controller module in the Zynq platform has the ability to halt
> >> and reset arbitrary devices, including the CPU. We use this feature to implement
> >> SMP Linux - the kernel halts CPU1 then rewrites the vector table to the
> >> secondary entry point and unhalts.
> >>
> >> The clock controller however is reasonable generic, in that in has the ability
> >> to halt and reset any arbitrary device in the Zynq SoC. The interface for doing
> >> this is reasonably consistent. So im looking for unified way to halt and reset
> >> arbitrary devices CPUs included. So given we already have reset() defined on the
> >> TYPE_DEVICE level, i've added halt as well.
> >>
> >> This series is based on v3 of the current qom-cpu queue to pick up the move of
> >> cpu halt out of the env. From this I attach the cpu::halted bit to the
> >> Device::(un)halt function.
> >
> > I have doubts whether that is a suitable API... My initial thought was:
> > Isn't that pretty much realize/unrealize according to Anthony's
> > definition via Vcc?
> 
> Loss of Vcc generally implies loss of state (unless your device state
> is non volatile storage) which is not what I am going for. I'm trying
> to suspend for later resumption. Could we hook up the cpu::halted bit
> to unrealize/realize and it would work?
> 
>  The technical difference I see is that in your case
> > devices are still migrated, not sure if that makes a difference,
> > assuming unhalt/realize puts them into a defined state (i.e., reset).
> >
> 
> The defined state for unhalt is exactly what it was when you halted.
> Not sure of the migration issues here.


IMO, you'd need to migrate the gated state. For hotunplug you
wouldn't want to migrate I guess.

Regarding the framework a thought is that in the future we might
need to propagte more detailed info about clocks. Not only gated
or enabled, but maybe also frequencies. Most models in QEMU don't
care but for example timers that change their timing depending
on clock-tree setup might benefit.

It would also make it possible to trap guest errors when for
example using I/O with wrong clock setups.

Best regards,
Edgar

Re: [Qemu-devel] [RFC PATCH v1 2/7] qom/cpu.c: Encapsulate cpu halting

[Edgar E. Iglesias]

On Mon, Mar 04, 2013 at 07:01:34PM +1000, Peter Crosthwaite wrote:
> Set up the Device level halting API to halt cpus. This allows holders of a
> pointer to the CPU to halt it from the TYPE_DEVICE abstraction. (i.e. they
> can halt it even if they dont know its a CPU).
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> 
>  qom/cpu.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index e242dcb..00a07d9 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -43,6 +43,20 @@ static void cpu_common_reset(CPUState *cpu)
>      cpu->halted = 0;
>  }
>  
> +static void cpu_common_halt(DeviceState *dev)
> +{
> +    CPUState *s = CPU(dev);
> +
> +    s->halted = 1;
> +}
> +
> +static void cpu_common_unhalt(DeviceState *dev)
> +{
> +    CPUState *s = CPU(dev);
> +
> +    s->halted = 0;
> +}


Hi,

I might be wrong, but doesn't this also need to raise/lower CPU_INTERRUPT_HALT?
(e.g if the CPU is i a tight tcg loop with interrupts disabled)

Cheers,
Edgar

Re: [Qemu-devel] [RFC PATCH v1 0/7] Reset and Halting modifications + Zynq SMP

[Edgar E. Iglesias]

On Mon, Mar 04, 2013 at 07:01:32PM +1000, Peter Crosthwaite wrote:
> Hi All. The clock controller module in the Zynq platform has the ability to halt
> and reset arbitrary devices, including the CPU. We use this feature to implement
> SMP Linux - the kernel halts CPU1 then rewrites the vector table to the
> secondary entry point and unhalts.
> 
> The clock controller however is reasonable generic, in that in has the ability
> to halt and reset any arbitrary device in the Zynq SoC. The interface for doing
> this is reasonably consistent. So im looking for unified way to halt and reset
> arbitrary devices CPUs included. So given we already have reset() defined on the
> TYPE_DEVICE level, i've added halt as well.
> 
> This series is based on v3 of the current qom-cpu queue to pick up the move of
> cpu halt out of the env. From this I attach the cpu::halted bit to the
> Device::(un)halt function.
> 
> Next up, CPUs seem to have a different reset path to normal devices. So I have
> trivially hooked up CPU::Reset to Device::Reset. This means that anyone who
> holds a DeviceState pointer to the CPU can reset it properly without actually
> knowing its a CPU.
> 
> With the halt API I played with changing an existing device over to use it in
> place of the CPU halted bit (sun4m).
> 
> I then finally add SMP support to the Zynq machine, and patch the Zynq SLCR
> (my clock controller) to have DeviceState pointers to the CPUs. device_reset()
> and device_halt() is where the magic happens.
> 
> Future work, more devices in Zynq will have halt and reset. My agenda for
> abstracting away the fact that attached device is a CPU is to allow for
> consistent implementation and a single code path for the clock controlled
> devices.


Hi Peter,

I like the approach. As I mentioned in the other thread, the interface
might need to be extended to propagate more info in the future.

IMO, something like this would be useful now.

Cheers,
Edgar

> 
> 
> Peter A. G. Crosthwaite (3):
>   xilinx_zynq: added smp support
>   zynq_slcr: Add links to the CPUs
>   zynq_slcr: Implement CPU reset and halting
> 
> Peter Crosthwaite (4):
>   qdev: Define halting API
>   qom/cpu.c: Encapsulate cpu halting
>   qom/cpu.c: Hook CPU reset up to device reset
>   sun4m: Use halting API to halt/unhalt CPUs
> 
>  hw/qdev-core.h   |   17 ++++++++++++++
>  hw/qdev.c        |   18 ++++++++++++++
>  hw/sun4m.c       |   24 +++++++++---------
>  hw/xilinx_zynq.c |   66 ++++++++++++++++++++++++++++++++++++++++++++---------
>  hw/zynq_slcr.c   |   30 ++++++++++++++++++++++++
>  qom/cpu.c        |   22 ++++++++++++++++++
>  6 files changed, 153 insertions(+), 24 deletions(-)
> 
>