All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v1 00/15]  data-driven device registers
@ 2015-07-29 20:24 Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 01/15] register: Add Register API Alistair Francis
                   ` (15 more replies)
  0 siblings, 16 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

>From Peter:
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.
P15: Connect the ZynqMP SLCR device

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

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.

Changes since RFC:
 - Connect the ZynqMP IOU SLCR device
 - Rebase

Changed from RFC 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.


Alistair Francis (1):
  xlnx-zynqmp: Connect the ZynqMP IOU SLCR

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/arm/xlnx-zynqmp.c                   |   15 ++
 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/arm/xlnx-zynqmp.h           |    2 +
 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                  |  274 +++++++++++++++++++++
 include/qemu/bitops.h                  |    2 +
 18 files changed, 1354 insertions(+), 0 deletions(-)
 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

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

* [Qemu-devel] [PATCH v1 01/15] register: Add Register API
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 02/15] register: Add Memory API glue Alistair Francis
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
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(+), 0 deletions(-)
 create mode 100644 hw/core/register.c
 create mode 100644 include/hw/register.h

diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index abb3560..bf95db5 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -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
diff --git a/hw/core/register.c b/hw/core/register.c
new file mode 100644
index 0000000..02a4376
--- /dev/null
+++ b/hw/core/register.c
@@ -0,0 +1,186 @@
+/*
+ * Register Definition API
+ *
+ * Copyright (c) 2013 Xilinx Inc.
+ * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "hw/register.h"
+#include "qemu/log.h"
+
+static inline void register_write_log(RegisterInfo *reg, int dir, uint64_t val,
+                                      int mask, const char *msg,
+                                      const char *reason)
+{
+    qemu_log_mask(mask, "%s:%s bits %#" PRIx64 " %s write of %d%s%s\n",
+                  reg->prefix, reg->access->name, val, msg, dir,
+                  reason ? ": " : "", reason ? reason : "");
+}
+
+static inline void register_write_val(RegisterInfo *reg, uint64_t val)
+{
+    if (!reg->data) {
+        return;
+    }
+    switch (reg->data_size) {
+    case 1:
+        *(uint8_t *)reg->data = val;
+        break;
+    case 2:
+        *(uint16_t *)reg->data = val;
+        break;
+    case 4:
+        *(uint32_t *)reg->data = val;
+        break;
+    case 8:
+        *(uint64_t *)reg->data = val;
+        break;
+    default:
+        abort();
+    }
+}
+
+static inline uint64_t register_read_val(RegisterInfo *reg)
+{
+    switch (reg->data_size) {
+    case 1:
+        return *(uint8_t *)reg->data;
+    case 2:
+        return *(uint16_t *)reg->data;
+    case 4:
+        return *(uint32_t *)reg->data;
+    case 8:
+        return *(uint64_t *)reg->data;
+    default:
+        abort();
+    }
+    return 0; /* unreachable */
+}
+
+void register_write(RegisterInfo *reg, uint64_t val, uint64_t we)
+{
+    uint64_t old_val, new_val, test, no_w_mask;
+    const RegisterAccessInfo *ac;
+    const RegisterAccessError *rae;
+
+    assert(reg);
+
+    ac = reg->access;
+    old_val = reg->data ? register_read_val(reg) : ac->reset;
+    if (reg->write_lite && !~we) { /* fast path!! */
+        new_val = val;
+        goto register_write_fast;
+    }
+
+    if (!ac || !ac->name) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: write to undefined device state "
+                      "(written value: %#" PRIx64 ")\n", reg->prefix, val);
+        return;
+    }
+
+    no_w_mask = ac->ro | ac->w1c | ~we;
+
+    if (reg->debug) {
+        qemu_log("%s:%s: write of value %#" PRIx64 "\n", reg->prefix, ac->name,
+                 val);
+    }
+
+    if (qemu_loglevel_mask(LOG_GUEST_ERROR)) {
+        test = (old_val ^ val) & ac->rsvd;
+        if (test) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: change of value in reserved bit"
+                          "fields: %#" PRIx64 ")\n", reg->prefix, test);
+        }
+        for (rae = ac->ge1; rae && rae->mask; rae++) {
+            test = val & rae->mask;
+            if (test) {
+                register_write_log(reg, 1, test, LOG_GUEST_ERROR,
+                                   "invalid", rae->reason);
+            }
+        }
+        for (rae = ac->ge0; rae && rae->mask; rae++) {
+            test = ~val & rae->mask;
+            if (test) {
+                register_write_log(reg, 0, test, LOG_GUEST_ERROR,
+                                   "invalid", rae->reason);
+            }
+        }
+    }
+
+    if (qemu_loglevel_mask(LOG_UNIMP)) {
+        for (rae = ac->ui1; rae && rae->mask; rae++) {
+            test = val & rae->mask;
+            if (test) {
+                register_write_log(reg, 1, test, LOG_GUEST_ERROR,
+                                   "unimplmented", rae->reason);
+            }
+        }
+        for (rae = ac->ui0; rae && rae->mask; rae++) {
+            test = ~val & rae->mask;
+            if (test) {
+                register_write_log(reg, 0, test, LOG_GUEST_ERROR,
+                                   "unimplemented", rae->reason);
+            }
+        }
+    }
+
+    new_val = (val & ~no_w_mask) | (old_val & no_w_mask);
+    new_val &= ~(val & ac->w1c);
+
+    if (ac->pre_write) {
+        new_val = ac->pre_write(reg, new_val);
+    }
+register_write_fast:
+    register_write_val(reg, new_val);
+    if (ac->post_write) {
+        ac->post_write(reg, new_val);
+    }
+}
+
+uint64_t register_read(RegisterInfo *reg)
+{
+    uint64_t ret;
+    const RegisterAccessInfo *ac;
+
+    assert(reg);
+
+    ac = reg->access;
+    if (!ac || !ac->name) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: read from undefined device state\n",
+                      reg->prefix);
+        return 0;
+    }
+
+    ret = reg->data ? register_read_val(reg) : ac->reset;
+
+    if (!reg->read_lite) {
+        register_write_val(reg, ret & ~ac->cor);
+    }
+
+    if (ac->post_read) {
+        ret = ac->post_read(reg, ret);
+    }
+
+    if (!reg->read_lite) {
+        if (reg->debug) {
+            qemu_log("%s:%s: read of value %#" PRIx64 "\n", reg->prefix,
+                     ac->name, ret);
+        }
+    }
+
+    return ret;
+}
+
+void register_reset(RegisterInfo *reg)
+{
+    assert(reg);
+
+    if (!reg->data || !reg->access) {
+        return;
+    }
+
+    register_write_val(reg, reg->access->reset);
+}
diff --git a/include/hw/register.h b/include/hw/register.h
new file mode 100644
index 0000000..249f458
--- /dev/null
+++ b/include/hw/register.h
@@ -0,0 +1,132 @@
+/*
+ * Register Definition API
+ *
+ * Copyright (c) 2013 Xilinx Inc.
+ * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef REGISTER_H
+#define REGISTER_H
+
+#include "exec/memory.h"
+
+typedef struct RegisterInfo RegisterInfo;
+typedef struct RegisterAccessInfo RegisterAccessInfo;
+
+/**
+ * A register access error message
+ * @mask: Bits in the register the error applies to
+ * @reason: Reason why this access is an error
+ */
+
+typedef struct RegisterAccessError {
+    uint64_t mask;
+    const char *reason;
+} RegisterAccessError;
+
+/**
+ * Access description for a register that is part of guest accessible device
+ * state.
+ *
+ * @name: String name of the register
+ * @ro: whether or not the bit is read-only
+ * @w1c: bits with the common write 1 to clear semantic.
+ * @reset: reset value.
+ * @cor: Bits that are clear on read
+ * @rsvd: Bits that are reserved and should not be changed
+ *
+ * @ge1: Bits that when written 1 indicate a guest error
+ * @ge0: Bits that when written 0 indicate a guest error
+ * @ui1: Bits that when written 1 indicate use of an unimplemented feature
+ * @ui0: Bits that when written 0 indicate use of an unimplemented feature
+ *
+ * @pre_write: Pre write callback. Passed the value that's to be written,
+ * immediately before the actual write. The returned value is what is written,
+ * giving the handler a chance to modify the written value.
+ * @post_write: Post write callback. Passed the written value. Most write side
+ * effects should be implemented here.
+ *
+ * @post_read: Post read callback. Passes the value that is about to be returned
+ * for a read. The return value from this function is what is ultimately read,
+ * allowing this function to modify the value before return to the client.
+ */
+
+struct RegisterAccessInfo {
+    const char *name;
+    uint64_t ro;
+    uint64_t w1c;
+    uint64_t reset;
+    uint64_t cor;
+    uint64_t rsvd;
+
+    const RegisterAccessError *ge0;
+    const RegisterAccessError *ge1;
+    const RegisterAccessError *ui0;
+    const RegisterAccessError *ui1;
+
+    uint64_t (*pre_write)(RegisterInfo *reg, uint64_t val);
+    void (*post_write)(RegisterInfo *reg, uint64_t val);
+
+    uint64_t (*post_read)(RegisterInfo *reg, uint64_t val);
+};
+
+/**
+ * A register that is part of guest accessible state
+ * @data: pointer to the register data. Will be cast
+ * to the relevant uint type depending on data_size.
+ * @data_size: Size of the register in bytes. Must be
+ * 1, 2, 4 or 8
+ *
+ * @access: Access desciption of this register
+ *
+ * @debug: Whether or not verbose debug is enabled
+ * @prefix: String prefix for log and debug messages
+ *
+ * @opaque: Opaque data for the register
+ */
+
+struct RegisterInfo {
+    void *data;
+    int data_size;
+
+    const RegisterAccessInfo *access;
+
+    bool debug;
+    const char *prefix;
+
+    void *opaque;
+
+    /*< private >*/
+
+    bool read_lite;
+    bool write_lite;
+};
+
+/**
+ * write a value to a register, subject to its restrictions
+ * @reg: register to write to
+ * @val: value to write
+ * @we: write enable mask
+ */
+
+void register_write(RegisterInfo *reg, uint64_t val, uint64_t we);
+
+/**
+ * read a value from a register, subject to its restrictions
+ * @reg: register to read from
+ * returns: value read
+ */
+
+uint64_t register_read(RegisterInfo *reg);
+
+/**
+ * reset a register
+ * @reg: register to reset
+ */
+
+void register_reset(RegisterInfo *reg);
+
+#endif
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 02/15] register: Add Memory API glue
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 01/15] register: Add Register API Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 03/15] register: Add support for decoding information Alistair Francis
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
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(+), 0 deletions(-)

diff --git a/hw/core/register.c b/hw/core/register.c
index 02a4376..ca10cff 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -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);
+}
diff --git a/include/hw/register.h b/include/hw/register.h
index 249f458..a3c41db 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -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
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 03/15] register: Add support for decoding information
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 01/15] register: Add Register API Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 02/15] register: Add Memory API glue Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 04/15] register: Define REG and FIELD macros Alistair Francis
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v4:
Remove extraneous unused defintions.

 include/hw/register.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/hw/register.h b/include/hw/register.h
index a3c41db..90c0185 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -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;
 };
 
 /**
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 04/15] register: Define REG and FIELD macros
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (2 preceding siblings ...)
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 03/15] register: Add support for decoding information Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 05/15] register: QOMify Alistair Francis
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
[ EI Changes:
  * Add Deposit macros
]
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
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]


 include/hw/register.h |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/include/hw/register.h b/include/hw/register.h
index 90c0185..0c6f03d 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -169,4 +169,42 @@ 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
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 05/15] register: QOMify
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (3 preceding siblings ...)
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 04/15] register: Define REG and FIELD macros Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 06/15] register: Add block initialise helper Alistair Francis
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/register.c    |   34 ++++++++++++++++++++++++++++++++++
 include/hw/register.h |   17 +++++++++++++++++
 2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/hw/core/register.c b/hw/core/register.c
index ca10cff..000b87f 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -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)
diff --git a/include/hw/register.h b/include/hw/register.h
index 0c6f03d..6677dee 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -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
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 06/15] register: Add block initialise helper
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (4 preceding siblings ...)
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 05/15] register: QOMify Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 07/15] bitops: Add ONES macro Alistair Francis
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/register.c    |   28 ++++++++++++++++++++++++++++
 include/hw/register.h |   21 +++++++++++++++++++++
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/hw/core/register.c b/hw/core/register.c
index 000b87f..4e59122 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -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,
diff --git a/include/hw/register.h b/include/hw/register.h
index 6677dee..f3e4c2c 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -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)                                                  \
     enum { A_ ## reg = (addr) };                                          \
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 07/15] bitops: Add ONES macro
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (5 preceding siblings ...)
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 06/15] register: Add block initialise helper Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 08/15] dma: Add Xilinx Zynq devcfg device model Alistair Francis
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/qemu/bitops.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 8164225..27bf98d 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -430,4 +430,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.7.1

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

* [Qemu-devel] [PATCH v1 08/15] dma: Add Xilinx Zynq devcfg device model
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (6 preceding siblings ...)
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 07/15] bitops: Add ONES macro Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 09/15] xilinx_zynq: add devcfg to machine model Alistair Francis
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
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(+), 0 deletions(-)
 create mode 100644 hw/dma/xlnx-zynq-devcfg.c
 create mode 100644 include/hw/dma/xlnx-zynq-devcfg.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 74f1db3..1222bce 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -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
diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
index 0e65ed0..eaf0a81 100644
--- a/hw/dma/Makefile.objs
+++ b/hw/dma/Makefile.objs
@@ -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
diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
new file mode 100644
index 0000000..c51b6e0
--- /dev/null
+++ b/hw/dma/xlnx-zynq-devcfg.c
@@ -0,0 +1,406 @@
+/*
+ * QEMU model of the Xilinx Zynq Devcfg Interface
+ *
+ * (C) 2011 PetaLogix Pty Ltd
+ * (C) 2014 Xilinx Inc.
+ * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/dma/xlnx-zynq-devcfg.h"
+#include "qemu/bitops.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/dma.h"
+
+#define FREQ_HZ 900000000
+
+#define BTT_MAX 0x400
+
+#ifndef XLNX_ZYNQ_DEVCFG_ERR_DEBUG
+#define XLNX_ZYNQ_DEVCFG_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT(...) do { \
+    if (XLNX_ZYNQ_DEVCFG_ERR_DEBUG) { \
+        qemu_log("%s: ", __func__); \
+        qemu_log(__VA_ARGS__); \
+    } \
+} while (0);
+
+REG32(CTRL, 0x00)
+    FIELD(CTRL,     FORCE_RST,          31,  1) /* Not supported, wr ignored */
+    FIELD(CTRL,     PCAP_PR,            27,  1) /* Forced to 0 on bad unlock */
+    FIELD(CTRL,     PCAP_MODE,          26,  1)
+    FIELD(CTRL,     MULTIBOOT_EN,       24,  1)
+    FIELD(CTRL,     USER_MODE,          15,  1)
+    FIELD(CTRL,     PCFG_AES_FUSE,      12,  1)
+    FIELD(CTRL,     PCFG_AES_EN,         9,  3)
+    FIELD(CTRL,     SEU_EN,              8,  1)
+    FIELD(CTRL,     SEC_EN,              7,  1)
+    FIELD(CTRL,     SPNIDEN,             6,  1)
+    FIELD(CTRL,     SPIDEN,              5,  1)
+    FIELD(CTRL,     NIDEN,               4,  1)
+    FIELD(CTRL,     DBGEN,               3,  1)
+    FIELD(CTRL,     DAP_EN,              0,  3)
+
+REG32(LOCK, 0x04)
+    #define AES_FUSE_LOCK        4
+    #define AES_EN_LOCK          3
+    #define SEU_LOCK             2
+    #define SEC_LOCK             1
+    #define DBG_LOCK             0
+
+/* mapping bits in R_LOCK to what they lock in R_CTRL */
+static const uint32_t lock_ctrl_map[] = {
+    [AES_FUSE_LOCK] = R_CTRL_PCFG_AES_FUSE_MASK,
+    [AES_EN_LOCK]   = R_CTRL_PCFG_AES_EN_MASK,
+    [SEU_LOCK]      = R_CTRL_SEU_EN_MASK,
+    [SEC_LOCK]      = R_CTRL_SEC_EN_MASK,
+    [DBG_LOCK]      = R_CTRL_SPNIDEN_MASK | R_CTRL_SPIDEN_MASK |
+                      R_CTRL_NIDEN_MASK   | R_CTRL_DBGEN_MASK  |
+                      R_CTRL_DAP_EN_MASK,
+};
+
+REG32(CFG, 0x08)
+    FIELD(CFG,      RFIFO_TH,           10,  2)
+    FIELD(CFG,      WFIFO_TH,            8,  2)
+    FIELD(CFG,      RCLK_EDGE,           7,  1)
+    FIELD(CFG,      WCLK_EDGE,           6,  1)
+    FIELD(CFG,      DISABLE_SRC_INC,     5,  1)
+    FIELD(CFG,      DISABLE_DST_INC,     4,  1)
+#define R_CFG_RO 0xFFFFF000
+#define R_CFG_RESET 0x50B
+
+REG32(INT_STS, 0x0C)
+    FIELD(INT_STS,  PSS_GTS_USR_B,      31,  1)
+    FIELD(INT_STS,  PSS_FST_CFG_B,      30,  1)
+    FIELD(INT_STS,  PSS_CFG_RESET_B,    27,  1)
+    FIELD(INT_STS,  RX_FIFO_OV,         18,  1)
+    FIELD(INT_STS,  WR_FIFO_LVL,        17,  1)
+    FIELD(INT_STS,  RD_FIFO_LVL,        16,  1)
+    FIELD(INT_STS,  DMA_CMD_ERR,        15,  1)
+    FIELD(INT_STS,  DMA_Q_OV,           14,  1)
+    FIELD(INT_STS,  DMA_DONE,           13,  1)
+    FIELD(INT_STS,  DMA_P_DONE,         12,  1)
+    FIELD(INT_STS,  P2D_LEN_ERR,        11,  1)
+    FIELD(INT_STS,  PCFG_DONE,           2,  1)
+    #define R_INT_STS_RSVD       ((0x7 << 24) | (0x1 << 19) | (0xF < 7))
+
+REG32(INT_MASK, 0x10)
+
+REG32(STATUS, 0x14)
+    FIELD(STATUS,   DMA_CMD_Q_F,        31,  1)
+    FIELD(STATUS,   DMA_CMD_Q_E,        30,  1)
+    FIELD(STATUS,   DMA_DONE_CNT,       28,  2)
+    FIELD(STATUS,   RX_FIFO_LVL,        20,  5)
+    FIELD(STATUS,   TX_FIFO_LVL,        12,  7)
+    FIELD(STATUS,   PSS_GTS_USR_B,      11,  1)
+    FIELD(STATUS,   PSS_FST_CFG_B,      10,  1)
+    FIELD(STATUS,   PSS_CFG_RESET_B,     5,  1)
+
+REG32(DMA_SRC_ADDR, 0x18)
+REG32(DMA_DST_ADDR, 0x1C)
+REG32(DMA_SRC_LEN, 0x20)
+REG32(DMA_DST_LEN, 0x24)
+REG32(ROM_SHADOW, 0x28)
+REG32(SW_ID, 0x30)
+REG32(UNLOCK, 0x34)
+
+#define R_UNLOCK_MAGIC 0x757BDF0D
+
+REG32(MCTRL, 0x80)
+    FIELD(MCTRL,    PS_VERSION,         28,  4)
+    FIELD(MCTRL,    PCFG_POR_B,          8,  1)
+    FIELD(MCTRL,    INT_PCAP_LPBK,       4,  1)
+    FIELD(MCTRL,    QEMU,                3,  1)
+
+static void xlnx_zynq_devcfg_update_ixr(XlnxZynqDevcfg *s)
+{
+    qemu_set_irq(s->irq, ~s->regs[R_INT_MASK] & s->regs[R_INT_STS]);
+}
+
+static void xlnx_zynq_devcfg_reset(DeviceState *dev)
+{
+    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(dev);
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQ_DEVCFG_R_MAX; ++i) {
+        register_reset(&s->regs_info[i]);
+    }
+}
+
+static void xlnx_zynq_devcfg_dma_go(XlnxZynqDevcfg *s)
+{
+    for (;;) {
+        uint8_t buf[BTT_MAX];
+        XlnxZynqDevcfgDMACmd *dmah = s->dma_cmd_fifo;
+        uint32_t btt = BTT_MAX;
+        bool loopback = s->regs[R_MCTRL] & R_MCTRL_INT_PCAP_LPBK_MASK;
+
+        btt = MIN(btt, dmah->src_len);
+        if (loopback) {
+            btt = MIN(btt, dmah->dest_len);
+        }
+        DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr);
+        dma_memory_read(&address_space_memory, dmah->src_addr, buf, btt);
+        dmah->src_len -= btt;
+        dmah->src_addr += btt;
+        if (loopback) {
+            DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr);
+            dma_memory_write(&address_space_memory, dmah->dest_addr, buf, btt);
+            dmah->dest_len -= btt;
+            dmah->dest_addr += btt;
+        }
+        if (!dmah->src_len && !dmah->dest_len) {
+            DB_PRINT("dma operation finished\n");
+            s->regs[R_INT_STS] |= R_INT_STS_DMA_DONE_MASK |
+                                  R_INT_STS_DMA_P_DONE_MASK;
+            s->dma_cmd_fifo_num--;
+            memmove(s->dma_cmd_fifo, &s->dma_cmd_fifo[1],
+                    sizeof(*s->dma_cmd_fifo) * XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN
+                                                                        - 1);
+        }
+        xlnx_zynq_devcfg_update_ixr(s);
+        if (!s->dma_cmd_fifo_num) { /* All done */
+            return;
+        }
+    }
+}
+
+static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
+{
+    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(reg->opaque);
+
+    xlnx_zynq_devcfg_update_ixr(s);
+}
+
+static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val)
+{
+    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(reg->opaque);
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(lock_ctrl_map); ++i) {
+        if (s->regs[R_LOCK] & 1 << i) {
+            val &= ~lock_ctrl_map[i];
+            val |= lock_ctrl_map[i] & s->regs[R_CTRL];
+        }
+    }
+    return val;
+}
+
+static void r_ctrl_post_write(RegisterInfo *reg, uint64_t val)
+{
+    uint32_t aes_en = F_EX32(val, CTRL, PCFG_AES_EN);
+
+    if (aes_en != 0 && aes_en != 7) {
+        qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
+                      "unimplemented security reset should happen!\n",
+                      reg->prefix);
+    }
+}
+
+static void r_unlock_post_write(RegisterInfo *reg, uint64_t val)
+{
+    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(reg->opaque);
+
+    if (val == R_UNLOCK_MAGIC) {
+        DB_PRINT("successful unlock\n");
+        /* BootROM will have already done the actual unlock so no need to do
+         * anything in successful subsequent unlock
+         */
+    } else { /* bad unlock attempt */
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->prefix);
+        s->regs[R_CTRL] &= ~R_CTRL_PCAP_PR_MASK;
+        s->regs[R_CTRL] &= ~R_CTRL_PCFG_AES_EN_MASK;
+        /* core becomes inaccessible */
+        memory_region_set_enabled(&s->iomem, false);
+    }
+}
+
+static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val)
+{
+    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(reg->opaque);
+
+    /* once bits are locked they stay locked */
+    return s->regs[R_LOCK] | val;
+}
+
+static void r_dma_dst_len_post_write(RegisterInfo *reg, uint64_t val)
+{
+    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(reg->opaque);
+
+    s->dma_cmd_fifo[s->dma_cmd_fifo_num] = (XlnxZynqDevcfgDMACmd) {
+            .src_addr = s->regs[R_DMA_SRC_ADDR] & ~0x3UL,
+            .dest_addr = s->regs[R_DMA_DST_ADDR] & ~0x3UL,
+            .src_len = s->regs[R_DMA_SRC_LEN] << 2,
+            .dest_len = s->regs[R_DMA_DST_LEN] << 2,
+    };
+    s->dma_cmd_fifo_num++;
+    DB_PRINT("dma transfer started; %d total transfers pending\n",
+             s->dma_cmd_fifo_num);
+    xlnx_zynq_devcfg_dma_go(s);
+}
+
+static const RegisterAccessInfo xlnx_zynq_devcfg_regs_info[] = {
+    {   .name = "CTRL",                 .decode.addr = A_CTRL,
+        .reset = R_CTRL_PCAP_PR_MASK | R_CTRL_PCAP_MODE_MASK | 0x3 << 13,
+        .rsvd = 0x1 << 28 | 0x3ff << 13 | 0x3 << 13,
+        .ui1 = (RegisterAccessError[]) {
+            { .mask = R_CTRL_FORCE_RST_MASK,
+                .reason = "PS reset not implemented" },
+            { .mask = R_CTRL_PCAP_MODE_MASK,
+                .reason = "FPGA Fabric doesn't exist" },
+            { .mask = R_CTRL_PCFG_AES_EN_MASK,
+                .reason = "AES not implemented" },
+            {},
+        },
+        .pre_write = r_ctrl_pre_write,
+        .post_write = r_ctrl_post_write,
+    },
+    {   .name = "LOCK",                 .decode.addr = A_LOCK,
+        .rsvd = ~ONES(5),
+        .pre_write = r_lock_pre_write,
+    },
+    {   .name = "CFG",                  .decode.addr = A_CFG,
+        .reset = 1 << R_CFG_RFIFO_TH_SHIFT | 1 << R_CFG_WFIFO_TH_SHIFT | 0x8,
+        .rsvd = 0xfffff00f,
+    },
+    {   .name = "INT_STS",              .decode.addr = A_INT_STS,
+        .w1c = ~R_INT_STS_RSVD,
+        .reset = R_INT_STS_PSS_GTS_USR_B_MASK   |
+                 R_INT_STS_PSS_CFG_RESET_B_MASK |
+                 R_INT_STS_WR_FIFO_LVL_MASK,
+        .rsvd = R_INT_STS_RSVD,
+        .post_write = r_ixr_post_write,
+    },
+    {   .name = "INT_MASK",            .decode.addr = A_INT_MASK,
+        .reset = ~0,
+        .rsvd = R_INT_STS_RSVD,
+        .post_write = r_ixr_post_write,
+    },
+    {   .name = "STATUS",               .decode.addr = A_STATUS,
+        .reset = R_STATUS_DMA_CMD_Q_E_MASK      |
+                 R_STATUS_PSS_GTS_USR_B_MASK    |
+                 R_STATUS_PSS_CFG_RESET_B_MASK,
+        .ro = ~0,
+    },
+    {   .name = "DMA_SRC_ADDR",         .decode.addr = A_DMA_SRC_ADDR, },
+    {   .name = "DMA_DST_ADDR",         .decode.addr = A_DMA_DST_ADDR, },
+    {   .name = "DMA_SRC_LEN",          .decode.addr = A_DMA_SRC_LEN,
+        .ro = ~ONES(27) },
+    {   .name = "DMA_DST_LEN",          .decode.addr = A_DMA_DST_LEN,
+        .ro = ~ONES(27),
+        .post_write = r_dma_dst_len_post_write,
+    },
+    {   .name = "ROM_SHADOW",           .decode.addr = A_ROM_SHADOW,
+        .rsvd = ~0ull,
+    },
+    {   .name = "SW_ID",                .decode.addr = A_SW_ID, },
+    {   .name = "UNLOCK",               .decode.addr = A_UNLOCK,
+        .post_write = r_unlock_post_write,
+    },
+    {   .name = "MCTRL",                .decode.addr = R_MCTRL * 4,
+       /* Silicon 3.0 for version field, the mysterious reserved bit 23
+        * and QEMU platform identifier.
+        */
+       .reset = 0x2 << R_MCTRL_PS_VERSION_SHIFT | 1 << 23 | R_MCTRL_QEMU_MASK,
+       .ro = ~R_MCTRL_INT_PCAP_LPBK_MASK,
+       .rsvd = 0x00f00303,
+    },
+};
+
+static const MemoryRegionOps xlnx_zynq_devcfg_reg_ops = {
+    .read = register_read_memory_le,
+    .write = register_write_memory_le,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    }
+};
+
+static const VMStateDescription vmstate_xlnx_zynq_devcfg_dma_cmd = {
+    .name = "xlnx_zynq_devcfg_dma_cmd",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(src_addr, XlnxZynqDevcfgDMACmd),
+        VMSTATE_UINT32(dest_addr, XlnxZynqDevcfgDMACmd),
+        VMSTATE_UINT32(src_len, XlnxZynqDevcfgDMACmd),
+        VMSTATE_UINT32(dest_len, XlnxZynqDevcfgDMACmd),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_xlnx_zynq_devcfg = {
+    .name = "xlnx_zynq_devcfg",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT_ARRAY(dma_cmd_fifo, XlnxZynqDevcfg,
+                             XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN, 0,
+                             vmstate_xlnx_zynq_devcfg_dma_cmd,
+                             XlnxZynqDevcfgDMACmd),
+        VMSTATE_UINT8(dma_cmd_fifo_num, XlnxZynqDevcfg),
+        VMSTATE_UINT32_ARRAY(regs, XlnxZynqDevcfg, XLNX_ZYNQ_DEVCFG_R_MAX),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void xlnx_zynq_devcfg_init(Object *obj)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(obj);
+
+    sysbus_init_irq(sbd, &s->irq);
+
+    memory_region_init(&s->iomem, obj, "devcfg", XLNX_ZYNQ_DEVCFG_R_MAX*4);
+    register_init_block32(DEVICE(obj), xlnx_zynq_devcfg_regs_info,
+                          ARRAY_SIZE(xlnx_zynq_devcfg_regs_info),
+                          s->regs_info, s->regs, &s->iomem,
+                          &xlnx_zynq_devcfg_reg_ops,
+                          XLNX_ZYNQ_DEVCFG_ERR_DEBUG);
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void xlnx_zynq_devcfg_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = xlnx_zynq_devcfg_reset;
+    dc->vmsd = &vmstate_xlnx_zynq_devcfg;
+}
+
+static const TypeInfo xlnx_zynq_devcfg_info = {
+    .name           = TYPE_XLNX_ZYNQ_DEVCFG,
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(XlnxZynqDevcfg),
+    .instance_init  = xlnx_zynq_devcfg_init,
+    .class_init     = xlnx_zynq_devcfg_class_init,
+};
+
+static void xlnx_zynq_devcfg_register_types(void)
+{
+    type_register_static(&xlnx_zynq_devcfg_info);
+}
+
+type_init(xlnx_zynq_devcfg_register_types)
diff --git a/include/hw/dma/xlnx-zynq-devcfg.h b/include/hw/dma/xlnx-zynq-devcfg.h
new file mode 100644
index 0000000..d40e5c8
--- /dev/null
+++ b/include/hw/dma/xlnx-zynq-devcfg.h
@@ -0,0 +1,62 @@
+/*
+ * QEMU model of the Xilinx Devcfg Interface
+ *
+ * (C) 2011 PetaLogix Pty Ltd
+ * (C) 2014 Xilinx Inc.
+ * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef XLNX_ZYNQ_DEVCFG_H
+
+#include "hw/register.h"
+#include "hw/sysbus.h"
+
+#define TYPE_XLNX_ZYNQ_DEVCFG "xlnx.ps7-dev-cfg"
+
+#define XLNX_ZYNQ_DEVCFG(obj) \
+    OBJECT_CHECK(XlnxZynqDevcfg, (obj), TYPE_XLNX_ZYNQ_DEVCFG)
+
+#define XLNX_ZYNQ_DEVCFG_R_MAX 0x118
+
+#define XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN 10
+
+typedef struct XlnxZynqDevcfgDMACmd {
+    uint32_t src_addr;
+    uint32_t dest_addr;
+    uint32_t src_len;
+    uint32_t dest_len;
+} XlnxZynqDevcfgDMACmd;
+
+typedef struct XlnxZynqDevcfg {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    qemu_irq irq;
+
+    XlnxZynqDevcfgDMACmd dma_cmd_fifo[XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN];
+    uint8_t dma_cmd_fifo_num;
+
+    uint32_t regs[XLNX_ZYNQ_DEVCFG_R_MAX];
+    RegisterInfo regs_info[XLNX_ZYNQ_DEVCFG_R_MAX];
+} XlnxZynqDevcfg;
+
+#define XLNX_ZYNQ_DEVCFG_H
+#endif
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 09/15] xilinx_zynq: add devcfg to machine model
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (7 preceding siblings ...)
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 08/15] dma: Add Xilinx Zynq devcfg device model Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 10/15] qdev: Define qdev_get_gpio_out Alistair Francis
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
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 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index a4e7b5c..df25cc6 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -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;
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 10/15] qdev: Define qdev_get_gpio_out
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (8 preceding siblings ...)
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 09/15] xilinx_zynq: add devcfg to machine model Alistair Francis
@ 2015-07-29 20:24 ` Alistair Francis
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 11/15] qdev: Add qdev_pass_all_gpios API Alistair Francis
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c         |   12 ++++++++++++
 include/hw/qdev-core.h |    2 ++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b2f404a..3594151 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -476,6 +476,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)
 {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 038b54d..0a21d57 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -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,
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 11/15] qdev: Add qdev_pass_all_gpios API
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (9 preceding siblings ...)
  2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 10/15] qdev: Define qdev_get_gpio_out Alistair Francis
