[PATCH v5] hw/dma/pl330: Add memory region to replace default

Wen, Jianxian posted 1 patch 2 years, 7 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/4C23C17B8E87E74E906A25A3254A03F4FA1FEC31@SHASXM03.verisilicon.com
Maintainers: Igor Mitsyanko <i.mitsyanko@gmail.com>, Alistair Francis <alistair@alistair23.me>, Peter Maydell <peter.maydell@linaro.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
hw/arm/exynos4210.c  |  3 +++
hw/arm/xilinx_zynq.c |  3 +++
hw/dma/pl330.c       | 26 ++++++++++++++++++++++----
3 files changed, 28 insertions(+), 4 deletions(-)
[PATCH v5] hw/dma/pl330: Add memory region to replace default
Posted by Wen, Jianxian 2 years, 7 months ago
Add property memory region which can connect with IOMMU region to support SMMU translate.

Signed-off-by: Jianxian Wen <jianxian.wen@verisilicon.com>
---
v5 (after review of Philippe Mathieu-Daudé):
 - Refine code and use memory_region_name() to get MR name.
v4 (after review of Philippe Mathieu-Daudé):
 - Avoid creating new AS for system memory, add AS if we connect with IOMMU region.
v3 (after review of Philippe Mathieu-Daudé):
 - Refine code to comply with code style, update error message if memory link is not set.
v2 (after review of Peter Maydell):
 - Use dma_memory_read/write functions to do data access, update function AddressSpace* parameter.

 hw/arm/exynos4210.c  |  3 +++
 hw/arm/xilinx_zynq.c |  3 +++
 hw/dma/pl330.c       | 26 ++++++++++++++++++++++----
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 5c7a51b..0299e81 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -173,6 +173,9 @@ static DeviceState *pl330_create(uint32_t base, qemu_or_irq *orgate,
     int i;
 
     dev = qdev_new("pl330");
+    object_property_set_link(OBJECT(dev), "memory",
+                             OBJECT(get_system_memory()),
+                             &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 245af81..69c333e 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -312,6 +312,9 @@ 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 944ba29..0cb4619 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,7 @@ static inline const PL330InsnDesc *pl330_fetch_insn(PL330Chan *ch)
     uint8_t opcode;
     int i;
 
-    dma_memory_read(&address_space_memory, ch->pc, &opcode, 1);
+    dma_memory_read(ch->parent->mem_as, ch->pc, &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 +1125,7 @@ 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);
+    dma_memory_read(ch->parent->mem_as, ch->pc, buf, insn->size);
     insn->exec(ch, buf[0], &buf[1], insn->size - 1);
 }
 
@@ -1186,7 +1189,7 @@ 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);
+        dma_memory_read(s->mem_as, q->addr, buf, len);
         trace_pl330_exec_cycle(q->addr, len);
         if (trace_event_get_state_backends(TRACE_PL330_HEXDUMP)) {
             pl330_hexdump(buf, len);
@@ -1217,7 +1220,7 @@ 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);
+            dma_memory_write(s->mem_as, q->addr, buf, len);
             trace_pl330_exec_cycle(q->addr, len);
             if (trace_event_get_state_backends(TRACE_PL330_HEXDUMP)) {
                 pl330_hexdump(buf, len);
@@ -1562,6 +1565,18 @@ 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, "'memory' link is not set");
+        return;
+    } else if (s->mem_mr == get_system_memory()) {
+        /* Avoid creating new AS for system memory. */
+        s->mem_as = &address_space_memory;
+    } else {
+        s->mem_as = g_new0(AddressSpace, 1);
+        address_space_init(s->mem_as, s->mem_mr,
+                           memory_region_name(s->mem_mr));
+    }
+
     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 +1671,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(),
 };
 
-- 
2.7.4

Re: [PATCH v5] hw/dma/pl330: Add memory region to replace default
Posted by Philippe Mathieu-Daudé 2 years, 7 months ago
On 8/18/21 12:17 PM, Wen, Jianxian wrote:
> Add property memory region which can connect with IOMMU region to support SMMU translate.
> 
> Signed-off-by: Jianxian Wen <jianxian.wen@verisilicon.com>
> ---
> v5 (after review of Philippe Mathieu-Daudé):
>  - Refine code and use memory_region_name() to get MR name.
> v4 (after review of Philippe Mathieu-Daudé):
>  - Avoid creating new AS for system memory, add AS if we connect with IOMMU region.
> v3 (after review of Philippe Mathieu-Daudé):
>  - Refine code to comply with code style, update error message if memory link is not set.
> v2 (after review of Peter Maydell):
>  - Use dma_memory_read/write functions to do data access, update function AddressSpace* parameter.
> 
>  hw/arm/exynos4210.c  |  3 +++
>  hw/arm/xilinx_zynq.c |  3 +++
>  hw/dma/pl330.c       | 26 ++++++++++++++++++++++----
>  3 files changed, 28 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v5] hw/dma/pl330: Add memory region to replace default
Posted by Peter Maydell 2 years, 7 months ago
On Wed, 18 Aug 2021 at 11:17, Wen, Jianxian
<Jianxian.Wen@verisilicon.com> wrote:
>
> Add property memory region which can connect with IOMMU region to support SMMU translate.
>
> Signed-off-by: Jianxian Wen <jianxian.wen@verisilicon.com>
> ---



Applied to target-arm.next for 6.2, thanks.

-- PMM