Skip to contentSkip to conversation

data-driven device registers

RFC PATCH In review Peter Crosthwaite 1 participant 16 messages Apr 27, 2015 – May 12, 2015 Arm/Zynq architecture

lore
Share & tools
lore
#0 lore raw

v5 cover data-driven device registers

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 P1 commit message for further discussion.

P1 is the main patch, adds the register definition functionality P2-3,6 add helpers that glue the register API to the Memory API P4 Defines a set of macros that minimise register and field definitions P5 is QOMfication
P7 is a trivial
P10-13 Work up to GPIO support
P8,9,14 add new devices (the Xilinx Zynq devcfg & ZynqMP SLCR) that use this
scheme.

This Zynq devcfg device was particularly finnicky with per-bit restrictions. 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

One thing that was suggested early on was built in GPIO support. This is now implemented (as of V5 of the series). To give an example of this, P14 is a (super minimal) implementation of the system controller for the new ZynqMP SoC. The first iteration of the machine model is currently in review stages this this dev cannot be added to a machine yet.

Remaking as RFC, as I think it should be added in two stages, one containing the base support then follow up for GPIOs. RFCing the whole thing here to demonstrate the GPIOs.

A future feature I am interested in is implementing TCG optimisation of side-effectless registers. The register API allows clear definition of what registers have txn side effects and which ones don't. You could even go a step further and translate such side-effectless accesses based on the data pointer for the register.

Changed from v4:
Rebased
Added QOMification
Added GPIO support
Refactored Devcfg device to use FIELD/REG/EX macros.
Update style of devcfg device
Added init_block help.
Changed from v3:
Rebased
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 (14):
register: Add Register API
register: Add Memory API glue
register: Add support for decoding information
register: Define REG and FIELD macros
register: QOMify
register: Add block initialise helper
bitops: Add ONES macro
dma: Add Xilinx Zynq devcfg device model
xilinx_zynq: add devcfg to machine model
qdev: Define qdev_get_gpio_out
qdev: Add qdev_pass_all_gpios API
irq: Add opaque setter routine
register: Add GPIO API
misc: Introduce ZynqMP IOU SLCR

 default-configs/arm-softmmu.mak        |   1 +
 hw/arm/xilinx_zynq.c                   |   8 +
 hw/core/Makefile.objs                  |   1 +
 hw/core/irq.c                          |   5 +
 hw/core/qdev.c                         |  21 ++
 hw/core/register.c                     | 390 +++++++++++++++++++++++++++++++
 hw/dma/Makefile.objs                   |   1 +
 hw/dma/xlnx-zynq-devcfg.c              | 406 +++++++++++++++++++++++++++++++++
 hw/misc/Makefile.objs                  |   1 +
 hw/misc/xlnx-zynqmp-iou-slcr.c         | 113 +++++++++
 include/hw/dma/xlnx-zynq-devcfg.h      |  62 +++++
 include/hw/irq.h                       |   2 +
 include/hw/misc/xlnx-zynqmp-iou-slcr.h |  47 ++++
 include/hw/qdev-core.h                 |   3 +
 include/hw/register.h                  | 276 ++++++++++++++++++++++
 include/qemu/bitops.h                  |   2 +
 16 files changed, 1339 insertions(+)
 create mode 100644 hw/core/register.c
 create mode 100644 hw/dma/xlnx-zynq-devcfg.c
 create mode 100644 hw/misc/xlnx-zynqmp-iou-slcr.c
 create mode 100644 include/hw/dma/xlnx-zynq-devcfg.h
 create mode 100644 include/hw/misc/xlnx-zynqmp-iou-slcr.h
 create mode 100644 include/hw/register.h
Signature · 1 line
2.3.6.3.g2cc70ee
#1 lore raw

RFC PATCH v5 1/14 register: Add Register API

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    | 186 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/register.h | 132 +++++++++++++++++++++++++++++++++++
 3 files changed, 319 insertions(+)
 create mode 100644 hw/core/register.c
 create mode 100644 include/hw/register.h
hw/core/Makefile.objs +1 -0
index abb3560..bf95db5 100644
@@ -14,4 +14,5 @@ common-obj-$(CONFIG_SOFTMMU) += machine.o
 common-obj-$(CONFIG_SOFTMMU) += null-machine.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
+common-obj-$(CONFIG_SOFTMMU) += register.o
 common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
hw/core/register.c new file +186 -0

186 lines not on this page · raw message

include/hw/register.h new file +132 -0

132 lines not on this page · raw message

Signature · 1 line
2.3.6.3.g2cc70ee
#2 lore raw

