qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP
@ 2014-01-02  7:30 Peter Crosthwaite
  2014-01-02  7:30 ` [Qemu-devel] [PATCH target-arm v4 1/3] xilinx_zynq: added SMP support: Peter Crosthwaite
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Peter Crosthwaite @ 2014-01-02  7:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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 DeviceState pointers to the CPUs. cpu_reset()
is then called on the appropriate register access.

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 v3:
Removed halting patches
Reduced to minimal change needed for SMP Zynq


Peter Crosthwaite (3):
  xilinx_zynq: added SMP support:
  zynq_slcr: Add links to the CPUs
  zynq_slcr: Implement CPU reset

 hw/arm/xilinx_zynq.c | 73 ++++++++++++++++++++++++++++++++++++++++------------
 hw/misc/zynq_slcr.c  | 23 +++++++++++++++++
 2 files changed, 80 insertions(+), 16 deletions(-)

-- 
1.8.5.2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH target-arm v4 1/3] xilinx_zynq: added SMP support:
  2014-01-02  7:30 [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP Peter Crosthwaite
@ 2014-01-02  7:30 ` Peter Crosthwaite
  2014-01-10 18:08   ` Peter Maydell
  2014-01-02  7:31 ` [Qemu-devel] [PATCH target-arm v4 2/3] zynq_slcr: Add links to the CPUs Peter Crosthwaite
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Peter Crosthwaite @ 2014-01-02  7:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed from v3:
Author reset
s/zynq_cpus/cpus
simplified custom secondary bootloader
Rebased
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/arm/xilinx_zynq.c | 69 ++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 16 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 17251c7..c09ff36 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -27,6 +27,8 @@
 #include "hw/ssi.h"
 #include "qemu/error-report.h"
 
+#define MAX_CPUS 2
+
 #define NUM_SPI_FLASHES 4
 #define NUM_QSPI_FLASHES 2
 #define NUM_QSPI_BUSSES 2
@@ -38,10 +40,37 @@
 
 #define MPCORE_PERIPHBASE 0xF8F00000
 
+/* Dummy bootreg addr to keep ARM bootloader happy. Very top of OCM */
+#define SMP_BOOTREG_ADDR 0xfffffffc
+/* Put SMP bootloader up top of OCM under bootreg  */
+#define SMP_BOOT_ADDR (SMP_BOOTREG_ADDR - sizeof(zynq_smpboot))
+
 static const int dma_irqs[8] = {
     46, 47, 48, 49, 72, 73, 74, 75
 };
 
+/* Entry point for secondary CPU. Zynq Linux SMP protocol is to just reset
+ * the secondary to unpen, so any infinite loop will do the trick. Use a WFI
+ * loop as that will cause the emulated CPU to halt (and remove itself from
+ * the work queue pending an interrupt that never comes).
+ */
+static uint32_t zynq_smpboot[] = {
+    0xe320f003, /* wfi */
+    0xeafffffd, /* b       <b wfi> */
+};
+
+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)
@@ -106,7 +135,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
     const char *kernel_cmdline = args->kernel_cmdline;
     const char *initrd_filename = args->initrd_filename;
     ObjectClass *cpu_oc;
-    ARMCPU *cpu;
+    ARMCPU *cpu[MAX_CPUS];
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
     MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
@@ -122,17 +151,20 @@ static void zynq_init(QEMUMachineInitArgs *args)
     }
     cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
 