@ 2015-07-29 20:25 ` Alistair Francis
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 12/15] irq: Add opaque setter routine Alistair Francis
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c         |    9 +++++++++
 include/hw/qdev-core.h |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 3594151..75f8139 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -576,6 +576,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;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 0a21d57..1cf44e1 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -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);
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 12/15] irq: Add opaque setter routine
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (10 preceding siblings ...)
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 11/15] qdev: Add qdev_pass_all_gpios API Alistair Francis
@ 2015-07-29 20:25 ` Alistair Francis
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 13/15] register: Add GPIO API Alistair Francis
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/irq.c    |    5 +++++
 include/hw/irq.h |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index 8a62a36..4a41059 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -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;
diff --git a/include/hw/irq.h b/include/hw/irq.h
index 4c4c2ea..edad0fc 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -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);
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 13/15] register: Add GPIO API
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (11 preceding siblings ...)
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 12/15] irq: Add opaque setter routine Alistair Francis
@ 2015-07-29 20:25 ` Alistair Francis
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 14/15] misc: Introduce ZynqMP IOU SLCR Alistair Francis
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
[ EI Changes:
  * register: Add a polarity field to GPIO connections
              Makes it possible to directly connect active low signals
              to generic interrupt pins.
]
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---

 hw/core/register.c    |   94 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/register.h |   26 +++++++++++++
 2 files changed, 120 insertions(+), 0 deletions(-)

diff --git a/hw/core/register.c b/hw/core/register.c
index 4e59122..9ab118b 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -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));
diff --git a/include/hw/register.h b/include/hw/register.h
index f3e4c2c..7a4c1c7 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -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
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 14/15] misc: Introduce ZynqMP IOU SLCR
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (12 preceding siblings ...)
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 13/15] register: Add GPIO API Alistair Francis
@ 2015-07-29 20:25 ` Alistair Francis
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 15/15] xlnx-zynqmp: Connect the " Alistair Francis
  2015-08-27 21:47 ` [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 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(+), 0 deletions(-)
 create mode 100644 hw/misc/xlnx-zynqmp-iou-slcr.c
 create mode 100644 include/hw/misc/xlnx-zynqmp-iou-slcr.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 4aa76ff..8aa6038 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -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
diff --git a/hw/misc/xlnx-zynqmp-iou-slcr.c b/hw/misc/xlnx-zynqmp-iou-slcr.c
new file mode 100644
index 0000000..35b989c
--- /dev/null
+++ b/hw/misc/xlnx-zynqmp-iou-slcr.c
@@ -0,0 +1,113 @@
+/*
+ * Xilinx ZynqMP IOU System Level Control Registers (SLCR)
+ *
+ * Copyright (c) 2013 Xilinx Inc
+ * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/misc/xlnx-zynqmp-iou-slcr.h"
+
+#ifndef XLNX_ZYNQMP_IOU_SLCR_ERR_DEBUG
+#define XLNX_ZYNQMP_IOU_SLCR_ERR_DEBUG 0
+#endif
+
+REG32(SD_SLOTTYPE, 0x310)
+    #define R_SD_SLOTTYPE_RSVD       0xffff7ffe
+
+static const RegisterAccessInfo xlnx_zynqmp_iou_slcr_regs_info[] = {
+    {   .name = "SD Slot TYPE",             .decode.addr = A_SD_SLOTTYPE,
+            .rsvd = R_SD_SLOTTYPE_RSVD,
+            .gpios = (RegisterGPIOMapping []) {
+                { .name = "SD0_SLOTTYPE",   .bit_pos = 0  },
+                { .name = "SD1_SLOTTYPE",   .bit_pos = 15 },
+                {},
+            }
+    }
+    /* FIXME: Complete device model */
+};
+
+static void xlnx_zynqmp_iou_slcr_reset(DeviceState *dev)
+{
+    XlnxZynqMPIOUSLCR *s = XLNX_ZYNQMP_IOU_SLCR(dev);
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQ_MP_IOU_SLCR_R_MAX; ++i) {
+        register_reset(&s->regs_info[i]);
+    }
+}
+
+static const MemoryRegionOps xlnx_zynqmp_iou_slcr_ops = {
+    .read = register_read_memory_le,
+    .write = register_write_memory_le,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    }
+};
+
+static void xlnx_zynqmp_iou_slcr_init(Object *obj)
+{
+    XlnxZynqMPIOUSLCR *s = XLNX_ZYNQMP_IOU_SLCR(obj);
+
+    memory_region_init(&s->iomem, obj, "MMIO", XLNX_ZYNQ_MP_IOU_SLCR_R_MAX * 4);
+    register_init_block32(DEVICE(obj), xlnx_zynqmp_iou_slcr_regs_info,
+                          ARRAY_SIZE(xlnx_zynqmp_iou_slcr_regs_info),
+                          s->regs_info, s->regs, &s->iomem,
+                          &xlnx_zynqmp_iou_slcr_ops,
+                          XLNX_ZYNQMP_IOU_SLCR_ERR_DEBUG);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static const VMStateDescription vmstate_xlnx_zynqmp_iou_slcr = {
+    .name = "xlnx_zynqmp_iou_slcr",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPIOUSLCR,
+                             XLNX_ZYNQ_MP_IOU_SLCR_R_MAX),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static void xlnx_zynqmp_iou_slcr_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = xlnx_zynqmp_iou_slcr_reset;
+    dc->vmsd = &vmstate_xlnx_zynqmp_iou_slcr;
+}
+
+static const TypeInfo xlnx_zynqmp_iou_slcr_info = {
+    .name          = TYPE_XLNX_ZYNQMP_IOU_SLCR,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(XlnxZynqMPIOUSLCR),
+    .class_init    = xlnx_zynqmp_iou_slcr_class_init,
+    .instance_init = xlnx_zynqmp_iou_slcr_init,
+};
+
+static void xlnx_zynqmp_iou_slcr_register_types(void)
+{
+    type_register_static(&xlnx_zynqmp_iou_slcr_info);
+}
+
+type_init(xlnx_zynqmp_iou_slcr_register_types)
diff --git a/include/hw/misc/xlnx-zynqmp-iou-slcr.h b/include/hw/misc/xlnx-zynqmp-iou-slcr.h
new file mode 100644
index 0000000..1a3f7e9
--- /dev/null
+++ b/include/hw/misc/xlnx-zynqmp-iou-slcr.h
@@ -0,0 +1,47 @@
+/*
+ * Xilinx ZynqMP IOU system level control registers (SLCR)
+ *
+ * Copyright (c) 2013 Xilinx Inc
+ * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/register.h"
+
+#define TYPE_XLNX_ZYNQMP_IOU_SLCR "xlnx_zynqmp-iou-slcr"
+
+#define XLNX_ZYNQMP_IOU_SLCR(obj) \
+     OBJECT_CHECK(XlnxZynqMPIOUSLCR, (obj), TYPE_XLNX_ZYNQMP_IOU_SLCR)
+
+#define XLNX_ZYNQ_MP_IOU_SLCR_R_MAX (0x314/4)
+
+typedef struct XlnxZynqMPIOUSLCR XlnxZynqMPIOUSLCR;
+
+struct XlnxZynqMPIOUSLCR {
+    /*< private >*/
+    SysBusDevice busdev;
+
+    /*< public >*/
+    MemoryRegion iomem;
+
+    uint32_t regs[XLNX_ZYNQ_MP_IOU_SLCR_R_MAX];
+    RegisterInfo regs_info[XLNX_ZYNQ_MP_IOU_SLCR_R_MAX];
+};
-- 
1.7.1

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

* [Qemu-devel] [PATCH v1 15/15] xlnx-zynqmp: Connect the ZynqMP IOU SLCR
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (13 preceding siblings ...)
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 14/15] misc: Introduce ZynqMP IOU SLCR Alistair Francis
@ 2015-07-29 20:25 ` Alistair Francis
  2015-08-27 21:47 ` [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
  15 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2015-07-29 20:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: edgar.iglesias, peter.maydell, alistair.francis,
	crosthwaitepeter, edgar.iglesias, afaerber

Connect the I/O Unit System Level Control Registers device
to the ZynqMP model. Unfortunately the GPIO links can not be
connected yet as the SD device is not yet attached to the
ZynqMP machine.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xlnx-zynqmp.c         |   15 +++++++++++++++
 include/hw/arm/xlnx-zynqmp.h |    2 ++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 62ef4ce..deddb63 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -28,6 +28,8 @@
 #define GIC_DIST_ADDR       0xf9010000
 #define GIC_CPU_ADDR        0xf9020000
 
+#define IOU_SLCR_ADDR       0xFF180000
+
 static const uint64_t gem_addr[XLNX_ZYNQMP_NUM_GEMS] = {
     0xFF0B0000, 0xFF0C0000, 0xFF0D0000, 0xFF0E0000,
 };
@@ -90,6 +92,10 @@ static void xlnx_zynqmp_init(Object *obj)
         object_initialize(&s->uart[i], sizeof(s->uart[i]), TYPE_CADENCE_UART);
         qdev_set_parent_bus(DEVICE(&s->uart[i]), sysbus_get_default());
     }
+
+    object_initialize(&s->iou_slcr, sizeof(s->iou_slcr),
+                      TYPE_XLNX_ZYNQMP_IOU_SLCR);
+    qdev_set_parent_bus(DEVICE(&s->iou_slcr), sysbus_get_default());
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -235,6 +241,15 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart[i]), 0,
                            gic_spi[uart_intr[i]]);
     }
+
+    object_property_set_bool(OBJECT(&s->iou_slcr), true, "realized", &err);
+    if (err) {
+        error_propagate((errp), (err));
+        return;
+    }
+
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->iou_slcr), 0,
+                    TYPE_XLNX_ZYNQMP_IOU_SLCR);
 }
 
 static Property xlnx_zynqmp_props[] = {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index c379632..96b1e97 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -22,6 +22,7 @@
 #include "hw/intc/arm_gic.h"
 #include "hw/net/cadence_gem.h"
 #include "hw/char/cadence_uart.h"
+#include "hw/misc/xlnx-zynqmp-iou-slcr.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -54,6 +55,7 @@ typedef struct XlnxZynqMPState {
     MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
     CadenceGEMState gem[XLNX_ZYNQMP_NUM_GEMS];
     CadenceUARTState uart[XLNX_ZYNQMP_NUM_UARTS];
+    XlnxZynqMPIOUSLCR iou_slcr;
 
     char *boot_cpu;
     ARMCPU *boot_cpu_ptr;
-- 
1.7.1

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
                   ` (14 preceding siblings ...)
  2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 15/15] xlnx-zynqmp: Connect the " Alistair Francis
