Re: [PATCH v3 3/3] hw/loongarch/virt: Use MemTxAttrs interface for misc ops

[maobibo]

On 2024/5/21 下午8:32, Song Gao wrote:
Use MemTxAttrs interface read_with_attrs/write_with_attrs
for virt_iocsr_misc_ops.

Signed-off-by: Song Gao <gaos...@loongson.cn>
---
  hw/loongarch/virt.c | 26 ++++++++++++++++----------
  1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index e7edc6c9f9..0ab2b6860a 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -866,8 +866,9 @@ static void virt_firmware_init(LoongArchVirtMachineState 
*lvms)
      }
  }
  
-static void virt_iocsr_misc_write(void *opaque, hwaddr addr,
-                                  uint64_t val, unsigned size)
+static MemTxResult virt_iocsr_misc_write(void *opaque, hwaddr addr,
+                                         uint64_t val, unsigned size,
+                                         MemTxAttrs attrs)
  {
      LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(opaque);
      uint64_t features;
@@ -875,12 +876,12 @@ static void virt_iocsr_misc_write(void *opaque, hwaddr 
addr,
      switch (addr) {
      case MISC_FUNC_REG:
          if (!virt_is_veiointc_enabled(lvms)) {
-            return;
+            return MEMTX_OK;
          }
  
          features = address_space_ldl(&lvms->as_iocsr,
                                       EXTIOI_VIRT_BASE + EXTIOI_VIRT_CONFIG,
-                                     MEMTXATTRS_UNSPECIFIED, NULL);
+                                     attrs, NULL);
          if (val & BIT_ULL(IOCSRM_EXTIOI_EN)) {
              features |= BIT(EXTIOI_ENABLE);
          }
@@ -890,11 +891,15 @@ static void virt_iocsr_misc_write(void *opaque, hwaddr 
addr,
  
          address_space_stl(&lvms->as_iocsr,
                            EXTIOI_VIRT_BASE + EXTIOI_VIRT_CONFIG,
-                          features, MEMTXATTRS_UNSPECIFIED, NULL);
+                          features, attrs, NULL);
      }
+
+    return MEMTX_OK;
  }
  
-static uint64_t virt_iocsr_misc_read(void *opaque, hwaddr addr, unsigned size)
+static MemTxResult virt_iocsr_misc_read(void *opaque, hwaddr addr,
+                                        uint64_t *data,
+                                        unsigned size, MemTxAttrs attrs)
  {
      LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(opaque);
      uint64_t ret = 0;
@@ -924,7 +929,7 @@ static uint64_t virt_iocsr_misc_read(void *opaque, hwaddr 
addr, unsigned size)
  
          features = address_space_ldl(&lvms->as_iocsr,
                                       EXTIOI_VIRT_BASE + EXTIOI_VIRT_CONFIG,
-                                     MEMTXATTRS_UNSPECIFIED, NULL);
+                                     attrs, NULL);
          if (features & BIT(EXTIOI_ENABLE)) {
              ret |= BIT_ULL(IOCSRM_EXTIOI_EN);
          }
@@ -933,12 +938,13 @@ static uint64_t virt_iocsr_misc_read(void *opaque, hwaddr 
addr, unsigned size)
          }
      }
  
-    return ret;
+    *data = ret;
+    return MEMTX_OK;
  }
  
  static const MemoryRegionOps virt_iocsr_misc_ops = {
-    .read  = virt_iocsr_misc_read,
-    .write = virt_iocsr_misc_write,
+    .read_with_attrs  = virt_iocsr_misc_read,
+    .write_with_attrs = virt_iocsr_misc_write,
      .endianness = DEVICE_LITTLE_ENDIAN,
      .valid = {
          .min_access_size = 4,

I think patch3 should be put ahead of patch2. Without patch3, there is 
potential problem to access iocsr EXTIOI_VIRT_CONFIG register with 
MEMTXATTRS_UNSPECIFIED attr.

Now extioi is emulated in user space, it will be better if hypervisor 
and VM know whether v-extioi extension is supported.

Regards
Bibo Mao