diff mbox series

dmaengine: idxd: don't load pasid config until needed

Message ID 164642891995.190921.7082046343492065429.stgit@djiang5-desk3.ch.intel.com (mailing list archive)
State Superseded
Headers show
Series dmaengine: idxd: don't load pasid config until needed | expand

Commit Message

Dave Jiang March 4, 2022, 9:21 p.m. UTC 
The driver currently programs the system pasid to the WQ preemptively when
system pasid is enabled. Given that a dwq will reprogram the pasid and
possibly a different pasid, the programming is not necessary. The pasid_en
bit can be set for swq as it does not need pasid pasid programming but
needs the pasid_en bit. Remove system pasid programming on device config
write. Add pasid programming for kernel wq type on wq driver enable. The
char dev driver already reprograms the dwq on ->open() call so there's no
change.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/dma/idxd/device.c    |   46 ++++++++++++++++++++++++++++++++++--------
 drivers/dma/idxd/registers.h |    1 +
 2 files changed, 38 insertions(+), 9 deletions(-)

Comments

Dave Jiang March 10, 2022, 4:14 p.m. UTC | #1 
On 3/4/2022 2:21 PM, Dave Jiang wrote:
> The driver currently programs the system pasid to the WQ preemptively when
> system pasid is enabled. Given that a dwq will reprogram the pasid and
> possibly a different pasid, the programming is not necessary. The pasid_en
> bit can be set for swq as it does not need pasid pasid programming but
> needs the pasid_en bit. Remove system pasid programming on device config
> write. Add pasid programming for kernel wq type on wq driver enable. The
> char dev driver already reprograms the dwq on ->open() call so there's no
> change.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Vinod, pls ignore this patch. There will be a v2.


