Data Driven device registers & Zynq DEVCFG

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

Signed-off-by Peter Crosthwaite <[email protected]>

---

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

include/qemu/bitops.h +0 -0

index affcc96..da47fc8 100644

Quoted 16 lines · Peter Crosthwaite

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

Signed-off-by Peter Crosthwaite <[email protected]>

---

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

include/qemu/bitops.h +0 -0

index affcc96..da47fc8 100644
@@ -273,4 +273,6 @@ static inline uint64_t deposit64(uint64_t value, int start, int length,
     return (value & ~mask) | ((fieldval << start) & mask);
 }
 

include/qemu/bitops.h static inline uint64_t deposit64(uint64_t value, int start, int length, · line 276

Quoted 1 line

include/qemu/bitops.h +1 -0

+#define ONES(num) ((num) == 64 ? ~0ull : (1ull << (num)) - 1)

You can avoid the ?: here (assuming you're happy to say that ONES(0) is a silly thing to ask for):

#define ONES(num) (~0ULL >> (64 - (num)))

Needs a documentation comment, anyway.

thanks
-- PMM

But I thought about this more, and perhaps just ditch the ? anyway, and rely on the shift by 64 ending up as 0.

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.

register.c new file +63 -0

+void register_write(RegisterInfo *reg, uint64_t val, uint64_t we)
+{
+    uint64_t old_val, new_val, test;
+    const RegisterAccessInfo *ac;
+    const RegisterAccessError *rae;
+
+    assert(reg);
+
+    ac = reg->access;
+    if (!ac || !ac->name) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: write to undefined device state "
+                      "(written value: %#" PRIx64 ")\n", reg->prefix, val);
+        return;
+    }
+
+    uint32_t no_w0_mask = ac->ro | ac->w1c | ac->nw0 | ~we;
+    uint32_t no_w1_mask = ac->ro | ac->w1c | ac->nw1 | ~we;
+
+    if (reg->debug) {
+        fprintf(stderr, "%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);
+            }
+        }
+    }
+
+    assert(reg->data);
+    old_val = register_read_val(reg);
+
+    new_val = val & ~(no_w1_mask & val);
+    new_val |= no_w1_mask & old_val & val;
+    new_val |= no_w0_mask & old_val & ~val;

Quoted 72 lines · Peter Crosthwaite

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.

register.c new file +67 -0

+void register_write(RegisterInfo *reg, uint64_t val, uint64_t we)
+{
+    uint64_t old_val, new_val, test;
+    const RegisterAccessInfo *ac;
+    const RegisterAccessError *rae;
+
+    assert(reg);
+
+    ac = reg->access;
+    if (!ac || !ac->name) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: write to undefined device state "
+                      "(written value: %#" PRIx64 ")\n", reg->prefix, val);
+        return;
+    }
+
+    uint32_t no_w0_mask = ac->ro | ac->w1c | ac->nw0 | ~we;
+    uint32_t no_w1_mask = ac->ro | ac->w1c | ac->nw1 | ~we;
+
+    if (reg->debug) {
+        fprintf(stderr, "%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);
+            }
+        }
+    }
+
+    assert(reg->data);
+    old_val = register_read_val(reg);
+
+    new_val = val & ~(no_w1_mask & val);
+    new_val |= no_w1_mask & old_val & val;
+    new_val |= no_w0_mask & old_val & ~val;
+    new_val &= ~(val & ac->w1c);
+
+    if (ac->pre_write) {
+        new_val = ac->pre_write(reg, new_val);

register.c line 117

Quoted 6 lines

register.c new file +6 -0

+    }
+    register_write_val(reg, new_val);
+    if (ac->post_write) {
+        ac->post_write(reg, new_val);
+    }
+}

Wow, this feels pretty heavyweight for "write a register"...

-- PMM

Quoted 74 lines · Peter Maydell

Quoted 73 lines · Peter Crosthwaite

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.

register.c new file +68 -0

+void register_write(RegisterInfo *reg, uint64_t val, uint64_t we)
+{
+    uint64_t old_val, new_val, test;
+    const RegisterAccessInfo *ac;
+    const RegisterAccessError *rae;
+
+    assert(reg);
+
+    ac = reg->access;
+    if (!ac || !ac->name) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: write to undefined device state "
+                      "(written value: %#" PRIx64 ")\n", reg->prefix, val);
+        return;
+    }
+
+    uint32_t no_w0_mask = ac->ro | ac->w1c | ac->nw0 | ~we;
+    uint32_t no_w1_mask = ac->ro | ac->w1c | ac->nw1 | ~we;
+
+    if (reg->debug) {
+        fprintf(stderr, "%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);
+            }
+        }
+    }
+
+    assert(reg->data);
+    old_val = register_read_val(reg);
+
+    new_val = val & ~(no_w1_mask & val);
+    new_val |= no_w1_mask & old_val & val;
+    new_val |= no_w0_mask & old_val & ~val;
+    new_val &= ~(val & ac->w1c);
+
+    if (ac->pre_write) {
+        new_val = ac->pre_write(reg, new_val);
+    }

Quoted 8 lines

register.c line 117

Quoted 5 lines

register.c new file +5 -0

+    register_write_val(reg, new_val);
+    if (ac->post_write) {
+        ac->post_write(reg, new_val);
+    }
+}

Wow, this feels pretty heavyweight for "write a register"...

Its a pritty fast path through if all the optionals turned off. If I was going to lighten it up, Id get rid of the reg_size and the byte loop first although that would mandate uint64_t as the data type for the backing store.

But the intended primary usage of the API is for device control and status registers, where you are generally not interested in performance, unless you are doing some form of PIO. In that case, you can opt out on a per register basis and make yourself a fast path for the critical registers (r/w fifo heads access regs etc). This is designed for developer and debugging convenience, where you have a large number or registers with a heteorgenous mix of accesses and side effects but at the same time the registers are infrequent access. This is true of most device registers.

Another solution is some sort of "lite" mode. A single check in the definition that disables a lot of this and cuts to the chase (the actual write).

Quoted 2 lines

-- PMM

- Add a "lite" flag that is (automatically) computed once that takes a fast path through the read/write handlers. A reg is lite if it requires no read-modify-write (no ro, nw, ge, ui, wtc). Only the actual write and post_write handlers are executed.

- Remove the byte loops. Just switch on register size and cast to uint_XXt. Remove support for non power-of-two size registers and non-host endianess (needed for potential PCI conversion) but should make the code faster.

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 <[email protected]>

---

 include/exec/register.h |   13 +++++++++++++

 register.c              |   43 +++++++++++++++++++++++++++++++++++++++++++