| Message ID | 1629878922-173270-5-git-send-email-chunming_li1234@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/smmuv3: Support non PCI/PCIe devices | expand |
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,
> -----Original Message----- > From: Eric Auger [mailto:eric.auger@redhat.com] > Sent: Tuesday, August 31, 2021 10:37 PM > To: chunming; peter.maydell@linaro.org > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, > Renwei; Li, Chunming > Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and > connect with SMMU v3 > > 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? I want this to be applied upstream. > > This static SID allocation may not work in general as it may collide > with PCIe RID space? I think SMMU support different devices connected with 1 SMMU and share same SID, even one is PCIe device and another one is peripheral platform device. They can share 1 SMMU page table and get right data translation. > > 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. Yes, these patches could support other platform devices connected with SMMUv3. They only should follow PL330 example to connect their memory region with SMMUv3 peripheral IOMMU memory region. > I guess this should work with virtio platform devices and VFIO platform > devices. How would you extend that work to those devices? I didn't get your point. I think virtio platform device should be Linux kernel SW part. These patches fixed the HW platform devices connection with SMMUv3. Could you help to list one virtio platform device then I can check? > > 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,
Hi, On 9/1/21 8:53 AM, Li, Chunming wrote: > >> -----Original Message----- >> From: Eric Auger [mailto:eric.auger@redhat.com] >> Sent: Tuesday, August 31, 2021 10:37 PM >> To: chunming; peter.maydell@linaro.org >> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, >> Renwei; Li, Chunming >> Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and >> connect with SMMU v3 >> >> 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? > I want this to be applied upstream. Then I think you need to bring a proper motivation behind adding the PL330 in machvirt besides a testing purpose. > >> This static SID allocation may not work in general as it may collide >> with PCIe RID space? > I think SMMU support different devices connected with 1 SMMU and share same > SID, even one is PCIe device and another one is peripheral platform device. > They can share 1 SMMU page table and get right data translation. Indeed I cannot find any statement that a streamid couldn't be used by more than 1 device behing the same smmu. However the 2 devices would be associated to the same context. I think this kind of mapping would be really platform specific and in general it does not make sense to me. what the point using the same context for the PL330 and a virtio-net-pci device for instance? > >> 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. > Yes, these patches could support other platform devices connected with SMMUv3. > They only should follow PL330 example to connect their memory region with SMMUv3 > peripheral IOMMU memory region. > >> I guess this should work with virtio platform devices and VFIO platform >> devices. How would you extend that work to those devices? > I didn't get your point. > I think virtio platform device should be Linux kernel SW part. > These patches fixed the HW platform devices connection with SMMUv3. > Could you help to list one virtio platform device then I can check? After this series you would get a single platform device connected to the SMMU, the PL330. What is the actual use case? By the way what about the virtio-iommu which is also supported in DT mode at the moment? Besides I meant virtio-net-pci and virtio-block-pci are protected by the SMMU. What does happen with their virtio-net-device and virtio-block-device sysbus device counterparts? Then possibly you can assign a VFIO platform device. You may want this latter to protected by the SMMU. How would you handle that case (SMMU is not yet integrated with VFIO but the virtio-iommu is). Thanks Eric > >> 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,
Hi, > -----Original Message----- > From: Eric Auger [mailto:eric.auger@redhat.com] > Sent: Wednesday, September 01, 2021 6:23 PM > To: Li, Chunming; chunming; peter.maydell@linaro.org > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, > Renwei > Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and > connect with SMMU v3 > > Hi, > > On 9/1/21 8:53 AM, Li, Chunming wrote: > > > >> -----Original Message----- > >> From: Eric Auger [mailto:eric.auger@redhat.com] > >> Sent: Tuesday, August 31, 2021 10:37 PM > >> To: chunming; peter.maydell@linaro.org > >> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, > >> Renwei; Li, Chunming > >> Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller > and > >> connect with SMMU v3 > >> > >> 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? > > I want this to be applied upstream. > Then I think you need to bring a proper motivation behind adding the > PL330 in machvirt besides a testing purpose. > > > >> This static SID allocation may not work in general as it may collide > >> with PCIe RID space? > > I think SMMU support different devices connected with 1 SMMU and > share same > > SID, even one is PCIe device and another one is peripheral platform > device. > > They can share 1 SMMU page table and get right data translation. > > Indeed I cannot find any statement that a streamid couldn't be used by > more than 1 device behing the same smmu. However the 2 devices would be > associated to the same context. > I think this kind of mapping would be really platform specific and in > general it does not make sense to me. what the point using the same > context for the PL330 and a virtio-net-pci device for instance? > > > >> 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. > > Yes, these patches could support other platform devices connected > with SMMUv3. > > They only should follow PL330 example to connect their memory region > with SMMUv3 > > peripheral IOMMU memory region. > > > >> I guess this should work with virtio platform devices and VFIO > platform > >> devices. How would you extend that work to those devices? > > I didn't get your point. > > I think virtio platform device should be Linux kernel SW part. > > These patches fixed the HW platform devices connection with SMMUv3. > > Could you help to list one virtio platform device then I can check? > After this series you would get a single platform device connected to > the SMMU, the PL330. What is the actual use case? The actual use case is this: 1. We will have a SoC which has SMMUv3 connected with our owned platform Video Encoder/Decoder and other IPs 2. We plan to use SMMUv3 stage 1 for continuous memory allocation and stage 2 for memory protection 3. We are developing our own IP QEMU models now 4. These models will be connected with SMMUv3 in QEMU 5. We will use the QEMU board to development IP driver and ensure the driver can work well with Linux SMMU and IOMMU framework > By the way what about the virtio-iommu which is also supported in DT > mode at the moment? > > Besides I meant virtio-net-pci and virtio-block-pci are protected by > the > SMMU. What does happen with their virtio-net-device and > virtio-block-device sysbus device counterparts? Then possibly you can > assign a VFIO platform device. You may want this latter to protected by > the SMMU. How would you handle that case (SMMU is not yet integrated > with VFIO but the virtio-iommu is). I get your point but I cannot give you a clear answer now. I didn't consider virtio-iommu before because our current use case doesn't need virtio-iommu. > > Thanks > > Eric > > > >> 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,
Hi, On 9/2/21 8:46 AM, Li, Chunming wrote: > Hi, > >> -----Original Message----- >> From: Eric Auger [mailto:eric.auger@redhat.com] >> Sent: Wednesday, September 01, 2021 6:23 PM >> To: Li, Chunming; chunming; peter.maydell@linaro.org >> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, >> Renwei >> Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and >> connect with SMMU v3 >> >> Hi, >> >> On 9/1/21 8:53 AM, Li, Chunming wrote: >>>> -----Original Message----- >>>> From: Eric Auger [mailto:eric.auger@redhat.com] >>>> Sent: Tuesday, August 31, 2021 10:37 PM >>>> To: chunming; peter.maydell@linaro.org >>>> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, >>>> Renwei; Li, Chunming >>>> Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller >> and >>>> connect with SMMU v3 >>>> >>>> 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? >>> I want this to be applied upstream. >> Then I think you need to bring a proper motivation behind adding the >> PL330 in machvirt besides a testing purpose. >>>> This static SID allocation may not work in general as it may collide >>>> with PCIe RID space? >>> I think SMMU support different devices connected with 1 SMMU and >> share same >>> SID, even one is PCIe device and another one is peripheral platform >> device. >>> They can share 1 SMMU page table and get right data translation. >> Indeed I cannot find any statement that a streamid couldn't be used by >> more than 1 device behing the same smmu. However the 2 devices would be >> associated to the same context. >> I think this kind of mapping would be really platform specific and in >> general it does not make sense to me. what the point using the same >> context for the PL330 and a virtio-net-pci device for instance >>>> 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. >>> Yes, these patches could support other platform devices connected >> with SMMUv3. >>> They only should follow PL330 example to connect their memory region >> with SMMUv3 >>> peripheral IOMMU memory region. >>> >>>> I guess this should work with virtio platform devices and VFIO >> platform >>>> devices. How would you extend that work to those devices? >>> I didn't get your point. >>> I think virtio platform device should be Linux kernel SW part. >>> These patches fixed the HW platform devices connection with SMMUv3. >>> Could you help to list one virtio platform device then I can check? >> After this series you would get a single platform device connected to >> the SMMU, the PL330. What is the actual use case? > The actual use case is this: > 1. We will have a SoC which has SMMUv3 connected with our owned platform > Video Encoder/Decoder and other IPs > 2. We plan to use SMMUv3 stage 1 for continuous memory allocation > and stage 2 for memory protection > 3. We are developing our own IP QEMU models now > 4. These models will be connected with SMMUv3 in QEMU > 5. We will use the QEMU board to development IP driver and ensure the driver > can work well with Linux SMMU and IOMMU framework I see and I understand your use case for system modeling purpose. This raises few questions/comments though. - supporting platform device protection from the vIOMMU/ARM makes sense to me globally. But above use case does not justify (to me) the introduction of PL330 in machvirt because it would be just for testing purpose. Peter may validate/invalidate though. Instead I think you should try to illustrate this feature with DMA capable platform devices such virtio-net and virtio-block sysbus devices as a counterpart of their PCIe flavour. > >> By the way what about the virtio-iommu which is also supported in DT >> mode at the moment? >> >> Besides I meant virtio-net-pci and virtio-block-pci are protected by >> the >> SMMU. What does happen with their virtio-net-device and >> virtio-block-device sysbus device counterparts? Then possibly you can >> assign a VFIO platform device. You may want this latter to protected by >> the SMMU. How would you handle that case (SMMU is not yet integrated >> with VFIO but the virtio-iommu is). > I get your point but I cannot give you a clear answer now. I didn't consider > virtio-iommu before because our current use case doesn't need virtio-iommu. I think you need to have consistency in the machvirt topology: with current series the PL330 would be protected with vSMMUv3 and not with virtio-iommu which does not seem acceptable to me. Either we need to devise a way to individually specify which sysbus device is protected and potentially also specify its streamid or all/none of them are. Thanks Eric > >> Thanks >> >> Eric >>>> 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,
Hi, > -----Original Message----- > From: Eric Auger [mailto:eric.auger@redhat.com] > Sent: Thursday, September 02, 2021 3:46 PM > To: Li, Chunming; chunming; peter.maydell@linaro.org > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, > Renwei > Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and > connect with SMMU v3 > > Hi, > > On 9/2/21 8:46 AM, Li, Chunming wrote: > > Hi, > > > >> -----Original Message----- > >> From: Eric Auger [mailto:eric.auger@redhat.com] > >> Sent: Wednesday, September 01, 2021 6:23 PM > >> To: Li, Chunming; chunming; peter.maydell@linaro.org > >> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, > >> Renwei > >> Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller > and > >> connect with SMMU v3 > >> > >> Hi, > >> > >> On 9/1/21 8:53 AM, Li, Chunming wrote: > >>>> -----Original Message----- > >>>> From: Eric Auger [mailto:eric.auger@redhat.com] > >>>> Sent: Tuesday, August 31, 2021 10:37 PM > >>>> To: chunming; peter.maydell@linaro.org > >>>> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; > Liu, > >>>> Renwei; Li, Chunming > >>>> Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller > >> and > >>>> connect with SMMU v3 > >>>> > >>>> 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? > >>> I want this to be applied upstream. > >> Then I think you need to bring a proper motivation behind adding the > >> PL330 in machvirt besides a testing purpose. > >>>> This static SID allocation may not work in general as it may > collide > >>>> with PCIe RID space? > >>> I think SMMU support different devices connected with 1 SMMU and > >> share same > >>> SID, even one is PCIe device and another one is peripheral platform > >> device. > >>> They can share 1 SMMU page table and get right data translation. > >> Indeed I cannot find any statement that a streamid couldn't be used > by > >> more than 1 device behing the same smmu. However the 2 devices would > be > >> associated to the same context. > >> I think this kind of mapping would be really platform specific and > in > >> general it does not make sense to me. what the point using the same > >> context for the PL330 and a virtio-net-pci device for instance > >>>> 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. > >>> Yes, these patches could support other platform devices connected > >> with SMMUv3. > >>> They only should follow PL330 example to connect their memory > region > >> with SMMUv3 > >>> peripheral IOMMU memory region. > >>> > >>>> I guess this should work with virtio platform devices and VFIO > >> platform > >>>> devices. How would you extend that work to those devices? > >>> I didn't get your point. > >>> I think virtio platform device should be Linux kernel SW part. > >>> These patches fixed the HW platform devices connection with SMMUv3. > >>> Could you help to list one virtio platform device then I can check? > >> After this series you would get a single platform device connected > to > >> the SMMU, the PL330. What is the actual use case? > > The actual use case is this: > > 1. We will have a SoC which has SMMUv3 connected with our owned > platform > > Video Encoder/Decoder and other IPs > > 2. We plan to use SMMUv3 stage 1 for continuous memory allocation > > and stage 2 for memory protection > > 3. We are developing our own IP QEMU models now > > 4. These models will be connected with SMMUv3 in QEMU > > 5. We will use the QEMU board to development IP driver and ensure the > driver > > can work well with Linux SMMU and IOMMU framework > I see and I understand your use case for system modeling purpose. > > This raises few questions/comments though. > - supporting platform device protection from the vIOMMU/ARM makes sense > to me globally. But above use case does not justify (to me) the > introduction of PL330 in machvirt because it would be just for testing > purpose. Peter may validate/invalidate though. Instead I think you > should try to illustrate this feature with DMA capable platform devices > such virtio-net and virtio-block sysbus devices as a counterpart of > their PCIe flavour. Thanks for your suggestion. I will try virtio-net and virtio-block sysbus devices in next step. But I hope to keep PL330 because it's not just for testing purpose. It's a good example to show how to connect platform devices with SMMUv3 based on this patch. I assume other developer may have same requirement. > > > >> By the way what about the virtio-iommu which is also supported in DT > >> mode at the moment? > >> > >> Besides I meant virtio-net-pci and virtio-block-pci are protected by > >> the > >> SMMU. What does happen with their virtio-net-device and > >> virtio-block-device sysbus device counterparts? Then possibly you > can > >> assign a VFIO platform device. You may want this latter to protected > by > >> the SMMU. How would you handle that case (SMMU is not yet integrated > >> with VFIO but the virtio-iommu is). > > I get your point but I cannot give you a clear answer now. I didn't > consider > > virtio-iommu before because our current use case doesn't need virtio- > iommu. > > I think you need to have consistency in the machvirt topology: with > current series the PL330 would be protected with vSMMUv3 and not with > virtio-iommu which does not seem acceptable to me. Either we need to > devise a way to individually specify which sysbus device is protected > and potentially also specify its streamid or all/none of them are. > > Thanks > > Eric Thanks for your suggestion. I created a v6 patch fixed your other feedback. Let us continue the virtio-iommu discussion and I will try it in next step. > > > >> Thanks > >> > >> Eric > >>>> 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,
Hi, > -----Original Message----- > From: Li, Chunming > Sent: Thursday, September 02, 2021 4:23 PM > To: 'eric.auger@redhat.com'; chunming; peter.maydell@linaro.org > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, > Renwei > Subject: RE: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and > connect with SMMU v3 > > Hi, > > > -----Original Message----- > > From: Eric Auger [mailto:eric.auger@redhat.com] > > Sent: Thursday, September 02, 2021 3:46 PM > > To: Li, Chunming; chunming; peter.maydell@linaro.org > > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu, > > Renwei > > Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and > > connect with SMMU v3 > > > > Hi, > > > > On 9/2/21 8:46 AM, Li, Chunming wrote: > > > Hi, > > > > > >> -----Original Message----- > > >> From: Eric Auger [mailto:eric.auger@redhat.com] > > >> Sent: Wednesday, September 01, 2021 6:23 PM > > >> To: Li, Chunming; chunming; peter.maydell@linaro.org > > >> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; > Liu, > > >> Renwei > > >> Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller > > and > > >> connect with SMMU v3 > > >> > > >> Hi, > > >> > > >> On 9/1/21 8:53 AM, Li, Chunming wrote: > > >>>> -----Original Message----- > > >>>> From: Eric Auger [mailto:eric.auger@redhat.com] > > >>>> Sent: Tuesday, August 31, 2021 10:37 PM > > >>>> To: chunming; peter.maydell@linaro.org > > >>>> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; > > Liu, > > >>>> Renwei; Li, Chunming > > >>>> Subject: Re: [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA > controller > > >> and > > >>>> connect with SMMU v3 > > >>>> > > >>>> 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? > > >>> I want this to be applied upstream. > > >> Then I think you need to bring a proper motivation behind adding > the > > >> PL330 in machvirt besides a testing purpose. > > >>>> This static SID allocation may not work in general as it may > > collide > > >>>> with PCIe RID space? > > >>> I think SMMU support different devices connected with 1 SMMU and > > >> share same > > >>> SID, even one is PCIe device and another one is peripheral > platform > > >> device. > > >>> They can share 1 SMMU page table and get right data translation. > > >> Indeed I cannot find any statement that a streamid couldn't be > used > > by > > >> more than 1 device behing the same smmu. However the 2 devices > would > > be > > >> associated to the same context. > > >> I think this kind of mapping would be really platform specific and > > in > > >> general it does not make sense to me. what the point using the > same > > >> context for the PL330 and a virtio-net-pci device for instance > > >>>> 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. > > >>> Yes, these patches could support other platform devices connected > > >> with SMMUv3. > > >>> They only should follow PL330 example to connect their memory > > region > > >> with SMMUv3 > > >>> peripheral IOMMU memory region. > > >>> > > >>>> I guess this should work with virtio platform devices and VFIO > > >> platform > > >>>> devices. How would you extend that work to those devices? > > >>> I didn't get your point. > > >>> I think virtio platform device should be Linux kernel SW part. > > >>> These patches fixed the HW platform devices connection with > SMMUv3. > > >>> Could you help to list one virtio platform device then I can > check? > > >> After this series you would get a single platform device connected > > to > > >> the SMMU, the PL330. What is the actual use case? > > > The actual use case is this: > > > 1. We will have a SoC which has SMMUv3 connected with our owned > > platform > > > Video Encoder/Decoder and other IPs > > > 2. We plan to use SMMUv3 stage 1 for continuous memory allocation > > > and stage 2 for memory protection > > > 3. We are developing our own IP QEMU models now > > > 4. These models will be connected with SMMUv3 in QEMU > > > 5. We will use the QEMU board to development IP driver and ensure > the > > driver > > > can work well with Linux SMMU and IOMMU framework > > I see and I understand your use case for system modeling purpose. > > > > This raises few questions/comments though. > > - supporting platform device protection from the vIOMMU/ARM makes > sense > > to me globally. But above use case does not justify (to me) the > > introduction of PL330 in machvirt because it would be just for > testing > > purpose. Peter may validate/invalidate though. Instead I think you > > should try to illustrate this feature with DMA capable platform > devices > > such virtio-net and virtio-block sysbus devices as a counterpart of > > their PCIe flavour. > > Thanks for your suggestion. I will try virtio-net and virtio-block > sysbus > devices in next step. > But I hope to keep PL330 because it's not just for testing purpose. > It's a good example to show how to connect platform devices with SMMUv3 > based > on this patch. > I assume other developer may have same requirement. > I tried virtio-net-device but looks like it doesn't match the application scenario of my patch series: 1. virtio-net-device is a unique parairtulization QEMU device which connected with virtio mmio bus like this: (qemu) info qom-tree /machine (virt-5.2-machine) /peripheral-anon (container) /device[2] (virtio-net-device) (qemu) qom-get /machine/peripheral-anon/device[2] parent_bus "/machine/unattached/device[43]/virtio-mmio-bus.30" 2. There is no virtio mmio bus or virtio-net-device IP in real HW SoC 3. ARM SMMUv3 IP support platform devices IP like PL330 but current SMMUv3 model cannot do this. 4. My patch series target to solve the #3 issue. 5. I agree we should think about how to update the current SMMUv3 model to support virtio-net-device but I do not think mix these 2 different use cases together is a good idea. > > > > > >> By the way what about the virtio-iommu which is also supported in > DT > > >> mode at the moment? > > >> > > >> Besides I meant virtio-net-pci and virtio-block-pci are protected > by > > >> the > > >> SMMU. What does happen with their virtio-net-device and > > >> virtio-block-device sysbus device counterparts? Then possibly you > > can > > >> assign a VFIO platform device. You may want this latter to > protected > > by > > >> the SMMU. How would you handle that case (SMMU is not yet > integrated > > >> with VFIO but the virtio-iommu is). > > > I get your point but I cannot give you a clear answer now. I didn't > > consider > > > virtio-iommu before because our current use case doesn't need > virtio- > > iommu. > > > > I think you need to have consistency in the machvirt topology: with > > current series the PL330 would be protected with vSMMUv3 and not with > > virtio-iommu which does not seem acceptable to me. Either we need to > > devise a way to individually specify which sysbus device is protected > > and potentially also specify its streamid or all/none of them are. > > > > Thanks > > > > Eric > > Thanks for your suggestion. I created a v6 patch fixed your other > feedback. > Let us continue the virtio-iommu discussion and I will try it in next > step. > I think virtio-iommu is same as virtio-net-device. My patch target to solve the SMMUv3 connected with platform IPs issue. In the real HW SoC, all the platform device IP should be protected by SMMUv3, there is no virtio-iommu IP at all. I assume virtio-iommu and virtio-net-device should be focus on QEMU/KVM virtualization but my target is focus on real HW virtualization. > > > > > >> Thanks > > >> > > >> Eric > > >>>> 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,
On Thu, 2 Sept 2021 at 09:23, Li, Chunming <Chunming.Li@verisilicon.com> wrote: > Eric Auger wrote: >> On 9/2/21 8:46 AM, Li, Chunming wrote: >>> Eric Auger wrote: >>>> Then I think you need to bring a proper motivation behind adding the >>>> PL330 in machvirt besides a testing purpose. >>>> After this series you would get a single platform device connected to >>>> the SMMU, the PL330. What is the actual use case? >>> The actual use case is this: >>> 1. We will have a SoC which has SMMUv3 connected with our owned platform >>> Video Encoder/Decoder and other IPs >>> 2. We plan to use SMMUv3 stage 1 for continuous memory allocation >>> and stage 2 for memory protection >>> 3. We are developing our own IP QEMU models now >>> 4. These models will be connected with SMMUv3 in QEMU >>> 5. We will use the QEMU board to development IP driver and ensure the driver >>> can work well with Linux SMMU and IOMMU framework >> I see and I understand your use case for system modeling purpose. >> >> This raises few questions/comments though. >> - supporting platform device protection from the vIOMMU/ARM makes sense >> to me globally. But above use case does not justify (to me) the >> introduction of PL330 in machvirt because it would be just for testing >> purpose. Peter may validate/invalidate though. Instead I think you >> should try to illustrate this feature with DMA capable platform devices >> such virtio-net and virtio-block sysbus devices as a counterpart of >> their PCIe flavour. > > Thanks for your suggestion. I will try virtio-net and virtio-block sysbus > devices in next step. > But I hope to keep PL330 because it's not just for testing purpose. > It's a good example to show how to connect platform devices with SMMUv3 based > on this patch. > I assume other developer may have same requirement. I agree with Eric here that the 'virt' board is not the right place to put this PL330. The 'virt' board is not intended as a dumping ground or experimental testbed for every possible arm device or system configuration -- we try to keep it at least reasonably targeted towards the "generic virtual platform or VM" usecase. The "sbsa-ref" platform is intended to look more like "real hardware". It's possible that "memory mapped hardware that sits behind an SMMU" is something that you would see on the sort of system that sbsa-ref is trying to model. I've cc'd Leif who would have a better idea about that. thanks -- PMM
Hi Peter, On Tue, Sep 07, 2021 at 12:00:45 +0100, Peter Maydell wrote: > On Thu, 2 Sept 2021 at 09:23, Li, Chunming <Chunming.Li@verisilicon.com> wrote: > > Eric Auger wrote: > >> On 9/2/21 8:46 AM, Li, Chunming wrote: > >>> Eric Auger wrote: > > >>>> Then I think you need to bring a proper motivation behind adding the > >>>> PL330 in machvirt besides a testing purpose. > > >>>> After this series you would get a single platform device connected to > >>>> the SMMU, the PL330. What is the actual use case? > > >>> The actual use case is this: > >>> 1. We will have a SoC which has SMMUv3 connected with our owned platform > >>> Video Encoder/Decoder and other IPs > >>> 2. We plan to use SMMUv3 stage 1 for continuous memory allocation > >>> and stage 2 for memory protection > >>> 3. We are developing our own IP QEMU models now > >>> 4. These models will be connected with SMMUv3 in QEMU > >>> 5. We will use the QEMU board to development IP driver and ensure the driver > >>> can work well with Linux SMMU and IOMMU framework > > >> I see and I understand your use case for system modeling purpose. > >> > >> This raises few questions/comments though. > >> - supporting platform device protection from the vIOMMU/ARM makes sense > >> to me globally. But above use case does not justify (to me) the > >> introduction of PL330 in machvirt because it would be just for testing > >> purpose. Peter may validate/invalidate though. Instead I think you > >> should try to illustrate this feature with DMA capable platform devices > >> such virtio-net and virtio-block sysbus devices as a counterpart of > >> their PCIe flavour. > > > > Thanks for your suggestion. I will try virtio-net and virtio-block sysbus > > devices in next step. > > But I hope to keep PL330 because it's not just for testing purpose. > > It's a good example to show how to connect platform devices with SMMUv3 based > > on this patch. > > I assume other developer may have same requirement. > > I agree with Eric here that the 'virt' board is not the right place > to put this PL330. The 'virt' board is not intended as a dumping > ground or experimental testbed for every possible arm device or > system configuration -- we try to keep it at least reasonably > targeted towards the "generic virtual platform or VM" usecase. Agreed. > The "sbsa-ref" platform is intended to look more like "real hardware". > It's possible that "memory mapped hardware that sits behind an SMMU" > is something that you would see on the sort of system that sbsa-ref > is trying to model. I've cc'd Leif who would have a better idea about > that. I don't know if there's a glovelike fit there, but I agree it's the closest fit of any platform in QEMU today. Most DMA on an SBSA system would normally be expected to come from PCIe, but some have certainly been released with random platform devices, and it's certainly conceivable that some platform devices might wants to make use of DMA. The potential conflict I can see with what this patchset is the use of a qemu-generated DT to describe the hw to the OS, whereas sbbr-ref uses ACPI, generated in-guest with a few hints from QEMU, to abstract it. It looks to me like the hand-in-glove fit for this use-case would be an ebbr-ref platform. (Or I guess SystemReady-ES-ref this week.) But if the ACPI thing isn't a showstopper, I don't particularly mind carrying an extra peripheral or two in the address space. / Leif
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,