@ 2015-08-27 21:47 ` Alistair Francis
  2015-10-14 18:42   ` Alistair Francis
  15 siblings, 1 reply; 32+ messages in thread
From: Alistair Francis @ 2015-08-27 21:47 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Edgar Iglesias, Peter Maydell, qemu-devel@nongnu.org Developers,
	Peter Crosthwaite, Edgar Iglesias, Andreas Färber

Ping!

On Wed, Jul 29, 2015 at 1:24 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> From Peter:
> 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.
> P15: Connect the ZynqMP SLCR device
>
> 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
>
> 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.
>
> Changes since RFC:
>  - Connect the ZynqMP IOU SLCR device
>  - Rebase
>
> Changed from RFC 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.
>
>
> Alistair Francis (1):
>   xlnx-zynqmp: Connect the ZynqMP IOU SLCR
>
> 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/arm/xlnx-zynqmp.c                   |   15 ++
>  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/arm/xlnx-zynqmp.h           |    2 +
>  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                  |  274 +++++++++++++++++++++
>  include/qemu/bitops.h                  |    2 +
>  18 files changed, 1354 insertions(+), 0 deletions(-)
>  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
>
>

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-08-27 21:47 ` [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
@ 2015-10-14 18:42   ` Alistair Francis
  2015-10-30  6:52     ` Peter Crosthwaite
  0 siblings, 1 reply; 32+ messages in thread
From: Alistair Francis @ 2015-10-14 18:42 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Edgar Iglesias, Peter Maydell, qemu-devel@nongnu.org Developers,
	Peter Crosthwaite, Edgar Iglesias, Andreas Färber

Ping^2

On Thu, Aug 27, 2015 at 2:47 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> Ping!
>
> On Wed, Jul 29, 2015 at 1:24 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> From Peter:
>> 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.
>> P15: Connect the ZynqMP SLCR device
>>
>> 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
>>
>> 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.
>>
>> Changes since RFC:
>>  - Connect the ZynqMP IOU SLCR device
>>  - Rebase
>>
>> Changed from RFC 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.
>>
>>
>> Alistair Francis (1):
>>   xlnx-zynqmp: Connect the ZynqMP IOU SLCR
>>
>> 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/arm/xlnx-zynqmp.c                   |   15 ++
>>  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/arm/xlnx-zynqmp.h           |    2 +
>>  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                  |  274 +++++++++++++++++++++
>>  include/qemu/bitops.h                  |    2 +
>>  18 files changed, 1354 insertions(+), 0 deletions(-)
>>  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
>>
>>

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-10-14 18:42   ` Alistair Francis
@ 2015-10-30  6:52     ` Peter Crosthwaite
  2015-10-30  8:06       ` Peter Maydell
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Crosthwaite @ 2015-10-30  6:52 UTC (permalink / raw)
  To: Alistair Francis, Peter Maydell
  Cc: Edgar Iglesias, Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Andreas Färber

