Data Driven device registers + Zynq DEVCFG

Hi All. This is a new scheme i've come up with handling device registers in a data driven way. My motivation for this is to factor out a lot of the access checking that seems to be replicated in every device. See P2 commit message for further discussion.

P1 is a trivial addition to bitops.h
P2 is the main patch, adds the register definition functionality P3 adds helpers that glue the register API to the Memory API
P4 is an example new device (the Xilinx Zynq devcfg) that uses this scheme. P5 adds devcfg to the Zynq machine model

This devcfg device was particularly finnicky with per-bit restrictions which prompted all this. I'm also looking for a higher-than-usual modelling fidelity on the register space, with semantics defined for random reserved bits in-between otherwise consistent fields.

Here's an example of the qemu_log output for the devcfg device. This is produced by now generic sharable code:

/machine/unattached/device[44]:Addr 0x000008:CFG: write of value 00000508 /machine/unattached/device[44]:Addr 0x000080:MCTRL: write of value 00800010 /machine/unattached/device[44]:Addr 0x000010:INT_MASK: write of value ffffffff /machine/unattached/device[44]:Addr 00000000:CTRL: write of value 0c00607f

And an example of a rogue guest banging on a bad bit:

/machine/unattached/device[44]:Addr 0x000014:STATUS bits 0x000001 may not be \
written to 1

Changed from v3:
Rebased (its been a while)
Added reserved bits.
Cleaner separation of decode and access components (Patch 3) Changed from v2:
Fixed for hw/ re-orginisation (Paolo review)
Simplified and optimized (PMM and Gerd review)
Changed from v1:
Added ONES macro patch
Dropped bogus former patch 1 (PMM review)
Addressed Blue, Gerd and MST comments.
Simplified to be more Memory API compatible.
Added Memory API helpers.
Please see discussion already on list and commit msgs for more detail.

Peter Crosthwaite (6):
bitops: Add ONES macro
register: Add Register API
register: Add Memory API glue
register: Add support for decoding information
xilinx_devcfg: Zynq devcfg device model
xilinx_zynq: added devcfg to machine model

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/xilinx_zynq.c            |   8 +
 hw/core/Makefile.objs           |   1 +
 hw/core/register.c              | 245 +++++++++++++++++++++
 hw/dma/Makefile.objs            |   1 +
 hw/dma/xilinx_devcfg.c          | 462 ++++++++++++++++++++++++++++++++++++++++
 include/hw/register.h           | 164 ++++++++++++++
 include/qemu/bitops.h           |   2 +
 8 files changed, 884 insertions(+)
 create mode 100644 hw/core/register.c
 create mode 100644 hw/dma/xilinx_devcfg.c
 create mode 100644 include/hw/register.h

1.9.1.1.gbb9f595

Little macro that just gives you N ones (justified to LSB).

---

 include/qemu/bitops.h | 2 ++
 1 file changed, 2 insertions(+)

index 340b1e7..b9fd60f 100644
@@ -403,4 +403,6 @@ static inline uint64_t deposit64(uint64_t value, int start, int length,
     return (value & ~mask) | ((fieldval << start) & mask);
 }
 
+#define ONES(num) ((num) == 64 ? ~0ull : (1ull << (num)) - 1)
+
 #endif

1.9.1.1.gbb9f595

This API provides some encapsulation of registers and factors our some common functionality to common code. Bits of device state (usually MMIO registers), often have all sorts of access restrictions and semantics associated with them. This API allow you to define what those restrictions are on a bit-by-bit basis.

Helper functions are then used to access the register which observe the semantics defined by the RegisterAccessInfo struct.

Some features:
Bits can be marked as read_only (ro field)
Bits can be marked as write-1-clear (w1c field)
Bits can be marked as reserved (rsvd field)
Reset values can be defined (reset)
Bits can throw guest errors when written certain values (ge0, ge1) Bits can throw unimp errors when written certain values (ui0, ui1) Bits can be marked clear on read (cor)
Pre and post action callbacks can be added to read and write ops Verbose debugging info can be enabled/disabled

Useful for defining device register spaces in a data driven way. Cuts down on a lot of the verbosity and repetition in the switch-case blocks in the standard foo_mmio_read/write functions.

Also useful for automated generation of device models from hardware design sources.

