* [Qemu-devel] [PATCH v3 0/5] Data Driven device registers & Zynq DEVCFG
@ 2013-05-24 5:46 peter.crosthwaite
2013-05-24 5:46 ` [Qemu-devel] [PATCH v3 1/5] bitops: Add ONES macro peter.crosthwaite
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: peter.crosthwaite @ 2013-05-24 5:46 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, edgar.iglesias, kraxel, pbonzini
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Hi All. This is a new scheme i've come up with handling device registers in a
data driven way. My motivation for this is to factor out a lot of the access
checking that seems to be replicated in every device. See P2 commit message for
further discussion.
P1 is a trivial addition to bitops.h
P2 is the main patch, adds the register definition functionality
P3 adds helpers that glue the register API to the Memory API
P4 is an example new device (the Xilinx Zynq devcfg) that uses this scheme.
P5 adds devcfg to the Zynq machine model
This devcfg device was particularly finnicky with per-bit restrictions which
prompted all this. I'm also looking for a higher-than-usual modelling fidelity
on the register space, with semantics defined for random reserved bits
in-between otherwise consistent fields.
Heres 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
Future work: Theres a lot of overlap here with what Peter did with the ARM
coprocessor definitions. We could go further and generalise ARM CP to use this
or some further evolution of it. That and converting existing models to this
scheme. Some device models will lose a lot of weight.
Changed from v2:
Fixed for hw/ re-orginisation (Paolo review)
Simplified and optimized (PMM and Gerd review)
Changed from v1:
Added ONES macro patch
Dropped bogus former patch 1 (PMM review)
Addressed Blue, Gerd and MST comments.
Simplified to be more Memory API compatible.
Added Memory API helpers.
Please see discussion already on list and commit msgs for more detail.
Peter A. G. Crosthwaite (2):
xilinx_devcfg: Zynq devcfg device model
xilinx_zynq: added devcfg to machine model
Peter Crosthwaite (3):
bitops: Add ONES macro
register: Add Register API
register: Add Memory API glue
default-configs/arm-softmmu.mak | 1 +
hw/arm/xilinx_zynq.c | 8 +
hw/core/Makefile.objs | 1 +
hw/core/register.c | 238 +++++++++++++++++++
hw/dma/Makefile.objs | 1 +
hw/dma/xilinx_devcfg.c | 495 ++++++++++++++++++++++++++++++++++++++++
include/hw/register.h | 141 ++++++++++++
include/qemu/bitops.h | 2 +
8 files changed, 887 insertions(+)
create mode 100644 hw/core/register.c
create mode 100644 hw/dma/xilinx_devcfg.c
create mode 100644 include/hw/register.h
--
1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 1/5] bitops: Add ONES macro
2013-05-24 5:46 [Qemu-devel] [PATCH v3 0/5] Data Driven device registers & Zynq DEVCFG peter.crosthwaite
@ 2013-05-24 5:46 ` peter.crosthwaite
2013-05-24 5:47 ` [Qemu-devel] [PATCH v3 2/5] register: Add Register API peter.crosthwaite
` (3 subsequent siblings)
4 siblings, 0 replies; 19+ messages in thread
From: peter.crosthwaite @ 2013-05-24 5:46 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, edgar.iglesias, kraxel, pbonzini
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 file changed, 2 insertions(+)
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index affcc96..da47fc8 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -273,4 +273,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.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 2/5] register: Add Register API
2013-05-24 5:46 [Qemu-devel] [PATCH v3 0/5] Data Driven device registers & Zynq DEVCFG peter.crosthwaite
2013-05-24 5:46 ` [Qemu-devel] [PATCH v3 1/5] bitops: Add ONES macro peter.crosthwaite
@ 2013-05-24 5:47 ` peter.crosthwaite
2013-05-29 17:52 ` Edgar E. Iglesias
2013-05-24 5:48 ` [Qemu-devel] [PATCH v3 3/5] register: Add Memory API glue peter.crosthwaite
` (2 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: peter.crosthwaite @ 2013-05-24 5:47 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, edgar.iglesias, kraxel, pbonzini
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)
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.
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 | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/register.h | 129 ++++++++++++++++++++++++++++++++++
3 files changed, 320 insertions(+)
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 950146c..210cb1a 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -2,6 +2,7 @@
common-obj-y += qdev.o qdev-properties.o
# irq.o needed for qdev GPIO handling:
common-obj-y += irq.o
+common-obj-y += register.o
common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
common-obj-$(CONFIG_XILINX_AXI) += stream.o
diff --git a/hw/core/register.c b/hw/core/register.c
new file mode 100644
index 0000000..b10212e
--- /dev/null
+++ b/hw/core/register.c
@@ -0,0 +1,190 @@
+/*
+ * 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;
+ 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)) {
+ 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);
+ }
+ }
+ }
+
+ old_val = reg->data ? register_read_val(reg) : ac->reset;
+
+ 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 (reg->debug) {
+ qemu_log("%s:%s: read of value %#" PRIx64 "\n", reg->prefix,
+ ac->name, ret);
+ }
+ }
+
+ if (ac->post_read) {
+ ret = ac->post_read(reg, ret);
+ }
+
+ return ret;
+}
+
+void register_reset(RegisterInfo *reg)
+{
+ assert(reg);
+ const RegisterAccessInfo *ac;
+
+ if (!reg->data || !reg->access) {
+ return;
+ }
+
+ 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;
+
+ 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..a4a8319
--- /dev/null
+++ b/include/hw/register.h
@@ -0,0 +1,129 @@
+/*
+ * 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
+ *
+ * @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;
+
+ 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
+ * @data_big_endian: Define endianess of data register
+ *
+ * @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.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 3/5] register: Add Memory API glue
2013-05-24 5:46 [Qemu-devel] [PATCH v3 0/5] Data Driven device registers & Zynq DEVCFG peter.crosthwaite
2013-05-24 5:46 ` [Qemu-devel] [PATCH v3 1/5] bitops: Add ONES macro peter.crosthwaite
2013-05-24 5:47 ` [Qemu-devel] [PATCH v3 2/5] register: Add Register API peter.crosthwaite
@ 2013-05-24 5:48 ` peter.crosthwaite
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model peter.crosthwaite
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 5/5] xilinx_zynq: added devcfg to machine model peter.crosthwaite
4 siblings, 0 replies; 19+ messages in thread
From: peter.crosthwaite @ 2013-05-24 5:48 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, edgar.iglesias, kraxel, pbonzini
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 | 12 ++++++++++++
2 files changed, 60 insertions(+)
diff --git a/hw/core/register.c b/hw/core/register.c
index b10212e..e743f16 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -188,3 +188,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 a4a8319..4333cd7 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -85,6 +85,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 {
@@ -100,6 +102,8 @@ struct RegisterInfo {
/* private */
bool read_lite;
bool write_lite;
+
+ MemoryRegion mem;
};
/**
@@ -126,4 +130,12 @@ uint64_t register_read(RegisterInfo *reg);
void register_reset(RegisterInfo *reg);
+void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value,
+ unsigned size);
+void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value,
+ unsigned size);
+
+uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size);
+uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size);
+
#endif
--
1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-24 5:46 [Qemu-devel] [PATCH v3 0/5] Data Driven device registers & Zynq DEVCFG peter.crosthwaite
` (2 preceding siblings ...)
2013-05-24 5:48 ` [Qemu-devel] [PATCH v3 3/5] register: Add Memory API glue peter.crosthwaite
@ 2013-05-24 5:49 ` peter.crosthwaite
2013-05-29 8:51 ` Paolo Bonzini
` (2 more replies)
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 5/5] xilinx_zynq: added devcfg to machine model peter.crosthwaite
4 siblings, 3 replies; 19+ messages in thread
From: peter.crosthwaite @ 2013-05-24 5:49 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, edgar.iglesias, kraxel, pbonzini
From: "Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com>
Minimal device model for devcfg module of Zynq. DMA capabilities and
interrupt generation supported.
Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
Changed since v2:
Some QOM styling updates.
Re-implemented nw0 for lock register as pre_write
Changed since v1:
Rebased against new version of Register API.
Use action callbacks for side effects rather than switch.
Documented reasons for ge0, ge1 (Verbatim from TRM)
Added ui1 definitions for unimplemented major features
Removed dead lock code
default-configs/arm-softmmu.mak | 1 +
hw/dma/Makefile.objs | 1 +
hw/dma/xilinx_devcfg.c | 495 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 497 insertions(+)
create mode 100644 hw/dma/xilinx_devcfg.c
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 27cbe3d..5a17f64 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -61,6 +61,7 @@ CONFIG_PXA2XX=y
CONFIG_BITBANG_I2C=y
CONFIG_FRAMEBUFFER=y
CONFIG_XILINX_SPIPS=y
+CONFIG_ZYNQ_DEVCFG=y
CONFIG_A9SCU=y
CONFIG_MARVELL_88W8618=y
diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
index 0e65ed0..96025cf 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) += xilinx_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/xilinx_devcfg.c b/hw/dma/xilinx_devcfg.c
new file mode 100644
index 0000000..b82b7cc
--- /dev/null
+++ b/hw/dma/xilinx_devcfg.c
@@ -0,0 +1,495 @@
+/*
+ * QEMU model of the Xilinx Devcfg Interface
+ *
+ * Copyright (c) 2011 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.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 "sysemu/sysemu.h"
+#include "sysemu/dma.h"
+#include "hw/sysbus.h"
+#include "hw/ptimer.h"
+#include "qemu/bitops.h"
+#include "hw/register.h"
+#include "qemu/bitops.h"
+
+#define TYPE_XILINX_DEVCFG "xlnx.ps7-dev-cfg"
+
+#define XILINX_DEVCFG(obj) \
+ OBJECT_CHECK(XilinxDevcfg, (obj), TYPE_XILINX_DEVCFG)
+
+/* FIXME: get rid of hardcoded nastiness */
+
+#define FREQ_HZ 900000000
+
+#define BTT_MAX 0x400 /* bytes to transfer per delay inteval */
+#define CYCLES_BTT_MAX 10000 /*approximate 10k cycles per delay interval */
+
+#ifndef XILINX_DEVCFG_ERR_DEBUG
+#define XILINX_DEVCFG_ERR_DEBUG 0
+#endif
+#define DB_PRINT(...) do { \
+ if (XILINX_DEVCFG_ERR_DEBUG) { \
+ fprintf(stderr, ": %s: ", __func__); \
+ fprintf(stderr, ## __VA_ARGS__); \
+ } \
+} while (0);
+
+#define R_CTRL (0x00/4)
+ #define FORCE_RST (1 << 31) /* Not supported, writes ignored */
+ #define PCAP_PR (1 << 27) /* Forced to 0 on bad unlock */
+ #define PCAP_MODE (1 << 26)
+ #define MULTIBOOT_EN (1 << 24)
+ #define USER_MODE (1 << 15)
+ #define PCFG_AES_FUSE (1 << 12) /* locked by AES_FUSE_LOCK */
+ #define PCFG_AES_EN_SHIFT 9 /* locked by AES_EN_LOCK */
+ #define PCFG_AES_EN_LEN 3 /* locked by AES_EN_LOCK */
+ #define PCFG_AES_EN_MASK (((1 << PCFG_AES_EN_LEN) - 1) \
+ << PCFG_AES_EN_SHIFT)
+ #define SEU_EN (1 << 8) /* locked by SEU_LOCK */
+ #define SEC_EN (1 << 7) /* locked by SEC_LOCK */
+ #define SPNIDEN (1 << 6) /* locked by DBG_LOCK */
+ #define SPIDEN (1 << 5) /* locked by DBG_LOCK */
+ #define NIDEN (1 << 4) /* locked by DBG_LOCK */
+ #define DBGEN (1 << 3) /* locked by DBG_LOCK */
+ #define DAP_EN (7 << 0) /* locked by DBG_LOCK */
+
+#define R_LOCK (0x04/4)
+ #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] = PCFG_AES_FUSE,
+ [AES_EN_LOCK] = PCFG_AES_EN_MASK,
+ [SEU_LOCK] = SEU_LOCK,
+ [SEC_LOCK] = SEC_LOCK,
+ [DBG_LOCK] = SPNIDEN | SPIDEN | NIDEN | DBGEN | DAP_EN,
+};
+
+#define R_CFG (0x08/4)
+ #define RFIFO_TH_SHIFT 10
+ #define RFIFO_TH_LEN 2
+ #define WFIFO_TH_SHIFT 8
+ #define WFIFO_TH_LEN 2
+ #define DISABLE_SRC_INC (1 << 5)
+ #define DISABLE_DST_INC (1 << 4)
+#define R_CFG_RO 0xFFFFF800
+#define R_CFG_RESET 0x50B
+
+#define R_INT_STS (0x0C/4)
+ #define PSS_GTS_USR_B_INT (1 << 31)
+ #define PSS_FST_CFG_B_INT (1 << 30)
+ #define PSS_CFG_RESET_B_INT (1 << 27)
+ #define RX_FIFO_OV_INT (1 << 18)
+ #define WR_FIFO_LVL_INT (1 << 17)
+ #define RD_FIFO_LVL_INT (1 << 16)
+ #define DMA_CMD_ERR_INT (1 << 15)
+ #define DMA_Q_OV_INT (1 << 14)
+ #define DMA_DONE_INT (1 << 13)
+ #define DMA_P_DONE_INT (1 << 12)
+ #define P2D_LEN_ERR_INT (1 << 11)
+ #define PCFG_DONE_INT (1 << 2)
+ #define R_INT_STS_RSVD ((0x7 << 24) | (0x1 << 19) | (0xF < 7))
+
+#define R_INT_MASK (0x10/4)
+
+#define R_STATUS (0x14/4)
+ #define DMA_CMD_Q_F (1 << 31)
+ #define DMA_CMD_Q_E (1 << 30)
+ #define DMA_DONE_CNT_SHIFT 28
+ #define DMA_DONE_CNT_LEN 2
+ #define RX_FIFO_LVL_SHIFT 20
+ #define RX_FIFO_LVL_LEN 5
+ #define TX_FIFO_LVL_SHIFT 12
+ #define TX_FIFO_LVL_LEN 7
+ #define TX_FIFO_LVL (0x7f << 12)
+ #define PSS_GTS_USR_B (1 << 11)
+ #define PSS_FST_CFG_B (1 << 10)
+ #define PSS_CFG_RESET_B (1 << 5)
+
+#define R_DMA_SRC_ADDR (0x18/4)
+#define R_DMA_DST_ADDR (0x1C/4)
+#define R_DMA_SRC_LEN (0x20/4)
+#define R_DMA_DST_LEN (0x24/4)
+#define R_ROM_SHADOW (0x28/4)
+#define R_SW_ID (0x30/4)
+#define R_UNLOCK (0x34/4)
+
+#define R_UNLOCK_MAGIC 0x757BDF0D
+
+#define R_MCTRL (0x80/4)
+ #define PS_VERSION_SHIFT 28
+ #define PS_VERSION_MASK (0xf << PS_VERSION_SHIFT)
+ #define PCFG_POR_B (1 << 8)
+ #define INT_PCAP_LPBK (1 << 4)
+
+#define R_MAX (0x118/4+1)
+
+#define RX_FIFO_LEN 32
+#define TX_FIFO_LEN 128
+
+#define DMA_COMMAND_FIFO_LEN 10
+
+typedef struct XilinxDevcfgDMACommand {
+ uint32_t src_addr;
+ uint32_t dest_addr;
+ uint32_t src_len;
+ uint32_t dest_len;
+} XilinxDevcfgDMACommand;
+
+static const VMStateDescription vmstate_xilinx_devcfg_dma_command = {
+ .name = "xilinx_devcfg_dma_command",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(src_addr, XilinxDevcfgDMACommand),
+ VMSTATE_UINT32(dest_addr, XilinxDevcfgDMACommand),
+ VMSTATE_UINT32(src_len, XilinxDevcfgDMACommand),
+ VMSTATE_UINT32(dest_len, XilinxDevcfgDMACommand),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+typedef struct XilinxDevcfg {
+ SysBusDevice parent_obj;
+
+ MemoryRegion iomem;
+ qemu_irq irq;
+
+ QEMUBH *timer_bh;
+ ptimer_state *timer;
+
+ XilinxDevcfgDMACommand dma_command_fifo[DMA_COMMAND_FIFO_LEN];
+ uint8_t dma_command_fifo_num;
+
+ uint32_t regs[R_MAX];
+ RegisterInfo regs_info[R_MAX];
+} XilinxDevcfg;
+
+static const VMStateDescription vmstate_xilinx_devcfg = {
+ .name = "xilinx_devcfg",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_PTIMER(timer, XilinxDevcfg),
+ VMSTATE_STRUCT_ARRAY(dma_command_fifo, XilinxDevcfg,
+ DMA_COMMAND_FIFO_LEN, 0,
+ vmstate_xilinx_devcfg_dma_command,
+ XilinxDevcfgDMACommand),
+ VMSTATE_UINT8(dma_command_fifo_num, XilinxDevcfg),
+ VMSTATE_UINT32_ARRAY(regs, XilinxDevcfg, R_MAX),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void xilinx_devcfg_update_ixr(XilinxDevcfg *s)
+{
+ qemu_set_irq(s->irq, !!(~s->regs[R_INT_MASK] & s->regs[R_INT_STS]));
+}
+
+static void xilinx_devcfg_reset(DeviceState *dev)
+{
+ XilinxDevcfg *s = XILINX_DEVCFG(dev);
+ int i;
+
+ for (i = 0; i < R_MAX; ++i) {
+ register_reset(&s->regs_info[i]);
+ }
+}
+
+#define min(a, b) ((a) < (b) ? (a) : (b))
+
+static void xilinx_devcfg_dma_go(void *opaque)
+{
+ XilinxDevcfg *s = XILINX_DEVCFG(opaque);
+ uint8_t buf[BTT_MAX];
+ XilinxDevcfgDMACommand *dmah = s->dma_command_fifo;
+ uint32_t btt = BTT_MAX;
+
+ btt = min(btt, dmah->src_len);
+ if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
+ btt = min(btt, dmah->dest_len);
+ }
+ DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr);
+ dma_memory_read(&dma_context_memory, dmah->src_addr, buf, btt);
+ dmah->src_len -= btt;
+ dmah->src_addr += btt;
+ if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
+ DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr);
+ dma_memory_write(&dma_context_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] |= DMA_DONE_INT | DMA_P_DONE_INT;
+ s->dma_command_fifo_num = s->dma_command_fifo_num - 1;
+ memcpy(s->dma_command_fifo, &s->dma_command_fifo[1],
+ sizeof(*s->dma_command_fifo) * DMA_COMMAND_FIFO_LEN - 1);
+ }
+ xilinx_devcfg_update_ixr(s);
+ if (s->dma_command_fifo_num) { /* there is still work to do */
+ DB_PRINT("dma work remains, setting up callback ptimer\n");
+ ptimer_set_freq(s->timer, FREQ_HZ);
+ ptimer_set_count(s->timer, CYCLES_BTT_MAX);
+ ptimer_run(s->timer, 1);
+ }
+}
+
+static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
+{
+ XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
+
+ xilinx_devcfg_update_ixr(s);
+}
+
+static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val)
+{
+ XilinxDevcfg *s = XILINX_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 = extract32(val, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
+
+ if (aes_en != 0 && aes_en != 7) {
+ qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
+ "unimplemeneted security reset should happen!\n",
+ reg->prefix);
+ }
+}
+
+static void r_unlock_post_write(RegisterInfo *reg, uint64_t val)
+{
+ XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
+
+ if (val == R_UNLOCK_MAGIC) {
+ DB_PRINT("successful unlock\n");
+ } else {/* bad unlock attempt */
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->prefix);
+ qemu_log_mask(LOG_UNIMP, "%s: failed unlock, unimplmeneted crash of"
+ "device should happen\n", reg->prefix);
+ s->regs[R_CTRL] &= ~PCAP_PR;
+ s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
+ }
+}
+
+static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val)
+{
+ XilinxDevcfg *s = XILINX_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)
+{
+ XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
+
+ /* TODO: implement command queue overflow check and interrupt */
+ s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
+ s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
+ s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
+ s->regs[R_DMA_DST_ADDR] & ~0x3UL;
+ s->dma_command_fifo[s->dma_command_fifo_num].src_len =
+ s->regs[R_DMA_SRC_LEN] << 2;
+ s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
+ s->regs[R_DMA_DST_LEN] << 2;
+ s->dma_command_fifo_num++;
+ DB_PRINT("dma transfer started; %d total transfers pending\n",
+ s->dma_command_fifo_num);
+ xilinx_devcfg_dma_go(s);
+}
+
+static const RegisterAccessInfo xilinx_devcfg_regs_info[] = {
+ [R_CTRL] = { .name = "CTRL",
+ .reset = PCAP_PR | PCAP_MODE | 0x3 << 13,
+ .ro = 0x107f6000,
+ .ge1 = (RegisterAccessError[]) {
+ { .mask = 0x1 << 15, .reason = "Reserved - do not modify" },
+ {},
+ },
+ .ge0 = (RegisterAccessError[]) {
+ { .mask = 0x3 << 13, .reason = "Reserved - always write with 1" },
+ {},
+ },
+ .ui1 = (RegisterAccessError[]) {
+ { .mask = FORCE_RST, .reason = "PS reset not implemented" },
+ { .mask = PCAP_MODE, .reason = "FPGA Fabric doesnt exist" },
+ { .mask = PCFG_AES_EN_MASK, .reason = "AES not implmented" },
+ {},
+ },
+ .pre_write = r_ctrl_pre_write,
+ .post_write = r_ctrl_post_write,
+ },
+ [R_LOCK] = { .name = "LOCK",
+ .ro = ~ONES(5),
+ .pre_write = r_lock_pre_write,
+ },
+ [R_CFG] = { .name = "CFG",
+ .reset = 1 << RFIFO_TH_SHIFT | 1 << WFIFO_TH_SHIFT | 0x8,
+ .ge1 = (RegisterAccessError[]) {
+ { .mask = 0x7, .reason = "Reserved - do not modify" },
+ {},
+ },
+ .ge0 = (RegisterAccessError[]) {
+ { .mask = 0x8, .reason = "Reserved - do not modify" },
+ {},
+ },
+ .ro = 0x00f | ~ONES(12),
+ },
+ [R_INT_STS] = { .name = "INT_STS",
+ .w1c = ~R_INT_STS_RSVD,
+ .reset = PSS_GTS_USR_B_INT | PSS_CFG_RESET_B_INT | WR_FIFO_LVL_INT,
+ .ro = R_INT_STS_RSVD,
+ .post_write = r_ixr_post_write,
+ },
+ [R_INT_MASK] = { .name = "INT_MASK",
+ .reset = ~0,
+ .ro = R_INT_STS_RSVD,
+ .post_write = r_ixr_post_write,
+ },
+ [R_STATUS] = { .name = "STATUS",
+ .reset = DMA_CMD_Q_E | PSS_GTS_USR_B | PSS_CFG_RESET_B,
+ .ro = ~0,
+ .ge1 = (RegisterAccessError[]) {
+ {.mask = 0x1, .reason = "Reserved - do not modify" },
+ {},
+ },
+ },
+ [R_DMA_SRC_ADDR] = { .name = "DMA_SRC_ADDR" },
+ [R_DMA_DST_ADDR] = { .name = "DMA_DST_ADDR" },
+ [R_DMA_SRC_LEN] = { .name = "DMA_SRC_LEN", .ro = ~ONES(27) },
+ [R_DMA_DST_LEN] = { .name = "DMA_DST_LEN",
+ .ro = ~ONES(27),
+ .post_write = r_dma_dst_len_post_write,
+ },
+ [R_ROM_SHADOW] = { .name = "ROM_SHADOW",
+ .ge1 = (RegisterAccessError[]) {
+ {.mask = ~0, .reason = "Reserved - do not modify" },
+ {},
+ },
+ },
+ [R_SW_ID] = { .name = "SW_ID" },
+ [R_UNLOCK] = { .name = "UNLOCK",
+ .post_write = r_unlock_post_write,
+ },
+ [R_MCTRL] = { .name = "MCTRL",
+ /* Silicon 3.0 for version field, and the mysterious reserved bit 23 */
+ .reset = 0x2 << PS_VERSION_SHIFT | 1 << 23,
+ /* some reserved bits are rw while others are ro */
+ .ro = ~INT_PCAP_LPBK,
+ .ge1 = (RegisterAccessError[]) {
+ { .mask = 0x00F00300, .reason = "Reserved - do not modify" },
+ { .mask = 0x00000003, .reason = "Reserved - always write with 0" },
+ {}
+ },
+ .ge0 = (RegisterAccessError[]) {
+ { .mask = 1 << 23, .reason = "Reserved - always write with 1" },
+ {}
+ },
+ },
+ [R_MAX] = {}
+};
+
+static const MemoryRegionOps 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 void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
+{
+ XilinxDevcfg *s = XILINX_DEVCFG(dev);
+ const char *prefix = object_get_canonical_path(OBJECT(dev));
+ int i;
+
+ for (i = 0; i < R_MAX; ++i) {
+ RegisterInfo *r = &s->regs_info[i];
+
+ *r = (RegisterInfo) {
+ .data = &s->regs[i],
+ .data_size = sizeof(uint32_t),
+ .access = &xilinx_devcfg_regs_info[i],
+ .debug = XILINX_DEVCFG_ERR_DEBUG,
+ .prefix = prefix,
+ .opaque = s,
+ };
+ memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
+ memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
+ }
+}
+
+static void xilinx_devcfg_init(Object *obj)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+ XilinxDevcfg *s = XILINX_DEVCFG(obj);
+
+ s->timer_bh = qemu_bh_new(xilinx_devcfg_dma_go, s);
+ s->timer = ptimer_init(s->timer_bh);
+
+ sysbus_init_irq(sbd, &s->irq);
+
+ memory_region_init(&s->iomem, "devcfg", R_MAX*4);
+ sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void xilinx_devcfg_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->reset = xilinx_devcfg_reset;
+ dc->vmsd = &vmstate_xilinx_devcfg;
+ dc->realize = xilinx_devcfg_realize;
+}
+
+static const TypeInfo xilinx_devcfg_info = {
+ .name = TYPE_XILINX_DEVCFG,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(XilinxDevcfg),
+ .instance_init = xilinx_devcfg_init,
+ .class_init = xilinx_devcfg_class_init,
+};
+
+static void xilinx_devcfg_register_types(void)
+{
+ type_register_static(&xilinx_devcfg_info);
+}
+
+type_init(xilinx_devcfg_register_types)
--
1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 5/5] xilinx_zynq: added devcfg to machine model
2013-05-24 5:46 [Qemu-devel] [PATCH v3 0/5] Data Driven device registers & Zynq DEVCFG peter.crosthwaite
` (3 preceding siblings ...)
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model peter.crosthwaite
@ 2013-05-24 5:49 ` peter.crosthwaite
4 siblings, 0 replies; 19+ messages in thread
From: peter.crosthwaite @ 2013-05-24 5:49 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, edgar.iglesias, kraxel, pbonzini
From: "Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com>
Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
Changed since v1:
Added manual parenting of devcfg node (evil but needed for early access
to canonical path by devcfgs realize fn).
hw/arm/xilinx_zynq.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 41505c3..16fea43 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -219,6 +219,14 @@ static void zynq_init(QEMUMachineInitArgs *args)
sysbus_connect_irq(busdev, n + 1, pic[dma_irqs[n] - IRQ_OFFSET]);
}
+ dev = qdev_create(NULL, "xlnx.ps7-dev-cfg");
+ object_property_add_child(qdev_get_machine(), "xilinx-devcfg", OBJECT(dev),
+ NULL);
+ qdev_init_nofail(dev);
+ busdev = SYS_BUS_DEVICE(dev);
+ sysbus_connect_irq(busdev, 0, pic[40-IRQ_OFFSET]);
+ sysbus_mmio_map(busdev, 0, 0xF8007000);
+
zynq_binfo.ram_size = ram_size;
zynq_binfo.kernel_filename = kernel_filename;
zynq_binfo.kernel_cmdline = kernel_cmdline;
--
1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model peter.crosthwaite
@ 2013-05-29 8:51 ` Paolo Bonzini
2013-05-29 13:54 ` Peter Crosthwaite
2013-05-29 17:04 ` Edgar E. Iglesias
2013-05-29 17:57 ` Anthony Liguori
2 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2013-05-29 8:51 UTC (permalink / raw)
To: peter.crosthwaite
Cc: blauwirbel, peter.maydell, edgar.iglesias, qemu-devel, kraxel
Il 24/05/2013 07:49, peter.crosthwaite@xilinx.com ha scritto:
> +static const MemoryRegionOps 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,
What happens if you have registers of mixed size within the same "bank"?
> + }
> +};
> +
> +static void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
> + const char *prefix = object_get_canonical_path(OBJECT(dev));
> + int i;
> +
> + for (i = 0; i < R_MAX; ++i) {
> + RegisterInfo *r = &s->regs_info[i];
> +
> + *r = (RegisterInfo) {
> + .data = &s->regs[i],
> + .data_size = sizeof(uint32_t),
> + .access = &xilinx_devcfg_regs_info[i],
> + .debug = XILINX_DEVCFG_ERR_DEBUG,
> + .prefix = prefix,
> + .opaque = s,
> + };
> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
Could you add a register_init function that does register_reset +
memory_region_init_io?
> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
Paolo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-29 8:51 ` Paolo Bonzini
@ 2013-05-29 13:54 ` Peter Crosthwaite
2013-05-29 14:03 ` Paolo Bonzini
0 siblings, 1 reply; 19+ messages in thread
From: Peter Crosthwaite @ 2013-05-29 13:54 UTC (permalink / raw)
To: Paolo Bonzini
Cc: blauwirbel, peter.maydell, kraxel, qemu-devel, edgar.iglesias
Hi Paolo,
On Wed, May 29, 2013 at 6:51 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 24/05/2013 07:49, peter.crosthwaite@xilinx.com ha scritto:
>> +static const MemoryRegionOps 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,
>
> What happens if you have registers of mixed size within the same "bank"?
>
You Probably should change these access restrictions per-register
accordingly. Note that min_access_size = 4 is a restriction of the
devcfg device and not the register API. We could change that to 1 no
problems. max_access should probably be no greater than the size of
the register, otherwise you have defined a non-nonsensical memory
region for the register. But note that defining it as less than the
register size is perfectly valid (for the weird case where you allow
e.g. only byte accesses to a register described as 32 bits).
The degenerate case is allowing greater-than-register size or
misaligned accesses, which is dependent on the behaviour of the memory
API. I can do some research, but do you know if the memory API
supports misaligned transactions that span a memory region boundary?
If so then there are no concerns, as the memory API will handle it for
us. If not then I don't see it as an issue as its very rare (at least
in my experience) for registers to support such strange misaligned or
mulit-register accesses. That or we can look into memory API for
answers.
>> + }
>> +};
>> +
>> +static void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>> + const char *prefix = object_get_canonical_path(OBJECT(dev));
>> + int i;
>> +
>> + for (i = 0; i < R_MAX; ++i) {
>> + RegisterInfo *r = &s->regs_info[i];
>> +
>> + *r = (RegisterInfo) {
>> + .data = &s->regs[i],
>> + .data_size = sizeof(uint32_t),
>> + .access = &xilinx_devcfg_regs_info[i],
>> + .debug = XILINX_DEVCFG_ERR_DEBUG,
>> + .prefix = prefix,
>> + .opaque = s,
>> + };
>> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
>
> Could you add a register_init function that does register_reset +
> memory_region_init_io?
>
I wanted to factor this loop out into a helper as a next step so I
think its already in my todo list, but i'm confused as to why reset
and memory_region_init would bundled together - arent they normally in
Device::reset and Device::realize (or Object::Init) respectively?
Regards,
Peter
>> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
>
> Paolo
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-29 13:54 ` Peter Crosthwaite
@ 2013-05-29 14:03 ` Paolo Bonzini
0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2013-05-29 14:03 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: blauwirbel, peter.maydell, kraxel, qemu-devel, edgar.iglesias
Il 29/05/2013 15:54, Peter Crosthwaite ha scritto:
> Hi Paolo,
>
> On Wed, May 29, 2013 at 6:51 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 24/05/2013 07:49, peter.crosthwaite@xilinx.com ha scritto:
>>> +static const MemoryRegionOps 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,
>>
>> What happens if you have registers of mixed size within the same "bank"?
>>
>
> You Probably should change these access restrictions per-register
> accordingly.
Can you add a generic .valid.accepts callback for the register API that
will check against the size of the register at that offset?
> The degenerate case is allowing greater-than-register size or
> misaligned accesses, which is dependent on the behaviour of the memory
> API. I can do some research, but do you know if the memory API
> supports misaligned transactions that span a memory region boundary?
I don't really care about that for now.
But the way to do that would be to add a .impl.accepts method (right now
there's only .valid.accepts). Then you would point the register API's
implementation to .impl.accepts, and use
.impl.accepts = register_accepts,
.valid = {
.min_access_size = 1,
.max_access_size = 4,
}
The memory API will do a RMW operation if needed. Won't make much sense
with write-1-clear bits and the like, but for simple registers it would
work.
Paolo
> If so then there are no concerns, as the memory API will handle it for
> us. If not then I don't see it as an issue as its very rare (at least
> in my experience) for registers to support such strange misaligned or
> mulit-register accesses. That or we can look into memory API for
> answers.
>
>>> + }
>>> +};
>>> +
>>> +static void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
>>> +{
>>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>>> + const char *prefix = object_get_canonical_path(OBJECT(dev));
>>> + int i;
>>> +
>>> + for (i = 0; i < R_MAX; ++i) {
>>> + RegisterInfo *r = &s->regs_info[i];
>>> +
>>> + *r = (RegisterInfo) {
>>> + .data = &s->regs[i],
>>> + .data_size = sizeof(uint32_t),
>>> + .access = &xilinx_devcfg_regs_info[i],
>>> + .debug = XILINX_DEVCFG_ERR_DEBUG,
>>> + .prefix = prefix,
>>> + .opaque = s,
>>> + };
>>> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
>>
>> Could you add a register_init function that does register_reset +
>> memory_region_init_io?
>>
>
> I wanted to factor this loop out into a helper as a next step so I
> think its already in my todo list, but i'm confused as to why reset
> and memory_region_init would bundled together - arent they normally in
> Device::reset and Device::realize (or Object::Init) respectively?
>
> Regards,
> Peter
>
>>> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
>>
>> Paolo
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model peter.crosthwaite
2013-05-29 8:51 ` Paolo Bonzini
@ 2013-05-29 17:04 ` Edgar E. Iglesias
2013-05-29 17:08 ` Paolo Bonzini
2013-05-29 17:57 ` Anthony Liguori
2 siblings, 1 reply; 19+ messages in thread
From: Edgar E. Iglesias @ 2013-05-29 17:04 UTC (permalink / raw)
To: peter.crosthwaite; +Cc: blauwirbel, peter.maydell, pbonzini, qemu-devel, kraxel
On Fri, May 24, 2013 at 03:49:00PM +1000, peter.crosthwaite@xilinx.com wrote:
> From: "Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com>
>
> Minimal device model for devcfg module of Zynq. DMA capabilities and
> interrupt generation supported.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
> Changed since v2:
> Some QOM styling updates.
> Re-implemented nw0 for lock register as pre_write
> Changed since v1:
> Rebased against new version of Register API.
> Use action callbacks for side effects rather than switch.
> Documented reasons for ge0, ge1 (Verbatim from TRM)
> Added ui1 definitions for unimplemented major features
> Removed dead lock code
>
> default-configs/arm-softmmu.mak | 1 +
> hw/dma/Makefile.objs | 1 +
> hw/dma/xilinx_devcfg.c | 495 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 497 insertions(+)
> create mode 100644 hw/dma/xilinx_devcfg.c
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 27cbe3d..5a17f64 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -61,6 +61,7 @@ CONFIG_PXA2XX=y
> CONFIG_BITBANG_I2C=y
> CONFIG_FRAMEBUFFER=y
> CONFIG_XILINX_SPIPS=y
> +CONFIG_ZYNQ_DEVCFG=y
>
> CONFIG_A9SCU=y
> CONFIG_MARVELL_88W8618=y
> diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
> index 0e65ed0..96025cf 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) += xilinx_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/xilinx_devcfg.c b/hw/dma/xilinx_devcfg.c
> new file mode 100644
> index 0000000..b82b7cc
> --- /dev/null
> +++ b/hw/dma/xilinx_devcfg.c
> @@ -0,0 +1,495 @@
> +/*
> + * QEMU model of the Xilinx Devcfg Interface
> + *
> + * Copyright (c) 2011 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.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 "sysemu/sysemu.h"
> +#include "sysemu/dma.h"
> +#include "hw/sysbus.h"
> +#include "hw/ptimer.h"
> +#include "qemu/bitops.h"
> +#include "hw/register.h"
> +#include "qemu/bitops.h"
> +
> +#define TYPE_XILINX_DEVCFG "xlnx.ps7-dev-cfg"
> +
> +#define XILINX_DEVCFG(obj) \
> + OBJECT_CHECK(XilinxDevcfg, (obj), TYPE_XILINX_DEVCFG)
> +
> +/* FIXME: get rid of hardcoded nastiness */
> +
> +#define FREQ_HZ 900000000
> +
> +#define BTT_MAX 0x400 /* bytes to transfer per delay inteval */
> +#define CYCLES_BTT_MAX 10000 /*approximate 10k cycles per delay interval */
> +
> +#ifndef XILINX_DEVCFG_ERR_DEBUG
> +#define XILINX_DEVCFG_ERR_DEBUG 0
> +#endif
> +#define DB_PRINT(...) do { \
> + if (XILINX_DEVCFG_ERR_DEBUG) { \
> + fprintf(stderr, ": %s: ", __func__); \
> + fprintf(stderr, ## __VA_ARGS__); \
> + } \
> +} while (0);
> +
> +#define R_CTRL (0x00/4)
> + #define FORCE_RST (1 << 31) /* Not supported, writes ignored */
> + #define PCAP_PR (1 << 27) /* Forced to 0 on bad unlock */
> + #define PCAP_MODE (1 << 26)
> + #define MULTIBOOT_EN (1 << 24)
> + #define USER_MODE (1 << 15)
> + #define PCFG_AES_FUSE (1 << 12) /* locked by AES_FUSE_LOCK */
> + #define PCFG_AES_EN_SHIFT 9 /* locked by AES_EN_LOCK */
> + #define PCFG_AES_EN_LEN 3 /* locked by AES_EN_LOCK */
> + #define PCFG_AES_EN_MASK (((1 << PCFG_AES_EN_LEN) - 1) \
> + << PCFG_AES_EN_SHIFT)
> + #define SEU_EN (1 << 8) /* locked by SEU_LOCK */
> + #define SEC_EN (1 << 7) /* locked by SEC_LOCK */
> + #define SPNIDEN (1 << 6) /* locked by DBG_LOCK */
> + #define SPIDEN (1 << 5) /* locked by DBG_LOCK */
> + #define NIDEN (1 << 4) /* locked by DBG_LOCK */
> + #define DBGEN (1 << 3) /* locked by DBG_LOCK */
> + #define DAP_EN (7 << 0) /* locked by DBG_LOCK */
> +
> +#define R_LOCK (0x04/4)
> + #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] = PCFG_AES_FUSE,
> + [AES_EN_LOCK] = PCFG_AES_EN_MASK,
> + [SEU_LOCK] = SEU_LOCK,
> + [SEC_LOCK] = SEC_LOCK,
> + [DBG_LOCK] = SPNIDEN | SPIDEN | NIDEN | DBGEN | DAP_EN,
> +};
> +
> +#define R_CFG (0x08/4)
> + #define RFIFO_TH_SHIFT 10
> + #define RFIFO_TH_LEN 2
> + #define WFIFO_TH_SHIFT 8
> + #define WFIFO_TH_LEN 2
> + #define DISABLE_SRC_INC (1 << 5)
> + #define DISABLE_DST_INC (1 << 4)
> +#define R_CFG_RO 0xFFFFF800
> +#define R_CFG_RESET 0x50B
> +
> +#define R_INT_STS (0x0C/4)
> + #define PSS_GTS_USR_B_INT (1 << 31)
> + #define PSS_FST_CFG_B_INT (1 << 30)
> + #define PSS_CFG_RESET_B_INT (1 << 27)
> + #define RX_FIFO_OV_INT (1 << 18)
> + #define WR_FIFO_LVL_INT (1 << 17)
> + #define RD_FIFO_LVL_INT (1 << 16)
> + #define DMA_CMD_ERR_INT (1 << 15)
> + #define DMA_Q_OV_INT (1 << 14)
> + #define DMA_DONE_INT (1 << 13)
> + #define DMA_P_DONE_INT (1 << 12)
> + #define P2D_LEN_ERR_INT (1 << 11)
> + #define PCFG_DONE_INT (1 << 2)
> + #define R_INT_STS_RSVD ((0x7 << 24) | (0x1 << 19) | (0xF < 7))
> +
> +#define R_INT_MASK (0x10/4)
> +
> +#define R_STATUS (0x14/4)
> + #define DMA_CMD_Q_F (1 << 31)
> + #define DMA_CMD_Q_E (1 << 30)
> + #define DMA_DONE_CNT_SHIFT 28
> + #define DMA_DONE_CNT_LEN 2
> + #define RX_FIFO_LVL_SHIFT 20
> + #define RX_FIFO_LVL_LEN 5
> + #define TX_FIFO_LVL_SHIFT 12
> + #define TX_FIFO_LVL_LEN 7
> + #define TX_FIFO_LVL (0x7f << 12)
> + #define PSS_GTS_USR_B (1 << 11)
> + #define PSS_FST_CFG_B (1 << 10)
> + #define PSS_CFG_RESET_B (1 << 5)
> +
> +#define R_DMA_SRC_ADDR (0x18/4)
> +#define R_DMA_DST_ADDR (0x1C/4)
> +#define R_DMA_SRC_LEN (0x20/4)
> +#define R_DMA_DST_LEN (0x24/4)
> +#define R_ROM_SHADOW (0x28/4)
> +#define R_SW_ID (0x30/4)
> +#define R_UNLOCK (0x34/4)
> +
> +#define R_UNLOCK_MAGIC 0x757BDF0D
> +
> +#define R_MCTRL (0x80/4)
> + #define PS_VERSION_SHIFT 28
> + #define PS_VERSION_MASK (0xf << PS_VERSION_SHIFT)
> + #define PCFG_POR_B (1 << 8)
> + #define INT_PCAP_LPBK (1 << 4)
> +
> +#define R_MAX (0x118/4+1)
> +
> +#define RX_FIFO_LEN 32
> +#define TX_FIFO_LEN 128
> +
> +#define DMA_COMMAND_FIFO_LEN 10
> +
> +typedef struct XilinxDevcfgDMACommand {
> + uint32_t src_addr;
> + uint32_t dest_addr;
> + uint32_t src_len;
> + uint32_t dest_len;
> +} XilinxDevcfgDMACommand;
> +
> +static const VMStateDescription vmstate_xilinx_devcfg_dma_command = {
> + .name = "xilinx_devcfg_dma_command",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(src_addr, XilinxDevcfgDMACommand),
> + VMSTATE_UINT32(dest_addr, XilinxDevcfgDMACommand),
> + VMSTATE_UINT32(src_len, XilinxDevcfgDMACommand),
> + VMSTATE_UINT32(dest_len, XilinxDevcfgDMACommand),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +typedef struct XilinxDevcfg {
> + SysBusDevice parent_obj;
> +
> + MemoryRegion iomem;
> + qemu_irq irq;
> +
> + QEMUBH *timer_bh;
> + ptimer_state *timer;
> +
> + XilinxDevcfgDMACommand dma_command_fifo[DMA_COMMAND_FIFO_LEN];
> + uint8_t dma_command_fifo_num;
> +
> + uint32_t regs[R_MAX];
> + RegisterInfo regs_info[R_MAX];
> +} XilinxDevcfg;
> +
> +static const VMStateDescription vmstate_xilinx_devcfg = {
> + .name = "xilinx_devcfg",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_PTIMER(timer, XilinxDevcfg),
> + VMSTATE_STRUCT_ARRAY(dma_command_fifo, XilinxDevcfg,
> + DMA_COMMAND_FIFO_LEN, 0,
> + vmstate_xilinx_devcfg_dma_command,
> + XilinxDevcfgDMACommand),
> + VMSTATE_UINT8(dma_command_fifo_num, XilinxDevcfg),
> + VMSTATE_UINT32_ARRAY(regs, XilinxDevcfg, R_MAX),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void xilinx_devcfg_update_ixr(XilinxDevcfg *s)
> +{
> + qemu_set_irq(s->irq, !!(~s->regs[R_INT_MASK] & s->regs[R_INT_STS]));
> +}
> +
> +static void xilinx_devcfg_reset(DeviceState *dev)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
> + int i;
> +
> + for (i = 0; i < R_MAX; ++i) {
> + register_reset(&s->regs_info[i]);
> + }
> +}
> +
> +#define min(a, b) ((a) < (b) ? (a) : (b))
> +
> +static void xilinx_devcfg_dma_go(void *opaque)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(opaque);
> + uint8_t buf[BTT_MAX];
> + XilinxDevcfgDMACommand *dmah = s->dma_command_fifo;
> + uint32_t btt = BTT_MAX;
> +
> + btt = min(btt, dmah->src_len);
> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
> + btt = min(btt, dmah->dest_len);
> + }
> + DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr);
> + dma_memory_read(&dma_context_memory, dmah->src_addr, buf, btt);
> + dmah->src_len -= btt;
> + dmah->src_addr += btt;
> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
> + DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr);
> + dma_memory_write(&dma_context_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] |= DMA_DONE_INT | DMA_P_DONE_INT;
> + s->dma_command_fifo_num = s->dma_command_fifo_num - 1;
> + memcpy(s->dma_command_fifo, &s->dma_command_fifo[1],
> + sizeof(*s->dma_command_fifo) * DMA_COMMAND_FIFO_LEN - 1);
> + }
> + xilinx_devcfg_update_ixr(s);
> + if (s->dma_command_fifo_num) { /* there is still work to do */
> + DB_PRINT("dma work remains, setting up callback ptimer\n");
> + ptimer_set_freq(s->timer, FREQ_HZ);
> + ptimer_set_count(s->timer, CYCLES_BTT_MAX);
> + ptimer_run(s->timer, 1);
> + }
> +}
> +
> +static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
> +
> + xilinx_devcfg_update_ixr(s);
> +}
> +
> +static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val)
> +{
> + XilinxDevcfg *s = XILINX_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 = extract32(val, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
> +
> + if (aes_en != 0 && aes_en != 7) {
> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
> + "unimplemeneted security reset should happen!\n",
> + reg->prefix);
> + }
> +}
> +
> +static void r_unlock_post_write(RegisterInfo *reg, uint64_t val)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
> +
> + if (val == R_UNLOCK_MAGIC) {
> + DB_PRINT("successful unlock\n");
> + } else {/* bad unlock attempt */
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->prefix);
> + qemu_log_mask(LOG_UNIMP, "%s: failed unlock, unimplmeneted crash of"
> + "device should happen\n", reg->prefix);
> + s->regs[R_CTRL] &= ~PCAP_PR;
> + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
> + }
> +}
> +
> +static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val)
> +{
> + XilinxDevcfg *s = XILINX_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)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
> +
> + /* TODO: implement command queue overflow check and interrupt */
> + s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
> + s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
> + s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
> + s->regs[R_DMA_DST_ADDR] & ~0x3UL;
> + s->dma_command_fifo[s->dma_command_fifo_num].src_len =
> + s->regs[R_DMA_SRC_LEN] << 2;
> + s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
> + s->regs[R_DMA_DST_LEN] << 2;
> + s->dma_command_fifo_num++;
> + DB_PRINT("dma transfer started; %d total transfers pending\n",
> + s->dma_command_fifo_num);
> + xilinx_devcfg_dma_go(s);
> +}
> +
> +static const RegisterAccessInfo xilinx_devcfg_regs_info[] = {
> + [R_CTRL] = { .name = "CTRL",
> + .reset = PCAP_PR | PCAP_MODE | 0x3 << 13,
> + .ro = 0x107f6000,
> + .ge1 = (RegisterAccessError[]) {
> + { .mask = 0x1 << 15, .reason = "Reserved - do not modify" },
> + {},
> + },
> + .ge0 = (RegisterAccessError[]) {
> + { .mask = 0x3 << 13, .reason = "Reserved - always write with 1" },
> + {},
> + },
> + .ui1 = (RegisterAccessError[]) {
> + { .mask = FORCE_RST, .reason = "PS reset not implemented" },
> + { .mask = PCAP_MODE, .reason = "FPGA Fabric doesnt exist" },
> + { .mask = PCFG_AES_EN_MASK, .reason = "AES not implmented" },
> + {},
> + },
> + .pre_write = r_ctrl_pre_write,
> + .post_write = r_ctrl_post_write,
> + },
> + [R_LOCK] = { .name = "LOCK",
> + .ro = ~ONES(5),
> + .pre_write = r_lock_pre_write,
> + },
> + [R_CFG] = { .name = "CFG",
> + .reset = 1 << RFIFO_TH_SHIFT | 1 << WFIFO_TH_SHIFT | 0x8,
> + .ge1 = (RegisterAccessError[]) {
> + { .mask = 0x7, .reason = "Reserved - do not modify" },
> + {},
> + },
> + .ge0 = (RegisterAccessError[]) {
> + { .mask = 0x8, .reason = "Reserved - do not modify" },
> + {},
> + },
> + .ro = 0x00f | ~ONES(12),
> + },
> + [R_INT_STS] = { .name = "INT_STS",
> + .w1c = ~R_INT_STS_RSVD,
> + .reset = PSS_GTS_USR_B_INT | PSS_CFG_RESET_B_INT | WR_FIFO_LVL_INT,
> + .ro = R_INT_STS_RSVD,
> + .post_write = r_ixr_post_write,
> + },
> + [R_INT_MASK] = { .name = "INT_MASK",
> + .reset = ~0,
> + .ro = R_INT_STS_RSVD,
> + .post_write = r_ixr_post_write,
> + },
> + [R_STATUS] = { .name = "STATUS",
> + .reset = DMA_CMD_Q_E | PSS_GTS_USR_B | PSS_CFG_RESET_B,
> + .ro = ~0,
> + .ge1 = (RegisterAccessError[]) {
> + {.mask = 0x1, .reason = "Reserved - do not modify" },
> + {},
> + },
> + },
> + [R_DMA_SRC_ADDR] = { .name = "DMA_SRC_ADDR" },
> + [R_DMA_DST_ADDR] = { .name = "DMA_DST_ADDR" },
> + [R_DMA_SRC_LEN] = { .name = "DMA_SRC_LEN", .ro = ~ONES(27) },
> + [R_DMA_DST_LEN] = { .name = "DMA_DST_LEN",
> + .ro = ~ONES(27),
> + .post_write = r_dma_dst_len_post_write,
> + },
> + [R_ROM_SHADOW] = { .name = "ROM_SHADOW",
> + .ge1 = (RegisterAccessError[]) {
> + {.mask = ~0, .reason = "Reserved - do not modify" },
> + {},
> + },
> + },
> + [R_SW_ID] = { .name = "SW_ID" },
> + [R_UNLOCK] = { .name = "UNLOCK",
> + .post_write = r_unlock_post_write,
> + },
> + [R_MCTRL] = { .name = "MCTRL",
> + /* Silicon 3.0 for version field, and the mysterious reserved bit 23 */
> + .reset = 0x2 << PS_VERSION_SHIFT | 1 << 23,
> + /* some reserved bits are rw while others are ro */
> + .ro = ~INT_PCAP_LPBK,
> + .ge1 = (RegisterAccessError[]) {
> + { .mask = 0x00F00300, .reason = "Reserved - do not modify" },
> + { .mask = 0x00000003, .reason = "Reserved - always write with 0" },
> + {}
> + },
> + .ge0 = (RegisterAccessError[]) {
> + { .mask = 1 << 23, .reason = "Reserved - always write with 1" },
> + {}
> + },
> + },
> + [R_MAX] = {}
> +};
> +
> +static const MemoryRegionOps 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 void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
> + const char *prefix = object_get_canonical_path(OBJECT(dev));
> + int i;
> +
> + for (i = 0; i < R_MAX; ++i) {
> + RegisterInfo *r = &s->regs_info[i];
> +
> + *r = (RegisterInfo) {
> + .data = &s->regs[i],
> + .data_size = sizeof(uint32_t),
> + .access = &xilinx_devcfg_regs_info[i],
> + .debug = XILINX_DEVCFG_ERR_DEBUG,
> + .prefix = prefix,
> + .opaque = s,
> + };
> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
Hi Peter, Should we be putting r->access->name here instead of "devcfg-regs"?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-29 17:04 ` Edgar E. Iglesias
@ 2013-05-29 17:08 ` Paolo Bonzini
2013-05-30 7:15 ` Peter Crosthwaite
0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2013-05-29 17:08 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: blauwirbel, peter.maydell, peter.crosthwaite, qemu-devel, kraxel
Il 29/05/2013 19:04, Edgar E. Iglesias ha scritto:
>> > + for (i = 0; i < R_MAX; ++i) {
>> > + RegisterInfo *r = &s->regs_info[i];
>> > +
>> > + *r = (RegisterInfo) {
>> > + .data = &s->regs[i],
>> > + .data_size = sizeof(uint32_t),
>> > + .access = &xilinx_devcfg_regs_info[i],
>> > + .debug = XILINX_DEVCFG_ERR_DEBUG,
>> > + .prefix = prefix,
>> > + .opaque = s,
>> > + };
>> > + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
> Hi Peter, Should we be putting r->access->name here instead of "devcfg-regs"?
Yes, that's why I preferred to wrap the memory_region_init_io into an
API that takes a RegisterInfo. :)
Paolo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/5] register: Add Register API
2013-05-24 5:47 ` [Qemu-devel] [PATCH v3 2/5] register: Add Register API peter.crosthwaite
@ 2013-05-29 17:52 ` Edgar E. Iglesias
0 siblings, 0 replies; 19+ messages in thread
From: Edgar E. Iglesias @ 2013-05-29 17:52 UTC (permalink / raw)
To: peter.crosthwaite; +Cc: blauwirbel, peter.maydell, pbonzini, qemu-devel, kraxel
On Fri, May 24, 2013 at 03:47:33PM +1000, peter.crosthwaite@xilinx.com wrote:
> 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)
> 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.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@gmail.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 | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/register.h | 129 ++++++++++++++++++++++++++++++++++
> 3 files changed, 320 insertions(+)
> 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 950146c..210cb1a 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -2,6 +2,7 @@
> common-obj-y += qdev.o qdev-properties.o
> # irq.o needed for qdev GPIO handling:
> common-obj-y += irq.o
> +common-obj-y += register.o
>
> common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
> common-obj-$(CONFIG_XILINX_AXI) += stream.o
> diff --git a/hw/core/register.c b/hw/core/register.c
> new file mode 100644
> index 0000000..b10212e
> --- /dev/null
> +++ b/hw/core/register.c
> @@ -0,0 +1,190 @@
> +/*
> + * 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;
> + 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)) {
> + 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);
> + }
> + }
> + }
> +
> + old_val = reg->data ? register_read_val(reg) : ac->reset;
> +
> + 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 (reg->debug) {
> + qemu_log("%s:%s: read of value %#" PRIx64 "\n", reg->prefix,
> + ac->name, ret);
> + }
> + }
> +
> + if (ac->post_read) {
> + ret = ac->post_read(reg, ret);
> + }
> +
> + return ret;
> +}
> +
> +void register_reset(RegisterInfo *reg)
> +{
> + assert(reg);
> + const RegisterAccessInfo *ac;
> +
> + if (!reg->data || !reg->access) {
> + return;
> + }
> +
> + 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;
> +
> + 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..a4a8319
> --- /dev/null
> +++ b/include/hw/register.h
> @@ -0,0 +1,129 @@
> +/*
> + * 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
> + *
> + * @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;
> +
> + 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
> + * @data_big_endian: Define endianess of data register
> + *
> + * @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.8.3.rc1.44.gb387c77.dirty
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model peter.crosthwaite
2013-05-29 8:51 ` Paolo Bonzini
2013-05-29 17:04 ` Edgar E. Iglesias
@ 2013-05-29 17:57 ` Anthony Liguori
2013-05-29 18:52 ` Anthony Liguori
2013-05-30 1:43 ` Peter Crosthwaite
2 siblings, 2 replies; 19+ messages in thread
From: Anthony Liguori @ 2013-05-29 17:57 UTC (permalink / raw)
To: peter.crosthwaite, qemu-devel
Cc: blauwirbel, peter.maydell, pbonzini, kraxel, edgar.iglesias
peter.crosthwaite@xilinx.com writes:
> From: "Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com>
>
> Minimal device model for devcfg module of Zynq. DMA capabilities and
> interrupt generation supported.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
> Changed since v2:
> Some QOM styling updates.
> Re-implemented nw0 for lock register as pre_write
> Changed since v1:
> Rebased against new version of Register API.
> Use action callbacks for side effects rather than switch.
> Documented reasons for ge0, ge1 (Verbatim from TRM)
> Added ui1 definitions for unimplemented major features
> Removed dead lock code
>
> default-configs/arm-softmmu.mak | 1 +
> hw/dma/Makefile.objs | 1 +
> hw/dma/xilinx_devcfg.c | 495 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 497 insertions(+)
> create mode 100644 hw/dma/xilinx_devcfg.c
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 27cbe3d..5a17f64 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -61,6 +61,7 @@ CONFIG_PXA2XX=y
> CONFIG_BITBANG_I2C=y
> CONFIG_FRAMEBUFFER=y
> CONFIG_XILINX_SPIPS=y
> +CONFIG_ZYNQ_DEVCFG=y
>
> CONFIG_A9SCU=y
> CONFIG_MARVELL_88W8618=y
> diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
> index 0e65ed0..96025cf 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) += xilinx_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/xilinx_devcfg.c b/hw/dma/xilinx_devcfg.c
> new file mode 100644
> index 0000000..b82b7cc
> --- /dev/null
> +++ b/hw/dma/xilinx_devcfg.c
> @@ -0,0 +1,495 @@
> +/*
> + * QEMU model of the Xilinx Devcfg Interface
> + *
> + * Copyright (c) 2011 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.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 "sysemu/sysemu.h"
> +#include "sysemu/dma.h"
> +#include "hw/sysbus.h"
> +#include "hw/ptimer.h"
> +#include "qemu/bitops.h"
> +#include "hw/register.h"
> +#include "qemu/bitops.h"
> +
> +#define TYPE_XILINX_DEVCFG "xlnx.ps7-dev-cfg"
> +
> +#define XILINX_DEVCFG(obj) \
> + OBJECT_CHECK(XilinxDevcfg, (obj), TYPE_XILINX_DEVCFG)
> +
> +/* FIXME: get rid of hardcoded nastiness */
> +
> +#define FREQ_HZ 900000000
> +
> +#define BTT_MAX 0x400 /* bytes to transfer per delay inteval */
> +#define CYCLES_BTT_MAX 10000 /*approximate 10k cycles per delay interval */
> +
> +#ifndef XILINX_DEVCFG_ERR_DEBUG
> +#define XILINX_DEVCFG_ERR_DEBUG 0
> +#endif
> +#define DB_PRINT(...) do { \
> + if (XILINX_DEVCFG_ERR_DEBUG) { \
> + fprintf(stderr, ": %s: ", __func__); \
> + fprintf(stderr, ## __VA_ARGS__); \
> + } \
> +} while (0);
> +
> +#define R_CTRL (0x00/4)
> + #define FORCE_RST (1 << 31) /* Not supported, writes ignored */
> + #define PCAP_PR (1 << 27) /* Forced to 0 on bad unlock */
> + #define PCAP_MODE (1 << 26)
> + #define MULTIBOOT_EN (1 << 24)
> + #define USER_MODE (1 << 15)
> + #define PCFG_AES_FUSE (1 << 12) /* locked by AES_FUSE_LOCK */
> + #define PCFG_AES_EN_SHIFT 9 /* locked by AES_EN_LOCK */
> + #define PCFG_AES_EN_LEN 3 /* locked by AES_EN_LOCK */
> + #define PCFG_AES_EN_MASK (((1 << PCFG_AES_EN_LEN) - 1) \
> + << PCFG_AES_EN_SHIFT)
> + #define SEU_EN (1 << 8) /* locked by SEU_LOCK */
> + #define SEC_EN (1 << 7) /* locked by SEC_LOCK */
> + #define SPNIDEN (1 << 6) /* locked by DBG_LOCK */
> + #define SPIDEN (1 << 5) /* locked by DBG_LOCK */
> + #define NIDEN (1 << 4) /* locked by DBG_LOCK */
> + #define DBGEN (1 << 3) /* locked by DBG_LOCK */
> + #define DAP_EN (7 << 0) /* locked by DBG_LOCK */
> +
> +#define R_LOCK (0x04/4)
> + #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] = PCFG_AES_FUSE,
> + [AES_EN_LOCK] = PCFG_AES_EN_MASK,
> + [SEU_LOCK] = SEU_LOCK,
> + [SEC_LOCK] = SEC_LOCK,
> + [DBG_LOCK] = SPNIDEN | SPIDEN | NIDEN | DBGEN | DAP_EN,
> +};
> +
> +#define R_CFG (0x08/4)
> + #define RFIFO_TH_SHIFT 10
> + #define RFIFO_TH_LEN 2
> + #define WFIFO_TH_SHIFT 8
> + #define WFIFO_TH_LEN 2
> + #define DISABLE_SRC_INC (1 << 5)
> + #define DISABLE_DST_INC (1 << 4)
> +#define R_CFG_RO 0xFFFFF800
> +#define R_CFG_RESET 0x50B
> +
> +#define R_INT_STS (0x0C/4)
> + #define PSS_GTS_USR_B_INT (1 << 31)
> + #define PSS_FST_CFG_B_INT (1 << 30)
> + #define PSS_CFG_RESET_B_INT (1 << 27)
> + #define RX_FIFO_OV_INT (1 << 18)
> + #define WR_FIFO_LVL_INT (1 << 17)
> + #define RD_FIFO_LVL_INT (1 << 16)
> + #define DMA_CMD_ERR_INT (1 << 15)
> + #define DMA_Q_OV_INT (1 << 14)
> + #define DMA_DONE_INT (1 << 13)
> + #define DMA_P_DONE_INT (1 << 12)
> + #define P2D_LEN_ERR_INT (1 << 11)
> + #define PCFG_DONE_INT (1 << 2)
> + #define R_INT_STS_RSVD ((0x7 << 24) | (0x1 << 19) | (0xF < 7))
> +
> +#define R_INT_MASK (0x10/4)
> +
> +#define R_STATUS (0x14/4)
> + #define DMA_CMD_Q_F (1 << 31)
> + #define DMA_CMD_Q_E (1 << 30)
> + #define DMA_DONE_CNT_SHIFT 28
> + #define DMA_DONE_CNT_LEN 2
> + #define RX_FIFO_LVL_SHIFT 20
> + #define RX_FIFO_LVL_LEN 5
> + #define TX_FIFO_LVL_SHIFT 12
> + #define TX_FIFO_LVL_LEN 7
> + #define TX_FIFO_LVL (0x7f << 12)
> + #define PSS_GTS_USR_B (1 << 11)
> + #define PSS_FST_CFG_B (1 << 10)
> + #define PSS_CFG_RESET_B (1 << 5)
> +
> +#define R_DMA_SRC_ADDR (0x18/4)
> +#define R_DMA_DST_ADDR (0x1C/4)
> +#define R_DMA_SRC_LEN (0x20/4)
> +#define R_DMA_DST_LEN (0x24/4)
> +#define R_ROM_SHADOW (0x28/4)
> +#define R_SW_ID (0x30/4)
> +#define R_UNLOCK (0x34/4)
> +
> +#define R_UNLOCK_MAGIC 0x757BDF0D
> +
> +#define R_MCTRL (0x80/4)
> + #define PS_VERSION_SHIFT 28
> + #define PS_VERSION_MASK (0xf << PS_VERSION_SHIFT)
> + #define PCFG_POR_B (1 << 8)
> + #define INT_PCAP_LPBK (1 << 4)
> +
> +#define R_MAX (0x118/4+1)
> +
> +#define RX_FIFO_LEN 32
> +#define TX_FIFO_LEN 128
> +
> +#define DMA_COMMAND_FIFO_LEN 10
> +
> +typedef struct XilinxDevcfgDMACommand {
> + uint32_t src_addr;
> + uint32_t dest_addr;
> + uint32_t src_len;
> + uint32_t dest_len;
> +} XilinxDevcfgDMACommand;
> +
> +static const VMStateDescription vmstate_xilinx_devcfg_dma_command = {
> + .name = "xilinx_devcfg_dma_command",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(src_addr, XilinxDevcfgDMACommand),
> + VMSTATE_UINT32(dest_addr, XilinxDevcfgDMACommand),
> + VMSTATE_UINT32(src_len, XilinxDevcfgDMACommand),
> + VMSTATE_UINT32(dest_len, XilinxDevcfgDMACommand),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +typedef struct XilinxDevcfg {
> + SysBusDevice parent_obj;
> +
> + MemoryRegion iomem;
> + qemu_irq irq;
> +
> + QEMUBH *timer_bh;
> + ptimer_state *timer;
> +
> + XilinxDevcfgDMACommand dma_command_fifo[DMA_COMMAND_FIFO_LEN];
> + uint8_t dma_command_fifo_num;
> +
> + uint32_t regs[R_MAX];
> + RegisterInfo regs_info[R_MAX];
> +} XilinxDevcfg;
> +
> +static const VMStateDescription vmstate_xilinx_devcfg = {
> + .name = "xilinx_devcfg",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_PTIMER(timer, XilinxDevcfg),
> + VMSTATE_STRUCT_ARRAY(dma_command_fifo, XilinxDevcfg,
> + DMA_COMMAND_FIFO_LEN, 0,
> + vmstate_xilinx_devcfg_dma_command,
> + XilinxDevcfgDMACommand),
> + VMSTATE_UINT8(dma_command_fifo_num, XilinxDevcfg),
> + VMSTATE_UINT32_ARRAY(regs, XilinxDevcfg, R_MAX),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void xilinx_devcfg_update_ixr(XilinxDevcfg *s)
> +{
> + qemu_set_irq(s->irq, !!(~s->regs[R_INT_MASK] & s->regs[R_INT_STS]));
> +}
> +
> +static void xilinx_devcfg_reset(DeviceState *dev)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
> + int i;
> +
> + for (i = 0; i < R_MAX; ++i) {
> + register_reset(&s->regs_info[i]);
> + }
> +}
> +
> +#define min(a, b) ((a) < (b) ? (a) : (b))
> +
> +static void xilinx_devcfg_dma_go(void *opaque)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(opaque);
> + uint8_t buf[BTT_MAX];
> + XilinxDevcfgDMACommand *dmah = s->dma_command_fifo;
> + uint32_t btt = BTT_MAX;
> +
> + btt = min(btt, dmah->src_len);
> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
> + btt = min(btt, dmah->dest_len);
> + }
> + DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr);
> + dma_memory_read(&dma_context_memory, dmah->src_addr, buf, btt);
> + dmah->src_len -= btt;
> + dmah->src_addr += btt;
> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
> + DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr);
> + dma_memory_write(&dma_context_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] |= DMA_DONE_INT | DMA_P_DONE_INT;
> + s->dma_command_fifo_num = s->dma_command_fifo_num - 1;
> + memcpy(s->dma_command_fifo, &s->dma_command_fifo[1],
> + sizeof(*s->dma_command_fifo) * DMA_COMMAND_FIFO_LEN - 1);
> + }
> + xilinx_devcfg_update_ixr(s);
> + if (s->dma_command_fifo_num) { /* there is still work to do */
> + DB_PRINT("dma work remains, setting up callback ptimer\n");
> + ptimer_set_freq(s->timer, FREQ_HZ);
> + ptimer_set_count(s->timer, CYCLES_BTT_MAX);
> + ptimer_run(s->timer, 1);
> + }
> +}
> +
> +static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
> +
> + xilinx_devcfg_update_ixr(s);
> +}
> +
> +static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val)
> +{
> + XilinxDevcfg *s = XILINX_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 = extract32(val, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
> +
> + if (aes_en != 0 && aes_en != 7) {
> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
> + "unimplemeneted security reset should happen!\n",
> + reg->prefix);
> + }
> +}
> +
> +static void r_unlock_post_write(RegisterInfo *reg, uint64_t val)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
> +
> + if (val == R_UNLOCK_MAGIC) {
> + DB_PRINT("successful unlock\n");
> + } else {/* bad unlock attempt */
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->prefix);
> + qemu_log_mask(LOG_UNIMP, "%s: failed unlock, unimplmeneted crash of"
> + "device should happen\n", reg->prefix);
> + s->regs[R_CTRL] &= ~PCAP_PR;
> + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
> + }
> +}
> +
> +static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val)
> +{
> + XilinxDevcfg *s = XILINX_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)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
> +
> + /* TODO: implement command queue overflow check and interrupt */
> + s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
> + s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
> + s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
> + s->regs[R_DMA_DST_ADDR] & ~0x3UL;
> + s->dma_command_fifo[s->dma_command_fifo_num].src_len =
> + s->regs[R_DMA_SRC_LEN] << 2;
> + s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
> + s->regs[R_DMA_DST_LEN] << 2;
> + s->dma_command_fifo_num++;
> + DB_PRINT("dma transfer started; %d total transfers pending\n",
> + s->dma_command_fifo_num);
> + xilinx_devcfg_dma_go(s);
> +}
> +
> +static const RegisterAccessInfo xilinx_devcfg_regs_info[] = {
> + [R_CTRL] = { .name = "CTRL",
> + .reset = PCAP_PR | PCAP_MODE | 0x3 << 13,
> + .ro = 0x107f6000,
> + .ge1 = (RegisterAccessError[]) {
> + { .mask = 0x1 << 15, .reason = "Reserved - do not modify" },
> + {},
> + },
> + .ge0 = (RegisterAccessError[]) {
> + { .mask = 0x3 << 13, .reason = "Reserved - always write with 1" },
> + {},
> + },
> + .ui1 = (RegisterAccessError[]) {
> + { .mask = FORCE_RST, .reason = "PS reset not implemented" },
> + { .mask = PCAP_MODE, .reason = "FPGA Fabric doesnt exist" },
> + { .mask = PCFG_AES_EN_MASK, .reason = "AES not implmented" },
> + {},
> + },
> + .pre_write = r_ctrl_pre_write,
> + .post_write = r_ctrl_post_write,
I dislike that this mechanism decentralizes register access. What about
a slightly different style so you could do something like:
static void my_device_io_write(...)
{
switch (register_decode(&my_device_reginfo, s, &info)) {
case R_CTRL:
// handle pre-write bits here
...
}
switch (register_write(&my_device_reginfo, s)) {
case R_CTRL:
//handle post write bits
...
}
}
It makes it much easier to convert existing code. We can then leave
most of the switch statements intact and just introduce register
descriptions.
I think splitting decode from data processing is useful too because it
makes it easier to support latching.
I think the current spin is probably over generalizing too. I don't
think supporting ge0/ge1 makes a lot of sense from the start. Decisions
aren't always that simple and it's weird to have sanity checking split
across two places.
BTW: I think it's also a good idea to model this as a QOM object so that
device state can be access through the QOM tree.
Regards,
Anthony Liguori
> + },
> + [R_LOCK] = { .name = "LOCK",
> + .ro = ~ONES(5),
> + .pre_write = r_lock_pre_write,
> + },
> + [R_CFG] = { .name = "CFG",
> + .reset = 1 << RFIFO_TH_SHIFT | 1 << WFIFO_TH_SHIFT | 0x8,
> + .ge1 = (RegisterAccessError[]) {
> + { .mask = 0x7, .reason = "Reserved - do not modify" },
> + {},
> + },
> + .ge0 = (RegisterAccessError[]) {
> + { .mask = 0x8, .reason = "Reserved - do not modify" },
> + {},
> + },
> + .ro = 0x00f | ~ONES(12),
> + },
> + [R_INT_STS] = { .name = "INT_STS",
> + .w1c = ~R_INT_STS_RSVD,
> + .reset = PSS_GTS_USR_B_INT | PSS_CFG_RESET_B_INT | WR_FIFO_LVL_INT,
> + .ro = R_INT_STS_RSVD,
> + .post_write = r_ixr_post_write,
> + },
> + [R_INT_MASK] = { .name = "INT_MASK",
> + .reset = ~0,
> + .ro = R_INT_STS_RSVD,
> + .post_write = r_ixr_post_write,
> + },
> + [R_STATUS] = { .name = "STATUS",
> + .reset = DMA_CMD_Q_E | PSS_GTS_USR_B | PSS_CFG_RESET_B,
> + .ro = ~0,
> + .ge1 = (RegisterAccessError[]) {
> + {.mask = 0x1, .reason = "Reserved - do not modify" },
> + {},
> + },
> + },
> + [R_DMA_SRC_ADDR] = { .name = "DMA_SRC_ADDR" },
> + [R_DMA_DST_ADDR] = { .name = "DMA_DST_ADDR" },
> + [R_DMA_SRC_LEN] = { .name = "DMA_SRC_LEN", .ro = ~ONES(27) },
> + [R_DMA_DST_LEN] = { .name = "DMA_DST_LEN",
> + .ro = ~ONES(27),
> + .post_write = r_dma_dst_len_post_write,
> + },
> + [R_ROM_SHADOW] = { .name = "ROM_SHADOW",
> + .ge1 = (RegisterAccessError[]) {
> + {.mask = ~0, .reason = "Reserved - do not modify" },
> + {},
> + },
> + },
> + [R_SW_ID] = { .name = "SW_ID" },
> + [R_UNLOCK] = { .name = "UNLOCK",
> + .post_write = r_unlock_post_write,
> + },
> + [R_MCTRL] = { .name = "MCTRL",
> + /* Silicon 3.0 for version field, and the mysterious reserved bit 23 */
> + .reset = 0x2 << PS_VERSION_SHIFT | 1 << 23,
> + /* some reserved bits are rw while others are ro */
> + .ro = ~INT_PCAP_LPBK,
> + .ge1 = (RegisterAccessError[]) {
> + { .mask = 0x00F00300, .reason = "Reserved - do not modify" },
> + { .mask = 0x00000003, .reason = "Reserved - always write with 0" },
> + {}
> + },
> + .ge0 = (RegisterAccessError[]) {
> + { .mask = 1 << 23, .reason = "Reserved - always write with 1" },
> + {}
> + },
> + },
> + [R_MAX] = {}
> +};
> +
> +static const MemoryRegionOps 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 void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
> +{
> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
> + const char *prefix = object_get_canonical_path(OBJECT(dev));
> + int i;
> +
> + for (i = 0; i < R_MAX; ++i) {
> + RegisterInfo *r = &s->regs_info[i];
> +
> + *r = (RegisterInfo) {
> + .data = &s->regs[i],
> + .data_size = sizeof(uint32_t),
> + .access = &xilinx_devcfg_regs_info[i],
> + .debug = XILINX_DEVCFG_ERR_DEBUG,
> + .prefix = prefix,
> + .opaque = s,
> + };
> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
> + }
> +}
> +
> +static void xilinx_devcfg_init(Object *obj)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> + XilinxDevcfg *s = XILINX_DEVCFG(obj);
> +
> + s->timer_bh = qemu_bh_new(xilinx_devcfg_dma_go, s);
> + s->timer = ptimer_init(s->timer_bh);
> +
> + sysbus_init_irq(sbd, &s->irq);
> +
> + memory_region_init(&s->iomem, "devcfg", R_MAX*4);
> + sysbus_init_mmio(sbd, &s->iomem);
> +}
> +
> +static void xilinx_devcfg_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->reset = xilinx_devcfg_reset;
> + dc->vmsd = &vmstate_xilinx_devcfg;
> + dc->realize = xilinx_devcfg_realize;
> +}
> +
> +static const TypeInfo xilinx_devcfg_info = {
> + .name = TYPE_XILINX_DEVCFG,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(XilinxDevcfg),
> + .instance_init = xilinx_devcfg_init,
> + .class_init = xilinx_devcfg_class_init,
> +};
> +
> +static void xilinx_devcfg_register_types(void)
> +{
> + type_register_static(&xilinx_devcfg_info);
> +}
> +
> +type_init(xilinx_devcfg_register_types)
> --
> 1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-29 17:57 ` Anthony Liguori
@ 2013-05-29 18:52 ` Anthony Liguori
2013-05-30 1:43 ` Peter Crosthwaite
1 sibling, 0 replies; 19+ messages in thread
From: Anthony Liguori @ 2013-05-29 18:52 UTC (permalink / raw)
To: peter.crosthwaite, qemu-devel
Cc: blauwirbel, peter.maydell, pbonzini, kraxel, edgar.iglesias
[-- Attachment #1: Type: text/plain, Size: 17273 bytes --]
Anthony Liguori <anthony@codemonkey.ws> writes:
> peter.crosthwaite@xilinx.com writes:
>
>> From: "Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com>
>>
>> Minimal device model for devcfg module of Zynq. DMA capabilities and
>> interrupt generation supported.
>>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>> Changed since v2:
>> Some QOM styling updates.
>> Re-implemented nw0 for lock register as pre_write
>> Changed since v1:
>> Rebased against new version of Register API.
>> Use action callbacks for side effects rather than switch.
>> Documented reasons for ge0, ge1 (Verbatim from TRM)
>> Added ui1 definitions for unimplemented major features
>> Removed dead lock code
>>
>> default-configs/arm-softmmu.mak | 1 +
>> hw/dma/Makefile.objs | 1 +
>> hw/dma/xilinx_devcfg.c | 495 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 497 insertions(+)
>> create mode 100644 hw/dma/xilinx_devcfg.c
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 27cbe3d..5a17f64 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -61,6 +61,7 @@ CONFIG_PXA2XX=y
>> CONFIG_BITBANG_I2C=y
>> CONFIG_FRAMEBUFFER=y
>> CONFIG_XILINX_SPIPS=y
>> +CONFIG_ZYNQ_DEVCFG=y
>>
>> CONFIG_A9SCU=y
>> CONFIG_MARVELL_88W8618=y
>> diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
>> index 0e65ed0..96025cf 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) += xilinx_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/xilinx_devcfg.c b/hw/dma/xilinx_devcfg.c
>> new file mode 100644
>> index 0000000..b82b7cc
>> --- /dev/null
>> +++ b/hw/dma/xilinx_devcfg.c
>> @@ -0,0 +1,495 @@
>> +/*
>> + * QEMU model of the Xilinx Devcfg Interface
>> + *
>> + * Copyright (c) 2011 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.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 "sysemu/sysemu.h"
>> +#include "sysemu/dma.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/ptimer.h"
>> +#include "qemu/bitops.h"
>> +#include "hw/register.h"
>> +#include "qemu/bitops.h"
>> +
>> +#define TYPE_XILINX_DEVCFG "xlnx.ps7-dev-cfg"
>> +
>> +#define XILINX_DEVCFG(obj) \
>> + OBJECT_CHECK(XilinxDevcfg, (obj), TYPE_XILINX_DEVCFG)
>> +
>> +/* FIXME: get rid of hardcoded nastiness */
>> +
>> +#define FREQ_HZ 900000000
>> +
>> +#define BTT_MAX 0x400 /* bytes to transfer per delay inteval */
>> +#define CYCLES_BTT_MAX 10000 /*approximate 10k cycles per delay interval */
>> +
>> +#ifndef XILINX_DEVCFG_ERR_DEBUG
>> +#define XILINX_DEVCFG_ERR_DEBUG 0
>> +#endif
>> +#define DB_PRINT(...) do { \
>> + if (XILINX_DEVCFG_ERR_DEBUG) { \
>> + fprintf(stderr, ": %s: ", __func__); \
>> + fprintf(stderr, ## __VA_ARGS__); \
>> + } \
>> +} while (0);
>> +
>> +#define R_CTRL (0x00/4)
>> + #define FORCE_RST (1 << 31) /* Not supported, writes ignored */
>> + #define PCAP_PR (1 << 27) /* Forced to 0 on bad unlock */
>> + #define PCAP_MODE (1 << 26)
>> + #define MULTIBOOT_EN (1 << 24)
>> + #define USER_MODE (1 << 15)
>> + #define PCFG_AES_FUSE (1 << 12) /* locked by AES_FUSE_LOCK */
>> + #define PCFG_AES_EN_SHIFT 9 /* locked by AES_EN_LOCK */
>> + #define PCFG_AES_EN_LEN 3 /* locked by AES_EN_LOCK */
>> + #define PCFG_AES_EN_MASK (((1 << PCFG_AES_EN_LEN) - 1) \
>> + << PCFG_AES_EN_SHIFT)
>> + #define SEU_EN (1 << 8) /* locked by SEU_LOCK */
>> + #define SEC_EN (1 << 7) /* locked by SEC_LOCK */
>> + #define SPNIDEN (1 << 6) /* locked by DBG_LOCK */
>> + #define SPIDEN (1 << 5) /* locked by DBG_LOCK */
>> + #define NIDEN (1 << 4) /* locked by DBG_LOCK */
>> + #define DBGEN (1 << 3) /* locked by DBG_LOCK */
>> + #define DAP_EN (7 << 0) /* locked by DBG_LOCK */
>> +
>> +#define R_LOCK (0x04/4)
>> + #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] = PCFG_AES_FUSE,
>> + [AES_EN_LOCK] = PCFG_AES_EN_MASK,
>> + [SEU_LOCK] = SEU_LOCK,
>> + [SEC_LOCK] = SEC_LOCK,
>> + [DBG_LOCK] = SPNIDEN | SPIDEN | NIDEN | DBGEN | DAP_EN,
>> +};
>> +
>> +#define R_CFG (0x08/4)
>> + #define RFIFO_TH_SHIFT 10
>> + #define RFIFO_TH_LEN 2
>> + #define WFIFO_TH_SHIFT 8
>> + #define WFIFO_TH_LEN 2
>> + #define DISABLE_SRC_INC (1 << 5)
>> + #define DISABLE_DST_INC (1 << 4)
>> +#define R_CFG_RO 0xFFFFF800
>> +#define R_CFG_RESET 0x50B
>> +
>> +#define R_INT_STS (0x0C/4)
>> + #define PSS_GTS_USR_B_INT (1 << 31)
>> + #define PSS_FST_CFG_B_INT (1 << 30)
>> + #define PSS_CFG_RESET_B_INT (1 << 27)
>> + #define RX_FIFO_OV_INT (1 << 18)
>> + #define WR_FIFO_LVL_INT (1 << 17)
>> + #define RD_FIFO_LVL_INT (1 << 16)
>> + #define DMA_CMD_ERR_INT (1 << 15)
>> + #define DMA_Q_OV_INT (1 << 14)
>> + #define DMA_DONE_INT (1 << 13)
>> + #define DMA_P_DONE_INT (1 << 12)
>> + #define P2D_LEN_ERR_INT (1 << 11)
>> + #define PCFG_DONE_INT (1 << 2)
>> + #define R_INT_STS_RSVD ((0x7 << 24) | (0x1 << 19) | (0xF < 7))
>> +
>> +#define R_INT_MASK (0x10/4)
>> +
>> +#define R_STATUS (0x14/4)
>> + #define DMA_CMD_Q_F (1 << 31)
>> + #define DMA_CMD_Q_E (1 << 30)
>> + #define DMA_DONE_CNT_SHIFT 28
>> + #define DMA_DONE_CNT_LEN 2
>> + #define RX_FIFO_LVL_SHIFT 20
>> + #define RX_FIFO_LVL_LEN 5
>> + #define TX_FIFO_LVL_SHIFT 12
>> + #define TX_FIFO_LVL_LEN 7
>> + #define TX_FIFO_LVL (0x7f << 12)
>> + #define PSS_GTS_USR_B (1 << 11)
>> + #define PSS_FST_CFG_B (1 << 10)
>> + #define PSS_CFG_RESET_B (1 << 5)
>> +
>> +#define R_DMA_SRC_ADDR (0x18/4)
>> +#define R_DMA_DST_ADDR (0x1C/4)
>> +#define R_DMA_SRC_LEN (0x20/4)
>> +#define R_DMA_DST_LEN (0x24/4)
>> +#define R_ROM_SHADOW (0x28/4)
>> +#define R_SW_ID (0x30/4)
>> +#define R_UNLOCK (0x34/4)
>> +
>> +#define R_UNLOCK_MAGIC 0x757BDF0D
>> +
>> +#define R_MCTRL (0x80/4)
>> + #define PS_VERSION_SHIFT 28
>> + #define PS_VERSION_MASK (0xf << PS_VERSION_SHIFT)
>> + #define PCFG_POR_B (1 << 8)
>> + #define INT_PCAP_LPBK (1 << 4)
>> +
>> +#define R_MAX (0x118/4+1)
>> +
>> +#define RX_FIFO_LEN 32
>> +#define TX_FIFO_LEN 128
>> +
>> +#define DMA_COMMAND_FIFO_LEN 10
>> +
>> +typedef struct XilinxDevcfgDMACommand {
>> + uint32_t src_addr;
>> + uint32_t dest_addr;
>> + uint32_t src_len;
>> + uint32_t dest_len;
>> +} XilinxDevcfgDMACommand;
>> +
>> +static const VMStateDescription vmstate_xilinx_devcfg_dma_command = {
>> + .name = "xilinx_devcfg_dma_command",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT32(src_addr, XilinxDevcfgDMACommand),
>> + VMSTATE_UINT32(dest_addr, XilinxDevcfgDMACommand),
>> + VMSTATE_UINT32(src_len, XilinxDevcfgDMACommand),
>> + VMSTATE_UINT32(dest_len, XilinxDevcfgDMACommand),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +typedef struct XilinxDevcfg {
>> + SysBusDevice parent_obj;
>> +
>> + MemoryRegion iomem;
>> + qemu_irq irq;
>> +
>> + QEMUBH *timer_bh;
>> + ptimer_state *timer;
>> +
>> + XilinxDevcfgDMACommand dma_command_fifo[DMA_COMMAND_FIFO_LEN];
>> + uint8_t dma_command_fifo_num;
>> +
>> + uint32_t regs[R_MAX];
>> + RegisterInfo regs_info[R_MAX];
>> +} XilinxDevcfg;
>> +
>> +static const VMStateDescription vmstate_xilinx_devcfg = {
>> + .name = "xilinx_devcfg",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_PTIMER(timer, XilinxDevcfg),
>> + VMSTATE_STRUCT_ARRAY(dma_command_fifo, XilinxDevcfg,
>> + DMA_COMMAND_FIFO_LEN, 0,
>> + vmstate_xilinx_devcfg_dma_command,
>> + XilinxDevcfgDMACommand),
>> + VMSTATE_UINT8(dma_command_fifo_num, XilinxDevcfg),
>> + VMSTATE_UINT32_ARRAY(regs, XilinxDevcfg, R_MAX),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static void xilinx_devcfg_update_ixr(XilinxDevcfg *s)
>> +{
>> + qemu_set_irq(s->irq, !!(~s->regs[R_INT_MASK] & s->regs[R_INT_STS]));
>> +}
>> +
>> +static void xilinx_devcfg_reset(DeviceState *dev)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>> + int i;
>> +
>> + for (i = 0; i < R_MAX; ++i) {
>> + register_reset(&s->regs_info[i]);
>> + }
>> +}
>> +
>> +#define min(a, b) ((a) < (b) ? (a) : (b))
>> +
>> +static void xilinx_devcfg_dma_go(void *opaque)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(opaque);
>> + uint8_t buf[BTT_MAX];
>> + XilinxDevcfgDMACommand *dmah = s->dma_command_fifo;
>> + uint32_t btt = BTT_MAX;
>> +
>> + btt = min(btt, dmah->src_len);
>> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
>> + btt = min(btt, dmah->dest_len);
>> + }
>> + DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr);
>> + dma_memory_read(&dma_context_memory, dmah->src_addr, buf, btt);
>> + dmah->src_len -= btt;
>> + dmah->src_addr += btt;
>> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
>> + DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr);
>> + dma_memory_write(&dma_context_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] |= DMA_DONE_INT | DMA_P_DONE_INT;
>> + s->dma_command_fifo_num = s->dma_command_fifo_num - 1;
>> + memcpy(s->dma_command_fifo, &s->dma_command_fifo[1],
>> + sizeof(*s->dma_command_fifo) * DMA_COMMAND_FIFO_LEN - 1);
>> + }
>> + xilinx_devcfg_update_ixr(s);
>> + if (s->dma_command_fifo_num) { /* there is still work to do */
>> + DB_PRINT("dma work remains, setting up callback ptimer\n");
>> + ptimer_set_freq(s->timer, FREQ_HZ);
>> + ptimer_set_count(s->timer, CYCLES_BTT_MAX);
>> + ptimer_run(s->timer, 1);
>> + }
>> +}
>> +
>> +static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>> +
>> + xilinx_devcfg_update_ixr(s);
>> +}
>> +
>> +static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val)
>> +{
>> + XilinxDevcfg *s = XILINX_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 = extract32(val, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
>> +
>> + if (aes_en != 0 && aes_en != 7) {
>> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
>> + "unimplemeneted security reset should happen!\n",
>> + reg->prefix);
>> + }
>> +}
>> +
>> +static void r_unlock_post_write(RegisterInfo *reg, uint64_t val)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>> +
>> + if (val == R_UNLOCK_MAGIC) {
>> + DB_PRINT("successful unlock\n");
>> + } else {/* bad unlock attempt */
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->prefix);
>> + qemu_log_mask(LOG_UNIMP, "%s: failed unlock, unimplmeneted crash of"
>> + "device should happen\n", reg->prefix);
>> + s->regs[R_CTRL] &= ~PCAP_PR;
>> + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
>> + }
>> +}
>> +
>> +static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val)
>> +{
>> + XilinxDevcfg *s = XILINX_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)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>> +
>> + /* TODO: implement command queue overflow check and interrupt */
>> + s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
>> + s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
>> + s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
>> + s->regs[R_DMA_DST_ADDR] & ~0x3UL;
>> + s->dma_command_fifo[s->dma_command_fifo_num].src_len =
>> + s->regs[R_DMA_SRC_LEN] << 2;
>> + s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
>> + s->regs[R_DMA_DST_LEN] << 2;
>> + s->dma_command_fifo_num++;
>> + DB_PRINT("dma transfer started; %d total transfers pending\n",
>> + s->dma_command_fifo_num);
>> + xilinx_devcfg_dma_go(s);
>> +}
>> +
>> +static const RegisterAccessInfo xilinx_devcfg_regs_info[] = {
>> + [R_CTRL] = { .name = "CTRL",
>> + .reset = PCAP_PR | PCAP_MODE | 0x3 << 13,
>> + .ro = 0x107f6000,
>> + .ge1 = (RegisterAccessError[]) {
>> + { .mask = 0x1 << 15, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + .ge0 = (RegisterAccessError[]) {
>> + { .mask = 0x3 << 13, .reason = "Reserved - always write with 1" },
>> + {},
>> + },
>> + .ui1 = (RegisterAccessError[]) {
>> + { .mask = FORCE_RST, .reason = "PS reset not implemented" },
>> + { .mask = PCAP_MODE, .reason = "FPGA Fabric doesnt exist" },
>> + { .mask = PCFG_AES_EN_MASK, .reason = "AES not implmented" },
>> + {},
>> + },
>> + .pre_write = r_ctrl_pre_write,
>> + .post_write = r_ctrl_post_write,
>
> I dislike that this mechanism decentralizes register access. What about
> a slightly different style so you could do something like:
>
> static void my_device_io_write(...)
> {
> switch (register_decode(&my_device_reginfo, s, &info)) {
> case R_CTRL:
> // handle pre-write bits here
> ...
> }
>
> switch (register_write(&my_device_reginfo, s)) {
> case R_CTRL:
> //handle post write bits
> ...
> }
> }
>
> It makes it much easier to convert existing code. We can then leave
> most of the switch statements intact and just introduce register
> descriptions.
>
> I think splitting decode from data processing is useful too because it
> makes it easier to support latching.
Here's a more illustrated example. I didn't try to make anything data
driven but I hope you can see how that would fit in.
I think there's a lot of value in splitting decode vs. load/store. I
think it's important to allow certain things to remain open coded and if
you see how I did it, there are a few registers that are decoded and
load/stored in an open coded fashion remaining.
By separating decode, we can have universal tracing of specific register
access (even if we're open coding it).
I think reset is also out of scope for an API like this. Particularly
when talking about latching, there isn't a 1-1 relationship anymore
between things with initial values and then the visibility within the
address space.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Demonstrate-separating-register-decode-from-get-set.patch --]
[-- Type: text/x-diff, Size: 9950 bytes --]
>From 8825ec58285957b8586e9439a84bbd5eca773fbb Mon Sep 17 00:00:00 2001
From: Anthony Liguori <aliguori@us.ibm.com>
Date: Wed, 29 May 2013 13:44:35 -0500
Subject: [PATCH] Demonstrate separating register decode from get/set
---
hw/char/serial.c | 253 ++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 167 insertions(+), 86 deletions(-)
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 66b6348..be637f5 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -299,6 +299,113 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
return FALSE;
}
+enum {
+ UART_INVALID = 0,
+ UART_DIVIDER,
+ UART_RBR,
+ UART_THR,
+ UART_IER,
+ UART_IIR,
+ UART_FCR,
+ UART_LCR,
+ UART_MCR,
+ UART_LSR,
+ UART_MSR,
+ UART_SCR,
+};
+
+static int serial_decode(void *opaque, hwaddr addr, unsigned size, bool is_write)
+{
+ SerialState *s = opaque;
+
+ switch (addr) {
+ case 0:
+ if (s->lcr & UART_LCR_DLAB) {
+ return UART_DIVIDER;
+ }
+ if (is_write) {
+ return UART_THR;
+ }
+ return UART_RBR;
+ case 1:
+ if (s->lcr & UART_LCR_DLAB) {
+ return UART_DIVIDER;
+ }
+ return UART_IER;
+ case 2:
+ if (is_write) {
+ return UART_FCR;
+ }
+ return UART_IIR;
+ case 3:
+ return UART_LCR;
+ case 4:
+ return UART_MCR;
+ case 5:
+ if (is_write) {
+ return UART_INVALID;
+ }
+ return UART_LSR;
+ case 6:
+ if (is_write) {
+ return UART_INVALID;
+ }
+ return UART_MSR;
+ case 7:
+ return UART_SCR;
+ default:
+ return UART_INVALID;
+ }
+}
+
+static int serial_load(void *opaque, int regno, unsigned size, uint64_t *ret)
+{
+ SerialState *s = opaque;
+
+ switch (regno) {
+ case UART_IER:
+ *ret = s->ier;
+ break;
+ case UART_IIR:
+ *ret = s->iir;
+ break;
+ case UART_LCR:
+ *ret = s->lcr;
+ break;
+ case UART_MCR:
+ *ret = s->mcr;
+ break;
+ case UART_LSR:
+ *ret = s->lsr;
+ break;
+ case UART_SCR:
+ *ret = s->scr;
+ break;
+ default:
+ break;
+ }
+
+ return regno;
+}
+
+static int serial_store(void *opaque, int regno, unsigned size, uint64_t val)
+{
+ SerialState *s = opaque;
+
+ switch (regno) {
+ case UART_THR:
+ s->thr = val;
+ break;
+ case UART_IER:
+ s->ier = val & 0x0f;
+ break;
+ case UART_LCR:
+ s->lcr = val;
+ break;
+ }
+
+ return regno;
+}
static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
@@ -307,52 +414,48 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
addr &= 7;
DPRINTF("write addr=0x%" HWADDR_PRIx " val=0x%" PRIx64 "\n", addr, val);
- switch(addr) {
+ switch(serial_store(s, serial_decode(s, addr, size, true), size, val)) {
default:
- case 0:
- if (s->lcr & UART_LCR_DLAB) {
+ case UART_DIVIDER:
+ if (addr == 0) {
s->divider = (s->divider & 0xff00) | val;
- serial_update_parameters(s);
} else {
- s->thr = (uint8_t) val;
- if(s->fcr & UART_FCR_FE) {
- fifo_put(s, XMIT_FIFO, s->thr);
- s->thr_ipending = 0;
- s->lsr &= ~UART_LSR_TEMT;
- s->lsr &= ~UART_LSR_THRE;
- serial_update_irq(s);
- } else {
- s->thr_ipending = 0;
- s->lsr &= ~UART_LSR_THRE;
- serial_update_irq(s);
- }
- serial_xmit(NULL, G_IO_OUT, s);
+ s->divider = (s->divider & 0x00ff) | (val << 8);
}
+ serial_update_parameters(s);
break;
- case 1:
- if (s->lcr & UART_LCR_DLAB) {
- s->divider = (s->divider & 0x00ff) | (val << 8);
- serial_update_parameters(s);
+ case UART_THR:
+ if(s->fcr & UART_FCR_FE) {
+ fifo_put(s, XMIT_FIFO, s->thr);
+ s->thr_ipending = 0;
+ s->lsr &= ~UART_LSR_TEMT;
+ s->lsr &= ~UART_LSR_THRE;
+ serial_update_irq(s);
} else {
- s->ier = val & 0x0f;
- /* If the backend device is a real serial port, turn polling of the modem
- status lines on physical port on or off depending on UART_IER_MSI state */
- if (s->poll_msl >= 0) {
- if (s->ier & UART_IER_MSI) {
- s->poll_msl = 1;
- serial_update_msl(s);
- } else {
- qemu_del_timer(s->modem_status_poll);
- s->poll_msl = 0;
- }
- }
- if (s->lsr & UART_LSR_THRE) {
- s->thr_ipending = 1;
- serial_update_irq(s);
+ s->thr_ipending = 0;
+ s->lsr &= ~UART_LSR_THRE;
+ serial_update_irq(s);
+ }
+ serial_xmit(NULL, G_IO_OUT, s);
+ break;
+ case UART_IER:
+ /* If the backend device is a real serial port, turn polling of the modem
+ status lines on physical port on or off depending on UART_IER_MSI state */
+ if (s->poll_msl >= 0) {
+ if (s->ier & UART_IER_MSI) {
+ s->poll_msl = 1;
+ serial_update_msl(s);
+ } else {
+ qemu_del_timer(s->modem_status_poll);
+ s->poll_msl = 0;
}
}
+ if (s->lsr & UART_LSR_THRE) {
+ s->thr_ipending = 1;
+ serial_update_irq(s);
+ }
break;
- case 2:
+ case UART_FCR:
val = val & 0xFF;
if (s->fcr == val)
@@ -398,10 +501,9 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
s->fcr = val & 0xC9;
serial_update_irq(s);
break;
- case 3:
+ case UART_LCR:
{
int break_enable;
- s->lcr = val;
serial_update_parameters(s);
break_enable = (val >> 6) & 1;
if (break_enable != s->last_break_enable) {
@@ -411,7 +513,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
}
}
break;
- case 4:
+ case UART_MCR:
{
int flags;
int old_mcr = s->mcr;
@@ -437,75 +539,57 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
}
}
break;
- case 5:
- break;
- case 6:
- break;
- case 7:
- s->scr = val;
- break;
}
}
static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
{
SerialState *s = opaque;
- uint32_t ret;
+ uint64_t ret = 0xFF;
addr &= 7;
- switch(addr) {
+ switch(serial_load(s, serial_decode(s, addr, size, false), size, &ret)) {
default:
- case 0:
- if (s->lcr & UART_LCR_DLAB) {
+ break;
+ case UART_DIVIDER:
+ if (addr == 0) {
ret = s->divider & 0xff;
} else {
- if(s->fcr & UART_FCR_FE) {
- ret = fifo_get(s,RECV_FIFO);
- if (s->recv_fifo.count == 0)
- s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
- else
- qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4);
- s->timeout_ipending = 0;
- } else {
- ret = s->rbr;
- s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
- }
- serial_update_irq(s);
- if (!(s->mcr & UART_MCR_LOOP)) {
- /* in loopback mode, don't receive any data */
- qemu_chr_accept_input(s->chr);
- }
+ ret = (s->divider >> 8) & 0xff;
}
break;
- case 1:
- if (s->lcr & UART_LCR_DLAB) {
- ret = (s->divider >> 8) & 0xff;
+ case UART_RBR:
+ if(s->fcr & UART_FCR_FE) {
+ ret = fifo_get(s,RECV_FIFO);
+ if (s->recv_fifo.count == 0)
+ s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
+ else
+ qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4);
+ s->timeout_ipending = 0;
} else {
- ret = s->ier;
+ ret = s->rbr;
+ s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
+ }
+ serial_update_irq(s);
+ if (!(s->mcr & UART_MCR_LOOP)) {
+ /* in loopback mode, don't receive any data */
+ qemu_chr_accept_input(s->chr);
}
break;
- case 2:
- ret = s->iir;
- if ((ret & UART_IIR_ID) == UART_IIR_THRI) {
+ case UART_IIR:
+ if ((s->iir & UART_IIR_ID) == UART_IIR_THRI) {
s->thr_ipending = 0;
serial_update_irq(s);
}
break;
- case 3:
- ret = s->lcr;
- break;
- case 4:
- ret = s->mcr;
- break;
- case 5:
- ret = s->lsr;
+ case UART_LSR:
/* Clear break and overrun interrupts */
if (s->lsr & (UART_LSR_BI|UART_LSR_OE)) {
s->lsr &= ~(UART_LSR_BI|UART_LSR_OE);
serial_update_irq(s);
}
break;
- case 6:
+ case UART_MSR:
if (s->mcr & UART_MCR_LOOP) {
/* in loopback, the modem output pins are connected to the
inputs */
@@ -523,9 +607,6 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
}
}
break;
- case 7:
- ret = s->scr;
- break;
}
DPRINTF("read addr=0x%" HWADDR_PRIx " val=0x%02x\n", addr, ret);
return ret;
--
1.8.0
[-- Attachment #3: Type: text/plain, Size: 5367 bytes --]
Regards,
Anthony Liguori
>
> I think the current spin is probably over generalizing too. I don't
> think supporting ge0/ge1 makes a lot of sense from the start. Decisions
> aren't always that simple and it's weird to have sanity checking split
> across two places.
>
> BTW: I think it's also a good idea to model this as a QOM object so that
> device state can be access through the QOM tree.
>
> Regards,
>
> Anthony Liguori
>
>> + },
>> + [R_LOCK] = { .name = "LOCK",
>> + .ro = ~ONES(5),
>> + .pre_write = r_lock_pre_write,
>> + },
>> + [R_CFG] = { .name = "CFG",
>> + .reset = 1 << RFIFO_TH_SHIFT | 1 << WFIFO_TH_SHIFT | 0x8,
>> + .ge1 = (RegisterAccessError[]) {
>> + { .mask = 0x7, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + .ge0 = (RegisterAccessError[]) {
>> + { .mask = 0x8, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + .ro = 0x00f | ~ONES(12),
>> + },
>> + [R_INT_STS] = { .name = "INT_STS",
>> + .w1c = ~R_INT_STS_RSVD,
>> + .reset = PSS_GTS_USR_B_INT | PSS_CFG_RESET_B_INT | WR_FIFO_LVL_INT,
>> + .ro = R_INT_STS_RSVD,
>> + .post_write = r_ixr_post_write,
>> + },
>> + [R_INT_MASK] = { .name = "INT_MASK",
>> + .reset = ~0,
>> + .ro = R_INT_STS_RSVD,
>> + .post_write = r_ixr_post_write,
>> + },
>> + [R_STATUS] = { .name = "STATUS",
>> + .reset = DMA_CMD_Q_E | PSS_GTS_USR_B | PSS_CFG_RESET_B,
>> + .ro = ~0,
>> + .ge1 = (RegisterAccessError[]) {
>> + {.mask = 0x1, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + },
>> + [R_DMA_SRC_ADDR] = { .name = "DMA_SRC_ADDR" },
>> + [R_DMA_DST_ADDR] = { .name = "DMA_DST_ADDR" },
>> + [R_DMA_SRC_LEN] = { .name = "DMA_SRC_LEN", .ro = ~ONES(27) },
>> + [R_DMA_DST_LEN] = { .name = "DMA_DST_LEN",
>> + .ro = ~ONES(27),
>> + .post_write = r_dma_dst_len_post_write,
>> + },
>> + [R_ROM_SHADOW] = { .name = "ROM_SHADOW",
>> + .ge1 = (RegisterAccessError[]) {
>> + {.mask = ~0, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + },
>> + [R_SW_ID] = { .name = "SW_ID" },
>> + [R_UNLOCK] = { .name = "UNLOCK",
>> + .post_write = r_unlock_post_write,
>> + },
>> + [R_MCTRL] = { .name = "MCTRL",
>> + /* Silicon 3.0 for version field, and the mysterious reserved bit 23 */
>> + .reset = 0x2 << PS_VERSION_SHIFT | 1 << 23,
>> + /* some reserved bits are rw while others are ro */
>> + .ro = ~INT_PCAP_LPBK,
>> + .ge1 = (RegisterAccessError[]) {
>> + { .mask = 0x00F00300, .reason = "Reserved - do not modify" },
>> + { .mask = 0x00000003, .reason = "Reserved - always write with 0" },
>> + {}
>> + },
>> + .ge0 = (RegisterAccessError[]) {
>> + { .mask = 1 << 23, .reason = "Reserved - always write with 1" },
>> + {}
>> + },
>> + },
>> + [R_MAX] = {}
>> +};
>> +
>> +static const MemoryRegionOps 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 void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>> + const char *prefix = object_get_canonical_path(OBJECT(dev));
>> + int i;
>> +
>> + for (i = 0; i < R_MAX; ++i) {
>> + RegisterInfo *r = &s->regs_info[i];
>> +
>> + *r = (RegisterInfo) {
>> + .data = &s->regs[i],
>> + .data_size = sizeof(uint32_t),
>> + .access = &xilinx_devcfg_regs_info[i],
>> + .debug = XILINX_DEVCFG_ERR_DEBUG,
>> + .prefix = prefix,
>> + .opaque = s,
>> + };
>> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
>> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
>> + }
>> +}
>> +
>> +static void xilinx_devcfg_init(Object *obj)
>> +{
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> + XilinxDevcfg *s = XILINX_DEVCFG(obj);
>> +
>> + s->timer_bh = qemu_bh_new(xilinx_devcfg_dma_go, s);
>> + s->timer = ptimer_init(s->timer_bh);
>> +
>> + sysbus_init_irq(sbd, &s->irq);
>> +
>> + memory_region_init(&s->iomem, "devcfg", R_MAX*4);
>> + sysbus_init_mmio(sbd, &s->iomem);
>> +}
>> +
>> +static void xilinx_devcfg_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->reset = xilinx_devcfg_reset;
>> + dc->vmsd = &vmstate_xilinx_devcfg;
>> + dc->realize = xilinx_devcfg_realize;
>> +}
>> +
>> +static const TypeInfo xilinx_devcfg_info = {
>> + .name = TYPE_XILINX_DEVCFG,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(XilinxDevcfg),
>> + .instance_init = xilinx_devcfg_init,
>> + .class_init = xilinx_devcfg_class_init,
>> +};
>> +
>> +static void xilinx_devcfg_register_types(void)
>> +{
>> + type_register_static(&xilinx_devcfg_info);
>> +}
>> +
>> +type_init(xilinx_devcfg_register_types)
>> --
>> 1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-29 17:57 ` Anthony Liguori
2013-05-29 18:52 ` Anthony Liguori
@ 2013-05-30 1:43 ` Peter Crosthwaite
2013-05-30 19:41 ` Anthony Liguori
1 sibling, 1 reply; 19+ messages in thread
From: Peter Crosthwaite @ 2013-05-30 1:43 UTC (permalink / raw)
To: Anthony Liguori
Cc: peter.maydell, qemu-devel, blauwirbel, kraxel, pbonzini, edgar.iglesias
Hi Anthony,
On Thu, May 30, 2013 at 3:57 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> peter.crosthwaite@xilinx.com writes:
>
>> From: "Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com>
>>
>> Minimal device model for devcfg module of Zynq. DMA capabilities and
>> interrupt generation supported.
>>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>> Changed since v2:
>> Some QOM styling updates.
>> Re-implemented nw0 for lock register as pre_write
>> Changed since v1:
>> Rebased against new version of Register API.
>> Use action callbacks for side effects rather than switch.
>> Documented reasons for ge0, ge1 (Verbatim from TRM)
>> Added ui1 definitions for unimplemented major features
>> Removed dead lock code
>>
>> default-configs/arm-softmmu.mak | 1 +
>> hw/dma/Makefile.objs | 1 +
>> hw/dma/xilinx_devcfg.c | 495 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 497 insertions(+)
>> create mode 100644 hw/dma/xilinx_devcfg.c
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 27cbe3d..5a17f64 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -61,6 +61,7 @@ CONFIG_PXA2XX=y
>> CONFIG_BITBANG_I2C=y
>> CONFIG_FRAMEBUFFER=y
>> CONFIG_XILINX_SPIPS=y
>> +CONFIG_ZYNQ_DEVCFG=y
>>
>> CONFIG_A9SCU=y
>> CONFIG_MARVELL_88W8618=y
>> diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
>> index 0e65ed0..96025cf 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) += xilinx_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/xilinx_devcfg.c b/hw/dma/xilinx_devcfg.c
>> new file mode 100644
>> index 0000000..b82b7cc
>> --- /dev/null
>> +++ b/hw/dma/xilinx_devcfg.c
>> @@ -0,0 +1,495 @@
>> +/*
>> + * QEMU model of the Xilinx Devcfg Interface
>> + *
>> + * Copyright (c) 2011 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.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 "sysemu/sysemu.h"
>> +#include "sysemu/dma.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/ptimer.h"
>> +#include "qemu/bitops.h"
>> +#include "hw/register.h"
>> +#include "qemu/bitops.h"
>> +
>> +#define TYPE_XILINX_DEVCFG "xlnx.ps7-dev-cfg"
>> +
>> +#define XILINX_DEVCFG(obj) \
>> + OBJECT_CHECK(XilinxDevcfg, (obj), TYPE_XILINX_DEVCFG)
>> +
>> +/* FIXME: get rid of hardcoded nastiness */
>> +
>> +#define FREQ_HZ 900000000
>> +
>> +#define BTT_MAX 0x400 /* bytes to transfer per delay inteval */
>> +#define CYCLES_BTT_MAX 10000 /*approximate 10k cycles per delay interval */
>> +
>> +#ifndef XILINX_DEVCFG_ERR_DEBUG
>> +#define XILINX_DEVCFG_ERR_DEBUG 0
>> +#endif
>> +#define DB_PRINT(...) do { \
>> + if (XILINX_DEVCFG_ERR_DEBUG) { \
>> + fprintf(stderr, ": %s: ", __func__); \
>> + fprintf(stderr, ## __VA_ARGS__); \
>> + } \
>> +} while (0);
>> +
>> +#define R_CTRL (0x00/4)
>> + #define FORCE_RST (1 << 31) /* Not supported, writes ignored */
>> + #define PCAP_PR (1 << 27) /* Forced to 0 on bad unlock */
>> + #define PCAP_MODE (1 << 26)
>> + #define MULTIBOOT_EN (1 << 24)
>> + #define USER_MODE (1 << 15)
>> + #define PCFG_AES_FUSE (1 << 12) /* locked by AES_FUSE_LOCK */
>> + #define PCFG_AES_EN_SHIFT 9 /* locked by AES_EN_LOCK */
>> + #define PCFG_AES_EN_LEN 3 /* locked by AES_EN_LOCK */
>> + #define PCFG_AES_EN_MASK (((1 << PCFG_AES_EN_LEN) - 1) \
>> + << PCFG_AES_EN_SHIFT)
>> + #define SEU_EN (1 << 8) /* locked by SEU_LOCK */
>> + #define SEC_EN (1 << 7) /* locked by SEC_LOCK */
>> + #define SPNIDEN (1 << 6) /* locked by DBG_LOCK */
>> + #define SPIDEN (1 << 5) /* locked by DBG_LOCK */
>> + #define NIDEN (1 << 4) /* locked by DBG_LOCK */
>> + #define DBGEN (1 << 3) /* locked by DBG_LOCK */
>> + #define DAP_EN (7 << 0) /* locked by DBG_LOCK */
>> +
>> +#define R_LOCK (0x04/4)
>> + #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] = PCFG_AES_FUSE,
>> + [AES_EN_LOCK] = PCFG_AES_EN_MASK,
>> + [SEU_LOCK] = SEU_LOCK,
>> + [SEC_LOCK] = SEC_LOCK,
>> + [DBG_LOCK] = SPNIDEN | SPIDEN | NIDEN | DBGEN | DAP_EN,
>> +};
>> +
>> +#define R_CFG (0x08/4)
>> + #define RFIFO_TH_SHIFT 10
>> + #define RFIFO_TH_LEN 2
>> + #define WFIFO_TH_SHIFT 8
>> + #define WFIFO_TH_LEN 2
>> + #define DISABLE_SRC_INC (1 << 5)
>> + #define DISABLE_DST_INC (1 << 4)
>> +#define R_CFG_RO 0xFFFFF800
>> +#define R_CFG_RESET 0x50B
>> +
>> +#define R_INT_STS (0x0C/4)
>> + #define PSS_GTS_USR_B_INT (1 << 31)
>> + #define PSS_FST_CFG_B_INT (1 << 30)
>> + #define PSS_CFG_RESET_B_INT (1 << 27)
>> + #define RX_FIFO_OV_INT (1 << 18)
>> + #define WR_FIFO_LVL_INT (1 << 17)
>> + #define RD_FIFO_LVL_INT (1 << 16)
>> + #define DMA_CMD_ERR_INT (1 << 15)
>> + #define DMA_Q_OV_INT (1 << 14)
>> + #define DMA_DONE_INT (1 << 13)
>> + #define DMA_P_DONE_INT (1 << 12)
>> + #define P2D_LEN_ERR_INT (1 << 11)
>> + #define PCFG_DONE_INT (1 << 2)
>> + #define R_INT_STS_RSVD ((0x7 << 24) | (0x1 << 19) | (0xF < 7))
>> +
>> +#define R_INT_MASK (0x10/4)
>> +
>> +#define R_STATUS (0x14/4)
>> + #define DMA_CMD_Q_F (1 << 31)
>> + #define DMA_CMD_Q_E (1 << 30)
>> + #define DMA_DONE_CNT_SHIFT 28
>> + #define DMA_DONE_CNT_LEN 2
>> + #define RX_FIFO_LVL_SHIFT 20
>> + #define RX_FIFO_LVL_LEN 5
>> + #define TX_FIFO_LVL_SHIFT 12
>> + #define TX_FIFO_LVL_LEN 7
>> + #define TX_FIFO_LVL (0x7f << 12)
>> + #define PSS_GTS_USR_B (1 << 11)
>> + #define PSS_FST_CFG_B (1 << 10)
>> + #define PSS_CFG_RESET_B (1 << 5)
>> +
>> +#define R_DMA_SRC_ADDR (0x18/4)
>> +#define R_DMA_DST_ADDR (0x1C/4)
>> +#define R_DMA_SRC_LEN (0x20/4)
>> +#define R_DMA_DST_LEN (0x24/4)
>> +#define R_ROM_SHADOW (0x28/4)
>> +#define R_SW_ID (0x30/4)
>> +#define R_UNLOCK (0x34/4)
>> +
>> +#define R_UNLOCK_MAGIC 0x757BDF0D
>> +
>> +#define R_MCTRL (0x80/4)
>> + #define PS_VERSION_SHIFT 28
>> + #define PS_VERSION_MASK (0xf << PS_VERSION_SHIFT)
>> + #define PCFG_POR_B (1 << 8)
>> + #define INT_PCAP_LPBK (1 << 4)
>> +
>> +#define R_MAX (0x118/4+1)
>> +
>> +#define RX_FIFO_LEN 32
>> +#define TX_FIFO_LEN 128
>> +
>> +#define DMA_COMMAND_FIFO_LEN 10
>> +
>> +typedef struct XilinxDevcfgDMACommand {
>> + uint32_t src_addr;
>> + uint32_t dest_addr;
>> + uint32_t src_len;
>> + uint32_t dest_len;
>> +} XilinxDevcfgDMACommand;
>> +
>> +static const VMStateDescription vmstate_xilinx_devcfg_dma_command = {
>> + .name = "xilinx_devcfg_dma_command",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT32(src_addr, XilinxDevcfgDMACommand),
>> + VMSTATE_UINT32(dest_addr, XilinxDevcfgDMACommand),
>> + VMSTATE_UINT32(src_len, XilinxDevcfgDMACommand),
>> + VMSTATE_UINT32(dest_len, XilinxDevcfgDMACommand),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +typedef struct XilinxDevcfg {
>> + SysBusDevice parent_obj;
>> +
>> + MemoryRegion iomem;
>> + qemu_irq irq;
>> +
>> + QEMUBH *timer_bh;
>> + ptimer_state *timer;
>> +
>> + XilinxDevcfgDMACommand dma_command_fifo[DMA_COMMAND_FIFO_LEN];
>> + uint8_t dma_command_fifo_num;
>> +
>> + uint32_t regs[R_MAX];
>> + RegisterInfo regs_info[R_MAX];
>> +} XilinxDevcfg;
>> +
>> +static const VMStateDescription vmstate_xilinx_devcfg = {
>> + .name = "xilinx_devcfg",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_PTIMER(timer, XilinxDevcfg),
>> + VMSTATE_STRUCT_ARRAY(dma_command_fifo, XilinxDevcfg,
>> + DMA_COMMAND_FIFO_LEN, 0,
>> + vmstate_xilinx_devcfg_dma_command,
>> + XilinxDevcfgDMACommand),
>> + VMSTATE_UINT8(dma_command_fifo_num, XilinxDevcfg),
>> + VMSTATE_UINT32_ARRAY(regs, XilinxDevcfg, R_MAX),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static void xilinx_devcfg_update_ixr(XilinxDevcfg *s)
>> +{
>> + qemu_set_irq(s->irq, !!(~s->regs[R_INT_MASK] & s->regs[R_INT_STS]));
>> +}
>> +
>> +static void xilinx_devcfg_reset(DeviceState *dev)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>> + int i;
>> +
>> + for (i = 0; i < R_MAX; ++i) {
>> + register_reset(&s->regs_info[i]);
>> + }
>> +}
>> +
>> +#define min(a, b) ((a) < (b) ? (a) : (b))
>> +
>> +static void xilinx_devcfg_dma_go(void *opaque)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(opaque);
>> + uint8_t buf[BTT_MAX];
>> + XilinxDevcfgDMACommand *dmah = s->dma_command_fifo;
>> + uint32_t btt = BTT_MAX;
>> +
>> + btt = min(btt, dmah->src_len);
>> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
>> + btt = min(btt, dmah->dest_len);
>> + }
>> + DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr);
>> + dma_memory_read(&dma_context_memory, dmah->src_addr, buf, btt);
>> + dmah->src_len -= btt;
>> + dmah->src_addr += btt;
>> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
>> + DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr);
>> + dma_memory_write(&dma_context_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] |= DMA_DONE_INT | DMA_P_DONE_INT;
>> + s->dma_command_fifo_num = s->dma_command_fifo_num - 1;
>> + memcpy(s->dma_command_fifo, &s->dma_command_fifo[1],
>> + sizeof(*s->dma_command_fifo) * DMA_COMMAND_FIFO_LEN - 1);
>> + }
>> + xilinx_devcfg_update_ixr(s);
>> + if (s->dma_command_fifo_num) { /* there is still work to do */
>> + DB_PRINT("dma work remains, setting up callback ptimer\n");
>> + ptimer_set_freq(s->timer, FREQ_HZ);
>> + ptimer_set_count(s->timer, CYCLES_BTT_MAX);
>> + ptimer_run(s->timer, 1);
>> + }
>> +}
>> +
>> +static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>> +
>> + xilinx_devcfg_update_ixr(s);
>> +}
>> +
>> +static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val)
>> +{
>> + XilinxDevcfg *s = XILINX_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 = extract32(val, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
>> +
>> + if (aes_en != 0 && aes_en != 7) {
>> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
>> + "unimplemeneted security reset should happen!\n",
>> + reg->prefix);
>> + }
>> +}
>> +
>> +static void r_unlock_post_write(RegisterInfo *reg, uint64_t val)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>> +
>> + if (val == R_UNLOCK_MAGIC) {
>> + DB_PRINT("successful unlock\n");
>> + } else {/* bad unlock attempt */
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->prefix);
>> + qemu_log_mask(LOG_UNIMP, "%s: failed unlock, unimplmeneted crash of"
>> + "device should happen\n", reg->prefix);
>> + s->regs[R_CTRL] &= ~PCAP_PR;
>> + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
>> + }
>> +}
>> +
>> +static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val)
>> +{
>> + XilinxDevcfg *s = XILINX_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)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>> +
>> + /* TODO: implement command queue overflow check and interrupt */
>> + s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
>> + s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
>> + s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
>> + s->regs[R_DMA_DST_ADDR] & ~0x3UL;
>> + s->dma_command_fifo[s->dma_command_fifo_num].src_len =
>> + s->regs[R_DMA_SRC_LEN] << 2;
>> + s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
>> + s->regs[R_DMA_DST_LEN] << 2;
>> + s->dma_command_fifo_num++;
>> + DB_PRINT("dma transfer started; %d total transfers pending\n",
>> + s->dma_command_fifo_num);
>> + xilinx_devcfg_dma_go(s);
>> +}
>> +
>> +static const RegisterAccessInfo xilinx_devcfg_regs_info[] = {
>> + [R_CTRL] = { .name = "CTRL",
>> + .reset = PCAP_PR | PCAP_MODE | 0x3 << 13,
>> + .ro = 0x107f6000,
>> + .ge1 = (RegisterAccessError[]) {
>> + { .mask = 0x1 << 15, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + .ge0 = (RegisterAccessError[]) {
>> + { .mask = 0x3 << 13, .reason = "Reserved - always write with 1" },
>> + {},
>> + },
>> + .ui1 = (RegisterAccessError[]) {
>> + { .mask = FORCE_RST, .reason = "PS reset not implemented" },
>> + { .mask = PCAP_MODE, .reason = "FPGA Fabric doesnt exist" },
>> + { .mask = PCFG_AES_EN_MASK, .reason = "AES not implmented" },
>> + {},
>> + },
>> + .pre_write = r_ctrl_pre_write,
>> + .post_write = r_ctrl_post_write,
>
> I dislike that this mechanism decentralizes register access. What about
> a slightly different style so you could do something like:
>
> static void my_device_io_write(...)
> {
> switch (register_decode(&my_device_reginfo, s, &info)) {
> case R_CTRL:
> // handle pre-write bits here
> ...
> }
>
> switch (register_write(&my_device_reginfo, s)) {
> case R_CTRL:
> //handle post write bits
> ...
> }
> }
>
That's still possible using just the register API (Patch 2 content
only) and throwing away the memory API glue. I think its actually
quite similar to V1 of the patch series where I did not have the
callbacks, and use the switch-cases for register side-effects only.
This looks like the intention of your patch. Gerd made a push for the
callbacks in an earlier review and there was push to integrate to
Memory API . Is it reasonable to leave it up to the developer whether
they want to do full conversion (Patches 2 & 3) or half conversion
(Patch 2 only + your decode refactoring). V1 of this patch at:
http://patchwork.ozlabs.org/patch/224534/
heres the relevant snippet (comments from me inline):
+static void xilinx_devcfg_write(void *opaque, hwaddr addr, uint64_t value,
+ unsigned size)
+{
+ XilinxDevcfg *s = opaque;
+ int i;
+ uint32_t aes_en;
+ const char *prefix = "";
+
+ /* FIXME: use tracing to avoid these gymnastics */
+ if (XILINX_DEVCFG_ERR_DEBUG || qemu_get_log_flags() & LOG_GUEST_ERROR) {
+ prefix = g_strdup_printf("%s:Addr %#08x",
+ object_get_canonical_path(OBJECT(s)),
+ (unsigned)addr);
+ }
+ addr >>= 2;
This is the open coded decode logic but is trivial in this case.
+ assert(addr < R_MAX);
+
+ if (s->lock && addr != R_UNLOCK) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s:write to non-lock register while"
+ " locked\n", prefix);
+ return;
+ }
+
some pre write logic (I dropped it later as it was actually unimplemented).
+ uint32_write(&s->regs[addr], &xilinx_devcfg_regs_info[addr], value, prefix,
+ XILINX_DEVCFG_ERR_DEBUG);
+
this is the data-driven register_write() call under its old name.
+ /*side effects */
+ switch (addr) {
+ case R_CTRL:
+ for (i = 0; i < ARRAY_SIZE(lock_ctrl_map); ++i) {
+ if (s->regs[R_LOCK] & 1 << i) {
+ value &= ~lock_ctrl_map[i];
+ value |= lock_ctrl_map[i] & s->regs[R_CTRL];
+ }
+ }
+ aes_en = extract32(value, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
+ if (aes_en != 0 && aes_en != 7) {
+ qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
+ "unimplemeneted security reset should happen!\n",
+ prefix);
+ }
+ break;
+
+ case R_DMA_DST_LEN:
+ /* TODO: implement command queue overflow check and interrupt */
+ s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
+ s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
+ s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
+ s->regs[R_DMA_DST_ADDR] & ~0x3UL;
+ s->dma_command_fifo[s->dma_command_fifo_num].src_len =
+ s->regs[R_DMA_SRC_LEN] << 2;
+ s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
+ s->regs[R_DMA_DST_LEN] << 2;
+ s->dma_command_fifo_num++;
+ DB_PRINT("dma transfer started; %d total transfers pending\n",
+ s->dma_command_fifo_num);
+ xilinx_devcfg_dma_go(s);
+ break;
+
+ case R_UNLOCK:
+ if (value == R_UNLOCK_MAGIC) {
+ s->lock = 0;
+ DB_PRINT("successful unlock\n");
+ } else if (s->lock) {/* bad unlock attempt */
+ qemu_log_mask(LOG_GUEST_ERROR, "%s:failed unlock\n", prefix);
+ s->regs[R_CTRL] &= ~PCAP_PR;
+ s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
+ }
+ break;
+ }
And the post-write logic.
+ xilinx_devcfg_update_ixr(s);
+
+ if (*prefix) {
+ g_free((void *)prefix);
+ }
+}
+
> It makes it much easier to convert existing code. We can then leave
> most of the switch statements intact and just introduce register
> descriptions.
>
> I think splitting decode from data processing is useful too because it
> makes it easier to support latching.
>
> I think the current spin is probably over generalizing too. I don't
> think supporting ge0/ge1 makes a lot of sense from the start. Decisions
> aren't always that simple and it's weird to have sanity checking split
> across two places.
>
My goal is to pretty much copy paste out the TRM for the clear GE
write. Some of my register fields in the TRM for this device say
things like "Reserved, always write as 1" which im trying to capture
in a data driven way without having to pollute my switch-cases with
this junk. It would be nice to autogenerate this as well.
> BTW: I think it's also a good idea to model this as a QOM object so that
> device state can be access through the QOM tree.
>
Hmm ill have to think on this one.
Regards,
Peter
> Regards,
>
> Anthony Liguori
>
>> + },
>> + [R_LOCK] = { .name = "LOCK",
>> + .ro = ~ONES(5),
>> + .pre_write = r_lock_pre_write,
>> + },
>> + [R_CFG] = { .name = "CFG",
>> + .reset = 1 << RFIFO_TH_SHIFT | 1 << WFIFO_TH_SHIFT | 0x8,
>> + .ge1 = (RegisterAccessError[]) {
>> + { .mask = 0x7, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + .ge0 = (RegisterAccessError[]) {
>> + { .mask = 0x8, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + .ro = 0x00f | ~ONES(12),
>> + },
>> + [R_INT_STS] = { .name = "INT_STS",
>> + .w1c = ~R_INT_STS_RSVD,
>> + .reset = PSS_GTS_USR_B_INT | PSS_CFG_RESET_B_INT | WR_FIFO_LVL_INT,
>> + .ro = R_INT_STS_RSVD,
>> + .post_write = r_ixr_post_write,
>> + },
>> + [R_INT_MASK] = { .name = "INT_MASK",
>> + .reset = ~0,
>> + .ro = R_INT_STS_RSVD,
>> + .post_write = r_ixr_post_write,
>> + },
>> + [R_STATUS] = { .name = "STATUS",
>> + .reset = DMA_CMD_Q_E | PSS_GTS_USR_B | PSS_CFG_RESET_B,
>> + .ro = ~0,
>> + .ge1 = (RegisterAccessError[]) {
>> + {.mask = 0x1, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + },
>> + [R_DMA_SRC_ADDR] = { .name = "DMA_SRC_ADDR" },
>> + [R_DMA_DST_ADDR] = { .name = "DMA_DST_ADDR" },
>> + [R_DMA_SRC_LEN] = { .name = "DMA_SRC_LEN", .ro = ~ONES(27) },
>> + [R_DMA_DST_LEN] = { .name = "DMA_DST_LEN",
>> + .ro = ~ONES(27),
>> + .post_write = r_dma_dst_len_post_write,
>> + },
>> + [R_ROM_SHADOW] = { .name = "ROM_SHADOW",
>> + .ge1 = (RegisterAccessError[]) {
>> + {.mask = ~0, .reason = "Reserved - do not modify" },
>> + {},
>> + },
>> + },
>> + [R_SW_ID] = { .name = "SW_ID" },
>> + [R_UNLOCK] = { .name = "UNLOCK",
>> + .post_write = r_unlock_post_write,
>> + },
>> + [R_MCTRL] = { .name = "MCTRL",
>> + /* Silicon 3.0 for version field, and the mysterious reserved bit 23 */
>> + .reset = 0x2 << PS_VERSION_SHIFT | 1 << 23,
>> + /* some reserved bits are rw while others are ro */
>> + .ro = ~INT_PCAP_LPBK,
>> + .ge1 = (RegisterAccessError[]) {
>> + { .mask = 0x00F00300, .reason = "Reserved - do not modify" },
>> + { .mask = 0x00000003, .reason = "Reserved - always write with 0" },
>> + {}
>> + },
>> + .ge0 = (RegisterAccessError[]) {
>> + { .mask = 1 << 23, .reason = "Reserved - always write with 1" },
>> + {}
>> + },
>> + },
>> + [R_MAX] = {}
>> +};
>> +
>> +static const MemoryRegionOps 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 void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
>> +{
>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>> + const char *prefix = object_get_canonical_path(OBJECT(dev));
>> + int i;
>> +
>> + for (i = 0; i < R_MAX; ++i) {
>> + RegisterInfo *r = &s->regs_info[i];
>> +
>> + *r = (RegisterInfo) {
>> + .data = &s->regs[i],
>> + .data_size = sizeof(uint32_t),
>> + .access = &xilinx_devcfg_regs_info[i],
>> + .debug = XILINX_DEVCFG_ERR_DEBUG,
>> + .prefix = prefix,
>> + .opaque = s,
>> + };
>> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
>> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
>> + }
>> +}
>> +
>> +static void xilinx_devcfg_init(Object *obj)
>> +{
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> + XilinxDevcfg *s = XILINX_DEVCFG(obj);
>> +
>> + s->timer_bh = qemu_bh_new(xilinx_devcfg_dma_go, s);
>> + s->timer = ptimer_init(s->timer_bh);
>> +
>> + sysbus_init_irq(sbd, &s->irq);
>> +
>> + memory_region_init(&s->iomem, "devcfg", R_MAX*4);
>> + sysbus_init_mmio(sbd, &s->iomem);
>> +}
>> +
>> +static void xilinx_devcfg_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->reset = xilinx_devcfg_reset;
>> + dc->vmsd = &vmstate_xilinx_devcfg;
>> + dc->realize = xilinx_devcfg_realize;
>> +}
>> +
>> +static const TypeInfo xilinx_devcfg_info = {
>> + .name = TYPE_XILINX_DEVCFG,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(XilinxDevcfg),
>> + .instance_init = xilinx_devcfg_init,
>> + .class_init = xilinx_devcfg_class_init,
>> +};
>> +
>> +static void xilinx_devcfg_register_types(void)
>> +{
>> + type_register_static(&xilinx_devcfg_info);
>> +}
>> +
>> +type_init(xilinx_devcfg_register_types)
>> --
>> 1.8.3.rc1.44.gb387c77.dirty
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-29 17:08 ` Paolo Bonzini
@ 2013-05-30 7:15 ` Peter Crosthwaite
2013-05-30 13:08 ` Paolo Bonzini
0 siblings, 1 reply; 19+ messages in thread
From: Peter Crosthwaite @ 2013-05-30 7:15 UTC (permalink / raw)
To: Paolo Bonzini
Cc: peter.maydell, qemu-devel, blauwirbel, kraxel, Anthony Liguori,
Edgar E. Iglesias
On Thu, May 30, 2013 at 3:08 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 29/05/2013 19:04, Edgar E. Iglesias ha scritto:
>>> > + for (i = 0; i < R_MAX; ++i) {
>>> > + RegisterInfo *r = &s->regs_info[i];
>>> > +
>>> > + *r = (RegisterInfo) {
>>> > + .data = &s->regs[i],
>>> > + .data_size = sizeof(uint32_t),
>>> > + .access = &xilinx_devcfg_regs_info[i],
>>> > + .debug = XILINX_DEVCFG_ERR_DEBUG,
>>> > + .prefix = prefix,
>>> > + .opaque = s,
>>> > + };
>>> > + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
>> Hi Peter, Should we be putting r->access->name here instead of "devcfg-regs"?
>
> Yes, that's why I preferred to wrap the memory_region_init_io into an
> API that takes a RegisterInfo. :)
ACK,
You've convinced me :). Will be in v4 (pending outcome of discussion
with Anthony RE decoding)
Regards,
Peter
>
> Paolo
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-30 7:15 ` Peter Crosthwaite
@ 2013-05-30 13:08 ` Paolo Bonzini
0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2013-05-30 13:08 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: peter.maydell, qemu-devel, blauwirbel, kraxel, Anthony Liguori,
Edgar E. Iglesias
Il 30/05/2013 09:15, Peter Crosthwaite ha scritto:
>>> >> Hi Peter, Should we be putting r->access->name here instead of "devcfg-regs"?
>> >
>> > Yes, that's why I preferred to wrap the memory_region_init_io into an
>> > API that takes a RegisterInfo. :)
> ACK,
>
> You've convinced me :). Will be in v4 (pending outcome of discussion
> with Anthony RE decoding)
I would also have preferred (I was too terse in mentioning
.accepts.valid earlier so this is the less concise explanation) an API
that does a single memory_region_init_io region for a whole array of
registers. Basically having a RegisterArrayInfo in addition to the
RegisterInfo.
Paolo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-30 1:43 ` Peter Crosthwaite
@ 2013-05-30 19:41 ` Anthony Liguori
2013-05-31 23:34 ` Peter Crosthwaite
0 siblings, 1 reply; 19+ messages in thread
From: Anthony Liguori @ 2013-05-30 19:41 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: peter.maydell, qemu-devel, blauwirbel, kraxel, pbonzini, edgar.iglesias
[-- Attachment #1: Type: text/plain, Size: 23332 bytes --]
Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
> Hi Anthony,
>
> On Thu, May 30, 2013 at 3:57 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>> peter.crosthwaite@xilinx.com writes:
>>
>>> From: "Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com>
>>>
>>> Minimal device model for devcfg module of Zynq. DMA capabilities and
>>> interrupt generation supported.
>>>
>>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>>> ---
>>> Changed since v2:
>>> Some QOM styling updates.
>>> Re-implemented nw0 for lock register as pre_write
>>> Changed since v1:
>>> Rebased against new version of Register API.
>>> Use action callbacks for side effects rather than switch.
>>> Documented reasons for ge0, ge1 (Verbatim from TRM)
>>> Added ui1 definitions for unimplemented major features
>>> Removed dead lock code
>>>
>>> default-configs/arm-softmmu.mak | 1 +
>>> hw/dma/Makefile.objs | 1 +
>>> hw/dma/xilinx_devcfg.c | 495 ++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 497 insertions(+)
>>> create mode 100644 hw/dma/xilinx_devcfg.c
>>>
>>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>>> index 27cbe3d..5a17f64 100644
>>> --- a/default-configs/arm-softmmu.mak
>>> +++ b/default-configs/arm-softmmu.mak
>>> @@ -61,6 +61,7 @@ CONFIG_PXA2XX=y
>>> CONFIG_BITBANG_I2C=y
>>> CONFIG_FRAMEBUFFER=y
>>> CONFIG_XILINX_SPIPS=y
>>> +CONFIG_ZYNQ_DEVCFG=y
>>>
>>> CONFIG_A9SCU=y
>>> CONFIG_MARVELL_88W8618=y
>>> diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
>>> index 0e65ed0..96025cf 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) += xilinx_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/xilinx_devcfg.c b/hw/dma/xilinx_devcfg.c
>>> new file mode 100644
>>> index 0000000..b82b7cc
>>> --- /dev/null
>>> +++ b/hw/dma/xilinx_devcfg.c
>>> @@ -0,0 +1,495 @@
>>> +/*
>>> + * QEMU model of the Xilinx Devcfg Interface
>>> + *
>>> + * Copyright (c) 2011 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.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 "sysemu/sysemu.h"
>>> +#include "sysemu/dma.h"
>>> +#include "hw/sysbus.h"
>>> +#include "hw/ptimer.h"
>>> +#include "qemu/bitops.h"
>>> +#include "hw/register.h"
>>> +#include "qemu/bitops.h"
>>> +
>>> +#define TYPE_XILINX_DEVCFG "xlnx.ps7-dev-cfg"
>>> +
>>> +#define XILINX_DEVCFG(obj) \
>>> + OBJECT_CHECK(XilinxDevcfg, (obj), TYPE_XILINX_DEVCFG)
>>> +
>>> +/* FIXME: get rid of hardcoded nastiness */
>>> +
>>> +#define FREQ_HZ 900000000
>>> +
>>> +#define BTT_MAX 0x400 /* bytes to transfer per delay inteval */
>>> +#define CYCLES_BTT_MAX 10000 /*approximate 10k cycles per delay interval */
>>> +
>>> +#ifndef XILINX_DEVCFG_ERR_DEBUG
>>> +#define XILINX_DEVCFG_ERR_DEBUG 0
>>> +#endif
>>> +#define DB_PRINT(...) do { \
>>> + if (XILINX_DEVCFG_ERR_DEBUG) { \
>>> + fprintf(stderr, ": %s: ", __func__); \
>>> + fprintf(stderr, ## __VA_ARGS__); \
>>> + } \
>>> +} while (0);
>>> +
>>> +#define R_CTRL (0x00/4)
>>> + #define FORCE_RST (1 << 31) /* Not supported, writes ignored */
>>> + #define PCAP_PR (1 << 27) /* Forced to 0 on bad unlock */
>>> + #define PCAP_MODE (1 << 26)
>>> + #define MULTIBOOT_EN (1 << 24)
>>> + #define USER_MODE (1 << 15)
>>> + #define PCFG_AES_FUSE (1 << 12) /* locked by AES_FUSE_LOCK */
>>> + #define PCFG_AES_EN_SHIFT 9 /* locked by AES_EN_LOCK */
>>> + #define PCFG_AES_EN_LEN 3 /* locked by AES_EN_LOCK */
>>> + #define PCFG_AES_EN_MASK (((1 << PCFG_AES_EN_LEN) - 1) \
>>> + << PCFG_AES_EN_SHIFT)
>>> + #define SEU_EN (1 << 8) /* locked by SEU_LOCK */
>>> + #define SEC_EN (1 << 7) /* locked by SEC_LOCK */
>>> + #define SPNIDEN (1 << 6) /* locked by DBG_LOCK */
>>> + #define SPIDEN (1 << 5) /* locked by DBG_LOCK */
>>> + #define NIDEN (1 << 4) /* locked by DBG_LOCK */
>>> + #define DBGEN (1 << 3) /* locked by DBG_LOCK */
>>> + #define DAP_EN (7 << 0) /* locked by DBG_LOCK */
>>> +
>>> +#define R_LOCK (0x04/4)
>>> + #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] = PCFG_AES_FUSE,
>>> + [AES_EN_LOCK] = PCFG_AES_EN_MASK,
>>> + [SEU_LOCK] = SEU_LOCK,
>>> + [SEC_LOCK] = SEC_LOCK,
>>> + [DBG_LOCK] = SPNIDEN | SPIDEN | NIDEN | DBGEN | DAP_EN,
>>> +};
>>> +
>>> +#define R_CFG (0x08/4)
>>> + #define RFIFO_TH_SHIFT 10
>>> + #define RFIFO_TH_LEN 2
>>> + #define WFIFO_TH_SHIFT 8
>>> + #define WFIFO_TH_LEN 2
>>> + #define DISABLE_SRC_INC (1 << 5)
>>> + #define DISABLE_DST_INC (1 << 4)
>>> +#define R_CFG_RO 0xFFFFF800
>>> +#define R_CFG_RESET 0x50B
>>> +
>>> +#define R_INT_STS (0x0C/4)
>>> + #define PSS_GTS_USR_B_INT (1 << 31)
>>> + #define PSS_FST_CFG_B_INT (1 << 30)
>>> + #define PSS_CFG_RESET_B_INT (1 << 27)
>>> + #define RX_FIFO_OV_INT (1 << 18)
>>> + #define WR_FIFO_LVL_INT (1 << 17)
>>> + #define RD_FIFO_LVL_INT (1 << 16)
>>> + #define DMA_CMD_ERR_INT (1 << 15)
>>> + #define DMA_Q_OV_INT (1 << 14)
>>> + #define DMA_DONE_INT (1 << 13)
>>> + #define DMA_P_DONE_INT (1 << 12)
>>> + #define P2D_LEN_ERR_INT (1 << 11)
>>> + #define PCFG_DONE_INT (1 << 2)
>>> + #define R_INT_STS_RSVD ((0x7 << 24) | (0x1 << 19) | (0xF < 7))
>>> +
>>> +#define R_INT_MASK (0x10/4)
>>> +
>>> +#define R_STATUS (0x14/4)
>>> + #define DMA_CMD_Q_F (1 << 31)
>>> + #define DMA_CMD_Q_E (1 << 30)
>>> + #define DMA_DONE_CNT_SHIFT 28
>>> + #define DMA_DONE_CNT_LEN 2
>>> + #define RX_FIFO_LVL_SHIFT 20
>>> + #define RX_FIFO_LVL_LEN 5
>>> + #define TX_FIFO_LVL_SHIFT 12
>>> + #define TX_FIFO_LVL_LEN 7
>>> + #define TX_FIFO_LVL (0x7f << 12)
>>> + #define PSS_GTS_USR_B (1 << 11)
>>> + #define PSS_FST_CFG_B (1 << 10)
>>> + #define PSS_CFG_RESET_B (1 << 5)
>>> +
>>> +#define R_DMA_SRC_ADDR (0x18/4)
>>> +#define R_DMA_DST_ADDR (0x1C/4)
>>> +#define R_DMA_SRC_LEN (0x20/4)
>>> +#define R_DMA_DST_LEN (0x24/4)
>>> +#define R_ROM_SHADOW (0x28/4)
>>> +#define R_SW_ID (0x30/4)
>>> +#define R_UNLOCK (0x34/4)
>>> +
>>> +#define R_UNLOCK_MAGIC 0x757BDF0D
>>> +
>>> +#define R_MCTRL (0x80/4)
>>> + #define PS_VERSION_SHIFT 28
>>> + #define PS_VERSION_MASK (0xf << PS_VERSION_SHIFT)
>>> + #define PCFG_POR_B (1 << 8)
>>> + #define INT_PCAP_LPBK (1 << 4)
>>> +
>>> +#define R_MAX (0x118/4+1)
>>> +
>>> +#define RX_FIFO_LEN 32
>>> +#define TX_FIFO_LEN 128
>>> +
>>> +#define DMA_COMMAND_FIFO_LEN 10
>>> +
>>> +typedef struct XilinxDevcfgDMACommand {
>>> + uint32_t src_addr;
>>> + uint32_t dest_addr;
>>> + uint32_t src_len;
>>> + uint32_t dest_len;
>>> +} XilinxDevcfgDMACommand;
>>> +
>>> +static const VMStateDescription vmstate_xilinx_devcfg_dma_command = {
>>> + .name = "xilinx_devcfg_dma_command",
>>> + .version_id = 1,
>>> + .minimum_version_id = 1,
>>> + .minimum_version_id_old = 1,
>>> + .fields = (VMStateField[]) {
>>> + VMSTATE_UINT32(src_addr, XilinxDevcfgDMACommand),
>>> + VMSTATE_UINT32(dest_addr, XilinxDevcfgDMACommand),
>>> + VMSTATE_UINT32(src_len, XilinxDevcfgDMACommand),
>>> + VMSTATE_UINT32(dest_len, XilinxDevcfgDMACommand),
>>> + VMSTATE_END_OF_LIST()
>>> + }
>>> +};
>>> +
>>> +typedef struct XilinxDevcfg {
>>> + SysBusDevice parent_obj;
>>> +
>>> + MemoryRegion iomem;
>>> + qemu_irq irq;
>>> +
>>> + QEMUBH *timer_bh;
>>> + ptimer_state *timer;
>>> +
>>> + XilinxDevcfgDMACommand dma_command_fifo[DMA_COMMAND_FIFO_LEN];
>>> + uint8_t dma_command_fifo_num;
>>> +
>>> + uint32_t regs[R_MAX];
>>> + RegisterInfo regs_info[R_MAX];
>>> +} XilinxDevcfg;
>>> +
>>> +static const VMStateDescription vmstate_xilinx_devcfg = {
>>> + .name = "xilinx_devcfg",
>>> + .version_id = 1,
>>> + .minimum_version_id = 1,
>>> + .minimum_version_id_old = 1,
>>> + .fields = (VMStateField[]) {
>>> + VMSTATE_PTIMER(timer, XilinxDevcfg),
>>> + VMSTATE_STRUCT_ARRAY(dma_command_fifo, XilinxDevcfg,
>>> + DMA_COMMAND_FIFO_LEN, 0,
>>> + vmstate_xilinx_devcfg_dma_command,
>>> + XilinxDevcfgDMACommand),
>>> + VMSTATE_UINT8(dma_command_fifo_num, XilinxDevcfg),
>>> + VMSTATE_UINT32_ARRAY(regs, XilinxDevcfg, R_MAX),
>>> + VMSTATE_END_OF_LIST()
>>> + }
>>> +};
>>> +
>>> +static void xilinx_devcfg_update_ixr(XilinxDevcfg *s)
>>> +{
>>> + qemu_set_irq(s->irq, !!(~s->regs[R_INT_MASK] & s->regs[R_INT_STS]));
>>> +}
>>> +
>>> +static void xilinx_devcfg_reset(DeviceState *dev)
>>> +{
>>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>>> + int i;
>>> +
>>> + for (i = 0; i < R_MAX; ++i) {
>>> + register_reset(&s->regs_info[i]);
>>> + }
>>> +}
>>> +
>>> +#define min(a, b) ((a) < (b) ? (a) : (b))
>>> +
>>> +static void xilinx_devcfg_dma_go(void *opaque)
>>> +{
>>> + XilinxDevcfg *s = XILINX_DEVCFG(opaque);
>>> + uint8_t buf[BTT_MAX];
>>> + XilinxDevcfgDMACommand *dmah = s->dma_command_fifo;
>>> + uint32_t btt = BTT_MAX;
>>> +
>>> + btt = min(btt, dmah->src_len);
>>> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
>>> + btt = min(btt, dmah->dest_len);
>>> + }
>>> + DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr);
>>> + dma_memory_read(&dma_context_memory, dmah->src_addr, buf, btt);
>>> + dmah->src_len -= btt;
>>> + dmah->src_addr += btt;
>>> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) {
>>> + DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr);
>>> + dma_memory_write(&dma_context_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] |= DMA_DONE_INT | DMA_P_DONE_INT;
>>> + s->dma_command_fifo_num = s->dma_command_fifo_num - 1;
>>> + memcpy(s->dma_command_fifo, &s->dma_command_fifo[1],
>>> + sizeof(*s->dma_command_fifo) * DMA_COMMAND_FIFO_LEN - 1);
>>> + }
>>> + xilinx_devcfg_update_ixr(s);
>>> + if (s->dma_command_fifo_num) { /* there is still work to do */
>>> + DB_PRINT("dma work remains, setting up callback ptimer\n");
>>> + ptimer_set_freq(s->timer, FREQ_HZ);
>>> + ptimer_set_count(s->timer, CYCLES_BTT_MAX);
>>> + ptimer_run(s->timer, 1);
>>> + }
>>> +}
>>> +
>>> +static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
>>> +{
>>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>>> +
>>> + xilinx_devcfg_update_ixr(s);
>>> +}
>>> +
>>> +static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val)
>>> +{
>>> + XilinxDevcfg *s = XILINX_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 = extract32(val, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
>>> +
>>> + if (aes_en != 0 && aes_en != 7) {
>>> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
>>> + "unimplemeneted security reset should happen!\n",
>>> + reg->prefix);
>>> + }
>>> +}
>>> +
>>> +static void r_unlock_post_write(RegisterInfo *reg, uint64_t val)
>>> +{
>>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>>> +
>>> + if (val == R_UNLOCK_MAGIC) {
>>> + DB_PRINT("successful unlock\n");
>>> + } else {/* bad unlock attempt */
>>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->prefix);
>>> + qemu_log_mask(LOG_UNIMP, "%s: failed unlock, unimplmeneted crash of"
>>> + "device should happen\n", reg->prefix);
>>> + s->regs[R_CTRL] &= ~PCAP_PR;
>>> + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
>>> + }
>>> +}
>>> +
>>> +static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val)
>>> +{
>>> + XilinxDevcfg *s = XILINX_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)
>>> +{
>>> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque);
>>> +
>>> + /* TODO: implement command queue overflow check and interrupt */
>>> + s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
>>> + s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
>>> + s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
>>> + s->regs[R_DMA_DST_ADDR] & ~0x3UL;
>>> + s->dma_command_fifo[s->dma_command_fifo_num].src_len =
>>> + s->regs[R_DMA_SRC_LEN] << 2;
>>> + s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
>>> + s->regs[R_DMA_DST_LEN] << 2;
>>> + s->dma_command_fifo_num++;
>>> + DB_PRINT("dma transfer started; %d total transfers pending\n",
>>> + s->dma_command_fifo_num);
>>> + xilinx_devcfg_dma_go(s);
>>> +}
>>> +
>>> +static const RegisterAccessInfo xilinx_devcfg_regs_info[] = {
>>> + [R_CTRL] = { .name = "CTRL",
>>> + .reset = PCAP_PR | PCAP_MODE | 0x3 << 13,
>>> + .ro = 0x107f6000,
>>> + .ge1 = (RegisterAccessError[]) {
>>> + { .mask = 0x1 << 15, .reason = "Reserved - do not modify" },
>>> + {},
>>> + },
>>> + .ge0 = (RegisterAccessError[]) {
>>> + { .mask = 0x3 << 13, .reason = "Reserved - always write with 1" },
>>> + {},
>>> + },
>>> + .ui1 = (RegisterAccessError[]) {
>>> + { .mask = FORCE_RST, .reason = "PS reset not implemented" },
>>> + { .mask = PCAP_MODE, .reason = "FPGA Fabric doesnt exist" },
>>> + { .mask = PCFG_AES_EN_MASK, .reason = "AES not implmented" },
>>> + {},
>>> + },
>>> + .pre_write = r_ctrl_pre_write,
>>> + .post_write = r_ctrl_post_write,
>>
>> I dislike that this mechanism decentralizes register access. What about
>> a slightly different style so you could do something like:
>>
>> static void my_device_io_write(...)
>> {
>> switch (register_decode(&my_device_reginfo, s, &info)) {
>> case R_CTRL:
>> // handle pre-write bits here
>> ...
>> }
>>
>> switch (register_write(&my_device_reginfo, s)) {
>> case R_CTRL:
>> //handle post write bits
>> ...
>> }
>> }
>>
>
> That's still possible using just the register API (Patch 2 content
> only) and throwing away the memory API glue. I think its actually
> quite similar to V1 of the patch series where I did not have the
> callbacks, and use the switch-cases for register side-effects only.
> This looks like the intention of your patch. Gerd made a push for the
> callbacks in an earlier review and there was push to integrate to
> Memory API . Is it reasonable to leave it up to the developer whether
> they want to do full conversion (Patches 2 & 3) or half conversion
> (Patch 2 only + your decode refactoring). V1 of this patch at:
>
> http://patchwork.ozlabs.org/patch/224534/
>
> heres the relevant snippet (comments from me inline):
>
> +static void xilinx_devcfg_write(void *opaque, hwaddr addr, uint64_t value,
> + unsigned size)
> +{
> + XilinxDevcfg *s = opaque;
> + int i;
> + uint32_t aes_en;
> + const char *prefix = "";
> +
> + /* FIXME: use tracing to avoid these gymnastics */
> + if (XILINX_DEVCFG_ERR_DEBUG || qemu_get_log_flags() & LOG_GUEST_ERROR) {
> + prefix = g_strdup_printf("%s:Addr %#08x",
> + object_get_canonical_path(OBJECT(s)),
> + (unsigned)addr);
> + }
> + addr >>= 2;
>
> This is the open coded decode logic but is trivial in this case.
But this is invisible to the common layer.
I think being able to have a common layer insight into "this value is
being written to this device register" would be extremely useful.
But my fear is that the current proposal will not work for a large set
of devices.
Let me provide another example (attached below) but highlighting the
description:
> static RegisterDecodeEntry decoder[] = {
> /* addresses 0-1 must be open decoded due to latching */
> { .addr = 2, .regno = UART_FCR, .flags = REG_WRITE },
> { .addr = 2, .regno = UART_IIR, .flags = REG_READ },
> { .addr = 3, .regno = UART_LCR, .flags = REG_RW },
> { .addr = 4, .regno = UART_MCR, .flags = REG_RW },
> { .addr = 5, .regno = UART_LSR, .flags = REG_READ },
> { .addr = 6, .regno = UART_MSR, .flags = REG_READ },
> { .addr = 7, .regno = UART_SCR, .flags = REG_RW },
> };
>
> static RegisterMapEntry regmap[] = {
> DEFINE_REG_U8(SerialState, ier, UART_IER),
> DEFINE_REG_U8(SerialState, iir, UART_IIR),
> DEFINE_REG_U8(SerialState, lcr, UART_LCR),
> DEFINE_REG_U8(SerialState, mcr, UART_MCR),
> DEFINE_REG_U8(SerialState, lsr, UART_LSR),
> DEFINE_REG_U8(SerialState, scr, UART_SCR),
> DEFINE_REG_U8(SerialState, thr, UART_THR),
> };
This is a clear separation between decode logic and load/store logic.
They are very different things from a hardware point of view.
> + assert(addr < R_MAX);
> +
> + if (s->lock && addr != R_UNLOCK) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s:write to non-lock register while"
> + " locked\n", prefix);
> + return;
> + }
> +
>
> some pre write logic (I dropped it later as it was actually unimplemented).
>
> + uint32_write(&s->regs[addr], &xilinx_devcfg_regs_info[addr], value, prefix,
> + XILINX_DEVCFG_ERR_DEBUG);
> +
>
> this is the data-driven register_write() call under its old name.
>
> + /*side effects */
> + switch (addr) {
> + case R_CTRL:
> + for (i = 0; i < ARRAY_SIZE(lock_ctrl_map); ++i) {
> + if (s->regs[R_LOCK] & 1 << i) {
> + value &= ~lock_ctrl_map[i];
> + value |= lock_ctrl_map[i] & s->regs[R_CTRL];
> + }
> + }
> + aes_en = extract32(value, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
> + if (aes_en != 0 && aes_en != 7) {
> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
> + "unimplemeneted security reset should happen!\n",
> + prefix);
> + }
> + break;
> +
> + case R_DMA_DST_LEN:
> + /* TODO: implement command queue overflow check and interrupt */
> + s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
> + s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
> + s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
> + s->regs[R_DMA_DST_ADDR] & ~0x3UL;
> + s->dma_command_fifo[s->dma_command_fifo_num].src_len =
> + s->regs[R_DMA_SRC_LEN] << 2;
> + s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
> + s->regs[R_DMA_DST_LEN] << 2;
> + s->dma_command_fifo_num++;
> + DB_PRINT("dma transfer started; %d total transfers pending\n",
> + s->dma_command_fifo_num);
> + xilinx_devcfg_dma_go(s);
> + break;
> +
> + case R_UNLOCK:
> + if (value == R_UNLOCK_MAGIC) {
> + s->lock = 0;
> + DB_PRINT("successful unlock\n");
> + } else if (s->lock) {/* bad unlock attempt */
> + qemu_log_mask(LOG_GUEST_ERROR, "%s:failed unlock\n", prefix);
> + s->regs[R_CTRL] &= ~PCAP_PR;
> + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
> + }
> + break;
> + }
>
> And the post-write logic.
>
> + xilinx_devcfg_update_ixr(s);
> +
> + if (*prefix) {
> + g_free((void *)prefix);
> + }
> +}
> +
>
>> It makes it much easier to convert existing code. We can then leave
>> most of the switch statements intact and just introduce register
>> descriptions.
>>
>> I think splitting decode from data processing is useful too because it
>> makes it easier to support latching.
>>
>> I think the current spin is probably over generalizing too. I don't
>> think supporting ge0/ge1 makes a lot of sense from the start. Decisions
>> aren't always that simple and it's weird to have sanity checking split
>> across two places.
>>
>
> My goal is to pretty much copy paste out the TRM for the clear GE
> write. Some of my register fields in the TRM for this device say
> things like "Reserved, always write as 1" which im trying to capture
> in a data driven way without having to pollute my switch-cases with
> this junk. It would be nice to autogenerate this as well.
I think you're trying to solve too many problems at once.
I don't think putting error messages in a register layout description is
a good idea.
>> BTW: I think it's also a good idea to model this as a QOM object so that
>> device state can be access through the QOM tree.
FWIW, I now think this is a bad idea :-)
Here's the full example:
[-- Attachment #2: foo --]
[-- Type: application/octet-stream, Size: 9871 bytes --]
commit cce15a1ed6aac51f8b7e05eba5c93466c3d96df7
Author: Anthony Liguori <aliguori@us.ibm.com>
Date: Wed May 29 13:44:35 2013 -0500
Demonstrate separating register decode from get/set
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 66b6348..34c8a2e 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -299,6 +299,89 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
return FALSE;
}
+enum {
+ UART_INVALID = 0,
+ UART_DIVIDER,
+ UART_RBR,
+ UART_THR,
+ UART_IER,
+ UART_IIR,
+ UART_FCR,
+ UART_LCR,
+ UART_MCR,
+ UART_LSR,
+ UART_MSR,
+ UART_SCR,
+};
+
+#define REG_READ 1
+#define REG_WRITE 2
+#define REG_RW (REG_READ | REG_WRITE)
+
+typedef struct RegisterDecodeEntry
+{
+ uint64_t addr;
+ unsigned access_size;
+ int regno;
+ int flags;
+} RegisterDecodeEntry;
+
+static RegisterDecodeEntry decoder[] = {
+ /* addresses 0-1 must be open decoded due to latching */
+ { .addr = 2, .regno = UART_FCR, .flags = REG_WRITE },
+ { .addr = 2, .regno = UART_IIR, .flags = REG_READ },
+ { .addr = 3, .regno = UART_LCR, .flags = REG_RW },
+ { .addr = 4, .regno = UART_MCR, .flags = REG_RW },
+ { .addr = 5, .regno = UART_LSR, .flags = REG_READ },
+ { .addr = 6, .regno = UART_MSR, .flags = REG_READ },
+ { .addr = 7, .regno = UART_SCR, .flags = REG_RW },
+};
+
+typedef struct RegisterMapEntry
+{
+ size_t offset;
+ int regno;
+ RegisterType type;
+ uint64_t mask;
+ int rshift;
+ int flags;
+} RegisterMapEntry;
+
+static int serial_decode(void *opaque, hwaddr addr, unsigned size,
+ bool is_write)
+{
+ SerialState *s = opaque;
+
+ switch (addr) {
+ case 0:
+ if (s->lcr & UART_LCR_DLAB) {
+ return UART_DIVIDER;
+ }
+ if (is_write) {
+ return UART_THR;
+ }
+ return UART_RBR;
+ case 1:
+ if (s->lcr & UART_LCR_DLAB) {
+ return UART_DIVIDER;
+ }
+ return UART_IER;
+ default:
+ return register_decode(decoder,
+ ARRAY_SIZE(decoder),
+ addr, size, is_write);
+ }
+}
+
+static RegisterMapEntry regmap[] = {
+ DEFINE_REG_U8(SerialState, ier, UART_IER),
+ DEFINE_REG_U8(SerialState, iir, UART_IIR),
+ DEFINE_REG_U8(SerialState, lcr, UART_LCR),
+ DEFINE_REG_U8(SerialState, mcr, UART_MCR),
+ DEFINE_REG_U8(SerialState, lsr, UART_LSR),
+ DEFINE_REG_U8(SerialState, scr, UART_SCR),
+ DEFINE_REG_U8(SerialState, thr, UART_THR),
+};
static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
@@ -307,52 +390,48 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
addr &= 7;
DPRINTF("write addr=0x%" HWADDR_PRIx " val=0x%" PRIx64 "\n", addr, val);
- switch(addr) {
+ switch(register_store(regmap, s, serial_decode(s, addr, size, true), size, val)) {
default:
- case 0:
- if (s->lcr & UART_LCR_DLAB) {
+ case UART_DIVIDER:
+ if (addr == 0) {
s->divider = (s->divider & 0xff00) | val;
- serial_update_parameters(s);
} else {
- s->thr = (uint8_t) val;
- if(s->fcr & UART_FCR_FE) {
- fifo_put(s, XMIT_FIFO, s->thr);
- s->thr_ipending = 0;
- s->lsr &= ~UART_LSR_TEMT;
- s->lsr &= ~UART_LSR_THRE;
- serial_update_irq(s);
- } else {
- s->thr_ipending = 0;
- s->lsr &= ~UART_LSR_THRE;
- serial_update_irq(s);
- }
- serial_xmit(NULL, G_IO_OUT, s);
+ s->divider = (s->divider & 0x00ff) | (val << 8);
}
+ serial_update_parameters(s);
break;
- case 1:
- if (s->lcr & UART_LCR_DLAB) {
- s->divider = (s->divider & 0x00ff) | (val << 8);
- serial_update_parameters(s);
+ case UART_THR:
+ if(s->fcr & UART_FCR_FE) {
+ fifo_put(s, XMIT_FIFO, s->thr);
+ s->thr_ipending = 0;
+ s->lsr &= ~UART_LSR_TEMT;
+ s->lsr &= ~UART_LSR_THRE;
+ serial_update_irq(s);
} else {
- s->ier = val & 0x0f;
- /* If the backend device is a real serial port, turn polling of the modem
- status lines on physical port on or off depending on UART_IER_MSI state */
- if (s->poll_msl >= 0) {
- if (s->ier & UART_IER_MSI) {
- s->poll_msl = 1;
- serial_update_msl(s);
- } else {
- qemu_del_timer(s->modem_status_poll);
- s->poll_msl = 0;
- }
- }
- if (s->lsr & UART_LSR_THRE) {
- s->thr_ipending = 1;
- serial_update_irq(s);
+ s->thr_ipending = 0;
+ s->lsr &= ~UART_LSR_THRE;
+ serial_update_irq(s);
+ }
+ serial_xmit(NULL, G_IO_OUT, s);
+ break;
+ case UART_IER:
+ /* If the backend device is a real serial port, turn polling of the modem
+ status lines on physical port on or off depending on UART_IER_MSI state */
+ if (s->poll_msl >= 0) {
+ if (s->ier & UART_IER_MSI) {
+ s->poll_msl = 1;
+ serial_update_msl(s);
+ } else {
+ qemu_del_timer(s->modem_status_poll);
+ s->poll_msl = 0;
}
}
+ if (s->lsr & UART_LSR_THRE) {
+ s->thr_ipending = 1;
+ serial_update_irq(s);
+ }
break;
- case 2:
+ case UART_FCR:
val = val & 0xFF;
if (s->fcr == val)
@@ -398,10 +477,9 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
s->fcr = val & 0xC9;
serial_update_irq(s);
break;
- case 3:
+ case UART_LCR:
{
int break_enable;
- s->lcr = val;
serial_update_parameters(s);
break_enable = (val >> 6) & 1;
if (break_enable != s->last_break_enable) {
@@ -411,7 +489,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
}
}
break;
- case 4:
+ case UART_MCR:
{
int flags;
int old_mcr = s->mcr;
@@ -437,75 +515,57 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
}
}
break;
- case 5:
- break;
- case 6:
- break;
- case 7:
- s->scr = val;
- break;
}
}
static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
{
SerialState *s = opaque;
- uint32_t ret;
+ uint64_t ret = 0xFF;
addr &= 7;
- switch(addr) {
+ switch(register_load(regmap, s, serial_decode(s, addr, size, false), size, &ret)) {
default:
- case 0:
- if (s->lcr & UART_LCR_DLAB) {
+ break;
+ case UART_DIVIDER:
+ if (addr == 0) {
ret = s->divider & 0xff;
} else {
- if(s->fcr & UART_FCR_FE) {
- ret = fifo_get(s,RECV_FIFO);
- if (s->recv_fifo.count == 0)
- s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
- else
- qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4);
- s->timeout_ipending = 0;
- } else {
- ret = s->rbr;
- s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
- }
- serial_update_irq(s);
- if (!(s->mcr & UART_MCR_LOOP)) {
- /* in loopback mode, don't receive any data */
- qemu_chr_accept_input(s->chr);
- }
+ ret = (s->divider >> 8) & 0xff;
}
break;
- case 1:
- if (s->lcr & UART_LCR_DLAB) {
- ret = (s->divider >> 8) & 0xff;
+ case UART_RBR:
+ if(s->fcr & UART_FCR_FE) {
+ ret = fifo_get(s,RECV_FIFO);
+ if (s->recv_fifo.count == 0)
+ s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
+ else
+ qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4);
+ s->timeout_ipending = 0;
} else {
- ret = s->ier;
+ ret = s->rbr;
+ s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
+ }
+ serial_update_irq(s);
+ if (!(s->mcr & UART_MCR_LOOP)) {
+ /* in loopback mode, don't receive any data */
+ qemu_chr_accept_input(s->chr);
}
break;
- case 2:
- ret = s->iir;
- if ((ret & UART_IIR_ID) == UART_IIR_THRI) {
+ case UART_IIR:
+ if ((s->iir & UART_IIR_ID) == UART_IIR_THRI) {
s->thr_ipending = 0;
serial_update_irq(s);
}
break;
- case 3:
- ret = s->lcr;
- break;
- case 4:
- ret = s->mcr;
- break;
- case 5:
- ret = s->lsr;
+ case UART_LSR:
/* Clear break and overrun interrupts */
if (s->lsr & (UART_LSR_BI|UART_LSR_OE)) {
s->lsr &= ~(UART_LSR_BI|UART_LSR_OE);
serial_update_irq(s);
}
break;
- case 6:
+ case UART_MSR:
if (s->mcr & UART_MCR_LOOP) {
/* in loopback, the modem output pins are connected to the
inputs */
@@ -523,9 +583,6 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
}
}
break;
- case 7:
- ret = s->scr;
- break;
}
DPRINTF("read addr=0x%" HWADDR_PRIx " val=0x%02x\n", addr, ret);
return ret;
[-- Attachment #3: Type: text/plain, Size: 5208 bytes --]
Regards,
Anthony Liguori
>>
>
> Hmm ill have to think on this one.
>
> Regards,
> Peter
>
>> Regards,
>>
>> Anthony Liguori
>>
>>> + },
>>> + [R_LOCK] = { .name = "LOCK",
>>> + .ro = ~ONES(5),
>>> + .pre_write = r_lock_pre_write,
>>> + },
>>> + [R_CFG] = { .name = "CFG",
>>> + .reset = 1 << RFIFO_TH_SHIFT | 1 << WFIFO_TH_SHIFT | 0x8,
>>> + .ge1 = (RegisterAccessError[]) {
>>> + { .mask = 0x7, .reason = "Reserved - do not modify" },
>>> + {},
>>> + },
>>> + .ge0 = (RegisterAccessError[]) {
>>> + { .mask = 0x8, .reason = "Reserved - do not modify" },
>>> + {},
>>> + },
>>> + .ro = 0x00f | ~ONES(12),
>>> + },
>>> + [R_INT_STS] = { .name = "INT_STS",
>>> + .w1c = ~R_INT_STS_RSVD,
>>> + .reset = PSS_GTS_USR_B_INT | PSS_CFG_RESET_B_INT | WR_FIFO_LVL_INT,
>>> + .ro = R_INT_STS_RSVD,
>>> + .post_write = r_ixr_post_write,
>>> + },
>>> + [R_INT_MASK] = { .name = "INT_MASK",
>>> + .reset = ~0,
>>> + .ro = R_INT_STS_RSVD,
>>> + .post_write = r_ixr_post_write,
>>> + },
>>> + [R_STATUS] = { .name = "STATUS",
>>> + .reset = DMA_CMD_Q_E | PSS_GTS_USR_B | PSS_CFG_RESET_B,
>>> + .ro = ~0,
>>> + .ge1 = (RegisterAccessError[]) {
>>> + {.mask = 0x1, .reason = "Reserved - do not modify" },
>>> + {},
>>> + },
>>> + },
>>> + [R_DMA_SRC_ADDR] = { .name = "DMA_SRC_ADDR" },
>>> + [R_DMA_DST_ADDR] = { .name = "DMA_DST_ADDR" },
>>> + [R_DMA_SRC_LEN] = { .name = "DMA_SRC_LEN", .ro = ~ONES(27) },
>>> + [R_DMA_DST_LEN] = { .name = "DMA_DST_LEN",
>>> + .ro = ~ONES(27),
>>> + .post_write = r_dma_dst_len_post_write,
>>> + },
>>> + [R_ROM_SHADOW] = { .name = "ROM_SHADOW",
>>> + .ge1 = (RegisterAccessError[]) {
>>> + {.mask = ~0, .reason = "Reserved - do not modify" },
>>> + {},
>>> + },
>>> + },
>>> + [R_SW_ID] = { .name = "SW_ID" },
>>> + [R_UNLOCK] = { .name = "UNLOCK",
>>> + .post_write = r_unlock_post_write,
>>> + },
>>> + [R_MCTRL] = { .name = "MCTRL",
>>> + /* Silicon 3.0 for version field, and the mysterious reserved bit 23 */
>>> + .reset = 0x2 << PS_VERSION_SHIFT | 1 << 23,
>>> + /* some reserved bits are rw while others are ro */
>>> + .ro = ~INT_PCAP_LPBK,
>>> + .ge1 = (RegisterAccessError[]) {
>>> + { .mask = 0x00F00300, .reason = "Reserved - do not modify" },
>>> + { .mask = 0x00000003, .reason = "Reserved - always write with 0" },
>>> + {}
>>> + },
>>> + .ge0 = (RegisterAccessError[]) {
>>> + { .mask = 1 << 23, .reason = "Reserved - always write with 1" },
>>> + {}
>>> + },
>>> + },
>>> + [R_MAX] = {}
>>> +};
>>> +
>>> +static const MemoryRegionOps 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 void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
>>> +{
>>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>>> + const char *prefix = object_get_canonical_path(OBJECT(dev));
>>> + int i;
>>> +
>>> + for (i = 0; i < R_MAX; ++i) {
>>> + RegisterInfo *r = &s->regs_info[i];
>>> +
>>> + *r = (RegisterInfo) {
>>> + .data = &s->regs[i],
>>> + .data_size = sizeof(uint32_t),
>>> + .access = &xilinx_devcfg_regs_info[i],
>>> + .debug = XILINX_DEVCFG_ERR_DEBUG,
>>> + .prefix = prefix,
>>> + .opaque = s,
>>> + };
>>> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
>>> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
>>> + }
>>> +}
>>> +
>>> +static void xilinx_devcfg_init(Object *obj)
>>> +{
>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>>> + XilinxDevcfg *s = XILINX_DEVCFG(obj);
>>> +
>>> + s->timer_bh = qemu_bh_new(xilinx_devcfg_dma_go, s);
>>> + s->timer = ptimer_init(s->timer_bh);
>>> +
>>> + sysbus_init_irq(sbd, &s->irq);
>>> +
>>> + memory_region_init(&s->iomem, "devcfg", R_MAX*4);
>>> + sysbus_init_mmio(sbd, &s->iomem);
>>> +}
>>> +
>>> +static void xilinx_devcfg_class_init(ObjectClass *klass, void *data)
>>> +{
>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> + dc->reset = xilinx_devcfg_reset;
>>> + dc->vmsd = &vmstate_xilinx_devcfg;
>>> + dc->realize = xilinx_devcfg_realize;
>>> +}
>>> +
>>> +static const TypeInfo xilinx_devcfg_info = {
>>> + .name = TYPE_XILINX_DEVCFG,
>>> + .parent = TYPE_SYS_BUS_DEVICE,
>>> + .instance_size = sizeof(XilinxDevcfg),
>>> + .instance_init = xilinx_devcfg_init,
>>> + .class_init = xilinx_devcfg_class_init,
>>> +};
>>> +
>>> +static void xilinx_devcfg_register_types(void)
>>> +{
>>> + type_register_static(&xilinx_devcfg_info);
>>> +}
>>> +
>>> +type_init(xilinx_devcfg_register_types)
>>> --
>>> 1.8.3.rc1.44.gb387c77.dirty
>>
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model
2013-05-30 19:41 ` Anthony Liguori
@ 2013-05-31 23:34 ` Peter Crosthwaite
0 siblings, 0 replies; 19+ messages in thread
From: Peter Crosthwaite @ 2013-05-31 23:34 UTC (permalink / raw)
To: Anthony Liguori
Cc: peter.maydell, qemu-devel, blauwirbel, kraxel, edgar.iglesias, pbonzini
Hi Anthony,
On Fri, May 31, 2013 at 5:41 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
[snip]
>>> }
>>>
>>
>> That's still possible using just the register API (Patch 2 content
>> only) and throwing away the memory API glue. I think its actually
>> quite similar to V1 of the patch series where I did not have the
>> callbacks, and use the switch-cases for register side-effects only.
>> This looks like the intention of your patch. Gerd made a push for the
>> callbacks in an earlier review and there was push to integrate to
>> Memory API . Is it reasonable to leave it up to the developer whether
>> they want to do full conversion (Patches 2 & 3) or half conversion
>> (Patch 2 only + your decode refactoring). V1 of this patch at:
>>
>> http://patchwork.ozlabs.org/patch/224534/
>>
>> heres the relevant snippet (comments from me inline):
>>
>> +static void xilinx_devcfg_write(void *opaque, hwaddr addr, uint64_t value,
>> + unsigned size)
>> +{
>> + XilinxDevcfg *s = opaque;
>> + int i;
>> + uint32_t aes_en;
>> + const char *prefix = "";
>> +
>> + /* FIXME: use tracing to avoid these gymnastics */
>> + if (XILINX_DEVCFG_ERR_DEBUG || qemu_get_log_flags() & LOG_GUEST_ERROR) {
>> + prefix = g_strdup_printf("%s:Addr %#08x",
>> + object_get_canonical_path(OBJECT(s)),
>> + (unsigned)addr);
>> + }
>> + addr >>= 2;
>>
>> This is the open coded decode logic but is trivial in this case.
>
> But this is invisible to the common layer.
>
> I think being able to have a common layer insight into "this value is
> being written to this device register" would be extremely useful.
>
> But my fear is that the current proposal will not work for a large set
> of devices.
>
> Let me provide another example (attached below) but highlighting the
> description:
>
>> static RegisterDecodeEntry decoder[] = {
>> /* addresses 0-1 must be open decoded due to latching */
>> { .addr = 2, .regno = UART_FCR, .flags = REG_WRITE },
>> { .addr = 2, .regno = UART_IIR, .flags = REG_READ },
The TRM im reading (for the Xilinx 16550 serial) has the read decoding
of FCR vs IIR dependent on LCR, so I think these two are open decoded
as well.
>> { .addr = 3, .regno = UART_LCR, .flags = REG_RW },
>> { .addr = 4, .regno = UART_MCR, .flags = REG_RW },
>> { .addr = 5, .regno = UART_LSR, .flags = REG_READ },
>> { .addr = 6, .regno = UART_MSR, .flags = REG_READ },
>> { .addr = 7, .regno = UART_SCR, .flags = REG_RW },
>> };
>>
>> static RegisterMapEntry regmap[] = {
>> DEFINE_REG_U8(SerialState, ier, UART_IER),
>> DEFINE_REG_U8(SerialState, iir, UART_IIR),
>> DEFINE_REG_U8(SerialState, lcr, UART_LCR),
>> DEFINE_REG_U8(SerialState, mcr, UART_MCR),
>> DEFINE_REG_U8(SerialState, lsr, UART_LSR),
>> DEFINE_REG_U8(SerialState, scr, UART_SCR),
>> DEFINE_REG_U8(SerialState, thr, UART_THR),
>> };
I like it, but can we merge the two to avoid the replicated lists?
Append the decode information to the existing RegisterMapEntry (or
other way round if you want to think of it like that). I know there is
not a 1:1 correlation between decode lines and storage, but register
API already supports no-storage entries for this very purpose - you
can describe a "register" with no storage and define only side effects
(currently implemented via post_foo callbacks). Then we dont need the
enum to perform mapping from decode to register (except for the open
decode case).
Perhaps its better to think of the RegisterAccessInfo array in the
patch as corresponding to decode lines, rather than storage, with
optional storage information appended
Will send rework shortly.
Regards,
Peter
>
> This is a clear separation between decode logic and load/store logic.
> They are very different things from a hardware point of view.
>
>
>> + assert(addr < R_MAX);
>> +
>> + if (s->lock && addr != R_UNLOCK) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s:write to non-lock register while"
>> + " locked\n", prefix);
>> + return;
>> + }
>> +
>>
>> some pre write logic (I dropped it later as it was actually unimplemented).
>>
>> + uint32_write(&s->regs[addr], &xilinx_devcfg_regs_info[addr], value, prefix,
>> + XILINX_DEVCFG_ERR_DEBUG);
>> +
>>
>> this is the data-driven register_write() call under its old name.
>>
>> + /*side effects */
>> + switch (addr) {
>> + case R_CTRL:
>> + for (i = 0; i < ARRAY_SIZE(lock_ctrl_map); ++i) {
>> + if (s->regs[R_LOCK] & 1 << i) {
>> + value &= ~lock_ctrl_map[i];
>> + value |= lock_ctrl_map[i] & s->regs[R_CTRL];
>> + }
>> + }
>> + aes_en = extract32(value, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN);
>> + if (aes_en != 0 && aes_en != 7) {
>> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent,"
>> + "unimplemeneted security reset should happen!\n",
>> + prefix);
>> + }
>> + break;
>> +
>> + case R_DMA_DST_LEN:
>> + /* TODO: implement command queue overflow check and interrupt */
>> + s->dma_command_fifo[s->dma_command_fifo_num].src_addr =
>> + s->regs[R_DMA_SRC_ADDR] & ~0x3UL;
>> + s->dma_command_fifo[s->dma_command_fifo_num].dest_addr =
>> + s->regs[R_DMA_DST_ADDR] & ~0x3UL;
>> + s->dma_command_fifo[s->dma_command_fifo_num].src_len =
>> + s->regs[R_DMA_SRC_LEN] << 2;
>> + s->dma_command_fifo[s->dma_command_fifo_num].dest_len =
>> + s->regs[R_DMA_DST_LEN] << 2;
>> + s->dma_command_fifo_num++;
>> + DB_PRINT("dma transfer started; %d total transfers pending\n",
>> + s->dma_command_fifo_num);
>> + xilinx_devcfg_dma_go(s);
>> + break;
>> +
>> + case R_UNLOCK:
>> + if (value == R_UNLOCK_MAGIC) {
>> + s->lock = 0;
>> + DB_PRINT("successful unlock\n");
>> + } else if (s->lock) {/* bad unlock attempt */
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s:failed unlock\n", prefix);
>> + s->regs[R_CTRL] &= ~PCAP_PR;
>> + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK;
>> + }
>> + break;
>> + }
>>
>> And the post-write logic.
>>
>> + xilinx_devcfg_update_ixr(s);
>> +
>> + if (*prefix) {
>> + g_free((void *)prefix);
>> + }
>> +}
>> +
>>
>>> It makes it much easier to convert existing code. We can then leave
>>> most of the switch statements intact and just introduce register
>>> descriptions.
>>>
>>> I think splitting decode from data processing is useful too because it
>>> makes it easier to support latching.
>>>
>>> I think the current spin is probably over generalizing too. I don't
>>> think supporting ge0/ge1 makes a lot of sense from the start. Decisions
>>> aren't always that simple and it's weird to have sanity checking split
>>> across two places.
>>>
>>
>> My goal is to pretty much copy paste out the TRM for the clear GE
>> write. Some of my register fields in the TRM for this device say
>> things like "Reserved, always write as 1" which im trying to capture
>> in a data driven way without having to pollute my switch-cases with
>> this junk. It would be nice to autogenerate this as well.
>
> I think you're trying to solve too many problems at once.
>
> I don't think putting error messages in a register layout description is
> a good idea.
>
>
>>> BTW: I think it's also a good idea to model this as a QOM object so that
>>> device state can be access through the QOM tree.
>
> FWIW, I now think this is a bad idea :-)
>
> Here's the full example:
>
>
>
> Regards,
>
> Anthony Liguori
>
>>>
>>
>> Hmm ill have to think on this one.
>>
>> Regards,
>> Peter
>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>> + },
>>>> + [R_LOCK] = { .name = "LOCK",
>>>> + .ro = ~ONES(5),
>>>> + .pre_write = r_lock_pre_write,
>>>> + },
>>>> + [R_CFG] = { .name = "CFG",
>>>> + .reset = 1 << RFIFO_TH_SHIFT | 1 << WFIFO_TH_SHIFT | 0x8,
>>>> + .ge1 = (RegisterAccessError[]) {
>>>> + { .mask = 0x7, .reason = "Reserved - do not modify" },
>>>> + {},
>>>> + },
>>>> + .ge0 = (RegisterAccessError[]) {
>>>> + { .mask = 0x8, .reason = "Reserved - do not modify" },
>>>> + {},
>>>> + },
>>>> + .ro = 0x00f | ~ONES(12),
>>>> + },
>>>> + [R_INT_STS] = { .name = "INT_STS",
>>>> + .w1c = ~R_INT_STS_RSVD,
>>>> + .reset = PSS_GTS_USR_B_INT | PSS_CFG_RESET_B_INT | WR_FIFO_LVL_INT,
>>>> + .ro = R_INT_STS_RSVD,
>>>> + .post_write = r_ixr_post_write,
>>>> + },
>>>> + [R_INT_MASK] = { .name = "INT_MASK",
>>>> + .reset = ~0,
>>>> + .ro = R_INT_STS_RSVD,
>>>> + .post_write = r_ixr_post_write,
>>>> + },
>>>> + [R_STATUS] = { .name = "STATUS",
>>>> + .reset = DMA_CMD_Q_E | PSS_GTS_USR_B | PSS_CFG_RESET_B,
>>>> + .ro = ~0,
>>>> + .ge1 = (RegisterAccessError[]) {
>>>> + {.mask = 0x1, .reason = "Reserved - do not modify" },
>>>> + {},
>>>> + },
>>>> + },
>>>> + [R_DMA_SRC_ADDR] = { .name = "DMA_SRC_ADDR" },
>>>> + [R_DMA_DST_ADDR] = { .name = "DMA_DST_ADDR" },
>>>> + [R_DMA_SRC_LEN] = { .name = "DMA_SRC_LEN", .ro = ~ONES(27) },
>>>> + [R_DMA_DST_LEN] = { .name = "DMA_DST_LEN",
>>>> + .ro = ~ONES(27),
>>>> + .post_write = r_dma_dst_len_post_write,
>>>> + },
>>>> + [R_ROM_SHADOW] = { .name = "ROM_SHADOW",
>>>> + .ge1 = (RegisterAccessError[]) {
>>>> + {.mask = ~0, .reason = "Reserved - do not modify" },
>>>> + {},
>>>> + },
>>>> + },
>>>> + [R_SW_ID] = { .name = "SW_ID" },
>>>> + [R_UNLOCK] = { .name = "UNLOCK",
>>>> + .post_write = r_unlock_post_write,
>>>> + },
>>>> + [R_MCTRL] = { .name = "MCTRL",
>>>> + /* Silicon 3.0 for version field, and the mysterious reserved bit 23 */
>>>> + .reset = 0x2 << PS_VERSION_SHIFT | 1 << 23,
>>>> + /* some reserved bits are rw while others are ro */
>>>> + .ro = ~INT_PCAP_LPBK,
>>>> + .ge1 = (RegisterAccessError[]) {
>>>> + { .mask = 0x00F00300, .reason = "Reserved - do not modify" },
>>>> + { .mask = 0x00000003, .reason = "Reserved - always write with 0" },
>>>> + {}
>>>> + },
>>>> + .ge0 = (RegisterAccessError[]) {
>>>> + { .mask = 1 << 23, .reason = "Reserved - always write with 1" },
>>>> + {}
>>>> + },
>>>> + },
>>>> + [R_MAX] = {}
>>>> +};
>>>> +
>>>> +static const MemoryRegionOps 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 void xilinx_devcfg_realize(DeviceState *dev, Error **errp)
>>>> +{
>>>> + XilinxDevcfg *s = XILINX_DEVCFG(dev);
>>>> + const char *prefix = object_get_canonical_path(OBJECT(dev));
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < R_MAX; ++i) {
>>>> + RegisterInfo *r = &s->regs_info[i];
>>>> +
>>>> + *r = (RegisterInfo) {
>>>> + .data = &s->regs[i],
>>>> + .data_size = sizeof(uint32_t),
>>>> + .access = &xilinx_devcfg_regs_info[i],
>>>> + .debug = XILINX_DEVCFG_ERR_DEBUG,
>>>> + .prefix = prefix,
>>>> + .opaque = s,
>>>> + };
>>>> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", 4);
>>>> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem);
>>>> + }
>>>> +}
>>>> +
>>>> +static void xilinx_devcfg_init(Object *obj)
>>>> +{
>>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>>>> + XilinxDevcfg *s = XILINX_DEVCFG(obj);
>>>> +
>>>> + s->timer_bh = qemu_bh_new(xilinx_devcfg_dma_go, s);
>>>> + s->timer = ptimer_init(s->timer_bh);
>>>> +
>>>> + sysbus_init_irq(sbd, &s->irq);
>>>> +
>>>> + memory_region_init(&s->iomem, "devcfg", R_MAX*4);
>>>> + sysbus_init_mmio(sbd, &s->iomem);
>>>> +}
>>>> +
>>>> +static void xilinx_devcfg_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +
>>>> + dc->reset = xilinx_devcfg_reset;
>>>> + dc->vmsd = &vmstate_xilinx_devcfg;
>>>> + dc->realize = xilinx_devcfg_realize;
>>>> +}
>>>> +
>>>> +static const TypeInfo xilinx_devcfg_info = {
>>>> + .name = TYPE_XILINX_DEVCFG,
>>>> + .parent = TYPE_SYS_BUS_DEVICE,
>>>> + .instance_size = sizeof(XilinxDevcfg),
>>>> + .instance_init = xilinx_devcfg_init,
>>>> + .class_init = xilinx_devcfg_class_init,
>>>> +};
>>>> +
>>>> +static void xilinx_devcfg_register_types(void)
>>>> +{
>>>> + type_register_static(&xilinx_devcfg_info);
>>>> +}
>>>> +
>>>> +type_init(xilinx_devcfg_register_types)
>>>> --
>>>> 1.8.3.rc1.44.gb387c77.dirty
>>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2013-05-31 23:34 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-24 5:46 [Qemu-devel] [PATCH v3 0/5] Data Driven device registers & Zynq DEVCFG peter.crosthwaite
2013-05-24 5:46 ` [Qemu-devel] [PATCH v3 1/5] bitops: Add ONES macro peter.crosthwaite
2013-05-24 5:47 ` [Qemu-devel] [PATCH v3 2/5] register: Add Register API peter.crosthwaite
2013-05-29 17:52 ` Edgar E. Iglesias
2013-05-24 5:48 ` [Qemu-devel] [PATCH v3 3/5] register: Add Memory API glue peter.crosthwaite
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model peter.crosthwaite
2013-05-29 8:51 ` Paolo Bonzini
2013-05-29 13:54 ` Peter Crosthwaite
2013-05-29 14:03 ` Paolo Bonzini
2013-05-29 17:04 ` Edgar E. Iglesias
2013-05-29 17:08 ` Paolo Bonzini
2013-05-30 7:15 ` Peter Crosthwaite
2013-05-30 13:08 ` Paolo Bonzini
2013-05-29 17:57 ` Anthony Liguori
2013-05-29 18:52 ` Anthony Liguori
2013-05-30 1:43 ` Peter Crosthwaite
2013-05-30 19:41 ` Anthony Liguori
2013-05-31 23:34 ` Peter Crosthwaite
2013-05-24 5:49 ` [Qemu-devel] [PATCH v3 5/5] xilinx_zynq: added devcfg to machine model peter.crosthwaite
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.