Ping^3

This has been on list for a very long time without 3rd party review.
Can I send a PULL?

Regards,
Peter

On Wed, Oct 14, 2015 at 11:42 AM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> Ping^2
>
> On Thu, Aug 27, 2015 at 2:47 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> Ping!
>>
>> On Wed, Jul 29, 2015 at 1:24 PM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> From Peter:
>>> 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.
>>> P15: Connect the ZynqMP SLCR device
>>>
>>> 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
>>>
>>> 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.
>>>
>>> Changes since RFC:
>>>  - Connect the ZynqMP IOU SLCR device
>>>  - Rebase
>>>
>>> Changed from RFC 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.
>>>
>>>
>>> Alistair Francis (1):
>>>   xlnx-zynqmp: Connect the ZynqMP IOU SLCR
>>>
>>> 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/arm/xlnx-zynqmp.c                   |   15 ++
>>>  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/arm/xlnx-zynqmp.h           |    2 +
>>>  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                  |  274 +++++++++++++++++++++
>>>  include/qemu/bitops.h                  |    2 +
>>>  18 files changed, 1354 insertions(+), 0 deletions(-)
>>>  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
>>>
>>>

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-10-30  6:52     ` Peter Crosthwaite
@ 2015-10-30  8:06       ` Peter Maydell
  2015-12-15 19:46         ` Peter Maydell
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-10-30  8:06 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Edgar Iglesias, Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Andreas Färber, Alistair Francis

On 30 October 2015 at 06:52, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> Ping^3
>
> This has been on list for a very long time without 3rd party review.
> Can I send a PULL?

I would prefer not to take a new unreviewed feature
in softfreeze for 2.5...

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-10-30  8:06       ` Peter Maydell
@ 2015-12-15 19:46         ` Peter Maydell
  2015-12-15 20:52           ` Peter Crosthwaite
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-12-15 19:46 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Edgar Iglesias, Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Andreas Färber, Alistair Francis

