Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Michal,

> On 12 May 2023, at 3:35 pm, Michal Orzel <michal.orzel@xxxxxxx> wrote:
> 
> At the moment, even in case of a SMMU being I/O coherent, we clean the
> updated PT as a result of not advertising the coherency feature. SMMUv3
> coherency feature means that page table walks, accesses to memory
> structures and queues are I/O coherent (refer ARM IHI 0070 E.A, 3.15).
> 
> Follow the same steps that were done for SMMU v1,v2 driver by the commit:
> 080dcb781e1bc3bb22f55a9dfdecb830ccbabe88
> 
> The same restrictions apply, meaning that in order to advertise coherent
> table walk platform feature, all the SMMU devices need to report coherency
> feature. This is because the page tables (we are sharing them with CPU)
> are populated before any device assignment and in case of a device being
> behind non-coherent SMMU, we would have to scan the tables and clean
> the cache.
> 
> It is to be noted that the SBSA/BSA (refer ARM DEN0094C 1.0C, section D)
> requires that all SMMUv3 devices support I/O coherency.
> 
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
> There are very few platforms out there with SMMUv3 but I have never seen
> a SMMUv3 that is not I/O coherent.
> ---
> xen/drivers/passthrough/arm/smmu-v3.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
> b/xen/drivers/passthrough/arm/smmu-v3.c
> index bf053cdb6d5c..2adaad0fa038 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -2526,6 +2526,15 @@ static const struct dt_device_match 
> arm_smmu_of_match[] = {
> };
> 
> /* Start of Xen specific code. */
> +
> +/*
> + * Platform features. It indicates the list of features supported by all
> + * SMMUs. Actually we only care about coherent table walk, which in case of
> + * SMMUv3 is implied by the overall coherency feature (refer ARM IHI 0070 
> E.A,
> + * section 3.15 and SMMU_IDR0.COHACC bit description).
> + */
> +static uint32_t platform_features = ARM_SMMU_FEAT_COHERENCY;
> +
> static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
> {
>       struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> @@ -2708,8 +2717,12 @@ static int arm_smmu_iommu_xen_domain_init(struct 
> domain *d)
>       INIT_LIST_HEAD(&xen_domain->contexts);
> 
>       dom_iommu(d)->arch.priv = xen_domain;
> -     return 0;
> 
> +     /* Coherent walk can be enabled only when all SMMUs support it. */
> +     if (platform_features & ARM_SMMU_FEAT_COHERENCY)
> +             iommu_set_feature(d, IOMMU_FEAT_COHERENT_WALK);
> +
> +     return 0;
> }
> 
> static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
> @@ -2738,6 +2751,7 @@ static __init int arm_smmu_dt_init(struct 
> dt_device_node *dev,
>                               const void *data)
> {
>       int rc;
> +     const struct arm_smmu_device *smmu;
> 
>       /*
>        * Even if the device can't be initialized, we don't want to
> @@ -2751,6 +2765,14 @@ static __init int arm_smmu_dt_init(struct 
> dt_device_node *dev,
> 
>       iommu_set_ops(&arm_smmu_iommu_ops);
> 
> +     /* Find the just added SMMU and retrieve its features. */
> +     smmu = arm_smmu_get_by_dev(dt_to_dev(dev));
> +
> +     /* It would be a bug not to find the SMMU we just added. */
> +     BUG_ON(!smmu);
> +
> +     platform_features &= smmu->features;
> +
>       return 0;
> }
> 
> -- 
> 2.25.1
>