-    cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
+    for (n = 0; n < smp_cpus; n++) {
+        cpu[n] = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
 
-    object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
-    if (err) {
-        error_report("%s", error_get_pretty(err));
-        exit(1);
-    }
-    object_property_set_bool(OBJECT(cpu), true, "realized", &err);
-    if (err) {
-        error_report("%s", error_get_pretty(err));
-        exit(1);
+        object_property_set_int(OBJECT(cpu[n]), MPCORE_PERIPHBASE, "reset-cbar",
+                                &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
+            exit(1);
+        }
+        object_property_set_bool(OBJECT(cpu[n]), true, "realized", &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
+            exit(1);
+        }
     }
 
     /* max 2GB ram */
@@ -164,12 +196,14 @@ 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, MPCORE_PERIPHBASE);
-    sysbus_connect_irq(busdev, 0,
-                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
+    for (n = 0; n < smp_cpus; n++) {
+        sysbus_connect_irq(busdev, n,
+                           qdev_get_gpio_in(DEVICE(cpu[n]), ARM_CPU_IRQ));
+    }
 
     for (n = 0; n < 64; n++) {
         pic[n] = qdev_get_gpio_in(dev, n);
@@ -233,7 +267,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_CPU(first_cpu), &zynq_binfo);
@@ -244,7 +281,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,
 };
 
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH target-arm v4 2/3] zynq_slcr: Add links to the CPUs
  2014-01-02  7:30 [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP Peter Crosthwaite
  2014-01-02  7:30 ` [Qemu-devel] [PATCH target-arm v4 1/3] xilinx_zynq: added SMP support: Peter Crosthwaite
@ 2014-01-02  7:31 ` Peter Crosthwaite
  2014-01-10 18:11   ` Peter Maydell
  2014-01-02  7:32 ` [Qemu-devel] [PATCH target-arm v4 3/3] zynq_slcr: Implement CPU reset Peter Crosthwaite
  2014-01-10  4:34 ` [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP Peter Crosthwaite
  3 siblings, 1 reply; 10+ messages in thread
From: Peter Crosthwaite @ 2014-01-02  7:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed since v3:
Revert back to using TYPE_CPU
Changed from v2:
Soften type of CPU to Device
Looped link creator

 hw/arm/xilinx_zynq.c |  4 ++++
 hw/misc/zynq_slcr.c  | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index c09ff36..1445c60 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -193,6 +193,10 @@ static void zynq_init(QEMUMachineInitArgs *args)
 
     dev = qdev_create(NULL, "xilinx,zynq_slcr");
     qdev_init_nofail(dev);
+    object_property_set_link(OBJECT(dev), OBJECT(cpu[0]), "cpu0", NULL);
+    if (smp_cpus > 1) {
+        object_property_set_link(OBJECT(dev), OBJECT(cpu[1]), "cpu1", NULL);
+    }
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
 
     dev = qdev_create(NULL, "a9mpcore_priv");
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index e42a5b0..83ffc3b 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -19,6 +19,8 @@
 #include "hw/sysbus.h"
 #include "sysemu/sysemu.h"
 
+#define NUM_CPUS 2
+
 #ifdef ZYNQ_ARM_SLCR_ERR_DEBUG
 #define DB_PRINT(...) do { \
     fprintf(stderr,  ": %s: ", __func__); \
@@ -122,6 +124,8 @@ typedef struct ZynqSLCRState {
 
     MemoryRegion iomem;
 
+    CPUState *cpus[NUM_CPUS];
+
     union {
         struct {
             uint16_t scl;
@@ -496,10 +500,17 @@ static const MemoryRegionOps slcr_ops = {
 static int zynq_slcr_init(SysBusDevice *dev)
 {
     ZynqSLCRState *s = ZYNQ_SLCR(dev);
+    int i;
 
     memory_region_init_io(&s->iomem, OBJECT(s), &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_CPU,
+                                 (Object **)&s->cpus[i], NULL);
+        g_free(name);
+    }
     return 0;
 }
 
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH target-arm v4 3/3] zynq_slcr: Implement CPU reset
  2014-01-02  7:30 [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP Peter Crosthwaite
  2014-01-02  7:30 ` [Qemu-devel] [PATCH target-arm v4 1/3] xilinx_zynq: added SMP support: Peter Crosthwaite
  2014-01-02  7:31 ` [Qemu-devel] [PATCH target-arm v4 2/3] zynq_slcr: Add links to the CPUs Peter Crosthwaite
@ 2014-01-02  7:32 ` Peter Crosthwaite
  2014-01-10  4:34 ` [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP Peter Crosthwaite
  3 siblings, 0 replies; 10+ messages in thread
From: Peter Crosthwaite @ 2014-01-02  7:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Implement the CPU reset function of the A9_CPU_RST_CTRL register
(offset 0x244).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed from v3:
Author reset
Use CPU reset rather than device reset
use extract32 rather than << &.
Removed halting functionality
changed from v2:
used device halting API instead of talking to the cpu.

 hw/misc/zynq_slcr.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index 83ffc3b..0f94e03 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -116,6 +116,8 @@ typedef enum {
   RESET_MAX
 } ResetValues;
 
+#define A9_CPU_RST_CTRL_RST_SHIFT 0
+
 #define TYPE_ZYNQ_SLCR "xilinx,zynq_slcr"
 #define ZYNQ_SLCR(obj) OBJECT_CHECK(ZynqSLCRState, (obj), TYPE_ZYNQ_SLCR)
 
@@ -349,6 +351,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);
 
@@ -403,6 +406,15 @@ 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 rst = extract32(val, A9_CPU_RST_CTRL_RST_SHIFT + i, 1);
+                    if (rst) {
+                        DB_PRINT("resetting cpu %d\n", i);
+                        cpu_reset(s->cpus[i]);
+                    }
+                }
+            }
             break;
         case 0x300:
             s->apu_ctrl = val;
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP
  2014-01-02  7:30 [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP Peter Crosthwaite
                   ` (2 preceding siblings ...)
  2014-01-02  7:32 ` [Qemu-devel] [PATCH target-arm v4 3/3] zynq_slcr: Implement CPU reset Peter Crosthwaite
@ 2014-01-10  4:34 ` Peter Crosthwaite
  3 siblings, 0 replies; 10+ messages in thread
From: Peter Crosthwaite @ 2014-01-10  4:34 UTC (permalink / raw)
  To: qemu-devel@nongnu.org Developers; +Cc: Peter Maydell

Ping!

On Thu, Jan 2, 2014 at 5:30 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> 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 the resets+unhalts.
>
> This series adds SMP support to the Zynq machine, and patches the Zynq SLCR
> (the clock controller) to have DeviceState pointers to the CPUs. cpu_reset()
> is then called on the appropriate register access.
>
> 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 v3:
> Removed halting patches
> Reduced to minimal change needed for SMP Zynq
>
>
> Peter Crosthwaite (3):
>   xilinx_zynq: added SMP support:
>   zynq_slcr: Add links to the CPUs
>   zynq_slcr: Implement CPU reset
>
>  hw/arm/xilinx_zynq.c | 73 ++++++++++++++++++++++++++++++++++++++++------------
>  hw/misc/zynq_slcr.c  | 23 +++++++++++++++++
>  2 files changed, 80 insertions(+), 16 deletions(-)
>
> --
> 1.8.5.2
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH target-arm v4 1/3] xilinx_zynq: added SMP support:
  2014-01-02  7:30 ` [Qemu-devel] [PATCH target-arm v4 1/3] xilinx_zynq: added SMP support: Peter Crosthwaite
@ 2014-01-10 18:08   ` Peter Maydell
  2014-01-15  6:47     ` Peter Crosthwaite
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2014-01-10 18:08 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: QEMU Developers

On 2 January 2014 07:30, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> Added Linux SMP support for the Xilinx Zynq platform (2x CPUs are
> supported)
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> Changed from v3:
> Author reset
> s/zynq_cpus/cpus
> simplified custom secondary bootloader
> Rebased
> 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/arm/xilinx_zynq.c | 69 ++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 53 insertions(+), 16 deletions(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 17251c7..c09ff36 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -27,6 +27,8 @@
>  #include "hw/ssi.h"
>  #include "qemu/error-report.h"
>
> +#define MAX_CPUS 2
> +
>  #define NUM_SPI_FLASHES 4
>  #define NUM_QSPI_FLASHES 2
>  #define NUM_QSPI_BUSSES 2
> @@ -38,10 +40,37 @@
>
>  #define MPCORE_PERIPHBASE 0xF8F00000
>
> +/* Dummy bootreg addr to keep ARM bootloader happy. Very top of OCM */
> +#define SMP_BOOTREG_ADDR 0xfffffffc

It would probably be nicer to provide your own
reset_secondary hook, and then hw/arm/boot.c won't
ever look at what you set in bootreg_addr.

Looks ok otherwise, though.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH target-arm v4 2/3] zynq_slcr: Add links to the CPUs
  2014-01-02  7:31 ` [Qemu-devel] [PATCH target-arm v4 2/3] zynq_slcr: Add links to the CPUs Peter Crosthwaite