RFC PATCH v5 2/14 register: Add Memory API glue

In reply to Peter Crosthwaite

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 | 30 ++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
hw/core/register.c +48 -0
index 02a4376..ca10cff 100644
@@ -184,3 +184,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);
+}
include/hw/register.h +30 -0
index 249f458..a3c41db 100644
@@ -86,6 +86,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 {
@@ -103,6 +105,8 @@ struct RegisterInfo {
 
     bool read_lite;
     bool write_lite;
+
+    MemoryRegion mem;
 };
 
 /**
@@ -129,4 +133,30 @@ uint64_t register_read(RegisterInfo *reg);
 
 void register_reset(RegisterInfo *reg);
 
+/**
+ * Memory API MMIO write handler that will write to a Register API register.
+ *  _be for big endian variant and _le for little endian.
+ * @opaque: RegisterInfo to write to
+ * @addr: Address to write
+ * @value: Value to write
+ * @size: Number of bytes to write
+ */
+
+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);
+
+/**
+ * Memory API MMIO read handler that will read from a Register API register.
+ *  _be for big endian variant and _le for little endian.
+ * @opaque: RegisterInfo to read from
+ * @addr: Address to read
+ * @size: Number of bytes to read
+ * returns: Value read from register
+ */
+
+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
Signature · 1 line
2.3.6.3.g2cc70ee
#3 lore raw

RFC PATCH v5 3/14 register: Add support for decoding information

In reply to Peter Crosthwaite

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

---
changed since v4:
Remove extraneous unused defintions.

 include/hw/register.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
include/hw/register.h +10 -0
index a3c41db..90c0185 100644
@@ -54,6 +54,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 +76,11 @@ struct RegisterAccessInfo {
     void (*post_write)(RegisterInfo *reg, uint64_t val);
 
     uint64_t (*post_read)(RegisterInfo *reg, uint64_t val);
+
+    struct {
+        hwaddr addr;
+        uint8_t flags;
+    } decode;
 };
 
 /**
Signature · 1 line
2.3.6.3.g2cc70ee
#4 lore raw

RFC PATCH v5 4/14 register: Define REG and FIELD macros

In reply to Peter Crosthwaite

Define some macros that can be used for defining registers and fields.

The REG32 macro will define A_FOO, for the byte address of a register as well as R_FOO for the uint32_t[] register number (A_FOO / 4).

The FIELD macro will define FOO_BAR_MASK, FOO_BAR_SHIFT and FOO_BAR_LENGTH constants for field BAR in register FOO.

Finally, there are some shorthand helpers for extracting/depositing fields from registers based on these naming schemes.

Usage can greatly reduce the verbosity of device code.

The deposit and extract macros (eg F_EX32, AF_DP32 etc.) can be used to generate extract and deposits without any repetition of the name stems.

E.g. Currently you have to define something like:

\#define R_FOOREG (0x84/4)
\#define R_FOOREG_BARFIELD_SHIFT 10
\#define R_FOOREG_BARFIELD_LENGTH 5

uint32_t foobar_val = extract32(s->regs[R_FOOREG],

                                R_FOOREG_BARFIELD_SHIFT,
                                R_FOOREG_BARFIELD_LENGTH);

Which has:
2 macro definitions per field
3 register names ("FOOREG") per extract
2 field names ("BARFIELD") per extract

With these macros this becomes:

REG32(FOOREG, 0x84)
FIELD(FOOREG, BARFIELD, 10, 5)

uint32_t foobar_val = AF_EX32(s->regs, FOOREG, BARFIELD)

Which has:
1 macro definition per field
1 register name per extract
1 field name per extract

If you are not using arrays for the register data you can just use the non-array "F_" variants and still save 2 name stems:

uint32_t foobar_val = F_EX32(s->fooreg, FOOREG, BARFIELD)

Deposit is similar for depositing values. Deposit has compile-time overflow checking for literals.
For example:

REG32(XYZ1, 0x84)
FIELD(XYZ1, TRC, 0, 4)

/* Correctly set XYZ1.TRC = 5. */
AF_DP32(s->regs, XYZ1, TRC, 5);

/* Incorrectly set XYZ1.TRC = 16. */
AF_DP32(s->regs, XYZ1, TRC, 16);

The latter assignment results in:
warning: large integer implicitly truncated to unsigned type [-Woverflow]

[ EI Changes:
* Add Deposit macros
]

---
 include/hw/register.h | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
include/hw/register.h +40 -0
index 90c0185..7a8671b 100644
@@ -169,4 +169,44 @@ void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value,
 uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size);
 uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size);
 
