From f780b0ee2ee36c562ab814915fff0e7217b25e63 Mon Sep 17 00:00:00 2001
From: Jianxian Wen <jianxian.wen@verisilicon.com>
Date: Tue, 3 Aug 2021 09:44:35 +0800
Subject: [PATCH] hw/dma/pl330: Add memory region to replace default
 address_space_memory

PL330 needs a memory region which can connect with SMMU translate IOMMU region to support SMMU.

Signed-off-by: Jianxian Wen <jianxian.wen@verisilicon.com>
---
 hw/arm/exynos4210.c  |  3 +++
 hw/arm/xilinx_zynq.c |  2 ++
 hw/dma/pl330.c       | 24 ++++++++++++++++++++----
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 5c7a51bba..af0e4820d 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -171,8 +171,11 @@ static DeviceState *pl330_create(uint32_t base, qemu_or_irq *orgate,
     SysBusDevice *busdev;
     DeviceState *dev;
     int i;
+    MemoryRegion *sysmem = get_system_memory();

     dev = qdev_new("pl330");
+    object_property_set_link(OBJECT(dev), "memory",
+                                    OBJECT(sysmem), &error_fatal);
     qdev_prop_set_uint8(dev, "num_events", nevents);
     qdev_prop_set_uint8(dev, "num_chnls",  8);
     qdev_prop_set_uint8(dev, "num_periph_req",  nreq);
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 245af81bb..e0b3a73b9 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -312,6 +312,8 @@ static void zynq_init(MachineState *machine)
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]);

     dev = qdev_new("pl330");
+    object_property_set_link(OBJECT(dev), "memory",
+                                    OBJECT(address_space_mem), &error_fatal);
     qdev_prop_set_uint8(dev, "num_chnls",  8);
     qdev_prop_set_uint8(dev, "num_periph_req",  4);
     qdev_prop_set_uint8(dev, "num_events",  16);
diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
index 944ba296b..06747ca0b 100644
--- a/hw/dma/pl330.c
+++ b/hw/dma/pl330.c
@@ -269,6 +269,9 @@ struct PL330State {
     uint8_t num_faulting;
     uint8_t periph_busy[PL330_PERIPH_NUM];

+    /* Memory region that DMA operation access */
+    MemoryRegion *mem_mr;
+    AddressSpace mem_as;
 };

 #define TYPE_PL330 "pl330"
@@ -1108,7 +1111,8 @@ static inline const PL330InsnDesc *pl330_fetch_insn(PL330Chan *ch)
     uint8_t opcode;
     int i;