@ 2014-01-10 18:11   ` Peter Maydell
  2014-01-10 20:20     ` Peter Crosthwaite
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2014-01-10 18:11 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: QEMU Developers

On 2 January 2014 07:31, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> The SLCR needs to be able to reset the CPUs, so link the CPUs to the
> SLCR.

> @@ -496,10 +500,17 @@ static const MemoryRegionOps slcr_ops = {
>  static int zynq_slcr_init(SysBusDevice *dev)
>  {
>      ZynqSLCRState *s = ZYNQ_SLCR(dev);
> +    int i;
>
>      memory_region_init_io(&s->iomem, OBJECT(s), &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_CPU,
> +                                 (Object **)&s->cpus[i], NULL);
> +        g_free(name);
> +    }

This is where we get into the nasty questions of how
we ought to be modelling reset. I don't think that
reset controllers ought to work by having direct links
to a pile of QOM device objects. I'd much rather we tried
to work towards modelling this the way the hardware does,
ie a QOM device has one or more inbound GPIO lines
corresponding to the hardware's reset signals, and the
SoC or board wires those up to the reset controller
appropriately.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH target-arm v4 2/3] zynq_slcr: Add links to the CPUs
  2014-01-10 18:11   ` Peter Maydell
@ 2014-01-10 20:20     ` Peter Crosthwaite
  2014-01-10 20:54       ` Andreas Färber
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Crosthwaite @ 2014-01-10 20:20 UTC (permalink / raw)
  To: Peter Maydell, Andreas Färber, Anthony Liguori; +Cc: QEMU Developers

On Sat, Jan 11, 2014 at 4:11 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 2 January 2014 07:31, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> The SLCR needs to be able to reset the CPUs, so link the CPUs to the
>> SLCR.
>
>> @@ -496,10 +500,17 @@ static const MemoryRegionOps slcr_ops = {
>>  static int zynq_slcr_init(SysBusDevice *dev)
>>  {
>>      ZynqSLCRState *s = ZYNQ_SLCR(dev);
>> +    int i;
>>
>>      memory_region_init_io(&s->iomem, OBJECT(s), &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_CPU,
>> +                                 (Object **)&s->cpus[i], NULL);
>> +        g_free(name);
>> +    }
>
> This is where we get into the nasty questions of how
> we ought to be modelling reset. I don't think that
> reset controllers ought to work by having direct links
> to a pile of QOM device objects. I'd much rather we tried
> to work towards modelling this the way the hardware does,
> ie a QOM device has one or more inbound GPIO lines
> corresponding to the hardware's reset signals, and the
> SoC or board wires those up to the reset controller
> appropriately.
>

So all nice solutions to this really want named GPIOs which is
something of a long term sore-point. Are you happy to take a simple
addition of a reset GPIO to ARMCPU  (which itself just calls
cpu_reset) without the need for the big planned GPIO fixups (whether
than be pins of Andreas' QOMification)?

Regards,
Peter

> thanks
> -- PMM
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH target-arm v4 2/3] zynq_slcr: Add links to the CPUs
  2014-01-10 20:20     ` Peter Crosthwaite
