Skip to contentSkip to conversation

iommu/tegra241-cmdqv: Fix initialization and uAPI related to HYP_OWN

PATCH Applied Nicolin Chen 5 participants 9 messages Mar 13 – Mar 24

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover iommu/tegra241-cmdqv: Fix initialization and uAPI related to HYP_OWN

Hi,

Eric and Shameer found a bug in kernel driver that supports_cmd is set in an incorrect order related to the hyp_own finalization. This results in a unnecessary command filter in the host environment that sends unsupported (non-invalidation) commands to the SMMU CMDQ.

Also, Eric pointed it out that the requirement of VMM wiring HYP_OWN to 0 is unclear.

Thus, I am sending this small series to address these issues.

Nicolin Chen (2):
iommu/tegra241-cmdqv: Set supports_cmd op in tegra241_vcmdq_hw_init() iommu/tegra241-cmdqv: Update uAPI to clarify HYP_OWN requirement

 include/uapi/linux/iommufd.h                   | 5 +++++
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 7 ++++---
 2 files changed, 9 insertions(+), 3 deletions(-)
Signature · 1 line
2.43.0
#1 lore raw

PATCH 1/2 iommu/tegra241-cmdqv: Set supports_cmd op in tegra241_vcmdq_hw_init()

vintf->hyp_own is finalized in tegra241_vintf_hw_init(). On the other hand, tegra241_vcmdq_alloc_smmu_cmdq() is called via an init_structures callback, which is earlier than tegra241_vintf_hw_init().

This results in the supports_cmd op always being set to the guest function, although this doesn't break any functionality nor have some noticeable perf impact since non-invalidation commands are not issued in the perf sensitive context.

Fix this by moving supports_cmd to tegra241_vcmdq_hw_init().

After this change,
- For a guest kernel, this will be a status quo
- For a host kernel, non-invalidation commands will be issued to VCMDQ(s)

---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +4 -3
index 6fe5563eaf9eb..83f6e9f6c51d6 100644
@@ -479,6 +479,10 @@ static int tegra241_vcmdq_hw_init(struct tegra241_vcmdq *vcmdq)
 	/* Reset VCMDQ */
 	tegra241_vcmdq_hw_deinit(vcmdq);
 
+	/* vintf->hyp_own is a HW state finalized in tegra241_vintf_hw_init() */
+	if (!vcmdq->vintf->hyp_own)
+		vcmdq->cmdq.supports_cmd = tegra241_guest_vcmdq_supports_cmd;
+
 	/* Configure and enable VCMDQ */
 	writeq_relaxed(vcmdq->cmdq.q.q_base, REG_VCMDQ_PAGE1(vcmdq, BASE));
 
@@ -639,9 +643,6 @@ static int tegra241_vcmdq_alloc_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
 	q->q_base = q->base_dma & VCMDQ_ADDR;
 	q->q_base |= FIELD_PREP(VCMDQ_LOG2SIZE, q->llq.max_n_shift);
 
-	if (!vcmdq->vintf->hyp_own)
-		cmdq->supports_cmd = tegra241_guest_vcmdq_supports_cmd;
-
 	return arm_smmu_cmdq_init(smmu, cmdq);
 }
 
Signature · 1 line
2.43.0
Tested-by on v1 1/2
#2 lore raw

RE: [PATCH 1/2] iommu/tegra241-cmdqv: Set supports_cmd op in tegra241_vcmdq_hw_init()

Quoted 30 lines

