[RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()

[RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()

[Kunkun Jiang]

Hi all,

This set of patches solves some errors when I tested the SMMU nested mode.

Test scenario description:
guest kernel: 4KB translation granule
host kernel: 16KB translation granule

errors:
1. encountered an endless loop in __arm_smmu_tlb_inv_range because
num_pages is 0
2. encountered CERROR_ILL because the fields of TLB invalidation
command are as follow: TG = 2, NUM = 0, SCALE = 0, TTL = 0. The
combination is exactly the kind of reserved combination pointed
out in the SMMUv3 spec(page 143-144, version D.a)

In my opinion, it is more appropriate to add parameter check in
__arm_smmu_tlb_inv_range(), although these problems only appeared
when I tested the SMMU nested mode. What do you think?

This series include patches as below:
Patch 1:
- align the invalid range with leaf page size upwards when smmu
supports RIL

Patch 2:
- add a check to standardize granule size when smmu supports RIL

Kunkun Jiang (2):
  iommu/arm-smmu-v3: Align invalid range with leaf page size upwards
    when support RIL
  iommu/arm-smmu-v3: Standardize granule size when support RIL

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++++++++
 1 file changed, 9 insertions(+)

-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH v1 1/2] iommu/arm-smmu-v3: Align invalid range with leaf page size upwards when support RIL

[Kunkun Jiang]

In the __arm_smmu_tlb_inv_range(), the fileds of TLBI CMD is
calculated based on the invalid range and the leaf page size,
when SMMU supports RIL. It will cause some errors when the
invalid range isn't aligned with the leaf page size.

1. The num_pages will be zero, if the invalid range is less
than b. Then it will enter an endless loop in
__arm_smmu_tlb_inv_range().
2. The actual invalid range will only be part of the invalid
range. If the invalid range is not an integral multiple of
the leaf page size.

To align invalid range with leaf page size upwards will solve
the two issues.

Reported-by: Nianyao Tang <tangnianyao@huawei.com>
Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 54b2f27b81d4..8a2cacbb1ef8 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1703,7 +1703,9 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
 
 	if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
 		/* Get the leaf page size */
+		size_t leaf_pgsize;
 		tg = __ffs(smmu_domain->domain.pgsize_bitmap);
+		leaf_pgsize = 1 << tg;
 
 		/* Convert page size of 12,14,16 (log2) to 1,2,3 */
 		cmd->tlbi.tg = (tg - 10) / 2;
@@ -1711,6 +1713,8 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
 		/* Determine what level the granule is at */
 		cmd->tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3));
 
+		/* Align size with the leaf page size upwards */
+		size = ALIGN(size, leaf_pgsize);
 		num_pages = size >> tg;
 	}
 
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH v1 2/2] iommu/arm-smmu-v3: Standardize granule size when support RIL

[Kunkun Jiang]

In __arm_smmu_tlb_inv_range(), the field 'ttl' of TLB invalidation
command is caculated based on granule size when the SMMU supports
RIL. There are some scenarious we need to avoid, which are pointed
out in the SMMUv3 spec(page 143-144, Version D.a). Adding a check
to ensure that the granule size is supported by the SMMU before set
the 'ttl' value.

Reported-by: Nianyao Tang <tangnianyao@huawei.com>
Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 8a2cacbb1ef8..77ff283ca329 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1711,6 +1711,11 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
 		cmd->tlbi.tg = (tg - 10) / 2;
 
 		/* Determine what level the granule is at */
+		if (!(granule & smmu_domain->domain.pgsize_bitmap) ||
+		    (granule & (granule - 1))) {
+			granule = leaf_pgsize;
+			iova = ALIGN_DOWN(iova, leaf_pgsize);
+		}
 		cmd->tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3));
 
 		/* Align size with the leaf page size upwards */
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()

[Robin Murphy]

On 2021-05-19 10:43, Kunkun Jiang wrote:
> Hi all,
> 
> This set of patches solves some errors when I tested the SMMU nested mode.
> 
> Test scenario description:
> guest kernel: 4KB translation granule
> host kernel: 16KB translation granule
> 
> errors:
> 1. encountered an endless loop in __arm_smmu_tlb_inv_range because
> num_pages is 0
> 2. encountered CERROR_ILL because the fields of TLB invalidation
> command are as follow: TG = 2, NUM = 0, SCALE = 0, TTL = 0. The
> combination is exactly the kind of reserved combination pointed
> out in the SMMUv3 spec(page 143-144, version D.a)
> 
> In my opinion, it is more appropriate to add parameter check in
> __arm_smmu_tlb_inv_range(), although these problems only appeared
> when I tested the SMMU nested mode. What do you think?

FWIW I think it would be better to fix the caller to not issue broken 
commands in the first place. The kernel shouldn't do so for itself (and 
definitely needs fixing if it ever does), so it sounds like the nesting 
implementation needs to do a bit more validation of what it's passing 
through.

Robin.

> This series include patches as below:
> Patch 1:
> - align the invalid range with leaf page size upwards when smmu
> supports RIL
> 
> Patch 2:
> - add a check to standardize granule size when smmu supports RIL
> 
> Kunkun Jiang (2):
>    iommu/arm-smmu-v3: Align invalid range with leaf page size upwards
>      when support RIL
>    iommu/arm-smmu-v3: Standardize granule size when support RIL
> 
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()

[Kunkun Jiang]

Hi Robin,

On 2021/5/19 18:01, Robin Murphy wrote:
> On 2021-05-19 10:43, Kunkun Jiang wrote:
>> Hi all,
>>
>> This set of patches solves some errors when I tested the SMMU nested 
>> mode.
>>
>> Test scenario description:
>> guest kernel: 4KB translation granule
>> host kernel: 16KB translation granule
>>
>> errors:
>> 1. encountered an endless loop in __arm_smmu_tlb_inv_range because
>> num_pages is 0
>> 2. encountered CERROR_ILL because the fields of TLB invalidation
>> command are as follow: TG = 2, NUM = 0, SCALE = 0, TTL = 0. The
>> combination is exactly the kind of reserved combination pointed
>> out in the SMMUv3 spec(page 143-144, version D.a)
>>
>> In my opinion, it is more appropriate to add parameter check in
>> __arm_smmu_tlb_inv_range(), although these problems only appeared
>> when I tested the SMMU nested mode. What do you think?
>
> FWIW I think it would be better to fix the caller to not issue broken 
> commands in the first place. The kernel shouldn't do so for itself 
> (and definitely needs fixing if it ever does), so it sounds like the 
> nesting implementation needs to do a bit more validation of what it's 
> passing through.
Thanks for your reply.
I will report these errors to Eric and discuss how to fix them.

Thanks,
Kunkun Jiang
>
> Robin.
>
>> This series include patches as below:
>> Patch 1:
>> - align the invalid range with leaf page size upwards when smmu
>> supports RIL
>>
>> Patch 2:
>> - add a check to standardize granule size when smmu supports RIL
>>
>> Kunkun Jiang (2):
>>    iommu/arm-smmu-v3: Align invalid range with leaf page size upwards
>>      when support RIL
>>    iommu/arm-smmu-v3: Standardize granule size when support RIL
>>
>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
> .



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel