1. Patchew
  2. linux
  3. [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2)
  4. View patch

[PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd

Nicolin Chen posted 6 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd
Posted by Nicolin Chen 1 year, 7 months ago 
There is an existing arm_smmu_cmdq_build_sync_cmd() so the driver should
call it at all places other than going through arm_smmu_cmdq_build_cmd()
separately. This helps the following patch that adds a CS_NONE quirk for
tegra241-cmdqv driver.

Note that this changes the type of CMD_SYNC in __arm_smmu_cmdq_skip_err,
for ARM_SMMU_OPT_MSIPOLL=true cases, from previously a non-MSI one to an
MSI one that is proven to still work by a hacking test:
  nvme: Adding to iommu group 10
  nvme: --------hacking-----------
  arm-smmu-v3: unexpected global error reported (0x00000001),
               this could be serious
  arm-smmu-v3: CMDQ error (cons 0x01000022): Illegal command
  arm-smmu-v3: skipping command in error state:
  arm-smmu-v3:  0x0000000000000000
  arm-smmu-v3:  0x0000000000000000
  nvme: -------recovered----------
  nvme nvme0: 72/0/0 default/read/poll queues
   nvme0n1: p1 p2

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 36 ++++++++-------------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  3 --
 2 files changed, 13 insertions(+), 26 deletions(-)

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 3d2962f6dbda..569bca1a8a1f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -325,16 +325,6 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
 		cmd[0] |= FIELD_PREP(CMDQ_RESUME_0_RESP, ent->resume.resp);
 		cmd[1] |= FIELD_PREP(CMDQ_RESUME_1_STAG, ent->resume.stag);
 		break;
-	case CMDQ_OP_CMD_SYNC:
-		if (ent->sync.msiaddr) {
-			cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_IRQ);
-			cmd[1] |= ent->sync.msiaddr & CMDQ_SYNC_1_MSIADDR_MASK;
-		} else {
-			cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_SEV);
-		}
-		cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_MSH, ARM_SMMU_SH_ISH);
-		cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_MSIATTR, ARM_SMMU_MEMATTR_OIWB);
-		break;
 	default:
 		return -ENOENT;
 	}
@@ -350,20 +340,23 @@ static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
 static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device *smmu,
 					 struct arm_smmu_queue *q, u32 prod)
 {
-	struct arm_smmu_cmdq_ent ent = {
-		.opcode = CMDQ_OP_CMD_SYNC,
-	};
+	memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
+	cmd[0] = FIELD_PREP(CMDQ_0_OP, CMDQ_OP_CMD_SYNC) |
+		 FIELD_PREP(CMDQ_SYNC_0_MSH, ARM_SMMU_SH_ISH) |
+		 FIELD_PREP(CMDQ_SYNC_0_MSIATTR, ARM_SMMU_MEMATTR_OIWB);
+
+	if (!(smmu->options & ARM_SMMU_OPT_MSIPOLL)) {
+		cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_SEV);
+		return;
+	}
 
 	/*
 	 * Beware that Hi16xx adds an extra 32 bits of goodness to its MSI
 	 * payload, so the write will zero the entire command on that platform.
 	 */
-	if (smmu->options & ARM_SMMU_OPT_MSIPOLL) {
-		ent.sync.msiaddr = q->base_dma + Q_IDX(&q->llq, prod) *
-				   q->ent_dwords * 8;
-	}
-
-	arm_smmu_cmdq_build_cmd(cmd, &ent);
+	cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_IRQ);
+	cmd[1] = (q->base_dma + Q_IDX(&q->llq, prod) * q->ent_dwords * 8) &
+		 CMDQ_SYNC_1_MSIADDR_MASK;
 }
 
 void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
@@ -380,9 +373,6 @@ void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
 	u64 cmd[CMDQ_ENT_DWORDS];
 	u32 cons = readl_relaxed(q->cons_reg);
 	u32 idx = FIELD_GET(CMDQ_CONS_ERR, cons);
-	struct arm_smmu_cmdq_ent cmd_sync = {
-		.opcode = CMDQ_OP_CMD_SYNC,
-	};
 
 	dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
 		idx < ARRAY_SIZE(cerror_str) ?  cerror_str[idx] : "Unknown");
@@ -416,7 +406,7 @@ void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
 		dev_err(smmu->dev, "\t0x%016llx\n", (unsigned long long)cmd[i]);
 
 	/* Convert the erroneous command into a CMD_SYNC */
