Re: [PATCH v6 1/1] hw/intc: Fix LoongArch extioi coreisr accessing

[Philippe Mathieu-Daudé]

On 19/10/22 11:15, Xiaojuan Yang wrote:
When cpu read or write extioi COREISR reg, it should access

"When the CPU reads or writes ..."

the reg belonged to itself, so the cpu index of 's->coreisr'
is current cpu number. Using MemTxAttrs' requester_id to get
the cpu index.

Signed-off-by: Xiaojuan Yang <yangxiaoj...@loongson.cn>
---
  hw/intc/loongarch_extioi.c      | 42 ++++++++++++++++++---------------
  hw/intc/trace-events            |  3 +--
  target/loongarch/iocsr_helper.c | 18 +++++++-------
  3 files changed, 34 insertions(+), 29 deletions(-)

-static uint64_t extioi_readw(void *opaque, hwaddr addr, unsigned size)
+static MemTxResult extioi_readw(void *opaque, hwaddr addr, uint64_t *data,
+                                unsigned size, MemTxAttrs attrs)
  {

This patch would be easier to review if you split it in 2, first 
converting the MemoryRegionOps read/write handlers to _with_attrs
ones, then another patch fetching the CPU id from attrs.requester_id.

      LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
      unsigned long offset = addr & 0xffff;
-    uint32_t index, cpu, ret = 0;
+    uint32_t index, cpu;
  
      switch (offset) {
      case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1:
          index = (offset - EXTIOI_NODETYPE_START) >> 2;
-        ret = s->nodetype[index];
+        *data = s->nodetype[index];
          break;
      case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
          index = (offset - EXTIOI_IPMAP_START) >> 2;
-        ret = s->ipmap[index];
+        *data = s->ipmap[index];
          break;
      case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
          index = (offset - EXTIOI_ENABLE_START) >> 2;
-        ret = s->enable[index];
+        *data = s->enable[index];
          break;
      case EXTIOI_BOUNCE_START ... EXTIOI_BOUNCE_END - 1:
          index = (offset - EXTIOI_BOUNCE_START) >> 2;
-        ret = s->bounce[index];
+        *data = s->bounce[index];
          break;
      case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
-        index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
-        cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
-        ret = s->coreisr[cpu][index];
+        index = (offset - EXTIOI_COREISR_START) >> 2;
+        /* using attrs to get current cpu index */
+        cpu = attrs.requester_id;

           assert(attrs.requester_type == MTRT_CPU);

+        *data = s->coreisr[cpu][index];
          break;
      case EXTIOI_COREMAP_START ... EXTIOI_COREMAP_END - 1:
          index = (offset - EXTIOI_COREMAP_START) >> 2;
-        ret = s->coremap[index];
+        *data = s->coremap[index];
          break;
      default:
          break;
      }
  
-    trace_loongarch_extioi_readw(addr, ret);
-    return ret;
+    trace_loongarch_extioi_readw(addr, *data);
+    return MEMTX_OK;
  }

-static void extioi_writew(void *opaque, hwaddr addr,
-                          uint64_t val, unsigned size)
+static MemTxResult extioi_writew(void *opaque, hwaddr addr,
+                          uint64_t val, unsigned size,
+                          MemTxAttrs attrs)
  {
      LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
      int i, cpu, index, old_data, irq;
@@ -183,8 +185,9 @@ static void extioi_writew(void *opaque, hwaddr addr,
          s->bounce[index] = val;
          break;
      case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
-        index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
-        cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
+        index = (offset - EXTIOI_COREISR_START) >> 2;
+        /* using attrs to get current cpu index */

           assert(attrs.requester_type == MTRT_CPU);

+        cpu = attrs.requester_id;
          old_data = s->coreisr[cpu][index];
          s->coreisr[cpu][index] = old_data & ~val;
          /* write 1 to clear interrrupt */
@@ -231,11 +234,12 @@ static void extioi_writew(void *opaque, hwaddr addr,
      default:
          break;
      }
+    return MEMTX_OK;
  }