Skip to contentSkip to conversation

Reset and Halting modifications + Zynq SMP

RFC PATCH Superseded Peter Crosthwaite 3 participants 13 messages Mar 4, 2013 – Mar 30, 2013 Arm/Zynq architecture

v1 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Peter Crosthwaite

author
#0 lore raw

cover Reset and Halting modifications + Zynq SMP

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(-)
author
#1 lore raw

RFC PATCH 1/7 qdev: Define halting API

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 <[email protected]>

---

 hw/qdev-core.h |   17 +++++++++++++++++
 hw/qdev.c      |   18 ++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)
hw/qdev-core.h +17 -0
index 2486f36..1fee53a 100644
@@ -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);
hw/qdev.c +18 -0
index 689cd54..c338b74 100644
@@ -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;
Signature · 1 line
1.7.0.4
author
#2 lore raw

RFC PATCH 2/7 qom/cpu.c: Encapsulate cpu halting

In reply to 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 <[email protected]>

---

 qom/cpu.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
qom/cpu.c +16 -0
index e242dcb..00a07d9 100644
@@ -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;
 }
Signature · 1 line
1.7.0.4
#3 lore raw

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

Quoted 21 lines · 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 <[email protected]>

---

 qom/cpu.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
qom/cpu.c +3 -0
index e242dcb..00a07d9 100644
@@ -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);

qom/cpu.c static void cpu_common_reset(CPUState *cpu) · line 58

Quoted 10 lines
qom/cpu.c +10 -0
+
+    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

author
#4 lore raw

RFC PATCH 3/7 qom/cpu.c: Hook CPU reset up to device reset

In reply to 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 <[email protected]>

---

 qom/cpu.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
qom/cpu.c +6 -0
index 00a07d9..7084a67 100644
@@ -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;
Signature · 1 line
1.7.0.4
author
#5 lore raw

RFC PATCH 4/7 sun4m: Use halting API to halt/unhalt CPUs

In reply to 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 <[email protected]>

---

 hw/sun4m.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)
hw/sun4m.c +12 -12
index d83d42c..b544782 100644
@@ -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;
Signature · 1 line
1.7.0.4
author
#6 lore raw

RFC PATCH 5/7 xilinx_zynq: added smp support

In reply to Peter Crosthwaite

From: Peter A. G. Crosthwaite <[email protected]>

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

Signed-off-by Peter A. G. Crosthwaite <[email protected]>

---
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(-)
hw/xilinx_zynq.c +47 -12
index 2f67d90..80a2777 100644
@@ -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,
 };
Signature · 1 line
1.7.0.4
author
#7 lore raw

RFC PATCH 6/7 zynq_slcr: Add links to the CPUs

In reply to Peter Crosthwaite

From: Peter A. G. Crosthwaite <[email protected]>

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

Signed-off-by Peter A. G. Crosthwaite <[email protected]>

---
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(-)
hw/xilinx_zynq.c +7 -0
index 80a2777..9be2b3b 100644
@@ -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");
hw/zynq_slcr.c +11 -0
index 27b00f0..4925358 100644
@@ -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;
 }
Signature · 1 line
1.7.0.4
author
#8 lore raw

RFC PATCH 7/7 zynq_slcr: Implement CPU reset and halting

In reply to Peter Crosthwaite

From: Peter A. G. Crosthwaite <[email protected]>

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

Signed-off-by Peter A. G. Crosthwaite <[email protected]>

---
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(-)
hw/zynq_slcr.c +19 -0
index 4925358..945355e 100644
@@ -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;
Signature · 1 line
1.7.0.4
#9 lore raw
In reply to Peter Crosthwaite

Hi Peter,

Am 04.03.2013 10:01, schrieb Peter Crosthwaite:

Quoted 11 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

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

Quoted 33 lines

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(-)
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#10 lore raw

Hi Andreas,

Quoted 14 lines · Andreas Färber

Hi Peter,

Am 04.03.2013 10:01, schrieb Peter Crosthwaite:

Quoted 11 lines

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.

Quoted 7 lines
Quoted 3 lines

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

Quoted 3 lines

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.

Quoted 5 lines

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?

Quoted 7 lines
Quoted 4 lines

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

Quoted 42 lines

Regards,
Andreas

Quoted 25 lines

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

Quoted 8 lines
 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

#11 lore raw

Hi

Quoted 30 lines · Peter Crosthwaite

Hi Andreas,

Quoted 14 lines · Andreas Färber

Hi Peter,

Am 04.03.2013 10:01, schrieb Peter Crosthwaite:

Quoted 11 lines

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.

Quoted 7 lines
Quoted 3 lines

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?

Quoted 7 lines

The technical difference I see is that in your case

Quoted 3 lines

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

#12 lore raw
In reply to Peter Crosthwaite
Quoted 23 lines · 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).

Quoted 8 lines

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

Quoted 22 lines

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(-)