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
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(+)
>
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 <[email protected]>
Signed-off-by: Kunkun Jiang <[email protected]>
---
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
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(+)
>>
> .