Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and connect with SMMU v3

[Eric Auger]

Hi Chunming,

On 8/25/21 10:08 AM, chunming wrote:
> From: chunming <chunming.li@verisilicon.com>
>
> Add PL330 DMA controller to test SMMU v3 connection and function.
> The default SID for PL330 is 1 but we test other values, it works well.
Is it just a patch for testing or would you want this to be applied
upstream too?

This static SID allocation may not work in general as it may collide
with PCIe RID space?

My feeling is if we want to enable platform device support in the SMMUv3
this should work for all platform devices doing DMA accesses and not
only for this PL330.
I guess this should work with virtio platform devices and VFIO platform
devices. How would you extend that work to those devices?

Thanks

Eric
>
> Signed-off-by: chunming <chunming.li@verisilicon.com>
> ---
>  hw/arm/virt.c         | 92 ++++++++++++++++++++++++++++++++++++++++++-
>  include/hw/arm/virt.h |  1 +
>  2 files changed, 92 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index c3fd30e071..8180e4a331 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -143,6 +143,7 @@ static const MemMapEntry base_memmap[] = {
>      [VIRT_GIC_REDIST] =         { 0x080A0000, 0x00F60000 },
>      [VIRT_UART] =               { 0x09000000, 0x00001000 },
>      [VIRT_RTC] =                { 0x09010000, 0x00001000 },
> +    [VIRT_DMA] =                { 0x09011000, 0x00001000 },
>      [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
>      [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
>      [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
> @@ -188,6 +189,7 @@ static const int a15irqmap[] = {
>      [VIRT_GPIO] = 7,
>      [VIRT_SECURE_UART] = 8,
>      [VIRT_ACPI_GED] = 9,
> +    [VIRT_DMA] = 10,
>      [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>      [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>      [VIRT_SMMU] = 74,    /* ...to 74 + NUM_SMMU_IRQS - 1 */
> @@ -205,7 +207,7 @@ static const char *valid_cpus[] = {
>  };
>  
>  static const uint16_t smmuv3_sidmap[] = {
> -
> +    [VIRT_DMA] = 1,
>  };
>  
>  static bool cpu_type_valid(const char *cpu)
> @@ -793,6 +795,92 @@ static void create_uart(const VirtMachineState *vms, int 
> uart,
>      g_free(nodename);
>  }
>  
> +static void create_dma(const VirtMachineState *vms)
> +{
> +    int i;
> +    char *nodename;
> +    hwaddr base = vms->memmap[VIRT_DMA].base;
> +    hwaddr size = vms->memmap[VIRT_DMA].size;
> +    int irq = vms->irqmap[VIRT_DMA];
> +    int sid = vms->sidmap[VIRT_DMA];
> +    const char compat[] = "arm,pl330\0arm,primecell";
> +    const char irq_names[] = 
> "abort\0dma0\0dma1\0dma2\0dma3\0dma4\0dma5\0dma6\0dma7";
> +    DeviceState *dev;
> +    MachineState *ms = MACHINE(vms);
> +    SysBusDevice *busdev;
> +    DeviceState *smmuv3_dev;
> +    SMMUState *smmuv3_sys;
> +    Object *smmuv3_memory;
> +
> +    dev = qdev_new("pl330");
> +
> +    if (vms->iommu == VIRT_IOMMU_SMMUV3 && vms->iommu_phandle) {
> +        smmuv3_dev = vms->smmuv3;
> +        smmuv3_sys = ARM_SMMU(smmuv3_dev);
> +        g_autofree char *memname = g_strdup_printf("%s-peri-%d[0]",
> +                                                   smmuv3_sys->mrtypename,
> +                                                   sid);
> +
> +        smmuv3_memory = object_property_get_link(OBJECT(smmuv3_dev),
> +                                memname, &error_abort);
> +
> +        object_property_set_link(OBJECT(dev), "memory",
> +                                 OBJECT(smmuv3_memory),
> +                                 &error_fatal);
> +    } else {
> +        object_property_set_link(OBJECT(dev), "memory",
> +                                 OBJECT(get_system_memory()),
> +                                 &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);
> +    qdev_prop_set_uint8(dev, "data_width",  64);
> +    qdev_prop_set_uint8(dev, "wr_cap",  8);
> +    qdev_prop_set_uint8(dev, "wr_q_dep",  16);
> +    qdev_prop_set_uint8(dev, "rd_cap",  8);
> +    qdev_prop_set_uint8(dev, "rd_q_dep",  16);
> +    qdev_prop_set_uint16(dev, "data_buffer_dep",  256);
> +
> +    busdev = SYS_BUS_DEVICE(dev);
> +    sysbus_realize_and_unref(busdev, &error_fatal);
> +    sysbus_mmio_map(busdev, 0, base);
> +
> +    for (i = 0; i < 9; ++i) {
> +        sysbus_connect_irq(busdev, i, qdev_get_gpio_in(vms->gic, irq + i));
> +    }
> +
> +    nodename = g_strdup_printf("/pl330@%" PRIx64, base);
> +    qemu_fdt_add_subnode(ms->fdt, nodename);
> +    qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, 
> sizeof(compat));
> +    qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
> +                                 2, base, 2, size);
> +    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
> +                    GIC_FDT_IRQ_TYPE_SPI, irq, GIC_FDT_IRQ_FLAGS_LEVEL_HI,
> +                    GIC_FDT_IRQ_TYPE_SPI, irq + 1, 
> GIC_FDT_IRQ_FLAGS_LEVEL_HI,
> +                    GIC_FDT_IRQ_TYPE_SPI, irq + 2, 
> GIC_FDT_IRQ_FLAGS_LEVEL_HI,
> +                    GIC_FDT_IRQ_TYPE_SPI, irq + 3, 
> GIC_FDT_IRQ_FLAGS_LEVEL_HI,
> +                    GIC_FDT_IRQ_TYPE_SPI, irq + 4, 
> GIC_FDT_IRQ_FLAGS_LEVEL_HI,
> +                    GIC_FDT_IRQ_TYPE_SPI, irq + 5, 
> GIC_FDT_IRQ_FLAGS_LEVEL_HI,
> +                    GIC_FDT_IRQ_TYPE_SPI, irq + 6, 
> GIC_FDT_IRQ_FLAGS_LEVEL_HI,
> +                    GIC_FDT_IRQ_TYPE_SPI, irq + 7, 
> GIC_FDT_IRQ_FLAGS_LEVEL_HI,
> +                    GIC_FDT_IRQ_TYPE_SPI, irq + 8, 
> GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> +
> +    qemu_fdt_setprop(ms->fdt, nodename, "interrupt-names", irq_names,
> +                     sizeof(irq_names));
> +
> +    qemu_fdt_setprop_cell(ms->fdt, nodename, "clocks", vms->clock_phandle);
> +    qemu_fdt_setprop_string(ms->fdt, nodename, "clock-names", "apb_pclk");
> +
> +    if (vms->iommu == VIRT_IOMMU_SMMUV3 && vms->iommu_phandle) {
> +        qemu_fdt_setprop_cells(ms->fdt, nodename, "iommus",
> +                               vms->iommu_phandle, sid);
> +        qemu_fdt_setprop(ms->fdt, nodename, "dma-coherent", NULL, 0);
> +    }
> +
> +    g_free(nodename);
> +}
>  static void create_rtc(const VirtMachineState *vms)
>  {
>      char *nodename;
> @@ -2081,6 +2169,8 @@ static void machvirt_init(MachineState *machine)
>  
>      create_pcie(vms);
>  
> +    create_dma(vms);
> +
>      if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
>          vms->acpi_dev = create_acpi_ged(vms);
>      } else {
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index d3402a43dd..f307b26587 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -72,6 +72,7 @@ enum {
>      VIRT_UART,
>      VIRT_MMIO,
>      VIRT_RTC,
> +    VIRT_DMA,
>      VIRT_FW_CFG,
>      VIRT_PCIE,
>      VIRT_PCIE_MMIO,