+/* Define constants for a 32 bit register */
+
+#define REG32(reg, addr)                                                  \
+enum { A_ ## reg = (addr) };                                              \
+enum { R_ ## reg = (addr) / 4 };
+
+/* Define SHIFT, LEGTH and MASK constants for a field within a register */
+
+#define FIELD(reg, field, shift, length)                                  \
+enum { R_ ## reg ## _ ## field ## _SHIFT = (shift)};                      \
+enum { R_ ## reg ## _ ## field ## _LENGTH = (length)};                    \
+enum { R_ ## reg ## _ ## field ## _MASK = (((1ULL << (length)) - 1)       \
+                                          << (shift)) };
+
+/* Extract a field from a register */
+
+#define F_EX32(storage, reg, field)                                       \
+    extract32((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
+              R_ ## reg ## _ ## field ## _LENGTH)
+
+/* Extract a field from an array of registers */
+
+#define AF_EX32(regs, reg, field)                                         \
+    F_EX32((regs)[R_ ## reg], reg, field)
+
+/* Deposit a register field.  */
+
+#define F_DP32(storage, reg, field, val) ({                               \
+    struct {                                                              \
+        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
+    } v = { .v = val };                                                   \
+    uint32_t d;                                                           \
+    d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
+                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
+    d; })
+
+/* Deposit a field to array of registers.  */
+
+#define AF_DP32(regs, reg, field, val)                                    \
+    (regs)[R_ ## reg] = F_DP32((regs)[R_ ## reg], reg, field, val);
 #endif
Signature · 1 line
2.3.6.3.g2cc70ee
#5 lore raw

RFC PATCH v5 5/14 register: QOMify

In reply to Peter Crosthwaite

QOMify registers as a child of TYPE_DEVICE. This allows registers to define GPIOs.

Define an init helper that will do QOM initialisation as well as setup the r/w fast paths.

---

 hw/core/register.c    | 34 ++++++++++++++++++++++++++++++++++
 include/hw/register.h | 17 +++++++++++++++++
 2 files changed, 51 insertions(+)
hw/core/register.c +34 -0
index ca10cff..000b87f 100644
@@ -185,6 +185,28 @@ void register_reset(RegisterInfo *reg)
     register_write_val(reg, reg->access->reset);
 }
 
+void register_init(RegisterInfo *reg)
+{
+    assert(reg);
+    const RegisterAccessInfo *ac;
+
+    if (!reg->data || !reg->access) {
+        return;
+    }
+
+    object_initialize((void *)reg, sizeof(*reg), TYPE_REGISTER);
+
+    ac = reg->access;
+
+    /* if there are no debug msgs and no RMW requirement, mark for fast write */
+    reg->write_lite = reg->debug || ac->ro || ac->w1c || ac->pre_write ||
+            ((ac->ge0 || ac->ge1) && qemu_loglevel_mask(LOG_GUEST_ERROR)) ||
+            ((ac->ui0 || ac->ui1) && qemu_loglevel_mask(LOG_UNIMP))
+             ? false : true;
+    /* no debug and no clear-on-read is a fast read */
+    reg->read_lite = reg->debug || ac->cor ? false : true;
+}
+
 static inline void register_write_memory(void *opaque, hwaddr addr,
                                          uint64_t value, unsigned size, bool be)
 {
@@ -232,3 +254,15 @@ uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size)
 {
     return register_read_memory(opaque, addr, size, false);
 }
+
+static const TypeInfo register_info = {
+    .name  = TYPE_REGISTER,
+    .parent = TYPE_DEVICE,
+};
+
+static void register_register_types(void)
+{
+    type_register_static(&register_info);
+}
+
+type_init(register_register_types)
include/hw/register.h +17 -0
index 7a8671b..a3d3f60 100644
@@ -11,6 +11,7 @@
 #ifndef REGISTER_H
 #define REGISTER_H
 
+#include "hw/qdev-core.h"
 #include "exec/memory.h"
 
 typedef struct RegisterInfo RegisterInfo;
@@ -101,6 +102,11 @@ struct RegisterAccessInfo {
  */
 
 struct RegisterInfo {
+    /*< private >*/
+    DeviceState parent_obj;
+
+    /*< public >*/
+
     void *data;
     int data_size;
 
@@ -119,6 +125,9 @@ struct RegisterInfo {
     MemoryRegion mem;
 };
 
+#define TYPE_REGISTER "qemu,register"
+#define REGISTER(obj) OBJECT_CHECK(RegisterInfo, (obj), TYPE_REGISTER)
+
 /**
  * write a value to a register, subject to its restrictions
  * @reg: register to write to
@@ -144,6 +153,14 @@ uint64_t register_read(RegisterInfo *reg);
 void register_reset(RegisterInfo *reg);
 
 /**
+ * Initialize a register. GPIO's are setup as IOs to the specified device.
+ * Fast paths for eligible registers are enabled.
+ * @reg: Register to initialize
+ */
+
+void register_init(RegisterInfo *reg);
+
+/**
  * Memory API MMIO write handler that will write to a Register API register.
  *  _be for big endian variant and _le for little endian.
  * @opaque: RegisterInfo to write to
Signature · 1 line
2.3.6.3.g2cc70ee
#6 lore raw

RFC PATCH v5 6/14 register: Add block initialise helper

In reply to Peter Crosthwaite

Add a helper that will scan a static RegisterAccessInfo Array and populate a container MemoryRegion with registers as defined.

---
 hw/core/register.c    | 28 ++++++++++++++++++++++++++++
 include/hw/register.h | 21 +++++++++++++++++++++
 2 files changed, 49 insertions(+)
hw/core/register.c +28 -0
index 000b87f..4e59122 100644
@@ -255,6 +255,34 @@ uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size)
     return register_read_memory(opaque, addr, size, false);
 }
 
+void register_init_block32(DeviceState *owner, const RegisterAccessInfo *rae,
+                           int num, RegisterInfo *ri, uint32_t *data,
+                           MemoryRegion *container, const MemoryRegionOps *ops,
+                           bool debug_enabled)
+{
+    const char *debug_prefix = object_get_typename(OBJECT(owner));
+    int i;
+
+    for (i = 0; i < num; i++) {
+        int index = rae[i].decode.addr / 4;
+        RegisterInfo *r = &ri[index];
+
+        *r = (RegisterInfo) {
+            .data = &data[index],
+            .data_size = sizeof(uint32_t),
+            .access = &rae[i],
+            .debug = debug_enabled,
+            .prefix = debug_prefix,
+            .opaque = owner,
+        };
+        register_init(r);
+
+        memory_region_init_io(&r->mem, OBJECT(owner), ops, r, r->access->name,
+                              sizeof(uint32_t));
+        memory_region_add_subregion(container, r->access->decode.addr, &r->mem);
+    }
+}
+
 static const TypeInfo register_info = {
     .name  = TYPE_REGISTER,
     .parent = TYPE_DEVICE,
include/hw/register.h +21 -0
index a3d3f60..7b6f732 100644
@@ -186,6 +186,27 @@ void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value,
 uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size);
 uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size);
 
+/**
+ * Init a block of consecutive registers into a container MemoryRegion. A
+ * number of constant register definitions are parsed to create a corresponding
+ * array of RegisterInfo's.
+ *
+ * @owner: device owning the registers
+ * @rae: Register definitions to init
+ * @num: number of registers to init (length of @rae)
+ * @ri: Register array to init
+ * @data: Array to use for register data
+ * @container: Memory region to contain new registers
+ * @ops: Memory region ops to use to access registers. Opaque data of handler
+ * with be a RegisterInfo * (from @ri)
+ * @debug enabled: turn on/off verbose debug information
+ */
+
+void register_init_block32(DeviceState *owner, const RegisterAccessInfo *rae,
+                           int num, RegisterInfo *ri, uint32_t *data,
+                           MemoryRegion *container, const MemoryRegionOps *ops,
+                           bool debug_enabled);
+
 /* Define constants for a 32 bit register */
 
 #define REG32(reg, addr)                                                  \
Signature · 1 line
2.3.6.3.g2cc70ee
#7 lore raw

RFC PATCH v5 7/14 bitops: Add ONES macro

In reply to Peter Crosthwaite

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

---

 include/qemu/bitops.h | 2 ++
 1 file changed, 2 insertions(+)
include/qemu/bitops.h +2 -0
index 90ca8df..30bad78 100644
@@ -415,4 +415,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
Signature · 1 line
2.3.6.3.g2cc70ee
#8 lore raw

RFC PATCH v5 8/14 dma: Add Xilinx Zynq devcfg device model

In reply to Peter Crosthwaite

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

---
Changed since v4:
Create device state header.
Use REG/FIELD/EX macros
Use register init_block32
Remove un-needed timer code
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/xlnx-zynq-devcfg.c         | 406 ++++++++++++++++++++++++++++++++++++++
 include/hw/dma/xlnx-zynq-devcfg.h |  62 ++++++
 4 files changed, 470 insertions(+)
 create mode 100644 hw/dma/xlnx-zynq-devcfg.c
 create mode 100644 include/hw/dma/xlnx-zynq-devcfg.h
default-configs/arm-softmmu.mak +1 -0
index a767e4b..b6c5ba8 100644
@@ -65,6 +65,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
hw/dma/Makefile.objs +1 -0
index 0e65ed0..eaf0a81 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) += xlnx-zynq-devcfg.o
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_dma.o
 common-obj-$(CONFIG_STP2000) += sparc32_dma.o
 common-obj-$(CONFIG_SUN4M) += sun4m_iommu.o
hw/dma/xlnx-zynq-devcfg.c new file +406 -0

406 lines not on this page · raw message

include/hw/dma/xlnx-zynq-devcfg.h new file +62 -0

62 lines not on this page · raw message

Signature · 1 line
2.3.6.3.g2cc70ee
#9 lore raw

RFC PATCH v5 9/14 xilinx_zynq: add devcfg to machine model

In reply to Peter Crosthwaite

---
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(+)
hw/arm/xilinx_zynq.c +8 -0
index a4e7b5c..df25cc6 100644
@@ -245,6 +245,14 @@ static void zynq_init(MachineState *machine)
         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(), "xlnx-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;
Signature · 1 line
2.3.6.3.g2cc70ee
#10 lore raw

RFC PATCH v5 10/14 qdev: Define qdev_get_gpio_out

In reply to Peter Crosthwaite

An API similar to the existing qdev_get_gpio_in() except gets outputs. Useful for:

1: Implementing lightweight devices that don't want to keep pointers to their own GPIOs. They can get their GPIO pointers at runtime from QOM using this API.

2: testing or debugging code which may wish to override the hardware generated value of of a GPIO with a user specified value (E.G. interrupt injection).

---

 hw/core/qdev.c         | 12 ++++++++++++
 include/hw/qdev-core.h |  2 ++
 2 files changed, 14 insertions(+)
hw/core/qdev.c +12 -0
index 6e6a65d..d736613 100644
@@ -490,6 +490,18 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
     return qdev_get_gpio_in_named(dev, NULL, n);
 }
 
+qemu_irq qdev_get_gpio_out_named(DeviceState *dev, const char *name, int n)
+{
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+    return (qemu_irq)object_property_get_link(OBJECT(dev), propname, NULL);
+}
+
+qemu_irq qdev_get_gpio_out(DeviceState *dev, int n)
+{
+    return qdev_get_gpio_out_named(dev, NULL, n);
+}
+
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
                                  qemu_irq pin)
 {
include/hw/qdev-core.h +2 -0
index 4e673f9..95f7f0e 100644
@@ -274,6 +274,8 @@ bool qdev_machine_modified(void);
 
 qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
 qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
+qemu_irq qdev_get_gpio_out(DeviceState *dev, int n);
+qemu_irq qdev_get_gpio_out_named(DeviceState *dev, const char *name, int n);
 
 void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
Signature · 1 line
2.3.6.3.g2cc70ee
#11 lore raw

RFC PATCH v5 11/14 qdev: Add qdev_pass_all_gpios API

In reply to Peter Crosthwaite

For passing all GPIOs of all names from a contained device to a container.

---
 hw/core/qdev.c         | 9 +++++++++
 include/hw/qdev-core.h | 1 +
 2 files changed, 10 insertions(+)
hw/core/qdev.c +9 -0
index d736613..f472a4e 100644
@@ -588,6 +588,15 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
     QLIST_INSERT_HEAD(&container->gpios, ngl, node);
 }
 
+void qdev_pass_all_gpios(DeviceState *dev, DeviceState *container)
+{
+    NamedGPIOList *ngl;
+
+    QLIST_FOREACH(ngl, &dev->gpios, node) {
+        qdev_pass_gpios(dev, container, ngl->name);
+    }
+}
+
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
include/hw/qdev-core.h +1 -0
index 95f7f0e..1200fb4 100644
@@ -299,6 +299,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
 
 void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
                      const char *name);
+void qdev_pass_all_gpios(DeviceState *dev, DeviceState *container);
 
 BusState *qdev_get_parent_bus(DeviceState *dev);
 
Signature · 1 line
2.3.6.3.g2cc70ee
#12 lore raw

RFC PATCH v5 12/14 irq: Add opaque setter routine

In reply to Peter Crosthwaite

Add a routine to set or override the opaque data of an IRQ.

Qdev currently always initialises IRQ opaque as the device itself. This allows you to override to a custom opaque in the case where there is extra or different data needed.

---
 hw/core/irq.c    | 5 +++++
 include/hw/irq.h | 2 ++
 2 files changed, 7 insertions(+)
hw/core/irq.c +5 -0
index 8a62a36..4a41059 100644
@@ -76,6 +76,11 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
     return irq;
 }
 
+void qemu_irq_set_opaque(qemu_irq irq, void *opaque)
+{
+    irq->opaque = opaque;
+}
+
 void qemu_free_irqs(qemu_irq *s, int n)
 {
     int i;
include/hw/irq.h +2 -0
index 4c4c2ea..edad0fc 100644
@@ -44,6 +44,8 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n);
 qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
                                 void *opaque, int n);
 
+void qemu_irq_set_opaque(qemu_irq irq, void *opaque);
+
 void qemu_free_irqs(qemu_irq *s, int n);
 void qemu_free_irq(qemu_irq irq);
 
Signature · 1 line
2.3.6.3.g2cc70ee
#13 lore raw

RFC PATCH v5 13/14 register: Add GPIO API

In reply to Peter Crosthwaite

Add GPIO functionality to the register API. This allows association and automatic connection of GPIOs to bits in registers. GPIO inputs will attach to handlers that automatically set read-only bits in registers. GPIO outputs will be updated to reflect their field value when their respective registers are written (or reset). Supports active low GPIOs.

This is particularly effective for implementing system level controllers, where heterogenous collections of control signals are placed is a SoC specific peripheral then propagated all over the system.

[ EI Changes:
* register: Add a polarity field to GPIO connections Makes it possible to directly connect active low signals to generic interrupt pins.
]

---
 hw/core/register.c    | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/register.h | 26 ++++++++++++++
 2 files changed, 120 insertions(+)
hw/core/register.c +94 -0
index 4e59122..9ab118b 100644
@@ -135,6 +135,8 @@ void register_write(RegisterInfo *reg, uint64_t val, uint64_t we)
     }
 register_write_fast:
     register_write_val(reg, new_val);
