Reset and Halting modifications + Zynq SMP

Convert magic numbers for OCM to Macros.

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

---

 hw/arm/xilinx_zynq.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

hw/arm/xilinx_zynq.c +0 -0

index 98e0958..33a47e0 100644

Added Linux SMP support for the Xilinx Zynq platform (2x CPUs are supported)

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

Implement the CPU reset IO line of the A9_CPU_RST_CTRL register (offset 0x244). This is trivial GPIO mapping straight to the register bits.

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

Implement a reset GPIO for ARM CPUs. This allows individual reset of ARM CPUs from device land without the need for the much unwanted reset API calls.

The CPU is halted as long as the pin is held in reset. Releasing the reset starts the CPU running again.

target-arm/cpu.c +2 -0

+static void arm_cpu_reset_gpio(void *opaque, int irq, int level)
+{

Quoted 16 lines · Peter Crosthwaite

Implement a reset GPIO for ARM CPUs. This allows individual reset of ARM CPUs from device land without the need for the much unwanted reset API calls.

The CPU is halted as long as the pin is held in reset. Releasing the reset starts the CPU running again.

target-arm/cpu.c +9 -0

+static void arm_cpu_reset_gpio(void *opaque, int irq, int level)
+{
+    CPUState *cpu = opaque;
+
+    if (level) {
+        cpu_reset(cpu);
+        cpu_interrupt(cpu, CPU_INTERRUPT_HALT);
+    } else {
+        cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);

target-arm/cpu.c static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level) · line 211

Quoted 3 lines

target-arm/cpu.c +3 -0

+        cpu_interrupt(cpu, CPU_INTERRUPT_EXITTB);
+    }
+}

I don't think this will work properly. For instance, cpu_exec() will bring the CPU out of halt if an inbound interrupt arrives, but we should stay in reset until the reset line is deasserted.

Also ideally speaking we should probably do the reset actions on the falling edge of reset, not the rising edge.

Does this work properly when we're running under KVM rather than using the TCG CPU?

Is there anything really ARM-specific in this reset_gpio function, or could it be implemented at a common level for all target architectures?

target-arm/cpu.h line 85

Quoted 8 lines

target-arm/cpu.h +5 -3

-/* Meanings of the ARMCPU object's two inbound GPIO lines */
-#define ARM_CPU_IRQ 0
-#define ARM_CPU_FIQ 1
+/* Meanings of the ARMCPU object's inbound GPIO lines.  */
+#define ARM_CPU_IRQ         0
+#define ARM_CPU_FIQ         1
+/* reset GPIO is inited after irqs, so its index is one past FIQ */
+#define ARM_CPU_RESET       (ARM_CPU_FIQ + 1)

ARMv8 also has an SError which probably ought to follow IRQ/FIQ, but I think we can safely renumber GPIO lines without breaking migration.

thanks
-- PMM

Quoted 18 lines · Peter Maydell

Quoted 16 lines · Peter Crosthwaite

Implement a reset GPIO for ARM CPUs. This allows individual reset of ARM CPUs from device land without the need for the much unwanted reset API calls.

The CPU is halted as long as the pin is held in reset. Releasing the reset starts the CPU running again.

target-arm/cpu.c +9 -0

+static void arm_cpu_reset_gpio(void *opaque, int irq, int level)
+{
+    CPUState *cpu = opaque;
+
+    if (level) {
+        cpu_reset(cpu);
+        cpu_interrupt(cpu, CPU_INTERRUPT_HALT);
+    } else {
+        cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);

Quoted 9 lines

target-arm/cpu.c static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level) · line 211

Quoted 3 lines

target-arm/cpu.c +3 -0

+        cpu_interrupt(cpu, CPU_INTERRUPT_EXITTB);
+    }
+}

I don't think this will work properly. For instance, cpu_exec() will bring the CPU out of halt if an inbound interrupt arrives, but we should stay in reset until the reset line is deasserted.

I see. I guess I'm going to have to save the reset pin state as a boolean in CPU state (and I guess that means is should be migratable). Then inhibit the true return from cpu_has_work when the pin is asserted.

Quoted 3 lines

Also ideally speaking we should probably do the reset actions on the falling edge of reset, not the rising edge.

Any particular reason? I would have thought that any externally visible state would best be reset ASAP. For level sensitive behavior, the transitionals should happen going into the active level. Unless ARM CPU resets are actually falling edge sensitive (in which case the CPU would continue to run while the reset is held).

Quoted 2 lines

Does this work properly when we're running under KVM rather than using the TCG CPU?

I must confess no, I explicitly LOG_UNIMP for KVM, as I have no means to develop or test ARM KVM.

Quoted 4 lines

Is there anything really ARM-specific in this reset_gpio function, or could it be implemented at a common level for all target architectures?

Not yet, but probably will be ARM specific once I add the cpu reset pin state. Unless Andreas is happy for that pin state and all this code to go up to the base TYPE_CPU class.

I wonder however, whether different arch will have level/edge/high/low variances in reset behavior that must be accommodated.

Quoted 29 lines · Peter Crosthwaite