---
changed from v2:
Simplified! Removed pre-read, nwx, wo
Removed byte loops (Gerd Review)
Made data pointer optional
Added fast paths for simple registers
Moved into hw/core and include/hw (Paolo Review)
changed from v1:
Rebranded as the "Register API" - I think thats probably what it is. Near total rewrite of implementation.
De-arrayified reset (this is client/Memory APIs job).
Moved out of bitops into its own file (Blue review)
Added debug, the register pointer, and prefix to a struct (Blue Review) Made 64-bit to play friendlier with memory API (Blue review) Made backend storage uint8_t (MST review)
Added read/write callbacks (Blue review)
Added ui0, ui1 (Blue review)
Moved re-purposed width (now byte width defining actual storage size) Arrayified ge0, ge1 (ui0, ui1 too) and added .reason
Added wo field (not an April fools joke - this has genuine meaning here) Added we mask to write accessor

 hw/core/Makefile.objs |   1 +
 hw/core/register.c    | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/register.h | 131 +++++++++++++++++++++++++++++++++
 3 files changed, 329 insertions(+)
 create mode 100644 hw/core/register.c
 create mode 100644 include/hw/register.h

index 5377d05..7839f46 100644
@@ -4,6 +4,7 @@ common-obj-y += fw-path-provider.o
 # irq.o needed for qdev GPIO handling:
 common-obj-y += irq.o
 common-obj-y += hotplug.o
+common-obj-y += register.o
 
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 common-obj-$(CONFIG_XILINX_AXI) += stream.o

1.9.1.1.gbb9f595

Add memory io handlers that glue the register API to the memory API. Just translation functions at this stage. Although it does allow for devices to be created without all-in-one mmio r/w handlers.

---
changed from v2:
Added fast path to register_write_memory to skip endianness bitbashing

 hw/core/register.c    | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/register.h | 12 ++++++++++++
 2 files changed, 60 insertions(+)

index 96712f2..29e5f04 100644
@@ -195,3 +195,51 @@ void register_reset(RegisterInfo *reg)
 
     register_write_val(reg, reg->access->reset);
 }
+
+static inline void register_write_memory(void *opaque, hwaddr addr,
+                                         uint64_t value, unsigned size, bool be)
+{
+    RegisterInfo *reg = opaque;
+    uint64_t we = ~0;
+    int shift = 0;
+
+    if (reg->data_size != size) {
+        we = (size == 8) ? ~0ull : (1ull << size * 8) - 1;
+        shift = 8 * (be ? reg->data_size - size - addr : addr);
+    }
+
+    assert(size + addr <= reg->data_size);
+    register_write(reg, value << shift, we << shift);
+}
+
+void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value,
+                              unsigned size)
+{
+    register_write_memory(opaque, addr, value, size, true);
+}
+
+
+void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value,
+                              unsigned size)
+{
+    register_write_memory(opaque, addr, value, size, false);
+}
+
+static inline uint64_t register_read_memory(void *opaque, hwaddr addr,
+                                            unsigned size, bool be)
+{
+    RegisterInfo *reg = opaque;
+    int shift = 8 * (be ? reg->data_size - size - addr : addr);
+
+    return register_read(reg) >> shift;
+}
+
+uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size)
+{
+    return register_read_memory(opaque, addr, size, true);
+}
+
+uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size)
+{
+    return register_read_memory(opaque, addr, size, false);
+}