On 30 October 2015 at 08:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 30 October 2015 at 06:52, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> Ping^3
>>
>> This has been on list for a very long time without 3rd party review.
>> Can I send a PULL?
>
> I would prefer not to take a new unreviewed feature
> in softfreeze for 2.5...

Since we're now about to come out of 2.5 freeze I guess I
should write something about this patchset. I don't have any
fundamental objections to it, but it doesn't really excite me
either. I would like to see it reviewed by somebody else who
does think it's a good idea, because I think that increases
the chances that we will get general use of the facilities
rather than it being an odd thing used by a few Xilinx device
models and nothing else.

I hope that makes sense and doesn't seem too arbitrary a
hurdle to make you jump?

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-12-15 19:46         ` Peter Maydell
@ 2015-12-15 20:52           ` Peter Crosthwaite
  2015-12-15 21:56             ` Peter Maydell
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Crosthwaite @ 2015-12-15 20:52 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar Iglesias, Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Andreas Färber, Alistair Francis

On Tue, Dec 15, 2015 at 11:46 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 30 October 2015 at 08:06, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 30 October 2015 at 06:52, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> Ping^3
>>>
>>> This has been on list for a very long time without 3rd party review.
>>> Can I send a PULL?
>>
>> I would prefer not to take a new unreviewed feature
>> in softfreeze for 2.5...
>
> Since we're now about to come out of 2.5 freeze I guess I
> should write something about this patchset. I don't have any
> fundamental objections to it, but it doesn't really excite me
> either. I would like to see it reviewed by somebody else who
> does think it's a good idea, because I think that increases
> the chances that we will get general use of the facilities
> rather than it being an odd thing used by a few Xilinx device
> models and nothing else.
>

It needs to exist before it can be used so there is a bit of a chicken
and egg problem there. It is originally my code and Alistair has taken
ownership of it so that excludes the both of us. Aside from yourself,
there aren't too many new device-model authors out there who are
active review. Do you have a nominee?

It is a developer-centric feature as you can do things like gen device
model templates from data. The philosophy here (which is contrary to
most maintainers thinking) is that hardware designers and people who
constantly change hardware design specs and create new bits of IP are
the consumer of QEMU and the QEMU source code is a consumable. So I
see why it is hard to excite your regular QEMU developer who is
thinking purely about non-engineering end consumers.

I'll throw a new argument into the mix that is closer to home for
yourself, it has a lot in common with the ARM CP API. If we converted
the ARM CP API to be data driven rather than code driven (which we
have), why are MMIO devices so different? CP accesses can be
side-effectless or require side-effect causing functions, and 99% of
sysbus devices fit this description. Ideally, I'd like to mass-covert
CP API to use something like this for one API to rule them all. If I
instead morphed the CP API to a generic feature in hw/core, extended
with the features of this patch set, would that work better for you?
Then both devices and custom register APIs (like CP) can be
standardised. Note that this is already two layered. The concept of
the register API which defines collections of registers is separate
from sysbus.

Regards,
Peter

> I hope that makes sense and doesn't seem too arbitrary a
> hurdle to make you jump?
>
> thanks
> -- PMM

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-12-15 20:52           ` Peter Crosthwaite
@ 2015-12-15 21:56             ` Peter Maydell
  2015-12-16 16:33               ` Alistair Francis
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-12-15 21:56 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Edgar Iglesias, Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Andreas Färber, Alistair Francis