+    register_refresh_gpios(reg, old_val);
+
     if (ac->post_write) {
         ac->post_write(reg, new_val);
     }
@@ -177,18 +179,85 @@ uint64_t register_read(RegisterInfo *reg)
 void register_reset(RegisterInfo *reg)
 {
     assert(reg);
+    uint64_t old_val;
 
     if (!reg->data || !reg->access) {
         return;
     }
 
+    old_val = register_read_val(reg);
+
     register_write_val(reg, reg->access->reset);
+    register_refresh_gpios(reg, old_val);
+}
+
+void register_refresh_gpios(RegisterInfo *reg, uint64_t old_value)
+{
+    const RegisterAccessInfo *ac;
+    const RegisterGPIOMapping *gpio;
+
+    ac = reg->access;
+    for (gpio = ac->gpios; gpio && gpio->name; gpio++) {
+        int i;
+
+        if (gpio->input) {
+            continue;
+        }
+
+        for (i = 0; i < gpio->num; ++i) {
+            uint64_t gpio_value, gpio_value_old;
+
+            qemu_irq gpo = qdev_get_gpio_out_named(DEVICE(reg), gpio->name, i);
+            gpio_value_old = extract64(old_value,
+                                       gpio->bit_pos + i * gpio->width,
+                                       gpio->width) ^ gpio->polarity;
+            gpio_value = extract64(register_read_val(reg),
+                                   gpio->bit_pos + i * gpio->width,
+                                   gpio->width) ^ gpio->polarity;
+            if (!(gpio_value_old ^ gpio_value)) {
+                continue;
+            }
+            if (reg->debug && gpo) {
+                qemu_log("refreshing gpio out %s to %" PRIx64 "\n",
+                         gpio->name, gpio_value);
+            }
+            qemu_set_irq(gpo, gpio_value);
+        }
+    }
+}
+
+typedef struct DeviceNamedGPIOHandlerOpaque {
+    DeviceState *dev;
+    const char *name;
+} DeviceNamedGPIOHandlerOpaque;
+
+static void register_gpio_handler(void *opaque, int n, int level)
+{
+    DeviceNamedGPIOHandlerOpaque *gho = opaque;
+    RegisterInfo *reg = REGISTER(gho->dev);
+
+    const RegisterAccessInfo *ac;
+    const RegisterGPIOMapping *gpio;
+
+    ac = reg->access;
+    for (gpio = ac->gpios; gpio && gpio->name; gpio++) {
+        if (gpio->input && !strcmp(gho->name, gpio->name)) {
+            register_write_val(reg, deposit64(register_read_val(reg),
+                                              gpio->bit_pos + n * gpio->width,
+                                              gpio->width,
+                                              level ^ gpio->polarity));
+            return;
+        }
+    }
+
+    abort();
 }
 
 void register_init(RegisterInfo *reg)
 {
     assert(reg);
     const RegisterAccessInfo *ac;
+    const RegisterGPIOMapping *gpio;
 
     if (!reg->data || !reg->access) {
         return;
@@ -197,6 +266,30 @@ void register_init(RegisterInfo *reg)
     object_initialize((void *)reg, sizeof(*reg), TYPE_REGISTER);
 
     ac = reg->access;
+    for (gpio = ac->gpios; gpio && gpio->name; gpio++) {
+        if (!gpio->num) {
+            ((RegisterGPIOMapping *)gpio)->num = 1;
+        }
+        if (!gpio->width) {
+            ((RegisterGPIOMapping *)gpio)->width = 1;
+        }
+        if (gpio->input) {
+            DeviceNamedGPIOHandlerOpaque gho = {
+                .name = gpio->name,
+                .dev = DEVICE(reg),
+            };
+            qemu_irq irq;
+
+            qdev_init_gpio_in_named(DEVICE(reg), register_gpio_handler,
+                                    gpio->name, gpio->num);
+            irq = qdev_get_gpio_in_named(DEVICE(reg), gpio->name, gpio->num);
+            qemu_irq_set_opaque(irq, g_memdup(&gho, sizeof(gho)));
+        } else {
+            qemu_irq *gpos = g_new0(qemu_irq, gpio->num);
+
+            qdev_init_gpio_out_named(DEVICE(reg), gpos, gpio->name, gpio->num);
+        }
+    }
 
     /* if there are no debug msgs and no RMW requirement, mark for fast write */
     reg->write_lite = reg->debug || ac->ro || ac->w1c || ac->pre_write ||
@@ -276,6 +369,7 @@ void register_init_block32(DeviceState *owner, const RegisterAccessInfo *rae,
             .opaque = owner,
         };
         register_init(r);
+        qdev_pass_all_gpios(DEVICE(r), owner);
 
         memory_region_init_io(&r->mem, OBJECT(owner), ops, r, r->access->name,
                               sizeof(uint32_t));
include/hw/register.h +26 -0
index 7b6f732..44c13dc 100644
@@ -13,6 +13,7 @@
 
 #include "hw/qdev-core.h"
 #include "exec/memory.h"
+#include "hw/irq.h"
 
 typedef struct RegisterInfo RegisterInfo;
 typedef struct RegisterAccessInfo RegisterAccessInfo;
@@ -28,6 +29,18 @@ typedef struct RegisterAccessError {
     const char *reason;
 } RegisterAccessError;
 
+#define REG_GPIO_POL_HIGH 0
+#define REG_GPIO_POL_LOW  1
+
+typedef struct RegisterGPIOMapping {
+    const char *name;
+    uint8_t bit_pos;
+    bool input;
+    bool polarity;
+    uint8_t num;
+    uint8_t width;
+} RegisterGPIOMapping;
+
 /**
  * Access description for a register that is part of guest accessible device
  * state.
@@ -78,6 +91,8 @@ struct RegisterAccessInfo {
 
     uint64_t (*post_read)(RegisterInfo *reg, uint64_t val);
 
+    const RegisterGPIOMapping *gpios;
+
     struct {
         hwaddr addr;
         uint8_t flags;
@@ -161,6 +176,17 @@ void register_reset(RegisterInfo *reg);
 void register_init(RegisterInfo *reg);
 
 /**
+ * Refresh GPIO outputs based on diff between old value register current value.
+ * GPIOs are refreshed for fields where the old value differs to the current
+ * value.
+ *
+ * @reg: Register to refresh GPIO outs
+ * @old_value: previous value of register
+ */
+
+void register_refresh_gpios(RegisterInfo *reg, uint64_t old_value);
+
+/**
  * Memory API MMIO write handler that will write to a Register API register.
  *  _be for big endian variant and _le for little endian.
  * @opaque: RegisterInfo to write to
Signature · 1 line
2.3.6.3.g2cc70ee
#14 lore raw

RFC PATCH v5 14/14 misc: Introduce ZynqMP IOU SLCR

In reply to Peter Crosthwaite

IOU = I/O Unit
SLCR = System Level Control Registers

This IP is a misc collections of control registers that switch various properties of system IPs. Currently the only thing implemented is the SD_SLOTTYPE control (implemented as a GPIO output).

---
 hw/misc/Makefile.objs                  |   1 +
 hw/misc/xlnx-zynqmp-iou-slcr.c         | 113 +++++++++++++++++++++++++++++++++
 include/hw/misc/xlnx-zynqmp-iou-slcr.h |  47 ++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 hw/misc/xlnx-zynqmp-iou-slcr.c
 create mode 100644 include/hw/misc/xlnx-zynqmp-iou-slcr.h
hw/misc/Makefile.objs +1 -0
index 4aa76ff..8aa6038 100644
@@ -36,6 +36,7 @@ obj-$(CONFIG_OMAP) += omap_sdrc.o
 obj-$(CONFIG_OMAP) += omap_tap.o
 obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
+obj-$(CONFIG_ZYNQ) += xlnx-zynqmp-iou-slcr.o
 obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
hw/misc/xlnx-zynqmp-iou-slcr.c new file +113 -0

113 lines not on this page · raw message

include/hw/misc/xlnx-zynqmp-iou-slcr.h new file +47 -0

47 lines not on this page · raw message

Signature · 1 line
2.3.6.3.g2cc70ee
#15 lore raw
In reply to Peter Crosthwaite

Ping!

Quoted 115 lines · Peter Crosthwaite

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 P1 commit message for further discussion.

P1 is the main patch, adds the register definition functionality P2-3,6 add helpers that glue the register API to the Memory API P4 Defines a set of macros that minimise register and field definitions P5 is QOMfication
P7 is a trivial
P10-13 Work up to GPIO support
P8,9,14 add new devices (the Xilinx Zynq devcfg & ZynqMP SLCR) that use this
scheme.

This Zynq devcfg device was particularly finnicky with per-bit restrictions. 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

One thing that was suggested early on was built in GPIO support. This is now implemented (as of V5 of the series). To give an example of this, P14 is a (super minimal) implementation of the system controller for the new ZynqMP SoC. The first iteration of the machine model is currently in review stages this this dev cannot be added to a machine yet.

Remaking as RFC, as I think it should be added in two stages, one containing the base support then follow up for GPIOs. RFCing the whole thing here to demonstrate the GPIOs.

A future feature I am interested in is implementing TCG optimisation of side-effectless registers. The register API allows clear definition of what registers have txn side effects and which ones don't. You could even go a step further and translate such side-effectless accesses based on the data pointer for the register.

Changed from v4:
Rebased
Added QOMification
Added GPIO support
Refactored Devcfg device to use FIELD/REG/EX macros.
Update style of devcfg device
Added init_block help.
Changed from v3:
Rebased
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 (14):
register: Add Register API
register: Add Memory API glue
register: Add support for decoding information
register: Define REG and FIELD macros
register: QOMify
register: Add block initialise helper
bitops: Add ONES macro
dma: Add Xilinx Zynq devcfg device model
xilinx_zynq: add devcfg to machine model
qdev: Define qdev_get_gpio_out
qdev: Add qdev_pass_all_gpios API
irq: Add opaque setter routine
register: Add GPIO API
misc: Introduce ZynqMP IOU SLCR

 default-configs/arm-softmmu.mak        |   1 +
 hw/arm/xilinx_zynq.c                   |   8 +
 hw/core/Makefile.objs                  |   1 +
 hw/core/irq.c                          |   5 +
 hw/core/qdev.c                         |  21 ++
 hw/core/register.c                     | 390 +++++++++++++++++++++++++++++++
 hw/dma/Makefile.objs                   |   1 +
 hw/dma/xlnx-zynq-devcfg.c              | 406 +++++++++++++++++++++++++++++++++
 hw/misc/Makefile.objs                  |   1 +
 hw/misc/xlnx-zynqmp-iou-slcr.c         | 113 +++++++++
 include/hw/dma/xlnx-zynq-devcfg.h      |  62 +++++
 include/hw/irq.h                       |   2 +
 include/hw/misc/xlnx-zynqmp-iou-slcr.h |  47 ++++
 include/hw/qdev-core.h                 |   3 +
 include/hw/register.h                  | 276 ++++++++++++++++++++++
 include/qemu/bitops.h                  |   2 +
 16 files changed, 1339 insertions(+)
 create mode 100644 hw/core/register.c
 create mode 100644 hw/dma/xlnx-zynq-devcfg.c
 create mode 100644 hw/misc/xlnx-zynqmp-iou-slcr.c
 create mode 100644 include/hw/dma/xlnx-zynq-devcfg.h
 create mode 100644 include/hw/misc/xlnx-zynqmp-iou-slcr.h
 create mode 100644 include/hw/register.h

--
2.3.6.3.g2cc70ee