Re: [PATCH v4 13/22] xen/arm: vsmmuv3: IOMMU device tree node for dom0

[Milan Djokic <milan_djokic@xxxxxxxx>]

Hi Oleksandr,

On 10/10/26 17:30, Oleksandr Tyshchenko wrote:
> On 10/7/26 03:07, Milan Djokic wrote:
>
> Hello Milan
>
> > From: Rahul Singh <rahul.singh@xxxxxxx>
> >
> > XEN will create an IOMMU device tree node in the device tree
> > to enable the dom0 to discover the virtual SMMUv3 during dom0 boot.
> > IOMMU device tree node will only be created when viommu is enabled
> > via Kconfig build options.
> >
> > "iommus" property will be added for dom0 devices to virtual
> > IOMMU node to enable the dom0 linux kernel to configure the IOMMU.
> >
> > Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> > Signed-off-by: Milan Djokic <milan_djokic@xxxxxxxx>
> > ---
> >    xen/arch/arm/domain_build.c       | 102 +++++++++++++++++++++++++++++-
> >    xen/arch/arm/include/asm/viommu.h |   1 +
> >    2 files changed, 101 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index cd2e09f005..6513140133 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -557,9 +557,12 @@ static int __init write_properties(struct domain *d, 
> > struct kernel_info *kinfo,
> >                continue;
> >            }
> >    
> > -        if ( iommu_node )
> > +        /*
> > +         * Expose IOMMU specific properties to hwdom when vIOMMU is
> > +         * enabled.
> > +         */
> > +        if ( iommu_node && viommu_get_type() == XEN_DOMCTL_CONFIG_VIOMMU_NONE )
> >            {
> > -            /* Don't expose IOMMU specific properties to hwdom */
> >                if ( dt_property_name_is_equal(prop, "iommus") )
> >                    continue;
> >    
> > @@ -1457,6 +1460,95 @@ int __init make_timer_node(const struct kernel_info *kinfo)
> >        return res;
> >    }
> >    
> > +#ifdef CONFIG_ARM_VIRTUAL_IOMMU
> > +static int __init make_hwdom_viommu_node(const struct kernel_info *kinfo)
> > +{
> > +    uint32_t len;
> > +    int res;
> > +    char buf[24];
> > +    void *fdt = kinfo->fdt;
> > +    const void *prop = NULL;
> > +    const struct dt_device_node *iommu = NULL;
> > +    struct host_iommu *iommu_data;
> > +    gic_interrupt_t intr;
> > +
> > +    if ( list_empty(&host_iommu_list) )
> > +        return 0;
> > +
> > +    list_for_each_entry( iommu_data, &host_iommu_list, entry )
> > +    {
> > +        if ( iommu_data->hwdom_node_created )
> > +            return 0;
> > +
> > +        iommu = iommu_data->dt_node;
> > +
> > +        snprintf(buf, sizeof(buf), "iommu@%"PRIx64, iommu_data->addr);
> > +
> > +        res = fdt_begin_node(fdt, buf);
> > +        if ( res )
> > +            return res;
> > +
> > +        prop = dt_get_property(iommu, "compatible", &len);
> > +        if ( !prop )
> > +        {
> > +            res = -FDT_ERR_XEN(ENOENT);
> > +            return res;
> > +        }
> > +
> > +        res = fdt_property(fdt, "compatible", prop, len);
> > +        if ( res )
> > +            return res;
> > +
> > +        if ( iommu->phandle )
> > +        {
> > +            res = fdt_property_cell(fdt, "phandle", iommu->phandle);
> > +            if ( res )
> > +                return res;
> > +        }
> > +
> > +        /* Use the same reg regions as the IOMMU node in host DTB. */
> > +        prop = dt_get_property(iommu, "reg", &len);
> > +        if ( !prop )
> > +        {
> > +            printk(XENLOG_ERR "vIOMMU: Can't find IOMMU reg property.\n");
> > +            res = -FDT_ERR_XEN(ENOENT);
> > +            return res;
> > +        }
> > +
> > +        res = fdt_property(fdt, "reg", prop, len);
> > +        if ( res )
> > +            return res;
> > +
> > +        prop = dt_get_property(iommu, "#iommu-cells", &len);
> > +        if ( !prop )
> > +        {
> > +            res = -FDT_ERR_XEN(ENOENT);
> > +            return res;
> > +        }
> > +
> > +        res = fdt_property(fdt, "#iommu-cells", prop, len);
> > +        if ( res )
> > +            return res;
> > +
> > +        res = fdt_property_string(fdt, "interrupt-names", "combined");
> > +        if ( res )
> > +            return res;
> > +
> > +        set_interrupt(intr, iommu_data->irq, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
> > +
> > +        res = fdt_property_interrupts(kinfo, &intr, 1);
> > +        if ( res )
> > +            return res;
> > +
> > +        iommu_data->hwdom_node_created = true;
> > +
> > +        fdt_end_node(fdt);
> > +    }
> > +
> > +    return res;
> > +}
> > +#endif
> > +
> >    static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> >                                  struct dt_device_node *node,
> >                                  p2m_type_t p2mt)
> > @@ -1525,6 +1617,12 @@ static int __init handle_node(struct domain *d, struct 
> > kernel_info *kinfo,
> >        if ( dt_match_node(timer_matches, node) )
> >            return make_timer_node(kinfo);
> >    
> > +#ifdef CONFIG_ARM_VIRTUAL_IOMMU
> > +    if ( device_get_class(node) == DEVICE_IOMMU &&
> > +         viommu_get_type() != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
> > +        return make_hwdom_viommu_node(kinfo);
> So all vIOMMU nodes are created at the position of the first pIOMMU
> node, right? If so, could it be possible in real life for IOMMUs to sit
> under different buses (under different parent nodes)?
Yes, for hardware domains, all vIOMMU nodes are inserted at the position 
of the first pIOMMU node.
If pIOMMUs are located under different buses, the guest DT hierarchy 
will differ from the host DT hierarchy, with all vIOMMU nodes placed 
under the same parent. This is not necessarily a problem. However, the 
``reg`` property is copied from the original pIOMMU node and may 
therefore contain an address relative to its parent bus, whereas the 
host IOMMU list and the registered MMIO regions use the translated 
physical address obtained during the SMMUv3 driver probe. This 
inconsistency could prevent the guest from accessing the vIOMMUs correctly.
I'll change the implementation so that vIOMMU nodes are created under 
their corresponding parent buses, preserving the original pIOMMU 
hierarchy from the host DT.
Thank you for noticing this.

> > +#endif
> > +
> >        /* Skip nodes used by Xen */
> >        if ( dt_device_used_by(node) == DOMID_XEN )
> >        {
> > diff --git a/xen/arch/arm/include/asm/viommu.h 
> > b/xen/arch/arm/include/asm/viommu.h
> > index 844cf34d56..5ae21042f0 100644
> > --- a/xen/arch/arm/include/asm/viommu.h
> > +++ b/xen/arch/arm/include/asm/viommu.h
> > @@ -20,6 +20,7 @@ struct host_iommu {
> >        paddr_t size;
> >        /* Used for dom0 EVTQ and GERROR interrupts (1-1 IRQ mapping) */
> >        uint32_t irq;
> > +    bool hwdom_node_created;
> >    };
> >    
> >    struct viommu_ops {
BR,
Milan