-    dma_memory_read(&address_space_memory, ch->pc, &opcode, 1);
+    address_space_read(&ch->parent->mem_as, ch->pc,
+                        MEMTXATTRS_UNSPECIFIED, &opcode, 1);
     for (i = 0; insn_desc[i].size; i++) {
         if ((opcode & insn_desc[i].opmask) == insn_desc[i].opcode) {
             return &insn_desc[i];
@@ -1122,7 +1126,8 @@ static inline void pl330_exec_insn(PL330Chan *ch, const PL330InsnDesc *insn)
     uint8_t buf[PL330_INSN_MAXSIZE];

     assert(insn->size <= PL330_INSN_MAXSIZE);
-    dma_memory_read(&address_space_memory, ch->pc, buf, insn->size);
+    address_space_read(&ch->parent->mem_as, ch->pc,
+                        MEMTXATTRS_UNSPECIFIED, buf, insn->size);
     insn->exec(ch, buf[0], &buf[1], insn->size - 1);
 }

@@ -1186,7 +1191,8 @@ static int pl330_exec_cycle(PL330Chan *channel)
     if (q != NULL && q->len <= pl330_fifo_num_free(&s->fifo)) {
         int len = q->len - (q->addr & (q->len - 1));

-        dma_memory_read(&address_space_memory, q->addr, buf, len);
+        address_space_read(&s->mem_as, q->addr,
+                            MEMTXATTRS_UNSPECIFIED, buf, len);
         trace_pl330_exec_cycle(q->addr, len);
         if (trace_event_get_state_backends(TRACE_PL330_HEXDUMP)) {
             pl330_hexdump(buf, len);
@@ -1217,7 +1223,8 @@ static int pl330_exec_cycle(PL330Chan *channel)
             fifo_res = pl330_fifo_get(&s->fifo, buf, len, q->tag);
         }
         if (fifo_res == PL330_FIFO_OK || q->z) {
-            dma_memory_write(&address_space_memory, q->addr, buf, len);
+            address_space_write(&s->mem_as, q->addr,
+                                MEMTXATTRS_UNSPECIFIED, buf, len);
             trace_pl330_exec_cycle(q->addr, len);
             if (trace_event_get_state_backends(TRACE_PL330_HEXDUMP)) {
                 pl330_hexdump(buf, len);
@@ -1562,6 +1569,12 @@ static void pl330_realize(DeviceState *dev, Error **errp)
                           "dma", PL330_IOMEM_SIZE);
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);

+    if (!s->mem_mr) {
+        error_setg(errp, "'mem_mr' link is not set");
+        return;
+    }
+    address_space_init(&s->mem_as, s->mem_mr, "pl330-memory");
+
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, pl330_exec_cycle_timer, s);

     s->cfg[0] = (s->mgr_ns_at_rst ? 0x4 : 0) |
@@ -1656,6 +1669,9 @@ static Property pl330_properties[] = {
     DEFINE_PROP_UINT8("rd_q_dep", PL330State, rd_q_dep, 16),
     DEFINE_PROP_UINT16("data_buffer_dep", PL330State, data_buffer_dep, 256),

+    DEFINE_PROP_LINK("memory", PL330State, mem_mr,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
+
     DEFINE_PROP_END_OF_LIST(),
 };

--

On Fri, 13 Aug 2021 at 07:44, Wen, Jianxian
<Jianxian.Wen@verisilicon.com> wrote:
>
> From f780b0ee2ee36c562ab814915fff0e7217b25e63 Mon Sep 17 00:00:00 2001
>
> From: Jianxian Wen <jianxian.wen@verisilicon.com>
>
> Date: Tue, 3 Aug 2021 09:44:35 +0800
>
> Subject: [PATCH] hw/dma/pl330: Add memory region to replace default
>
>  address_space_memory
>
>
>
> PL330 needs a memory region which can connect with SMMU translate IOMMU region to support SMMU.
>
>
>
> Signed-off-by: Jianxian Wen <jianxian.wen@verisilicon.com>

Hi; thanks for sending this patch. It mostly looks good to me.
I have a review comment (which you can find further down in this
email), and one note on the patch-sending mechanics:

Your mail client is sending these patch emails as multipart/alternative
with both a plain text and an HTML version. In general the list prefers
plain text only, but this is particularly important for patch emails because
the HTML can confuse automatic patch handling tools.

If you haven't read the SubmitAPatch guidelines you can find them here:
https://wiki.qemu.org/Contribute/SubmitAPatch

Using 'git send-email' is usually the most reliable way to send patches
if you're a regular submitter, but it can take a bit of setup to get
working. You might also be interested in the sourcehut web-based UI for
sending patch emails described in this subsection:
https://wiki.qemu.org/Contribute/SubmitAPatch#If_you_cannot_send_patch_emails

>
> ---
> @@ -1108,7 +1111,8 @@ static inline const PL330InsnDesc *pl330_fetch_insn(PL330Chan *ch)
>
>      uint8_t opcode;
>
>      int i;
>
>
>
> -    dma_memory_read(&address_space_memory, ch->pc, &opcode, 1);
>
> +    address_space_read(&ch->parent->mem_as, ch->pc,
>
> +                        MEMTXATTRS_UNSPECIFIED, &opcode, 1);

dma_memory_read() can already take an AddressSpace* -- you don't need
to change these calls to address_space_read(), just change the AS*
we pass them.

Otherwise the change looks OK to me.

thanks
-- PMM