Re: [Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM

2013-08-06 Thread Andreas Färber
Am 06.08.2013 02:00, schrieb pe...@chubb.wattle.id.au:
 Andreas == Andreas Färber afaer...@suse.de writes:
 
 Andreas Am 05.08.2013 11:18, schrieb Peter Maydell:
 On 5 August 2013 02:21, Peter Chubb peter.ch...@nicta.com.au
 wrote:
 Reads to unassigned memory now return non-zero (since patch
 9b8c69243585). This breaks guests runnong on i.MX31 that use the
 cache controller --- they poll forever waiting for the L2CC cache
 invalidate regsiter to be zero.

 Andreas Peter Ch., if you know the exact differences, why don't you
 Andreas just derive an imx-l2cc type (or so) derived from ARM's type,
 Andreas overriding the values mentioned above? Sounds trivial to me.
 
 Because I don't know how -- can you point me at some documentation?

There's no official how-to, but QOM is documented in include/qom/object.h.

May I simply point you to an example:
http://git.qemu.org/?p=qemu.git;a=commit;h=692a76d1c4a32573bf3cc19110c7fa6cc8c93f60

pl061 has Luminary and ARM IDs, with ARM in the base type and Luminary
overriding values. Another idea is to use an abstract base type and
several derived types if the differences are bigger.

Andreas

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



Re: [Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM

2013-08-06 Thread Peter Chubb
OK, this is what I've come up with.  Dunno whether it's right or not
-- the object model is decoupled from the memory model, so  there's no
straightforward way to override just a few of the registers.

At this stage this is just for comment, as I don't really have that
much of a clue about how the object/class hierarchy is meant to work.

---
 hw/arm/kzm.c   |3 +
 hw/misc/arm_l2x0.c |  129 ++---
 2 files changed, 96 insertions(+), 36 deletions(-)

Index: qemu/hw/arm/kzm.c
===
--- qemu.orig/hw/arm/kzm.c  2013-08-07 11:21:48.864692846 +1000
+++ qemu/hw/arm/kzm.c   2013-08-07 11:22:40.292983604 +1000
@@ -33,6 +33,7 @@
  * 0x1fffc000-0x1fff RAM  EMULATED
  * 0x2000-0x2fff Reserved IGNORED
  * 0x3000-0x7fff I.MX31 Internal Register Space
+ *   0x3000-0x3fff L2 Cache Controller PARTIALLY EMULATED
  *   0x43f0 IO_AREA0
  *   0x43f9 UART1 EMULATED
  *   0x43f94000 UART2 EMULATED
@@ -134,6 +135,8 @@ static void kzm_init(QEMUMachineInitArgs
DEVICE_NATIVE_ENDIAN);
 }
 
+sysbus_create_varargs(imx_l2cc, 0x3000, NULL);
+
 kzm_binfo.ram_size = ram_size;
 kzm_binfo.kernel_filename = kernel_filename;
 kzm_binfo.kernel_cmdline = kernel_cmdline;
Index: qemu/hw/misc/arm_l2x0.c
===
--- qemu.orig/hw/misc/arm_l2x0.c2013-08-07 11:21:48.864692846 +1000
+++ qemu/hw/misc/arm_l2x0.c 2013-08-07 11:21:48.860692824 +1000
@@ -21,7 +21,9 @@
 #include hw/sysbus.h
 
 /* L2C-310 r3p2 */
-#define CACHE_ID 0x41c8
+#define PL310_CACHE_ID 0x41c8
+/* L2CC from Freescale */
+#define IMX_PL2CC_CACHE_ID 0xD541
 
 #define TYPE_ARM_L2X0 l2x0
 #define ARM_L2X0(obj) OBJECT_CHECK(L2x0State, (obj), TYPE_ARM_L2X0)
@@ -30,6 +32,7 @@ typedef struct L2x0State {
 SysBusDevice parent_obj;
 
 MemoryRegion iomem;
+uint32_t cache_id;
 uint32_t cache_type;
 uint32_t ctrl;
 uint32_t aux_ctrl;
@@ -66,7 +69,7 @@ static uint64_t l2x0_priv_read(void *opa
 }
 switch (offset) {
 case 0:
-return CACHE_ID;
+return s-cache_id;
 case 0x4:
 /* aux_ctrl values affect cache_type values */
 cache_data = (s-aux_ctrl  (7  17))  15;
@@ -78,23 +81,25 @@ static uint64_t l2x0_priv_read(void *opa
 return s-aux_ctrl;
 case 0x108:
 return s-tag_ctrl;
-case 0x10C:
-return s-data_ctrl;
-case 0xC00:
-return s-filter_start;
-case 0xC04:
-return s-filter_end;
 case 0xF40:
 return 0;
-case 0xF60:
-return 0;
-case 0xF80:
-return 0;
-default:
-qemu_log_mask(LOG_GUEST_ERROR,
-  l2x0_priv_read: Bad offset %x\n, (int)offset);
-break;
 }
+if (s-cache_id == PL310_CACHE_ID) {
+switch (offset) {
+case 0x10C:
+return s-data_ctrl;
+case 0xC00:
+return s-filter_start;
+case 0xC04:
+return s-filter_end;
+case 0xF60:
+return 0;
+case 0xF80:
+return 0;
+}
+}
+qemu_log_mask(LOG_GUEST_ERROR,
+  l2x0_priv_read: Bad offset %x\n, (int)offset);
 return 0;
 }
 
@@ -107,6 +112,7 @@ static void l2x0_priv_write(void *opaque
 /* ignore */
 return;
 }
+
 switch (offset) {
 case 0x100:
 s-ctrl = value  1;
@@ -114,29 +120,32 @@ static void l2x0_priv_write(void *opaque
 case 0x104:
 s-aux_ctrl = value;
 break;
-case 0x108:
-s-tag_ctrl = value;
-break;
-case 0x10C:
-s-data_ctrl = value;
-break;
-case 0xC00:
-s-filter_start = value;
-break;
-case 0xC04:
-s-filter_end = value;
-break;
 case 0xF40:
 return;
-case 0xF60:
-return;
-case 0xF80:
-return;
-default:
-qemu_log_mask(LOG_GUEST_ERROR,
-  l2x0_priv_write: Bad offset %x\n, (int)offset);
-break;
 }
+
+if (s-cache_id == PL310_CACHE_ID) {
+switch (offset) {
+case 0x108:
+s-tag_ctrl = value;
+break;
+case 0x10C:
+s-data_ctrl = value;
+break;
+case 0xC00:
+s-filter_start = value;
+break;
+case 0xC04:
+s-filter_end = value;
+break;
+case 0xF60:
+return;
+case 0xF80:
+return;
+}
+}
+qemu_log_mask(LOG_GUEST_ERROR,
+  l2x0_priv_write: Bad offset %x\n, (int)offset);
 }
 
 static void l2x0_priv_reset(DeviceState *dev)
@@ -184,16 +193,64 @@ static void l2x0_class_init(ObjectClass
 dc-reset = l2x0_priv_reset;
 }

Re: [Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM

2013-08-05 Thread Peter Maydell
On 5 August 2013 02:21, Peter Chubb peter.ch...@nicta.com.au wrote:
 Reads to unassigned memory now return non-zero (since patch
 9b8c69243585). This breaks guests runnong on i.MX31 that use the
 cache controller --- they poll forever waiting for the L2CC cache
 invalidate regsiter to be zero.

That commit claims it was just restoring the previous
behaviour, so it shouldn't break guests, surely?

-- PMM



Re: [Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM

2013-08-05 Thread Andreas Färber
Am 05.08.2013 11:18, schrieb Peter Maydell:
 On 5 August 2013 02:21, Peter Chubb peter.ch...@nicta.com.au wrote:
 Reads to unassigned memory now return non-zero (since patch
 9b8c69243585). This breaks guests runnong on i.MX31 that use the
 cache controller --- they poll forever waiting for the L2CC cache
 invalidate regsiter to be zero.
 
 That commit claims it was just restoring the previous
 behaviour, so it shouldn't break guests, surely?

See Jan's patches on the list. PReP was reported affected, too.

Peter Ch., if you know the exact differences, why don't you just derive
an imx-l2cc type (or so) derived from ARM's type, overriding the values
mentioned above? Sounds trivial to me.

Regards,
Andreas

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



Re: [Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM

2013-08-05 Thread Peter Chubb
 Peter == Peter Maydell peter.mayd...@linaro.org writes:

Peter On 5 August 2013 02:21, Peter Chubb peter.ch...@nicta.com.au
Peter wrote:
 Reads to unassigned memory now return non-zero (since patch
 9b8c69243585). This breaks guests runnong on i.MX31 that use the
 cache controller --- they poll forever waiting for the L2CC cache
 invalidate regsiter to be zero.

Peter That commit claims it was just restoring the previous
Peter behaviour, so it shouldn't break guests, surely?

The behaviour was introduced in 2008 in commit e18231a3 --- The KZM
port only went in last year and assumed the then-current behaviour.

Peter C
--
Dr Peter Chubb  peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au  Software Systems Research Group/NICTA



Re: [Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM

2013-08-05 Thread peter
 Andreas == Andreas Färber afaer...@suse.de writes:

Andreas Am 05.08.2013 11:18, schrieb Peter Maydell:
 On 5 August 2013 02:21, Peter Chubb peter.ch...@nicta.com.au
 wrote:
 Reads to unassigned memory now return non-zero (since patch
 9b8c69243585). This breaks guests runnong on i.MX31 that use the
 cache controller --- they poll forever waiting for the L2CC cache
 invalidate regsiter to be zero.
 
Andreas Peter Ch., if you know the exact differences, why don't you
Andreas just derive an imx-l2cc type (or so) derived from ARM's type,
Andreas overriding the values mentioned above? Sounds trivial to me.

Because I don't know how -- can you point me at some documentation?

Peter C



Re: [Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM

2013-08-05 Thread Peter Chubb
 Andreas == Andreas Färber afaer...@suse.de writes:


Andreas Peter Ch., if you know the exact differences, why don't you
Andreas just derive an imx-l2cc type (or so) derived from ARM's type,
Andreas overriding the values mentioned above? Sounds trivial to me.

For what it's worth, here's a diff between the arm_l2x0.c
implementation and a working imx_l2cc.c implementation.

Most of the diffs are name change; but there are substantive
differences to the initial values, and to which registers are
supported.  This makes the State variable smaller.

So it's a bit more than just overriding a few constants.

--- arm_l2x0.c  2013-08-06 09:59:30.008468028 +1000
+++ imx-l2cc-indep.c2013-08-06 10:49:38.021514073 +1000
@@ -1,7 +1,10 @@
 /*
- * ARM dummy L210, L220, PL310 cache controller.
+ * IMX dummy level 2 cache controller
  *
- * Copyright (c) 2010-2012 Calxeda
+ * Copyright (c) 2013 NICTA Peter Chubb
+ *
+ * Based on the PL210 implementation in arm_l2x0.c
+ * Differences: different Cache ID and aux control register values.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -20,46 +23,40 @@
 
 #include hw/sysbus.h
 
-/* L2C-310 r3p2 */
-#define CACHE_ID 0x41c8
 
-#define TYPE_ARM_L2X0 l2x0
-#define ARM_L2X0(obj) OBJECT_CHECK(L2x0State, (obj), TYPE_ARM_L2X0)
+#define CACHE_ID 0xd541
+#define CACHE_TYPE_DEFAULT 0x1C100100 
+
+#define TYPE_IMX_L2CC imx_l2cc
+#define IMX_L2CC(obj) OBJECT_CHECK(L2ccState, (obj), TYPE_IMX_L2CC)
 
-typedef struct L2x0State {
+typedef struct L2ccState {
 SysBusDevice parent_obj;
 
 MemoryRegion iomem;
 uint32_t cache_type;
 uint32_t ctrl;
 uint32_t aux_ctrl;
-uint32_t data_ctrl;
-uint32_t tag_ctrl;
-uint32_t filter_start;
-uint32_t filter_end;
-} L2x0State;
+} L2ccState;
 
-static const VMStateDescription vmstate_l2x0 = {
-.name = l2x0,
+static const VMStateDescription vmstate_l2cc = {
+.name = imx_l2cc,
 .version_id = 1,
 .minimum_version_id = 1,
 .fields = (VMStateField[]) {
-VMSTATE_UINT32(ctrl, L2x0State),
-VMSTATE_UINT32(aux_ctrl, L2x0State),
-VMSTATE_UINT32(data_ctrl, L2x0State),
-VMSTATE_UINT32(tag_ctrl, L2x0State),
-VMSTATE_UINT32(filter_start, L2x0State),
-VMSTATE_UINT32(filter_end, L2x0State),
+VMSTATE_UINT32(ctrl, L2ccState),
+VMSTATE_UINT32(cache_type, L2ccState),
+VMSTATE_UINT32(aux_ctrl, L2ccState),
 VMSTATE_END_OF_LIST()
 }
 };
 
 
-static uint64_t l2x0_priv_read(void *opaque, hwaddr offset,
+static uint64_t l2cc_priv_read(void *opaque, hwaddr offset,
unsigned size)
 {
 uint32_t cache_data;
-L2x0State *s = (L2x0State *)opaque;
+L2ccState *s = (L2ccState *)opaque;
 offset = 0xfff;
 if (offset = 0x730  offset  0x800) {
 return 0; /* cache ops complete */
@@ -76,32 +73,20 @@
 return s-ctrl;
 case 0x104:
 return s-aux_ctrl;
-case 0x108:
-return s-tag_ctrl;
-case 0x10C:
-return s-data_ctrl;
-case 0xC00:
-return s-filter_start;
-case 0xC04:
-return s-filter_end;
 case 0xF40:
 return 0;
-case 0xF60:
-return 0;
-case 0xF80:
-return 0;
 default:
 qemu_log_mask(LOG_GUEST_ERROR,
-  l2x0_priv_read: Bad offset %x\n, (int)offset);
+  l2cc_priv_read: Bad offset %x\n, (int)offset);
 break;
 }
 return 0;
 }
 
-static void l2x0_priv_write(void *opaque, hwaddr offset,
+static void l2cc_priv_write(void *opaque, hwaddr offset,
 uint64_t value, unsigned size)
 {
-L2x0State *s = (L2x0State *)opaque;
+L2ccState *s = (L2ccState *)opaque;
 offset = 0xfff;
 if (offset = 0x730  offset  0x800) {
 /* ignore */
@@ -114,86 +99,61 @@
 case 0x104:
 s-aux_ctrl = value;
 break;
-case 0x108:
-s-tag_ctrl = value;
-break;
-case 0x10C:
-s-data_ctrl = value;
-break;
-case 0xC00:
-s-filter_start = value;
-break;
-case 0xC04:
-s-filter_end = value;
-break;
 case 0xF40:
 return;
-case 0xF60:
-return;
-case 0xF80:
-return;
 default:
 qemu_log_mask(LOG_GUEST_ERROR,
-  l2x0_priv_write: Bad offset %x\n, (int)offset);
+  l2cc_priv_write: Bad offset %x\n, (int)offset);
 break;
 }
 }
 
-static void l2x0_priv_reset(DeviceState *dev)
+static void l2cc_priv_reset(DeviceState *dev)
 {
-L2x0State *s = ARM_L2X0(dev);
+L2ccState *s = IMX_L2CC(dev);

 s-ctrl = 0;
-s-aux_ctrl = 0x0202;
-s-tag_ctrl = 0;
-s-data_ctrl = 0;
-s-filter_start = 0;
-s-filter_end = 0;
+s-cache_type = CACHE_TYPE_DEFAULT;
+s-aux_ctrl = 0xE4020FFF;
 }
 
-static const MemoryRegionOps 

[Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM

2013-08-04 Thread Peter Chubb

Reads to unassigned memory now return non-zero (since patch
9b8c69243585).  This breaks guests runnong on i.MX31 that use the
cache controller --- they poll forever waiting for the L2CC cache
invalidate regsiter to be zero.

This patch adds in an L2 cache controller.  It's not quite right ---
it reuses the PL2x0 implementation that  is already in QEMU.  The
differences however are minor --- a different ID, a different initial
value for the aux control register (because Freescale have used
some of the reserved bits), and the pl2x0 implements registers that
are not present in the Freescale cache controller.

Signed-off-by: Peter Chubb peter.ch...@nicta.com.au

diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index bd6c05c..018fc81 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -33,6 +33,7 @@
  * 0x1fffc000-0x1fff RAM  EMULATED
  * 0x2000-0x2fff Reserved IGNORED
  * 0x3000-0x7fff I.MX31 Internal Register Space
+ *   0x3000-0x3fff L2 Cache Controller PARTIALLY EMULATED
  *   0x43f0 IO_AREA0
  *   0x43f9 UART1 EMULATED
  *   0x43f94000 UART2 EMULATED
@@ -134,6 +135,15 @@ static void kzm_init(QEMUMachineInitArgs *args)
DEVICE_NATIVE_ENDIAN);
 }
 
+/*
+ * The i.MX L2CC  is almost the same as the PL210
+ * except for a different ID (the implementor bits are different)
+ * and the `reserved' bits in the auxilliary control register
+ * are implemented.  The l2x0 qemu implementation is for a superset
+ * of the PL210.
+ */
+sysbus_create_varargs(l2x0, 0x3000, NULL);
+
 kzm_binfo.ram_size = ram_size;
 kzm_binfo.kernel_filename = kernel_filename;
 kzm_binfo.kernel_cmdline = kernel_cmdline;


--
Dr Peter Chubb  peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au  Software Systems Research Group/NICTA