On 15 December 2015 at 20:52, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> It needs to exist before it can be used so there is a bit of a chicken
> and egg problem there.

Right, but the hope would be that there's somebody who cares
about device models who thinks it's worthwhile... if we really
do only have three people who write devices then that's a bit sad.

> It is originally my code and Alistair has taken
> ownership of it so that excludes the both of us. Aside from yourself,
> there aren't too many new device-model authors out there who are
> active review. Do you have a nominee?

For instance, do any of the MIPS, PPC or SPARC maintainers see
it as useful for their devices?

> I'll throw a new argument into the mix that is closer to home for
> yourself, it has a lot in common with the ARM CP API. If we converted
> the ARM CP API to be data driven rather than code driven (which we
> have), why are MMIO devices so different? CP accesses can be
> side-effectless or require side-effect causing functions, and 99% of
> sysbus devices fit this description. Ideally, I'd like to mass-covert
> CP API to use something like this for one API to rule them all. If I
> instead morphed the CP API to a generic feature in hw/core, extended
> with the features of this patch set, would that work better for you?
> Then both devices and custom register APIs (like CP) can be
> standardised. Note that this is already two layered. The concept of
> the register API which defines collections of registers is separate
> from sysbus.

The coprocessor APIs are data driven because we had the previous
lots-of-switch-statements approach and it was terrible to
maintain. On the other hand I look at the average device
(say hw/misc/a9scu.c or hw/misc/arm_sysctl.c) and it doesn't
really seem that hard to deal with (in particular you only
have one level of switching, on the register address, rather
than four levels with opc1/opc2/crn/crm, and your average
device has tens of registers rather than hundreds).

Basically, the cp API change was a bit of an upheaval but
clearly worthwhile because we were feeling the pain of not
having it. I'm not convinced we're feeling that much pain from
our current approach to device registers, and I'm definitely
not convinced that we'd make a wholesale conversion to use
this new API the way we did with the cp API.

As I say, I don't fundamentally object to this, but I'd be
happier if you convinced some other developer of the utility...

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-12-15 21:56             ` Peter Maydell
@ 2015-12-16 16:33               ` Alistair Francis
  2016-01-08  0:39                 ` Alistair Francis
  0 siblings, 1 reply; 32+ messages in thread
From: Alistair Francis @ 2015-12-16 16:33 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Alistair Francis, Peter Crosthwaite, Edgar Iglesias,
	Andreas Färber

On Tue, Dec 15, 2015 at 1:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 15 December 2015 at 20:52, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> It needs to exist before it can be used so there is a bit of a chicken
>> and egg problem there.
>
> Right, but the hope would be that there's somebody who cares
> about device models who thinks it's worthwhile... if we really
> do only have three people who write devices then that's a bit sad.
>
>> It is originally my code and Alistair has taken
>> ownership of it so that excludes the both of us. Aside from yourself,
>> there aren't too many new device-model authors out there who are
>> active review. Do you have a nominee?
>
> For instance, do any of the MIPS, PPC or SPARC maintainers see
> it as useful for their devices?
>
>> I'll throw a new argument into the mix that is closer to home for
>> yourself, it has a lot in common with the ARM CP API. If we converted
>> the ARM CP API to be data driven rather than code driven (which we
>> have), why are MMIO devices so different? CP accesses can be
>> side-effectless or require side-effect causing functions, and 99% of
>> sysbus devices fit this description. Ideally, I'd like to mass-covert
>> CP API to use something like this for one API to rule them all. If I
>> instead morphed the CP API to a generic feature in hw/core, extended
>> with the features of this patch set, would that work better for you?
>> Then both devices and custom register APIs (like CP) can be
>> standardised. Note that this is already two layered. The concept of
>> the register API which defines collections of registers is separate
>> from sysbus.
>
> The coprocessor APIs are data driven because we had the previous
> lots-of-switch-statements approach and it was terrible to
> maintain. On the other hand I look at the average device
> (say hw/misc/a9scu.c or hw/misc/arm_sysctl.c) and it doesn't
> really seem that hard to deal with (in particular you only
> have one level of switching, on the register address, rather
> than four levels with opc1/opc2/crn/crm, and your average
> device has tens of registers rather than hundreds).

I think this problem is only going to get worse though.

Look at the ARM SMMU implementation which we have, it has hundreds of
registers (https://github.com/Xilinx/qemu/blob/master/hw/misc/arm-smmu.c).
Admittedly it is still only one level of switch statements, but
devices are only getting more complex.

Thanks,

Alistair

>
> Basically, the cp API change was a bit of an upheaval but
> clearly worthwhile because we were feeling the pain of not
> having it. I'm not convinced we're feeling that much pain from
> our current approach to device registers, and I'm definitely
> not convinced that we'd make a wholesale conversion to use
> this new API the way we did with the cp API.
>
> As I say, I don't fundamentally object to this, but I'd be
> happier if you convinced some other developer of the utility...
>
> thanks
> -- PMM
>

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2015-12-16 16:33               ` Alistair Francis
@ 2016-01-08  0:39                 ` Alistair Francis
  2016-01-08 10:40                   ` Peter Maydell
  0 siblings, 1 reply; 32+ messages in thread
