All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
@ 2021-05-19  9:43 ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, Keqian Zhu, Zenghui Yu, 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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
@ 2021-05-19  9:43 ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, 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

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
@ 2021-05-19  9:43 ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, Keqian Zhu, Zenghui Yu, 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFC PATCH v1 1/2] iommu/arm-smmu-v3: Align invalid range with leaf page size upwards when support RIL
  2021-05-19  9:43 ` Kunkun Jiang
  (?)
@ 2021-05-19  9:43   ` Kunkun Jiang
  -1 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, Keqian Zhu, Zenghui Yu, 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH v1 1/2] iommu/arm-smmu-v3: Align invalid range with leaf page size upwards when support RIL
@ 2021-05-19  9:43   ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, 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

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH v1 1/2] iommu/arm-smmu-v3: Align invalid range with leaf page size upwards when support RIL
@ 2021-05-19  9:43   ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, Keqian Zhu, Zenghui Yu, 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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH v1 2/2] iommu/arm-smmu-v3: Standardize granule size when support RIL
  2021-05-19  9:43 ` Kunkun Jiang
  (?)
@ 2021-05-19  9:43   ` Kunkun Jiang
  -1 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, Keqian Zhu, Zenghui Yu, 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH v1 2/2] iommu/arm-smmu-v3: Standardize granule size when support RIL
@ 2021-05-19  9:43   ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, 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

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH v1 2/2] iommu/arm-smmu-v3: Standardize granule size when support RIL
@ 2021-05-19  9:43   ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-19  9:43 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang, Keqian Zhu, Zenghui Yu, 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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
  2021-05-19  9:43 ` Kunkun Jiang
  (?)
@ 2021-05-19 10:01   ` Robin Murphy
  -1 siblings, 0 replies; 15+ messages in thread
From: Robin Murphy @ 2021-05-19 10:01 UTC (permalink / raw)
  To: Kunkun Jiang, Will Deacon, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang

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(+)
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
@ 2021-05-19 10:01   ` Robin Murphy
  0 siblings, 0 replies; 15+ messages in thread
From: Robin Murphy @ 2021-05-19 10:01 UTC (permalink / raw)
  To: Kunkun Jiang, Will Deacon, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang

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(+)
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
@ 2021-05-19 10:01   ` Robin Murphy
  0 siblings, 0 replies; 15+ messages in thread
From: Robin Murphy @ 2021-05-19 10:01 UTC (permalink / raw)
  To: Kunkun Jiang, Will Deacon, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
  2021-05-19 10:01   ` Robin Murphy
  (?)
@ 2021-05-21  6:53     ` Kunkun Jiang
  -1 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-21  6:53 UTC (permalink / raw)
  To: Robin Murphy, Will Deacon, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang

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(+)
>>
> .



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
@ 2021-05-21  6:53     ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-21  6:53 UTC (permalink / raw)
  To: Robin Murphy, Will Deacon, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang

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(+)
>>
> .


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range()
@ 2021-05-21  6:53     ` Kunkun Jiang
  0 siblings, 0 replies; 15+ messages in thread
From: Kunkun Jiang @ 2021-05-21  6:53 UTC (permalink / raw)
  To: Robin Murphy, Will Deacon, Eric Auger,
	moderated list:ARM SMMU DRIVERS, open list:IOMMU DRIVERS,
	open list
  Cc: wanghaibin.wang

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-05-21  6:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19  9:43 [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range() Kunkun Jiang
2021-05-19  9:43 ` Kunkun Jiang
2021-05-19  9:43 ` Kunkun Jiang
2021-05-19  9:43 ` [RFC PATCH v1 1/2] iommu/arm-smmu-v3: Align invalid range with leaf page size upwards when support RIL Kunkun Jiang
2021-05-19  9:43   ` Kunkun Jiang
2021-05-19  9:43   ` Kunkun Jiang
2021-05-19  9:43 ` [RFC PATCH v1 2/2] iommu/arm-smmu-v3: Standardize granule size " Kunkun Jiang
2021-05-19  9:43   ` Kunkun Jiang
2021-05-19  9:43   ` Kunkun Jiang
2021-05-19 10:01 ` [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in __arm_smmu_tlb_inv_range() Robin Murphy
2021-05-19 10:01   ` Robin Murphy
2021-05-19 10:01   ` Robin Murphy
2021-05-21  6:53   ` Kunkun Jiang
2021-05-21  6:53     ` Kunkun Jiang
2021-05-21  6:53     ` Kunkun Jiang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.