-	arm_smmu_cmdq_build_cmd(cmd, &cmd_sync);
+	arm_smmu_cmdq_build_sync_cmd(cmd, smmu, q, cons);
 
 	queue_write(Q_ENT(q, cons), cmd, q->ent_dwords);
 }
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 8e4fbf4f50f3..180c0b1e0658 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -512,9 +512,6 @@ struct arm_smmu_cmdq_ent {
 		} resume;
 
 		#define CMDQ_OP_CMD_SYNC	0x46
-		struct {
-			u64			msiaddr;
-		} sync;
 	};
 };
 
-- 
2.43.0
Re: [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd
Posted by Jason Gunthorpe 1 year, 7 months ago 
On Tue, May 07, 2024 at 10:56:51PM -0700, Nicolin Chen wrote:
> There is an existing arm_smmu_cmdq_build_sync_cmd() so the driver should
> call it at all places other than going through arm_smmu_cmdq_build_cmd()
> separately. This helps the following patch that adds a CS_NONE quirk for
> tegra241-cmdqv driver.
> 
> Note that this changes the type of CMD_SYNC in __arm_smmu_cmdq_skip_err,
> for ARM_SMMU_OPT_MSIPOLL=true cases, from previously a non-MSI one to an
> MSI one that is proven to still work by a hacking test:
>   nvme: Adding to iommu group 10
>   nvme: --------hacking-----------
>   arm-smmu-v3: unexpected global error reported (0x00000001),
>                this could be serious
>   arm-smmu-v3: CMDQ error (cons 0x01000022): Illegal command
>   arm-smmu-v3: skipping command in error state:
>   arm-smmu-v3:  0x0000000000000000
>   arm-smmu-v3:  0x0000000000000000
>   nvme: -------recovered----------
>   nvme nvme0: 72/0/0 default/read/poll queues
>    nvme0n1: p1 p2

Nice
 
> @@ -350,20 +340,23 @@ static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
>  static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device *smmu,
>  					 struct arm_smmu_queue *q, u32 prod)
>  {
> -	struct arm_smmu_cmdq_ent ent = {
> -		.opcode = CMDQ_OP_CMD_SYNC,
> -	};
> +	memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);

The command would also benifit from its own type someday :\

Maybe this should just be cmd[1] = 0 ?

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason
Re: [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd
Posted by Nicolin Chen 1 year, 7 months ago 
On Sun, May 12, 2024 at 12:39:43PM -0300, Jason Gunthorpe wrote:
> On Tue, May 07, 2024 at 10:56:51PM -0700, Nicolin Chen wrote:
> > There is an existing arm_smmu_cmdq_build_sync_cmd() so the driver should
> > call it at all places other than going through arm_smmu_cmdq_build_cmd()
> > separately. This helps the following patch that adds a CS_NONE quirk for
> > tegra241-cmdqv driver.
> > 
> > Note that this changes the type of CMD_SYNC in __arm_smmu_cmdq_skip_err,
> > for ARM_SMMU_OPT_MSIPOLL=true cases, from previously a non-MSI one to an
> > MSI one that is proven to still work by a hacking test:
> >   nvme: Adding to iommu group 10
> >   nvme: --------hacking-----------
> >   arm-smmu-v3: unexpected global error reported (0x00000001),
> >                this could be serious
> >   arm-smmu-v3: CMDQ error (cons 0x01000022): Illegal command
> >   arm-smmu-v3: skipping command in error state:
> >   arm-smmu-v3:  0x0000000000000000
> >   arm-smmu-v3:  0x0000000000000000
> >   nvme: -------recovered----------
> >   nvme nvme0: 72/0/0 default/read/poll queues
> >    nvme0n1: p1 p2
> 
> Nice
>  
> > @@ -350,20 +340,23 @@ static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
> >  static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device *smmu,
> >  					 struct arm_smmu_queue *q, u32 prod)
> >  {
> > -	struct arm_smmu_cmdq_ent ent = {
> > -		.opcode = CMDQ_OP_CMD_SYNC,
> > -	};
> > +	memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
> 
> The command would also benifit from its own type someday :\
> 
> Maybe this should just be cmd[1] = 0 ?

Yes. I will add that.

> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Thanks!
Nicolin

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.