From: Alistair Francis @ 2016-01-08  0:39 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Edgar Iglesias, Peter Maydell, qemu-devel@nongnu.org Developers,
	Peter Crosthwaite, Edgar Iglesias, Andreas Färber

On Wed, Dec 16, 2015 at 8:33 AM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Dec 15, 2015 at 1:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 15 December 2015 at 20:52, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> It needs to exist before it can be used so there is a bit of a chicken
>>> and egg problem there.

No one seems to be jumping at reviewing this. Can we just send a pull request?

Thanks,

Alistair

>>
>> Right, but the hope would be that there's somebody who cares
>> about device models who thinks it's worthwhile... if we really
>> do only have three people who write devices then that's a bit sad.
>>
>>> It is originally my code and Alistair has taken
>>> ownership of it so that excludes the both of us. Aside from yourself,
>>> there aren't too many new device-model authors out there who are
>>> active review. Do you have a nominee?
>>
>> For instance, do any of the MIPS, PPC or SPARC maintainers see
>> it as useful for their devices?
>>
>>> I'll throw a new argument into the mix that is closer to home for
>>> yourself, it has a lot in common with the ARM CP API. If we converted
>>> the ARM CP API to be data driven rather than code driven (which we
>>> have), why are MMIO devices so different? CP accesses can be
>>> side-effectless or require side-effect causing functions, and 99% of
>>> sysbus devices fit this description. Ideally, I'd like to mass-covert
>>> CP API to use something like this for one API to rule them all. If I
>>> instead morphed the CP API to a generic feature in hw/core, extended
>>> with the features of this patch set, would that work better for you?
>>> Then both devices and custom register APIs (like CP) can be
>>> standardised. Note that this is already two layered. The concept of
>>> the register API which defines collections of registers is separate
>>> from sysbus.
>>
>> The coprocessor APIs are data driven because we had the previous
>> lots-of-switch-statements approach and it was terrible to
>> maintain. On the other hand I look at the average device
>> (say hw/misc/a9scu.c or hw/misc/arm_sysctl.c) and it doesn't
>> really seem that hard to deal with (in particular you only
>> have one level of switching, on the register address, rather
>> than four levels with opc1/opc2/crn/crm, and your average
>> device has tens of registers rather than hundreds).
>
> I think this problem is only going to get worse though.
>
> Look at the ARM SMMU implementation which we have, it has hundreds of
> registers (https://github.com/Xilinx/qemu/blob/master/hw/misc/arm-smmu.c).
> Admittedly it is still only one level of switch statements, but
> devices are only getting more complex.
>
> Thanks,
>
> Alistair
>
>>
>> Basically, the cp API change was a bit of an upheaval but
>> clearly worthwhile because we were feeling the pain of not
>> having it. I'm not convinced we're feeling that much pain from
>> our current approach to device registers, and I'm definitely
>> not convinced that we'd make a wholesale conversion to use
>> this new API the way we did with the cp API.
>>
>> As I say, I don't fundamentally object to this, but I'd be
>> happier if you convinced some other developer of the utility...
>>
>> thanks
>> -- PMM
>>

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2016-01-08  0:39                 ` Alistair Francis
@ 2016-01-08 10:40                   ` Peter Maydell
  2016-01-08 11:05                     ` Edgar E. Iglesias
  2016-01-28 16:31                     ` Frederic Konrad
  0 siblings, 2 replies; 32+ messages in thread
From: Peter Maydell @ 2016-01-08 10:40 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Edgar Iglesias, Edgar Iglesias, Peter Crosthwaite,
	qemu-devel@nongnu.org Developers, Andreas Färber

On 8 January 2016 at 00:39, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Wed, Dec 16, 2015 at 8:33 AM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Tue, Dec 15, 2015 at 1:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 15 December 2015 at 20:52, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> It needs to exist before it can be used so there is a bit of a chicken
>>>> and egg problem there.
>
> No one seems to be jumping at reviewing this. Can we just send a pull request?

I don't necessarily require review [*]. I would like *somebody* other
than you Xilinx folk to say "yes, I think I would use this for
modelling devices". Otherwise all we have is "weird thing used
only in two or three Xilinx devices and nowhere else", which I'm
a bit reluctant to let into the tree. We already have a pretty
wide divergence in how devices look just based on the various
transitions from older to newer qdev/QOM/etc that are not complete.

[*] by which I mean, I will review this series if you can find
somebody else who's going to say they'd use it.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2016-01-08 10:40                   ` Peter Maydell
@ 2016-01-08 11:05                     ` Edgar E. Iglesias
  2016-01-19 19:51                       ` Alistair Francis
  2016-01-28 16:31                     ` Frederic Konrad
  1 sibling, 1 reply; 32+ messages in thread
From: Edgar E. Iglesias @ 2016-01-08 11:05 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar Iglesias, Peter Crosthwaite,
	qemu-devel@nongnu.org Developers, Andreas Färber,
	Alistair Francis

On Fri, Jan 08, 2016 at 10:40:28AM +0000, Peter Maydell wrote:
> On 8 January 2016 at 00:39, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
> > On Wed, Dec 16, 2015 at 8:33 AM, Alistair Francis
> > <alistair.francis@xilinx.com> wrote:
> >> On Tue, Dec 15, 2015 at 1:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> >>> On 15 December 2015 at 20:52, Peter Crosthwaite
> >>> <crosthwaitepeter@gmail.com> wrote:
> >>>> It needs to exist before it can be used so there is a bit of a chicken
> >>>> and egg problem there.
> >
> > No one seems to be jumping at reviewing this. Can we just send a pull request?
> 
> I don't necessarily require review [*]. I would like *somebody* other
> than you Xilinx folk to say "yes, I think I would use this for
> modelling devices". Otherwise all we have is "weird thing used
> only in two or three Xilinx devices and nowhere else", which I'm
> a bit reluctant to let into the tree. We already have a pretty
> wide divergence in how devices look just based on the various
> transitions from older to newer qdev/QOM/etc that are not complete.
> 
> [*] by which I mean, I will review this series if you can find
> somebody else who's going to say they'd use it.
>

Hi,

I have two general comments to the series.

1. I think we need to do something to allow mem-attributes to be passed to the reg-access callbacks and possibly also to add a way to filter on attrs in the data structure.

2. We had trouble in the Xilinx tree with the number of memory regions created when using the style were each reg becomes an MR. I think that style should either be disallowed or we need to fix the low limit (IIRC it was at around 1K MRs/Regs).

I'm part of the Xilinx team so I this outside of Peters review request but anyawy....

Cheers,
Edgar

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2016-01-08 11:05                     ` Edgar E. Iglesias
@ 2016-01-19 19:51                       ` Alistair Francis
  2016-01-19 21:35                         ` Edgar E. Iglesias
  0 siblings, 1 reply; 32+ messages in thread
From: Alistair Francis @ 2016-01-19 19:51 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: Edgar Iglesias, Peter Maydell, qemu-devel@nongnu.org Developers,
	Alistair Francis, Peter Crosthwaite, Andreas Färber

On Fri, Jan 8, 2016 at 3:05 AM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> On Fri, Jan 08, 2016 at 10:40:28AM +0000, Peter Maydell wrote:
>> On 8 January 2016 at 00:39, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>> > On Wed, Dec 16, 2015 at 8:33 AM, Alistair Francis
>> > <alistair.francis@xilinx.com> wrote:
>> >> On Tue, Dec 15, 2015 at 1:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> >>> On 15 December 2015 at 20:52, Peter Crosthwaite
>> >>> <crosthwaitepeter@gmail.com> wrote:
>> >>>> It needs to exist before it can be used so there is a bit of a chicken
>> >>>> and egg problem there.
>> >
>> > No one seems to be jumping at reviewing this. Can we just send a pull request?
>>
>> I don't necessarily require review [*]. I would like *somebody* other
>> than you Xilinx folk to say "yes, I think I would use this for
>> modelling devices". Otherwise all we have is "weird thing used
>> only in two or three Xilinx devices and nowhere else", which I'm
>> a bit reluctant to let into the tree. We already have a pretty
>> wide divergence in how devices look just based on the various
>> transitions from older to newer qdev/QOM/etc that are not complete.
>>
>> [*] by which I mean, I will review this series if you can find
>> somebody else who's going to say they'd use it.
>>
>
> Hi,
>
> I have two general comments to the series.
>
> 1. I think we need to do something to allow mem-attributes to be passed to the reg-access callbacks and possibly also to add a way to filter on attrs in the data structure.

I would prefer to add this in the future. We are already having enough
trouble getting it accepted now and this will add complexity.

In saying that, it shouldn't be too hard to add in the future. The
basic infrastructure is all there.

>
> 2. We had trouble in the Xilinx tree with the number of memory regions created when using the style were each reg becomes an MR. I think that style should either be disallowed or we need to fix the low limit (IIRC it was at around 1K MRs/Regs).

This I can help with. I am sending a V2 which doesn't print the memory
regions with infro mtree. When you start getting hundreds of registers
this becomes really painful. I can't see a nice way to avoid actually
adding the memory subregions though. It is just a linked list, what
limit is there for memory subregions?

Thanks,

Alistair

>
> I'm part of the Xilinx team so I this outside of Peters review request but anyawy....
>
> Cheers,
> Edgar
>

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2016-01-19 19:51                       ` Alistair Francis
@ 2016-01-19 21:35                         ` Edgar E. Iglesias
  0 siblings, 0 replies; 32+ messages in thread
From: Edgar E. Iglesias @ 2016-01-19 21:35 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Peter Maydell, Peter Crosthwaite, Andreas Färber,
	qemu-devel@nongnu.org Developers

[-- Attachment #1: Type: text/plain, Size: 2931 bytes --]

On 19 Jan 2016 20:52, "Alistair Francis" <alistair.francis@xilinx.com>
wrote:
>
> On Fri, Jan 8, 2016 at 3:05 AM, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > On Fri, Jan 08, 2016 at 10:40:28AM +0000, Peter Maydell wrote:
> >> On 8 January 2016 at 00:39, Alistair Francis
> >> <alistair.francis@xilinx.com> wrote:
> >> > On Wed, Dec 16, 2015 at 8:33 AM, Alistair Francis
> >> > <alistair.francis@xilinx.com> wrote:
> >> >> On Tue, Dec 15, 2015 at 1:56 PM, Peter Maydell <
peter.maydell@linaro.org> wrote:
> >> >>> On 15 December 2015 at 20:52, Peter Crosthwaite
> >> >>> <crosthwaitepeter@gmail.com> wrote:
> >> >>>> It needs to exist before it can be used so there is a bit of a
chicken
> >> >>>> and egg problem there.
> >> >
> >> > No one seems to be jumping at reviewing this. Can we just send a
pull request?
> >>
> >> I don't necessarily require review [*]. I would like *somebody* other
> >> than you Xilinx folk to say "yes, I think I would use this for
> >> modelling devices". Otherwise all we have is "weird thing used
> >> only in two or three Xilinx devices and nowhere else", which I'm
> >> a bit reluctant to let into the tree. We already have a pretty
> >> wide divergence in how devices look just based on the various
> >> transitions from older to newer qdev/QOM/etc that are not complete.
> >>
> >> [*] by which I mean, I will review this series if you can find
> >> somebody else who's going to say they'd use it.
> >>
> >
> > Hi,
> >
> > I have two general comments to the series.
> >
> > 1. I think we need to do something to allow mem-attributes to be passed
to the reg-access callbacks and possibly also to add a way to filter on
attrs in the data structure.
>
> I would prefer to add this in the future. We are already having enough
> trouble getting it accepted now and this will add complexity.
>
> In saying that, it shouldn't be too hard to add in the future. The
> basic infrastructure is all there.
>

Sounds good

> >
> > 2. We had trouble in the Xilinx tree with the number of memory regions
created when using the style were each reg becomes an MR. I think that
style should either be disallowed or we need to fix the low limit (IIRC it
was at around 1K MRs/Regs).
>
> This I can help with. I am sending a V2 which doesn't print the memory
> regions with infro mtree. When you start getting hundreds of registers
> this becomes really painful. I can't see a nice way to avoid actually
> adding the memory subregions though. It is just a linked list, what
> limit is there for memory subregions?
>

I don't remember all the details but it was not directly due to the large
amount of MRs but indirectly with the resulting AS. We hit aborts and
died.  Our skeleton generator does not use the mr style anymore for
example.

Cheers,
Edgar

> Thanks,
>
> Alistair
>
> >
> > I'm part of the Xilinx team so I this outside of Peters review request
but anyawy....
> >
> > Cheers,
> > Edgar
> >

[-- Attachment #2: Type: text/html, Size: 4152 bytes --]

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2016-01-08 10:40                   ` Peter Maydell
  2016-01-08 11:05                     ` Edgar E. Iglesias
@ 2016-01-28 16:31                     ` Frederic Konrad
  2016-01-28 16:34                       ` Peter Maydell
  1 sibling, 1 reply; 32+ messages in thread
From: Frederic Konrad @ 2016-01-28 16:31 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Alistair Francis, Peter Crosthwaite, Edgar Iglesias,
	Andreas Färber

On 08/01/2016 11:40, Peter Maydell wrote:
> On 8 January 2016 at 00:39, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Wed, Dec 16, 2015 at 8:33 AM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> On Tue, Dec 15, 2015 at 1:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> On 15 December 2015 at 20:52, Peter Crosthwaite
>>>> <crosthwaitepeter@gmail.com> wrote:
>>>>> It needs to exist before it can be used so there is a bit of a chicken
>>>>> and egg problem there.
>> No one seems to be jumping at reviewing this. Can we just send a pull request?
> I don't necessarily require review [*]. I would like *somebody* other
> than you Xilinx folk to say "yes, I think I would use this for
> modelling devices". Otherwise all we have is "weird thing used
> only in two or three Xilinx devices and nowhere else", which I'm
> a bit reluctant to let into the tree. We already have a pretty
> wide divergence in how devices look just based on the various
> transitions from older to newer qdev/QOM/etc that are not complete.
>
> [*] by which I mean, I will review this series if you can find
> somebody else who's going to say they'd use it.
>
> thanks
> -- PMM
>

Hi Peter,

This is useful for us as well.
My point view is that it is an easy and clean way of implementing devices.
BTW we have the same mechanism to model devices in SystemC.

Thanks,
Fred

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2016-01-28 16:31                     ` Frederic Konrad
@ 2016-01-28 16:34                       ` Peter Maydell
  2016-01-30  0:56                         ` Alistair Francis
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2016-01-28 16:34 UTC (permalink / raw)
  To: Frederic Konrad
  Cc: Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Alistair Francis, Peter Crosthwaite, Edgar Iglesias,
	Andreas Färber

On 28 January 2016 at 16:31, Frederic Konrad <fred.konrad@greensocs.com> wrote:
> On 08/01/2016 11:40, Peter Maydell wrote:
>> [*] by which I mean, I will review this series if you can find
>> somebody else who's going to say they'd use it.

> This is useful for us as well.
> My point view is that it is an easy and clean way of implementing devices.
> BTW we have the same mechanism to model devices in SystemC.

OK, that is the second user that I asked for, so I will put this
on my queue to review at some point.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers
  2016-01-28 16:34                       ` Peter Maydell
@ 2016-01-30  0:56                         ` Alistair Francis
  0 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2016-01-30  0:56 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar Iglesias, qemu-devel@nongnu.org Developers,
	Alistair Francis, Peter Crosthwaite, Edgar Iglesias,
	Andreas Färber, Frederic Konrad

On Thu, Jan 28, 2016 at 8:34 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 28 January 2016 at 16:31, Frederic Konrad <fred.konrad@greensocs.com> wrote:
>> On 08/01/2016 11:40, Peter Maydell wrote:
>>> [*] by which I mean, I will review this series if you can find
>>> somebody else who's going to say they'd use it.
>
>> This is useful for us as well.
>> My point view is that it is an easy and clean way of implementing devices.
>> BTW we have the same mechanism to model devices in SystemC.
>
> OK, that is the second user that I asked for, so I will put this
> on my queue to review at some point.

Great!

Thanks for looking at it Fred. I am sending out a V3 addressing your comments.

Thanks,

Alistair

>
> thanks
> -- PMM
>

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

end of thread, other threads:[~2016-01-30  0:57 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-29 20:24 [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 01/15] register: Add Register API Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 02/15] register: Add Memory API glue Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 03/15] register: Add support for decoding information Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 04/15] register: Define REG and FIELD macros Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 05/15] register: QOMify Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 06/15] register: Add block initialise helper Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 07/15] bitops: Add ONES macro Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 08/15] dma: Add Xilinx Zynq devcfg device model Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 09/15] xilinx_zynq: add devcfg to machine model Alistair Francis
2015-07-29 20:24 ` [Qemu-devel] [PATCH v1 10/15] qdev: Define qdev_get_gpio_out Alistair Francis
2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 11/15] qdev: Add qdev_pass_all_gpios API Alistair Francis
2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 12/15] irq: Add opaque setter routine Alistair Francis
2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 13/15] register: Add GPIO API Alistair Francis
2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 14/15] misc: Introduce ZynqMP IOU SLCR Alistair Francis
2015-07-29 20:25 ` [Qemu-devel] [PATCH v1 15/15] xlnx-zynqmp: Connect the " Alistair Francis
2015-08-27 21:47 ` [Qemu-devel] [PATCH v1 00/15] data-driven device registers Alistair Francis
2015-10-14 18:42   ` Alistair Francis
2015-10-30  6:52     ` Peter Crosthwaite
2015-10-30  8:06       ` Peter Maydell
2015-12-15 19:46         ` Peter Maydell
2015-12-15 20:52           ` Peter Crosthwaite
2015-12-15 21:56             ` Peter Maydell
2015-12-16 16:33               ` Alistair Francis
2016-01-08  0:39                 ` Alistair Francis
2016-01-08 10:40                   ` Peter Maydell
2016-01-08 11:05                     ` Edgar E. Iglesias
2016-01-19 19:51                       ` Alistair Francis
2016-01-19 21:35                         ` Edgar E. Iglesias
2016-01-28 16:31                     ` Frederic Konrad
2016-01-28 16:34                       ` Peter Maydell
2016-01-30  0:56                         ` Alistair Francis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.