index f747a7f..c595ed6 100644
@@ -87,6 +87,8 @@ struct RegisterAccessInfo {
  * @prefix: String prefix for log and debug messages
  *
  * @opaque: Opaque data for the register
+ *
+ * @mem: optional Memory region for the register
  */
 
 struct RegisterInfo {

@@ -102,6 +104,8 @@ struct RegisterInfo {
     /* private */
     bool read_lite;
     bool write_lite;
+
+    MemoryRegion mem;
 };
 
 /**

@@ -128,4 +132,12 @@ uint64_t register_read(RegisterInfo *reg);
 
 void register_reset(RegisterInfo *reg);
 
+void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value,
+                              unsigned size);
+void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value,
+                              unsigned size);
+
+uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size);
+uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size);
+
 #endif

1.9.1.1.gbb9f595

Allow defining of optional address decoding information in register definitions. This is useful for clients that want to associate registers with specific addresses.

---

 include/hw/register.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

index c595ed6..d7f602f 100644
@@ -15,6 +15,7 @@
 
 typedef struct RegisterInfo RegisterInfo;
 typedef struct RegisterAccessInfo RegisterAccessInfo;
+typedef struct RegisterDecodeInfo RegisterDecodeInfo;
 
 /**
  * A register access error message

@@ -54,6 +55,11 @@ typedef struct RegisterAccessError {
  * allowing this function to modify the value before return to the client.
  */
 
+#define REG_DECODE_READ (1 << 0)
+#define REG_DECODE_WRITE (1 << 1)
+#define REG_DECODE_EXECUTE (1 << 2)
+#define REG_DECODE_RW (REG_DECODE_READ | REG_DECODE_WRITE)
+
 struct RegisterAccessInfo {
     const char *name;
     uint64_t ro;

@@ -71,6 +77,14 @@ struct RegisterAccessInfo {
     void (*post_write)(RegisterInfo *reg, uint64_t val);
 
     uint64_t (*post_read)(RegisterInfo *reg, uint64_t val);
+
+    size_t storage;
+    int data_size;
+
+    struct {
+        hwaddr addr;
+        uint8_t flags;
+    } decode;
 };
 
 /**

@@ -108,6 +122,13 @@ struct RegisterInfo {
     MemoryRegion mem;
 };
 
+
+struct RegisterDecodeInfo {
+    RegisterInfo *reg;
+    hwaddr addr;
+    unsigned len;
+};
+
 /**
  * write a value to a register, subject to its restrictions
  * @reg: register to write to

1.9.1.1.gbb9f595

Minimal device model for devcfg module of Zynq. DMA capabilities and interrupt generation supported.

---
Changed since v3:
Stylistic updates.
Changed over to new decoding scheme.
Use .rsvd in definitions as appropriate.
Author reset (s/petalogix/xilinx).
Changed since v2:
Some QOM styling updates.
Re-implemented nw0 for lock register as pre_write Changed since v1:
Rebased against new version of Register API.
Use action callbacks for side effects rather than switch. Documented reasons for ge0, ge1 (Verbatim from TRM) Added ui1 definitions for unimplemented major features Removed dead lock code

 default-configs/arm-softmmu.mak |   1 +
 hw/dma/Makefile.objs            |   1 +
 hw/dma/xilinx_devcfg.c          | 462 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 464 insertions(+)
 create mode 100644 hw/dma/xilinx_devcfg.c

index f3513fa..e0e518c 100644
@@ -62,6 +62,7 @@ CONFIG_PXA2XX=y
 CONFIG_BITBANG_I2C=y
 CONFIG_FRAMEBUFFER=y
 CONFIG_XILINX_SPIPS=y
+CONFIG_ZYNQ_DEVCFG=y
 
 CONFIG_ARM11SCU=y
 CONFIG_A9SCU=y

index 0e65ed0..96025cf 100644
@@ -5,6 +5,7 @@ common-obj-$(CONFIG_PL330) += pl330.o
 common-obj-$(CONFIG_I82374) += i82374.o
 common-obj-$(CONFIG_I8257) += i8257.o
 common-obj-$(CONFIG_XILINX_AXI) += xilinx_axidma.o
+common-obj-$(CONFIG_ZYNQ_DEVCFG) += xilinx_devcfg.o
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_dma.o
 common-obj-$(CONFIG_STP2000) += sparc32_dma.o
 common-obj-$(CONFIG_SUN4M) += sun4m_iommu.o

1.9.1.1.gbb9f595

---
Changed since v3:
Author reset.
Changed since v1:
Added manual parenting of devcfg node (evil but needed for early access to canonical path by devcfgs realize fn).

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

index 9ee21e7..e3505fa 100644
@@ -231,6 +231,14 @@ static void zynq_init(QEMUMachineInitArgs *args)
         sysbus_connect_irq(busdev, n + 1, pic[dma_irqs[n] - IRQ_OFFSET]);
     }
 
+    dev = qdev_create(NULL, "xlnx.ps7-dev-cfg");
+    object_property_add_child(qdev_get_machine(), "xilinx-devcfg", OBJECT(dev),
+                              NULL);
+    qdev_init_nofail(dev);
+    busdev = SYS_BUS_DEVICE(dev);
+    sysbus_connect_irq(busdev, 0, pic[40-IRQ_OFFSET]);
+    sysbus_mmio_map(busdev, 0, 0xF8007000);
+
     zynq_binfo.ram_size = ram_size;
     zynq_binfo.kernel_filename = kernel_filename;
     zynq_binfo.kernel_cmdline = kernel_cmdline;

1.9.1.1.gbb9f595

Ping^2!