-----Original Message-----
From: Nicolin Chen <[email protected]>
Sent: 13 March 2026 00:37
To: [email protected]; [email protected]; Jason Gunthorpe <[email protected]>; [email protected]
Cc: [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; linux- [email protected]; [email protected]; Shameer Kolothum Thodi <[email protected]>
Subject: [PATCH 1/2] iommu/tegra241-cmdqv: Set supports_cmd op in tegra241_vcmdq_hw_init()

vintf->hyp_own is finalized in tegra241_vintf_hw_init(). On the other hand, tegra241_vcmdq_alloc_smmu_cmdq() is called via an init_structures callback, which is earlier than tegra241_vintf_hw_init().

This results in the supports_cmd op always being set to the guest function, although this doesn't break any functionality nor have some noticeable perf impact since non-invalidation commands are not issued in the perf sensitive context.

Fix this by moving supports_cmd to tegra241_vcmdq_hw_init().

After this change,
- For a guest kernel, this will be a status quo
- For a host kernel, non-invalidation commands will be issued to VCMDQ(s)

Fixes a9d40285bdef ("iommu/tegra241-cmdqv: Limit CMDs for VCMDQs of a
Quoted 8 lines

guest owned VINTF")

devel/CH3PR12MB754836BEE54E39B30C7210C0AB44A@CH3PR12MB7548 .namprd12.prod.outlook.com/

---

Ran few tests with QEMU toggling HYP_OWN bit, and this now enforces that QEMU sets HYP_OWN for VCMDQ to work properly.

Thanks,
Shameer

Quoted 37 lines
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +4 -0
index 6fe5563eaf9eb..83f6e9f6c51d6 100644
@@ -479,6 +479,10 @@ static int tegra241_vcmdq_hw_init(struct
 tegra241_vcmdq *vcmdq)
 	/* Reset VCMDQ */
 	tegra241_vcmdq_hw_deinit(vcmdq);
 
+	/* vintf->hyp_own is a HW state finalized in tegra241_vintf_hw_init()
 */
+	if (!vcmdq->vintf->hyp_own)
+		vcmdq->cmdq.supports_cmd =
 tegra241_guest_vcmdq_supports_cmd;
+
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +0 -0
 	/* Configure and enable VCMDQ */
 	writeq_relaxed(vcmdq->cmdq.q.q_base,

REG_VCMDQ_PAGE1(vcmdq, BASE));

drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +0 -3
@@ -639,9 +643,6 @@ static int tegra241_vcmdq_alloc_smmu_cmdq(struct
 tegra241_vcmdq *vcmdq)
 	q->q_base = q->base_dma & VCMDQ_ADDR;
 	q->q_base |= FIELD_PREP(VCMDQ_LOG2SIZE, q->llq.max_n_shift);
 
-	if (!vcmdq->vintf->hyp_own)
-		cmdq->supports_cmd =
 tegra241_guest_vcmdq_supports_cmd;
-
 	return arm_smmu_cmdq_init(smmu, cmdq);

}

--
2.43.0

Reviewed-by on v1 1/2
#3 lore raw

Re: [PATCH 1/2] iommu/tegra241-cmdqv: Set supports_cmd op in tegra241_vcmdq_hw_init()

In reply to Nicolin Chen

Hi Nicolin,

Quoted 15 lines · Nicolin Chen

vintf->hyp_own is finalized in tegra241_vintf_hw_init(). On the other hand, tegra241_vcmdq_alloc_smmu_cmdq() is called via an init_structures callback, which is earlier than tegra241_vintf_hw_init().

This results in the supports_cmd op always being set to the guest function, although this doesn't break any functionality nor have some noticeable perf impact since non-invalidation commands are not issued in the perf sensitive context.

Fix this by moving supports_cmd to tegra241_vcmdq_hw_init().

After this change,
- For a guest kernel, this will be a status quo
- For a host kernel, non-invalidation commands will be issued to VCMDQ(s)

Quoted 5 lines

Eric

Quoted 29 lines
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +4 -3
index 6fe5563eaf9eb..83f6e9f6c51d6 100644
@@ -479,6 +479,10 @@ static int tegra241_vcmdq_hw_init(struct tegra241_vcmdq *vcmdq)
 	/* Reset VCMDQ */
 	tegra241_vcmdq_hw_deinit(vcmdq);
 
+	/* vintf->hyp_own is a HW state finalized in tegra241_vintf_hw_init() */
+	if (!vcmdq->vintf->hyp_own)
+		vcmdq->cmdq.supports_cmd = tegra241_guest_vcmdq_supports_cmd;
+
 	/* Configure and enable VCMDQ */
 	writeq_relaxed(vcmdq->cmdq.q.q_base, REG_VCMDQ_PAGE1(vcmdq, BASE));
 
@@ -639,9 +643,6 @@ static int tegra241_vcmdq_alloc_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
 	q->q_base = q->base_dma & VCMDQ_ADDR;
 	q->q_base |= FIELD_PREP(VCMDQ_LOG2SIZE, q->llq.max_n_shift);
 
-	if (!vcmdq->vintf->hyp_own)
-		cmdq->supports_cmd = tegra241_guest_vcmdq_supports_cmd;
-
 	return arm_smmu_cmdq_init(smmu, cmdq);
 }
 
