[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V2] libxl/arm: Create specific IOMMU node to be referred by virtio-mmio device
On Tue, 31 May 2022, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx> > > Reuse generic IOMMU device tree bindings to communicate Xen specific > information for the virtio devices for which the restricted memory > access using Xen grant mappings need to be enabled. > > Insert "iommus" property pointed to the IOMMU node with "xen,grant-dma" > compatible to all virtio devices which backends are going to run in > non-hardware domains (which are non-trusted by default). > > Based on device-tree binding from Linux: > Documentation/devicetree/bindings/iommu/xen,grant-dma.yaml > > The example of generated nodes: > > xen_iommu { > compatible = "xen,grant-dma"; > #iommu-cells = <0x01>; > phandle = <0xfde9>; > }; > > virtio@2000000 { > compatible = "virtio,mmio"; > reg = <0x00 0x2000000 0x00 0x200>; > interrupts = <0x00 0x01 0xf01>; > interrupt-parent = <0xfde8>; > dma-coherent; > iommus = <0xfde9 0x01>; > }; > > virtio@2000200 { > compatible = "virtio,mmio"; > reg = <0x00 0x2000200 0x00 0x200>; > interrupts = <0x00 0x02 0xf01>; > interrupt-parent = <0xfde8>; > dma-coherent; > iommus = <0xfde9 0x01>; > }; > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx> > --- > !!! This patch is based on non upstreamed yet “Virtio support for toolstack > on Arm” V8 series which is on review now: > https://lore.kernel.org/xen-devel/1651598763-12162-1-git-send-email-olekstysh@xxxxxxxxx/ > > New device-tree binding (commit #5) is a part of solution to restrict memory > access under Xen using xen-grant DMA-mapping layer (which is also on review): > https://lore.kernel.org/xen-devel/1653944417-17168-1-git-send-email-olekstysh@xxxxxxxxx/ > > Changes RFC -> V1: > - update commit description > - rebase according to the recent changes to > "libxl: Introduce basic virtio-mmio support on Arm" > > Changes V1 -> V2: > - Henry already gave his Reviewed-by, I dropped it due to the changes > - use generic IOMMU device tree bindings instead of custom property > "xen,dev-domid" > - change commit subject and description, was > "libxl/arm: Insert "xen,dev-domid" property to virtio-mmio device node" > --- > tools/libs/light/libxl_arm.c | 49 > ++++++++++++++++++++++++++++++++--- > xen/include/public/device_tree_defs.h | 1 + > 2 files changed, 47 insertions(+), 3 deletions(-) > > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c > index 9be9b2a..72da3b1 100644 > --- a/tools/libs/light/libxl_arm.c > +++ b/tools/libs/light/libxl_arm.c > @@ -865,9 +865,32 @@ static int make_vpci_node(libxl__gc *gc, void *fdt, > return 0; > } > > +static int make_xen_iommu_node(libxl__gc *gc, void *fdt) > +{ > + int res; > + > + /* See Linux Documentation/devicetree/bindings/iommu/xen,grant-dma.yaml > */ > + res = fdt_begin_node(fdt, "xen_iommu"); > + if (res) return res; > + > + res = fdt_property_compat(gc, fdt, 1, "xen,grant-dma"); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "#iommu-cells", 1); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_IOMMU); > + if (res) return res; > + > + res = fdt_end_node(fdt); > + if (res) return res; > + > + return 0; > +} > > static int make_virtio_mmio_node(libxl__gc *gc, void *fdt, > - uint64_t base, uint32_t irq) > + uint64_t base, uint32_t irq, > + uint32_t backend_domid) > { > int res; > gic_interrupt intr; > @@ -890,6 +913,16 @@ static int make_virtio_mmio_node(libxl__gc *gc, void > *fdt, > res = fdt_property(fdt, "dma-coherent", NULL, 0); > if (res) return res; > > + if (backend_domid != LIBXL_TOOLSTACK_DOMID) { > + uint32_t iommus_prop[2]; > + > + iommus_prop[0] = cpu_to_fdt32(GUEST_PHANDLE_IOMMU); > + iommus_prop[1] = cpu_to_fdt32(backend_domid); > + > + res = fdt_property(fdt, "iommus", iommus_prop, sizeof(iommus_prop)); > + if (res) return res; > + } > + > res = fdt_end_node(fdt); > if (res) return res; > > @@ -1097,6 +1130,7 @@ static int libxl__prepare_dtb(libxl__gc *gc, > libxl_domain_config *d_config, > size_t fdt_size = 0; > int pfdt_size = 0; > libxl_domain_build_info *const info = &d_config->b_info; > + bool iommu_created; > unsigned int i; > > const libxl_version_info *vers; > @@ -1204,11 +1238,20 @@ next_resize: > if (d_config->num_pcidevs) > FDT( make_vpci_node(gc, fdt, ainfo, dom) ); > > + iommu_created = false; > for (i = 0; i < d_config->num_disks; i++) { > libxl_device_disk *disk = &d_config->disks[i]; > > - if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) > - FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) ); > + if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) { > + if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID && > + !iommu_created) { > + FDT( make_xen_iommu_node(gc, fdt) ); > + iommu_created = true; > + } > + > + FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq, > + disk->backend_domid) ); > + } This is a matter of taste as the code would also work as is, but I would do the following instead: if ( d_config->num_disks > 0 && disk->backend_domid != LIBXL_TOOLSTACK_DOMID) { FDT( make_xen_iommu_node(gc, fdt) ); } for (i = 0; i < d_config->num_disks; i++) { libxl_device_disk *disk = &d_config->disks[i]; if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) ); } but I would give my acked-by anyway > } > > if (pfdt) > diff --git a/xen/include/public/device_tree_defs.h > b/xen/include/public/device_tree_defs.h > index 209d43d..df58944 100644 > --- a/xen/include/public/device_tree_defs.h > +++ b/xen/include/public/device_tree_defs.h > @@ -7,6 +7,7 @@ > * onwards. Reserve a high value for the GIC phandle. > */ > #define GUEST_PHANDLE_GIC (65000) > +#define GUEST_PHANDLE_IOMMU (GUEST_PHANDLE_GIC + 1) > > #define GUEST_ROOT_ADDRESS_CELLS 2 > #define GUEST_ROOT_SIZE_CELLS 2 > -- > 2.7.4 >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |