Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
On 15/05/2023 10:07, Michal Orzel wrote: Reviewed-by: Julien Grall Thanks, Bare in mind that Rahul responded in HTML so there will be a Thanks for the reminder! The patch is now committed. Cheers, -- Julien Grall
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
On 15/05/2023 11:06, Julien Grall wrote:
>
>
> On 15/05/2023 10:03, Michal Orzel wrote:
>> Hi Julien,
>
> Hi Michal,
>
>>
>> On 15/05/2023 10:56, Julien Grall wrote:
>>>
>>>
>>> Hi,
>>>
>>> On 12/05/2023 15:35, Michal Orzel 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
---
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;
>>>
>>> AFAICT, this variable is not meant to change after boot. So please add
>>> the attribute __ro_after_init.
>> Yes, that makes total sense. After probing this variable is not meant to be
>> modified.
>> Is it something that can be done on commit or would you want me to respin
>> this patch?
>
> I can do it on commit. With that:
>
> Reviewed-by: Julien Grall
Thanks,
Bare in mind that Rahul responded in HTML so there will be a
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
On 15/05/2023 10:03, Michal Orzel wrote:
Hi Julien,
Hi Michal,
On 15/05/2023 10:56, Julien Grall wrote:
Hi,
On 12/05/2023 15:35, Michal Orzel 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
---
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;
AFAICT, this variable is not meant to change after boot. So please add
the attribute __ro_after_init.
Yes, that makes total sense. After probing this variable is not meant to be
modified.
Is it something that can be done on commit or would you want me to respin this
patch?
I can do it on commit. With that:
Reviewed-by: Julien Grall
Cheers,
--
Julien Grall
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
Hi Julien,
On 15/05/2023 10:56, Julien Grall wrote:
>
>
> Hi,
>
> On 12/05/2023 15:35, Michal Orzel 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
>> ---
>> 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;
>
> AFAICT, this variable is not meant to change after boot. So please add
> the attribute __ro_after_init.
Yes, that makes total sense. After probing this variable is not meant to be
modified.
Is it something that can be done on commit or would you want me to respin this
patch?
~Michal
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
On 15/05/2023 07:46, Michal Orzel wrote:
Hi Ayan,
Hi Michal,
On 12/05/2023 18:59, Ayan Kumar Halder wrote:
Hi Michal,
On 12/05/2023 15:35, Michal Orzel 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
Reviewed-by: Ayan Kumar Halder
---
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;
}
All good till here.
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;
+
Can you explain this change in the commit message ?
I think it is already explained by saying that in order to advertise the
*platform* feature, all
SMMUs need to report it. If at least one doesn't, the feature is disabled. This
is exactly
what this line is doing. It is comparing platform features with a just probed
SMMU features (arm_smmu_dt_init()
is called for each SMMU).
All good.
~Michal
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
Hi,
On 12/05/2023 15:35, Michal Orzel 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
---
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;
AFAICT, this variable is not meant to change after boot. So please add
the attribute __ro_after_init.
With that:
Reviewed-by: Julien Grall
Cheers,
--
Julien Grall
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
Hi Michal, On 12 May 2023, at 3:35 pm, Michal Orzel 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 Reviewed-by: Rahul Singh mailto:rahul.si...@arm.com>> Regards, Rahul
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
Hi Michal,
> On 12 May 2023, at 3:35 pm, Michal Orzel 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
> ---
> 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
>
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
Hi Ayan,
On 12/05/2023 18:59, Ayan Kumar Halder wrote:
> Hi Michal,
>
> On 12/05/2023 15:35, Michal Orzel 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
>> ---
>> 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;
>> }
>>
> All good till here.
>> 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;
>> +
>
> Can you explain this change in the commit message ?
I think it is already explained by saying that in order to advertise the
*platform* feature, all
SMMUs need to report it. If at least one doesn't, the feature is disabled. This
is exactly
what this line is doing. It is comparing platform features with a just probed
SMMU features (arm_smmu_dt_init()
is called for each SMMU).
~Michal
Re: [PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
Hi Michal,
On 12/05/2023 15:35, Michal Orzel 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
---
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;
}
All good till here.
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;
+
Can you explain this change in the commit message ?
- Ayan
return 0;
}
[PATCH 2/2] xen/arm: smmuv3: Advertise coherent table walk if supported
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
---
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
