Hi Rahul, On 01/12/2022 17:02, Rahul Singh wrote: > > > To configure IOMMU in guest for passthrough devices, user will need to > copy the unmodified "iommus" property from host device tree to partial > device tree. To enable the dom0 linux kernel to confiure the IOMMU > correctly replace the phandle in partial device tree with virtual > IOMMU phandle when "iommus" property is set. > > Signed-off-by: Rahul Singh <[email protected]> > --- > xen/arch/arm/domain_build.c | 31 ++++++++++++++++++++++++++++++- > 1 file changed, 30 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 7cd99a6771..afb3e76409 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -3235,7 +3235,35 @@ static int __init handle_prop_pfdt(struct kernel_info > *kinfo, > return ( propoff != -FDT_ERR_NOTFOUND ) ? propoff : 0; > } > > -static int __init scan_pfdt_node(struct kernel_info *kinfo, const void *pfdt, > +static void modify_pfdt_node(void *pfdt, int nodeoff) > +{ > + int proplen, i, rc; > + const fdt32_t *prop; > + fdt32_t *prop_c; > + > + prop = fdt_getprop(pfdt, nodeoff, "iommus", &proplen); > + if ( !prop ) > + return; > + > + prop_c = xzalloc_bytes(proplen); > + > + for ( i = 0; i < proplen / 8; ++i ) > + { > + prop_c[i * 2] = cpu_to_fdt32(GUEST_PHANDLE_VSMMUV3); > + prop_c[i * 2 + 1] = prop[i * 2 + 1]; > + } > + > + rc = fdt_setprop(pfdt, nodeoff, "iommus", prop_c, proplen); > + if ( rc ) > + { > + dprintk(XENLOG_ERR, "Can't set the iommus property in partial FDT"); > + return; > + } > + > + return; > +} > + > +static int __init scan_pfdt_node(struct kernel_info *kinfo, void *pfdt, > int nodeoff, > uint32_t address_cells, uint32_t size_cells, > bool scan_passthrough_prop) > @@ -3261,6 +3289,7 @@ static int __init scan_pfdt_node(struct kernel_info > *kinfo, const void *pfdt, > node_next = fdt_first_subnode(pfdt, nodeoff); > while ( node_next > 0 ) > { > + modify_pfdt_node(pfdt, node_next); You do not protect this call in any way and there is no check inside it whether viommu is enabled or not. This means that even with viommu disabled, if a user copies the iommus property to partial dtb (which is understandable as the flow is to copy almost everything), you will perform the phandle replacement. I do not think we should do that (no need for an extra code).
> scan_pfdt_node(kinfo, pfdt, node_next, address_cells, size_cells, > scan_passthrough_prop); > node_next = fdt_next_subnode(pfdt, node_next); > -- > 2.25.1 > > ~Michal