@ 2014-01-10 20:54       ` Andreas Färber
  0 siblings, 0 replies; 10+ messages in thread
From: Andreas Färber @ 2014-01-10 20:54 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: Peter Maydell, QEMU Developers, Anthony Liguori

Am 10.01.2014 21:20, schrieb Peter Crosthwaite:
> On Sat, Jan 11, 2014 at 4:11 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 2 January 2014 07:31, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>>> The SLCR needs to be able to reset the CPUs, so link the CPUs to the
>>> SLCR.
>>
>>> @@ -496,10 +500,17 @@ static const MemoryRegionOps slcr_ops = {
>>>  static int zynq_slcr_init(SysBusDevice *dev)
>>>  {
>>>      ZynqSLCRState *s = ZYNQ_SLCR(dev);
>>> +    int i;
>>>
>>>      memory_region_init_io(&s->iomem, OBJECT(s), &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_CPU,
>>> +                                 (Object **)&s->cpus[i], NULL);
>>> +        g_free(name);
>>> +    }
>>
>> This is where we get into the nasty questions of how
>> we ought to be modelling reset. I don't think that
>> reset controllers ought to work by having direct links
>> to a pile of QOM device objects. I'd much rather we tried
>> to work towards modelling this the way the hardware does,
>> ie a QOM device has one or more inbound GPIO lines
>> corresponding to the hardware's reset signals, and the
>> SoC or board wires those up to the reset controller
>> appropriately.
>>
> 
> So all nice solutions to this really want named GPIOs which is
> something of a long term sore-point. Are you happy to take a simple
> addition of a reset GPIO to ARMCPU  (which itself just calls
> cpu_reset) without the need for the big planned GPIO fixups (whether
> than be pins of Andreas' QOMification)?

Pins are Anthony's topic, not mine. :) I rather recently suggested to do
a transparent QOM'ification. I thus have no objections against adding a
reset IRQ! That had BTW been discussed as possible solution for
partial/soft resets in PReP and x86 context.

Andreas

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH target-arm v4 1/3] xilinx_zynq: added SMP support:
  2014-01-10 18:08   ` Peter Maydell
