From: Peter Maydell <[email protected]>

The xtensa mx-pic interrupt controller has a rather odd register
setup, where some registers are 32 bits but are decoded at offsets
only one apart from each other.  The QEMU implementation handles this
correctly, but it did not set .impl.unaligned = true.  This has
worked up til now because QEMU has entirely ignored .impl.unaligned,
and just allowed through unaligned accesses when
.valid.unaligned is set.

To allow the possibility of properly implementing synthesis of
unaligned accesses by the memory subsystem when they are valid but
the device doesn't implement them, and for clarity of intention,
state explicitly that this MR's read and write functions directly
handle unaligned accesses, by setting .impl.unaligned = true.

While we are adjusting the MemoryRegionOps, we set also the minimum
and maximum allowed access sizes.  Since the only way to get at this
device is via the CPU's RER and WER instructions, which always
operate at 32-bit sizes (see the HELPER(rer) and HELPER(wer)
functions in target/xtensa/op_helper.c), we know we will always get
32-bit accesses.  Specify explicitly that that is what is valid and
implemented for the MR.

Add a comment to clarify that the hardware behaviour here is not
"true memory-mapped registers", so the odd-looking implementation is
correct.

Based-on-a-patch-by: CJ Chen <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Max Filippov <[email protected]>
Link: 
https://lore.kernel.org/r/[email protected]
Signed-off-by: Peter Xu <[email protected]>
---
 hw/xtensa/mx_pic.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/hw/xtensa/mx_pic.c b/hw/xtensa/mx_pic.c
index 07c3731aef..098c1aaf85 100644
--- a/hw/xtensa/mx_pic.c
+++ b/hw/xtensa/mx_pic.c
@@ -69,6 +69,26 @@ struct XtensaMxPic {
     } cpu[MX_MAX_CPU];
 };
 
+/*
+ * Note that decode for these registers is rather strange by the usual
+ * MMIO standards -- the MIROUT and MIPICAUSE areas can be read and
+ * written at 32-bit length, returning different values for each byte
+ * offset, because the low bits of the address are treated as selecting
+ * an IRQ or a processor:
+ *
+ * 00nn         0...0p..p       Interrupt Routing, route IRQ n to processor p
+ * 01pp         0...0d..d       16 bits (d) 'ored' as single IPI to processor p
+ *
+ * This is because (like x86 IO port in/out accesses) the offset is
+ * not a memory-mapped address but is really a register number,
+ * accessed via the Xtensa RER/WER "external register" instructions.
+ *
+ * We set .valid and .impl to both allow unaligned = true to permit
+ * these byte-offsets. Because this device is not a true memory mapped
+ * device but is accessible only via the Xtensa RER/WER "external
+ * register" interface, all accesses are guaranteed 32 bits.
+ */
+
 static uint64_t xtensa_mx_pic_ext_reg_read(void *opaque, hwaddr offset,
                                            unsigned size)
 {
@@ -267,7 +287,14 @@ static const MemoryRegionOps xtensa_mx_pic_ops = {
     .read = xtensa_mx_pic_ext_reg_read,
     .write = xtensa_mx_pic_ext_reg_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+        .unaligned = true,
+    },
     .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
         .unaligned = true,
     },
 };
-- 
2.53.0


Reply via email to