> ---
>   drivers/dma/idxd/device.c    |   46 ++++++++++++++++++++++++++++++++++--------
>   drivers/dma/idxd/registers.h |    1 +
>   2 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
> index 573ad8b86804..b2aaf4b64a8b 100644
> --- a/drivers/dma/idxd/device.c
> +++ b/drivers/dma/idxd/device.c
> @@ -299,16 +299,25 @@ void idxd_wqs_unmap_portal(struct idxd_device *idxd)
>   	}
>   }
>   
> -int idxd_wq_set_pasid(struct idxd_wq *wq, int pasid)
> +static void __idxd_wq_set_priv_locked(struct idxd_wq *wq)
>   {
>   	struct idxd_device *idxd = wq->idxd;
> -	int rc;
>   	union wqcfg wqcfg;
>   	unsigned int offset;
>   
> -	rc = idxd_wq_disable(wq, false);
> -	if (rc < 0)
> -		return rc;
> +	offset = WQCFG_OFFSET(idxd, wq->id, WQCFG_PRIVL_IDX);
> +	spin_lock(&idxd->dev_lock);
> +	wqcfg.bits[WQCFG_PRIVL_IDX] = ioread32(idxd->reg_base + offset);
> +	wqcfg.priv = 1;
> +	iowrite32(wqcfg.bits[WQCFG_PRIVL_IDX], idxd->reg_base + offset);
> +	spin_unlock(&idxd->dev_lock);
> +}
> +
> +static void __idxd_wq_set_pasid_locked(struct idxd_wq *wq, int pasid)
> +{
> +	struct idxd_device *idxd = wq->idxd;
> +	union wqcfg wqcfg;
> +	unsigned int offset;
>   
>   	offset = WQCFG_OFFSET(idxd, wq->id, WQCFG_PASID_IDX);
>   	spin_lock(&idxd->dev_lock);
> @@ -317,6 +326,17 @@ int idxd_wq_set_pasid(struct idxd_wq *wq, int pasid)
>   	wqcfg.pasid = pasid;
>   	iowrite32(wqcfg.bits[WQCFG_PASID_IDX], idxd->reg_base + offset);
>   	spin_unlock(&idxd->dev_lock);
> +}
> +
> +int idxd_wq_set_pasid(struct idxd_wq *wq, int pasid)
> +{
> +	int rc;
> +
> +	rc = idxd_wq_disable(wq, false);
> +	if (rc < 0)
> +		return rc;
> +
> +	__idxd_wq_set_pasid_locked(wq, pasid);
>   
>   	rc = idxd_wq_enable(wq);
>   	if (rc < 0)
> @@ -808,11 +828,13 @@ static int idxd_wq_config_write(struct idxd_wq *wq)
>   	if (wq_dedicated(wq))
>   		wq->wqcfg->mode = 1;
>   
> -	if (device_pasid_enabled(idxd)) {
> +	/*
> +	 * Enable this for shared WQ. swq does not need to program the pasid field,
> +	 * but pasid_en needs to be set. Programming here prevents swq needing to
> +	 * be disabled and re-enabled for reprogramming, which is something to avoid.
> +	 */
> +	if (device_pasid_enabled(idxd))
>   		wq->wqcfg->pasid_en = 1;
> -		if (wq->type == IDXD_WQT_KERNEL && wq_dedicated(wq))
> -			wq->wqcfg->pasid = idxd->pasid;
> -	}
>   
>   	/*
>   	 * Here the priv bit is set depending on the WQ type. priv = 1 if the
> @@ -1258,6 +1280,12 @@ int __drv_enable_wq(struct idxd_wq *wq)
>   		}
>   	}
>   
> +	if (is_idxd_wq_kernel(wq)) {
> +		if (device_pasid_enabled(idxd) && wq_dedicated(wq))
> +			__idxd_wq_set_pasid_locked(wq, idxd->pasid);
> +		__idxd_wq_set_priv_locked(wq);
> +	}
> +
>   	rc = 0;
>   	spin_lock(&idxd->dev_lock);
>   	if (test_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags))
> diff --git a/drivers/dma/idxd/registers.h b/drivers/dma/idxd/registers.h
> index aa642aecdc0b..02449aa9c454 100644
> --- a/drivers/dma/idxd/registers.h
> +++ b/drivers/dma/idxd/registers.h
> @@ -353,6 +353,7 @@ union wqcfg {
>   } __packed;
>   
>   #define WQCFG_PASID_IDX                2
> +#define WQCFG_PRIVL_IDX		2
>   #define WQCFG_OCCUP_IDX		6
>   
>   #define WQCFG_OCCUP_MASK	0xffff
>
>
diff mbox series

Patch

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 573ad8b86804..b2aaf4b64a8b 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -299,16 +299,25 @@  void idxd_wqs_unmap_portal(struct idxd_device *idxd)
 	}
 }
 
-int idxd_wq_set_pasid(struct idxd_wq *wq, int pasid)
+static void __idxd_wq_set_priv_locked(struct idxd_wq *wq)
 {
 	struct idxd_device *idxd = wq->idxd;
-	int rc;
 	union wqcfg wqcfg;
 	unsigned int offset;
 
-	rc = idxd_wq_disable(wq, false);
-	if (rc < 0)
-		return rc;
+	offset = WQCFG_OFFSET(idxd, wq->id, WQCFG_PRIVL_IDX);
+	spin_lock(&idxd->dev_lock);
+	wqcfg.bits[WQCFG_PRIVL_IDX] = ioread32(idxd->reg_base + offset);
+	wqcfg.priv = 1;
+	iowrite32(wqcfg.bits[WQCFG_PRIVL_IDX], idxd->reg_base + offset);
+	spin_unlock(&idxd->dev_lock);
+}
+
+static void __idxd_wq_set_pasid_locked(struct idxd_wq *wq, int pasid)
+{
+	struct idxd_device *idxd = wq->idxd;
+	union wqcfg wqcfg;
+	unsigned int offset;
 
 	offset = WQCFG_OFFSET(idxd, wq->id, WQCFG_PASID_IDX);
 	spin_lock(&idxd->dev_lock);
@@ -317,6 +326,17 @@  int idxd_wq_set_pasid(struct idxd_wq *wq, int pasid)
 	wqcfg.pasid = pasid;
 	iowrite32(wqcfg.bits[WQCFG_PASID_IDX], idxd->reg_base + offset);
 	spin_unlock(&idxd->dev_lock);
+}
+
+int idxd_wq_set_pasid(struct idxd_wq *wq, int pasid)
+{
+	int rc;
+
+	rc = idxd_wq_disable(wq, false);
+	if (rc < 0)
+		return rc;
+
+	__idxd_wq_set_pasid_locked(wq, pasid);
 
 	rc = idxd_wq_enable(wq);
 	if (rc < 0)
@@ -808,11 +828,13 @@  static int idxd_wq_config_write(struct idxd_wq *wq)
 	if (wq_dedicated(wq))
 		wq->wqcfg->mode = 1;
 
-	if (device_pasid_enabled(idxd)) {
+	/*
+	 * Enable this for shared WQ. swq does not need to program the pasid field,
+	 * but pasid_en needs to be set. Programming here prevents swq needing to
+	 * be disabled and re-enabled for reprogramming, which is something to avoid.
+	 */
+	if (device_pasid_enabled(idxd))
 		wq->wqcfg->pasid_en = 1;
-		if (wq->type == IDXD_WQT_KERNEL && wq_dedicated(wq))
-			wq->wqcfg->pasid = idxd->pasid;
-	}
 
 	/*
 	 * Here the priv bit is set depending on the WQ type. priv = 1 if the
@@ -1258,6 +1280,12 @@  int __drv_enable_wq(struct idxd_wq *wq)
 		}
 	}
 
+	if (is_idxd_wq_kernel(wq)) {
+		if (device_pasid_enabled(idxd) && wq_dedicated(wq))
+			__idxd_wq_set_pasid_locked(wq, idxd->pasid);
+		__idxd_wq_set_priv_locked(wq);
+	}
+
 	rc = 0;
 	spin_lock(&idxd->dev_lock);
 	if (test_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags))
diff --git a/drivers/dma/idxd/registers.h b/drivers/dma/idxd/registers.h
index aa642aecdc0b..02449aa9c454 100644
--- a/drivers/dma/idxd/registers.h
+++ b/drivers/dma/idxd/registers.h
@@ -353,6 +353,7 @@  union wqcfg {
 } __packed;
 
 #define WQCFG_PASID_IDX                2
+#define WQCFG_PRIVL_IDX		2
 #define WQCFG_OCCUP_IDX		6
 
 #define WQCFG_OCCUP_MASK	0xffff