@ 2014-01-15  6:47     ` Peter Crosthwaite
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Crosthwaite @ 2014-01-15  6:47 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On Sat, Jan 11, 2014 at 4:08 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 2 January 2014 07:30, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> Added Linux SMP support for the Xilinx Zynq platform (2x CPUs are
>> supported)
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>> Changed from v3:
>> Author reset
>> s/zynq_cpus/cpus
>> simplified custom secondary bootloader
>> Rebased
>> 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/arm/xilinx_zynq.c | 69 ++++++++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 53 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
>> index 17251c7..c09ff36 100644
>> --- a/hw/arm/xilinx_zynq.c
>> +++ b/hw/arm/xilinx_zynq.c
>> @@ -27,6 +27,8 @@
>>  #include "hw/ssi.h"
>>  #include "qemu/error-report.h"
>>
>> +#define MAX_CPUS 2
>> +
>>  #define NUM_SPI_FLASHES 4
>>  #define NUM_QSPI_FLASHES 2
>>  #define NUM_QSPI_BUSSES 2
>> @@ -38,10 +40,37 @@
>>
>>  #define MPCORE_PERIPHBASE 0xF8F00000
>>
>> +/* Dummy bootreg addr to keep ARM bootloader happy. Very top of OCM */
>> +#define SMP_BOOTREG_ADDR 0xfffffffc
>
> It would probably be nicer to provide your own
> reset_secondary hook, and then hw/arm/boot.c won't
> ever look at what you set in bootreg_addr.
>

Yep, thatll be much cleaner. Thanks. All fixed in v2.

Regards,
Peter

> Looks ok otherwise, though.
>
> thanks
> -- PMM
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-01-15  6:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-02  7:30 [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP Peter Crosthwaite
2014-01-02  7:30 ` [Qemu-devel] [PATCH target-arm v4 1/3] xilinx_zynq: added SMP support: Peter Crosthwaite
2014-01-10 18:08   ` Peter Maydell
2014-01-15  6:47     ` Peter Crosthwaite
2014-01-02  7:31 ` [Qemu-devel] [PATCH target-arm v4 2/3] zynq_slcr: Add links to the CPUs Peter Crosthwaite
2014-01-10 18:11   ` Peter Maydell
2014-01-10 20:20     ` Peter Crosthwaite
2014-01-10 20:54       ` Andreas Färber
2014-01-02  7:32 ` [Qemu-devel] [PATCH target-arm v4 3/3] zynq_slcr: Implement CPU reset Peter Crosthwaite
2014-01-10  4:34 ` [Qemu-devel] [PATCH target-arm v4 0/3] Reset and Halting modifications + Zynq SMP Peter Crosthwaite

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).