Quoted 18 lines · Peter Maydell

Quoted 16 lines · Peter Crosthwaite

Implement a reset GPIO for ARM CPUs. This allows individual reset of ARM CPUs from device land without the need for the much unwanted reset API calls.

The CPU is halted as long as the pin is held in reset. Releasing the reset starts the CPU running again.

target-arm/cpu.c +9 -0

+static void arm_cpu_reset_gpio(void *opaque, int irq, int level)
+{
+    CPUState *cpu = opaque;
+
+    if (level) {
+        cpu_reset(cpu);
+        cpu_interrupt(cpu, CPU_INTERRUPT_HALT);
+    } else {
+        cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);

Quoted 9 lines

target-arm/cpu.c static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level) · line 211

Quoted 3 lines

target-arm/cpu.c +3 -0

+        cpu_interrupt(cpu, CPU_INTERRUPT_EXITTB);
+    }
+}

I don't think this will work properly. For instance, cpu_exec() will bring the CPU out of halt if an inbound interrupt arrives, but we should stay in reset until the reset line is deasserted.

Quoted 4 lines

I see. I guess I'm going to have to save the reset pin state as a boolean in CPU state (and I guess that means is should be migratable). Then inhibit the true return from cpu_has_work when the pin is asserted.

Maybe we can do it with a CPU_INTERRUPT_RESET ?

Quoted 9 lines

Quoted 3 lines

Also ideally speaking we should probably do the reset actions on the falling edge of reset, not the rising edge.

Any particular reason? I would have thought that any externally visible state would best be reset ASAP. For level sensitive behavior, the transitionals should happen going into the active level. Unless ARM CPU resets are actually falling edge sensitive (in which case the CPU would continue to run while the reset is held).

I'm not an expert here, but what I thought happened was:
* on asserting edge of reset CPU stops running and starts resetting its internal state
* on real hardware you have to hold the line asserted for N clocks for this to finish
* on deasserting edge of reset, CPU samples config lines and starts running again

so you should be able to say 'assert reset; change state of configuration signals; deassert reset', for instance. (On QEMU that would probably be 'assert reset; change r/w QOM properties; deassert reset'). Slightly less theoretically, M-class CPUs do the initial read of the PC from memory in cpu_reset(), so if you do that on asserting edge then you prevent "hold CPU in reset and modify the vector table before releasing CPU".

So at least some of what we do in cpu_reset() ought I think to be done only on deasserting; unless you really want to split reset into two phases, it seems easiest just to hold the CPU not-running while reset is asserted and call cpu_reset() when it is deasserted.

Quoted 5 lines

Quoted 2 lines

Does this work properly when we're running under KVM rather than using the TCG CPU?

I must confess no, I explicitly LOG_UNIMP for KVM, as I have no means to develop or test ARM KVM.

I definitely don't want to take this if it doesn't cope with KVM. I guess I'll have to take a look at how that would work.

Quoted 8 lines

Quoted 4 lines

Is there anything really ARM-specific in this reset_gpio function, or could it be implemented at a common level for all target architectures?

Not yet, but probably will be ARM specific once I add the cpu reset pin state. Unless Andreas is happy for that pin state and all this code to go up to the base TYPE_CPU class.

It might be possible for most of the mechanics to be shared (CPU_INTERRUPT_RESET and what setting/clearing it does), just not the gpio pin.

Quoted 2 lines

I wonder however, whether different arch will have level/edge/high/low variances in reset behavior that must be accommodated.

Mmm.

thanks
-- PMM

Quoted 4 lines

Is there anything really ARM-specific in this reset_gpio function, or could it be implemented at a common level for all target architectures?

Not yet, but probably will be ARM specific once I add the cpu reset pin state. Unless Andreas is happy for that pin state and all this code to go up to the base TYPE_CPU class.

I wonder however, whether different arch will have level/edge/high/low variances in reset behavior that must be accommodated.

Andreas, you want this in CPU or should we leave it here in ARM land?

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 the resets+unhalts.

This series adds SMP support to the Zynq machine, and patches the Zynq SLCR (the clock controller) to have GPIOs connected to the CPUs. The GPIOs cause and ARM CPU reset.

Only the reset side is implemented (which is good enough for SMP linux as it stands). Future work is to implement the halting behaviour as well.

changed since v4 (PMM review):
Convert to GPIO scheme
Implemented custom secondary cpu reset
OCM Macro cleanup
changed since v3:
Removed halting patches
Reduced to minimal change needed for SMP Zynq

Peter Crosthwaite (5):
arm: zynq: Macroify OCM Base and Size
arm: zynq: added SMP support
zynq_slcr: Implement CPU reset
arm: Implement reset GPIO.
arm: zynq: Connect CPU resets to SLCR

 hw/arm/xilinx_zynq.c | 87 ++++++++++++++++++++++++++++++++++++++++------------
 hw/misc/zynq_slcr.c  | 16 ++++++++++
 target-arm/cpu.c     | 23 ++++++++++++++
 target-arm/cpu.h     |  8 +++--
 4 files changed, 112 insertions(+), 22 deletions(-)

--
1.8.5.3