#4 lore raw

PATCH 2/2 iommu/tegra241-cmdqv: Update uAPI to clarify HYP_OWN requirement

In reply to Nicolin Chen

From hardware implementation perspective, a guest tegra241-cmdqv hardware is different than the host hardware:
- Host HW is backed by a VINTF (HYP_OWN=1)
- Guest HW is backed by a VINTF (HYP_OWN=0)

The kernel driver has an implementation requirement of the HYP_OWN bit in the VM. So, VMM must follow that to allow the same copy of Linux to work.

Add this requirement to the uAPI, which is currently missing.

Fixes 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
---
 include/uapi/linux/iommufd.h | 5 +++++
 1 file changed, 5 insertions(+)
include/uapi/linux/iommufd.h +5 -0
index 1dafbc552d37d..f63edbe71d542 100644
@@ -1052,6 +1052,11 @@ struct iommu_fault_alloc {
 enum iommu_viommu_type {
 	IOMMU_VIOMMU_TYPE_DEFAULT = 0,
 	IOMMU_VIOMMU_TYPE_ARM_SMMUV3 = 1,
+	/*
+	 * TEGRA241_CMDQV requirements (otherwise, VCMDQs will not work)
+	 * - Kernel will allocate a VINTF (HYP_OWN=0) to back this VIOMMU. So,
+	 *   VMM must wire the HYP_OWN bit to 0 in guest VINTF_CONFIG register
+	 */
 	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV = 2,
 };
 
Signature · 1 line
2.43.0
maintainer
#5 lore raw

Re: [PATCH 2/2] iommu/tegra241-cmdqv: Update uAPI to clarify HYP_OWN requirement

Quoted 21 lines · Nicolin Chen

From hardware implementation perspective, a guest tegra241-cmdqv hardware is different than the host hardware:
- Host HW is backed by a VINTF (HYP_OWN=1)
- Guest HW is backed by a VINTF (HYP_OWN=0)

The kernel driver has an implementation requirement of the HYP_OWN bit in the VM. So, VMM must follow that to allow the same copy of Linux to work.

Add this requirement to the uAPI, which is currently missing.

Fixes 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
---
 include/uapi/linux/iommufd.h | 5 +++++
 1 file changed, 5 insertions(+)
include/uapi/linux/iommufd.h +0 -0
index 1dafbc552d37d..f63edbe71d542 100644

include/uapi/linux/iommufd.h struct iommu_fault_alloc { · line 1061

Quoted 10 lines
include/uapi/linux/iommufd.h +5 -0
@@ -1052,6 +1052,11 @@ struct iommu_fault_alloc {
 enum iommu_viommu_type {
 	IOMMU_VIOMMU_TYPE_DEFAULT = 0,
 	IOMMU_VIOMMU_TYPE_ARM_SMMUV3 = 1,
+	/*
+	 * TEGRA241_CMDQV requirements (otherwise, VCMDQs will not work)
+	 * - Kernel will allocate a VINTF (HYP_OWN=0) to back this VIOMMU. So,
+	 *   VMM must wire the HYP_OWN bit to 0 in guest VINTF_CONFIG register
+	 */
 	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV = 2,
 };

Jason, are you happy with this one?

Will

maintainer Reviewed-by
#6 lore raw

Re: [PATCH 2/2] iommu/tegra241-cmdqv: Update uAPI to clarify HYP_OWN requirement

Quoted 26 lines · Will Deacon
Quoted 25 lines · Nicolin Chen

From hardware implementation perspective, a guest tegra241-cmdqv hardware is different than the host hardware:
- Host HW is backed by a VINTF (HYP_OWN=1)
- Guest HW is backed by a VINTF (HYP_OWN=0)

The kernel driver has an implementation requirement of the HYP_OWN bit in the VM. So, VMM must follow that to allow the same copy of Linux to work.

Add this requirement to the uAPI, which is currently missing.

Fixes 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
---
 include/uapi/linux/iommufd.h | 5 +++++
 1 file changed, 5 insertions(+)
include/uapi/linux/iommufd.h +1 -0
index 1dafbc552d37d..f63edbe71d542 100644
@@ -1052,6 +1052,11 @@ struct iommu_fault_alloc {
 enum iommu_viommu_type {
 	IOMMU_VIOMMU_TYPE_DEFAULT = 0,
 	IOMMU_VIOMMU_TYPE_ARM_SMMUV3 = 1,
+	/*
Quoted 8 lines

include/uapi/linux/iommufd.h struct iommu_fault_alloc { · line 1061

Quoted 6 lines
include/uapi/linux/iommufd.h +4 -0
+	 * TEGRA241_CMDQV requirements (otherwise, VCMDQs will not work)
+	 * - Kernel will allocate a VINTF (HYP_OWN=0) to back this VIOMMU. So,
+	 *   VMM must wire the HYP_OWN bit to 0 in guest VINTF_CONFIG register
+	 */
 	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV = 2,
 };

Jason, are you happy with this one?

Yes, please pick up the series

Jason

Reviewed-by on v1 2/2
#7 lore raw

Re: [PATCH 2/2] iommu/tegra241-cmdqv: Update uAPI to clarify HYP_OWN requirement

In reply to Nicolin Chen
Quoted 5 lines · Nicolin Chen

From hardware implementation perspective, a guest tegra241-cmdqv hardware is different than the host hardware:
- Host HW is backed by a VINTF (HYP_OWN=1)
- Guest HW is backed by a VINTF (HYP_OWN=0)

Quoted 7 lines

The kernel driver has an implementation requirement of the HYP_OWN bit in the VM. So, VMM must follow that to allow the same copy of Linux to work.

Add this requirement to the uAPI, which is currently missing.

Fixes 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")

Eric

Quoted 20 lines
---
 include/uapi/linux/iommufd.h | 5 +++++
 1 file changed, 5 insertions(+)
include/uapi/linux/iommufd.h +5 -0
index 1dafbc552d37d..f63edbe71d542 100644
@@ -1052,6 +1052,11 @@ struct iommu_fault_alloc {
 enum iommu_viommu_type {
 	IOMMU_VIOMMU_TYPE_DEFAULT = 0,
 	IOMMU_VIOMMU_TYPE_ARM_SMMUV3 = 1,
+	/*
+	 * TEGRA241_CMDQV requirements (otherwise, VCMDQs will not work)
+	 * - Kernel will allocate a VINTF (HYP_OWN=0) to back this VIOMMU. So,
+	 *   VMM must wire the HYP_OWN bit to 0 in guest VINTF_CONFIG register
+	 */
 	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV = 2,
 };
 
maintainer Applied
#8 lore raw
In reply to Nicolin Chen
Quoted 9 lines · Nicolin Chen

Eric and Shameer found a bug in kernel driver that supports_cmd is set in an incorrect order related to the hyp_own finalization. This results in a unnecessary command filter in the host environment that sends unsupported (non-invalidation) commands to the SMMU CMDQ.

Also, Eric pointed it out that the requirement of VMM wiring HYP_OWN to 0 is unclear.

[...]

Applied to iommu (arm/smmu/updates), thanks!

[1/2] iommu/tegra241-cmdqv: Set supports_cmd op in tegra241_vcmdq_hw_init()

      https://git.kernel.org/iommu/c/803e41f36d22
[2/2] iommu/tegra241-cmdqv: Update uAPI to clarify HYP_OWN requirement
      https://git.kernel.org/iommu/c/9dcef98dbee3

Cheers,

Signature · 5 lines
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev