All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dmaengine: idxd: Only call idxd_enable_system_pasid if succeeded in enabling SVA feature
@ 2022-06-25 22:13 Jerry Snitselaar
  2022-06-26  4:20 ` Fenghua Yu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jerry Snitselaar @ 2022-06-25 22:13 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul, linux-kernel, Dave Jiang, Fenghua Yu

iommu_sva_bind_device requires that iommu_dev_enable_feature has been
previously called with IOMMU_DEV_FEAT_SVA, and succeeded. Without this
it is possible to run into a situation where you will dereference a
null pointer if the intel_iommu driver is not enabled.

Note: checkpatch didn't like the suggested addition of braces for the
      first arm of the "if (idxd_enable_system_pasid)" block.

Fixes: 42a1b73852c4 ("dmaengine: idxd: Separate user and kernel pasid enabling")
Cc: Vinod Koul <vkoul@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 drivers/dma/idxd/init.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 355fb3ef4cbf..5b49fd5c1e25 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -514,13 +514,14 @@ static int idxd_probe(struct idxd_device *idxd)
 	if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
 		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA))
 			dev_warn(dev, "Unable to turn on user SVA feature.\n");
-		else
+		else {
 			set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
 
-		if (idxd_enable_system_pasid(idxd))
-			dev_warn(dev, "No in-kernel DMA with PASID.\n");
-		else
-			set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
+			if (idxd_enable_system_pasid(idxd))
+				dev_warn(dev, "No in-kernel DMA with PASID.\n");
+			else
+				set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
+		}
 	} else if (!sva) {
 		dev_warn(dev, "User forced SVA off via module param.\n");
 	}
-- 
2.36.1


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

* Re: [PATCH] dmaengine: idxd: Only call idxd_enable_system_pasid if succeeded in enabling SVA feature
  2022-06-25 22:13 [PATCH] dmaengine: idxd: Only call idxd_enable_system_pasid if succeeded in enabling SVA feature Jerry Snitselaar
@ 2022-06-26  4:20 ` Fenghua Yu
  2022-06-26  4:51 ` [PATCH v2] dmaengine: idxd: Only call idxd_enable_system_pasid() " Jerry Snitselaar
  2022-06-26  5:16 ` [PATCH v3] " Jerry Snitselaar
  2 siblings, 0 replies; 8+ messages in thread
From: Fenghua Yu @ 2022-06-26  4:20 UTC (permalink / raw)
  To: Jerry Snitselaar; +Cc: dmaengine, Vinod Koul, linux-kernel, Dave Jiang

Hi, Jerry,

On Sat, Jun 25, 2022 at 03:13:33PM -0700, Jerry Snitselaar wrote:

> Subject: [PATCH] dmaengine: idxd: Only call idxd_enable_system_pasid if
>        succeeded in enabling SVA feature

s/idxd_enable_system_pasid/idxd_enable_system_pasid()/


Please add your original error info in the commit message:

On a Sapphire Rapids system if you boot without intel_iommu=on, the IDXD
driver will crash during probe in iommu_sva_bind_device().

[   21.423729] BUG: kernel NULL pointer dereference, address: 0000000000000038 
[   21.445108] #PF: supervisor read access in kernel mode 
[   21.450912] #PF: error_code(0x0000) - not-present page 
[   21.456706] PGD 0  
[   21.459047] Oops: 0000 [#1] PREEMPT SMP NOPTI 
[   21.464004] CPU: 0 PID: 1420 Comm: kworker/0:3 Not tainted 5.19.0-0.rc3.27.eln120.x86_64 #1 
[   21.464011] Hardware name: Intel Corporation EAGLESTREAM/EAGLESTREAM, BIOS EGSDCRB1.SYS.0067.D12.2110190954 10/19/2021 
[   21.464015] Workqueue: events work_for_cpu_fn 
[   21.464030] RIP: 0010:iommu_sva_bind_device+0x1d/0xe0 
[   21.464046] Code: c3 cc 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 57 41 56 49 89 d6 41 55 41 54 55 53 48 83 ec 08 48 8b 87 d8 02 00 00 <48> 8b 40 38 48 8b 50 10 48 83 7a 70 00 48 89 14 24 0f 84 91 00 00 
[   21.464050] RSP: 0018:ff7245d9096b7db8 EFLAGS: 00010296 
[   21.464054] RAX: 0000000000000000 RBX: ff1eadeec8a51000 RCX: 0000000000000000 
[   21.464058] RDX: ff7245d9096b7e24 RSI: 0000000000000000 RDI: ff1eadeec8a510d0 
[   21.464060] RBP: ff1eadeec8a51000 R08: ffffffffb1a12300 R09: ff1eadffbfce25b4 
[   21.464062] R10: ffffffffffffffff R11: 0000000000000038 R12: ffffffffc09f8000 
[   21.464065] R13: ff1eadeec8a510d0 R14: ff7245d9096b7e24 R15: ff1eaddf54429000 
[   21.464067] FS:  0000000000000000(0000) GS:ff1eadee7f600000(0000) knlGS:0000000000000000 
[   21.464070] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033 
[   21.464072] CR2: 0000000000000038 CR3: 00000008c0e10006 CR4: 0000000000771ef0 
[   21.464074] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 
[   21.464076] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 
[   21.464078] PKRU: 55555554 
[   21.464079] Call Trace: 
[   21.464083]  <TASK> 
[   21.464092]  idxd_pci_probe+0x259/0x1070 [idxd] 
[   21.464121]  local_pci_probe+0x3e/0x80 
[   21.464132]  work_for_cpu_fn+0x13/0x20 
[   21.464136]  process_one_work+0x1c4/0x380 
[   21.464143]  worker_thread+0x1ab/0x380 
[   21.464147]  ? _raw_spin_lock_irqsave+0x23/0x50 
[   21.464158]  ? process_one_work+0x380/0x380 
[   21.464161]  kthread+0xe6/0x110 
[   21.464168]  ? kthread_complete_and_exit+0x20/0x20 
[   21.464172]  ret_from_fork+0x1f/0x30

> iommu_sva_bind_device requires that iommu_dev_enable_feature has been
> previously called with IOMMU_DEV_FEAT_SVA, and succeeded. Without this
> it is possible to run into a situation where you will dereference a
> null pointer if the intel_iommu driver is not enabled.
 
How about this commit message?

iommu_sva_bind_device() requires SVA has been enabled successfully on
the IDXD device before it's called. Otherwise, iommu_sva_bind_device()
will access a NULL pointer. If Intel IOMMU is disabled, SVA cannot be
enabled and thus idxd_enable_system_pasid() and iommu_sva_bind_device()
should not be called.

> Note: checkpatch didn't like the suggested addition of braces for the
>       first arm of the "if (idxd_enable_system_pasid)" block.

Please see my comment in the code.

> 
> Fixes: 42a1b73852c4 ("dmaengine: idxd: Separate user and kernel pasid enabling")
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>

Please add bug report link here:
Link: https://lore.kernel.org/dmaengine/20220623170232.6whonfjuh3m5vcoy@cantor/

> ---
>  drivers/dma/idxd/init.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 355fb3ef4cbf..5b49fd5c1e25 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -514,13 +514,14 @@ static int idxd_probe(struct idxd_device *idxd)
>  	if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
>  		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA))

Need balanced braces in if-else.

Add "{" after this "if (...)".

>  			dev_warn(dev, "Unable to turn on user SVA feature.\n");
> -		else
> +		else {

Add "}" before this "else {".

>  			set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
>  
> -		if (idxd_enable_system_pasid(idxd))
> -			dev_warn(dev, "No in-kernel DMA with PASID.\n");
> -		else
> -			set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
> +			if (idxd_enable_system_pasid(idxd))
> +				dev_warn(dev, "No in-kernel DMA with PASID.\n");
> +			else
> +				set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
> +		}
>  	} else if (!sva) {
>  		dev_warn(dev, "User forced SVA off via module param.\n");
>  	}
> -- 
> 2.36.1
> 

Thanks.

-Fenghua

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

* [PATCH v2] dmaengine: idxd: Only call idxd_enable_system_pasid() if succeeded in enabling SVA feature
  2022-06-25 22:13 [PATCH] dmaengine: idxd: Only call idxd_enable_system_pasid if succeeded in enabling SVA feature Jerry Snitselaar
  2022-06-26  4:20 ` Fenghua Yu
@ 2022-06-26  4:51 ` Jerry Snitselaar
  2022-06-26  5:06   ` Fenghua Yu
  2022-06-26  5:16 ` [PATCH v3] " Jerry Snitselaar
  2 siblings, 1 reply; 8+ messages in thread
From: Jerry Snitselaar @ 2022-06-26  4:51 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul, linux-kernel, Dave Jiang, Fenghua Yu

On a Sapphire Rapids system if you boot without intel_iommu=on, the IDXD
driver will crash during probe in iommu_sva_bind_device().

[   21.423729] BUG: kernel NULL pointer dereference, address: 0000000000000038
[   21.445108] #PF: supervisor read access in kernel mode
[   21.450912] #PF: error_code(0x0000) - not-present page
[   21.456706] PGD 0
[   21.459047] Oops: 0000 [#1] PREEMPT SMP NOPTI
[   21.464004] CPU: 0 PID: 1420 Comm: kworker/0:3 Not tainted 5.19.0-0.rc3.27.eln120.x86_64 #1
[   21.464011] Hardware name: Intel Corporation EAGLESTREAM/EAGLESTREAM, BIOS EGSDCRB1.SYS.0067.D12.2110190954 10/19/2021
[   21.464015] Workqueue: events work_for_cpu_fn
[   21.464030] RIP: 0010:iommu_sva_bind_device+0x1d/0xe0
[   21.464046] Code: c3 cc 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 57 41 56 49 89 d6 41 55 41 54 55 53 48 83 ec 08 48 8b 87 d8 02 00 00 <48> 8b 40 38 48 8b 50 10 48 83 7a 70 00 48 89 14 24 0f 84 91 00 00
[   21.464050] RSP: 0018:ff7245d9096b7db8 EFLAGS: 00010296
[   21.464054] RAX: 0000000000000000 RBX: ff1eadeec8a51000 RCX: 0000000000000000
[   21.464058] RDX: ff7245d9096b7e24 RSI: 0000000000000000 RDI: ff1eadeec8a510d0
[   21.464060] RBP: ff1eadeec8a51000 R08: ffffffffb1a12300 R09: ff1eadffbfce25b4
[   21.464062] R10: ffffffffffffffff R11: 0000000000000038 R12: ffffffffc09f8000
[   21.464065] R13: ff1eadeec8a510d0 R14: ff7245d9096b7e24 R15: ff1eaddf54429000
[   21.464067] FS:  0000000000000000(0000) GS:ff1eadee7f600000(0000) knlGS:0000000000000000
[   21.464070] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   21.464072] CR2: 0000000000000038 CR3: 00000008c0e10006 CR4: 0000000000771ef0
[   21.464074] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   21.464076] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
[   21.464078] PKRU: 55555554
[   21.464079] Call Trace:
[   21.464083]  <TASK>
[   21.464092]  idxd_pci_probe+0x259/0x1070 [idxd]
[   21.464121]  local_pci_probe+0x3e/0x80
[   21.464132]  work_for_cpu_fn+0x13/0x20
[   21.464136]  process_one_work+0x1c4/0x380
[   21.464143]  worker_thread+0x1ab/0x380
[   21.464147]  ? _raw_spin_lock_irqsave+0x23/0x50
[   21.464158]  ? process_one_work+0x380/0x380
[   21.464161]  kthread+0xe6/0x110
[   21.464168]  ? kthread_complete_and_exit+0x20/0x20
[   21.464172]  ret_from_fork+0x1f/0x30

iommu_sva_bind_device() requires SVA has been enabled successfully on
the IDXD device before it's called. Otherwise, iommu_sva_bind_device()
will access a NULL pointer. If Intel IOMMU is disabled, SVA cannot be
enabled and thus idxd_enable_system_pasid() and iommu_sva_bind_device()
should not be called.

v2: Balance braces on if else block. Fix up commit description.

Fixes: 42a1b73852c4 ("dmaengine: idxd: Separate user and kernel pasid enabling")
Cc: Vinod Koul <vkoul@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 drivers/dma/idxd/init.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 355fb3ef4cbf..aa3478257ddb 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -512,15 +512,16 @@ static int idxd_probe(struct idxd_device *idxd)
 	dev_dbg(dev, "IDXD reset complete\n");
 
 	if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
-		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA))
+		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA)) {
 			dev_warn(dev, "Unable to turn on user SVA feature.\n");
-		else
+		} else {
 			set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
 
-		if (idxd_enable_system_pasid(idxd))
-			dev_warn(dev, "No in-kernel DMA with PASID.\n");
-		else
-			set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
+			if (idxd_enable_system_pasid(idxd))
+				dev_warn(dev, "No in-kernel DMA with PASID.\n");
+			else
+				set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
+		}
 	} else if (!sva) {
 		dev_warn(dev, "User forced SVA off via module param.\n");
 	}
-- 
2.36.1


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

* Re: [PATCH v2] dmaengine: idxd: Only call idxd_enable_system_pasid() if succeeded in enabling SVA feature
  2022-06-26  4:51 ` [PATCH v2] dmaengine: idxd: Only call idxd_enable_system_pasid() " Jerry Snitselaar
@ 2022-06-26  5:06   ` Fenghua Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Fenghua Yu @ 2022-06-26  5:06 UTC (permalink / raw)
  To: Jerry Snitselaar; +Cc: dmaengine, Vinod Koul, linux-kernel, Dave Jiang

Hi, Jerry,

On Sat, Jun 25, 2022 at 09:51:44PM -0700, Jerry Snitselaar wrote:
> On a Sapphire Rapids system if you boot without intel_iommu=on, the IDXD
                               s/you//

> driver will crash during probe in iommu_sva_bind_device().
.... 
> v2: Balance braces on if else block. Fix up commit description.

This change log should be moved after "---".

> 
> Fixes: 42a1b73852c4 ("dmaengine: idxd: Separate user and kernel pasid enabling")
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>

Please put the bug report link here:

Link: https://lore.kernel.org/dmaengine/20220623170232.6whonfjuh3m5vcoy@cantor/

> ---

i.e. put the change log here:

v2: Balance braces on if else block. Fix up commit description.

>  drivers/dma/idxd/init.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 355fb3ef4cbf..aa3478257ddb 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -512,15 +512,16 @@ static int idxd_probe(struct idxd_device *idxd)
>  	dev_dbg(dev, "IDXD reset complete\n");
>  
>  	if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
> -		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA))
> +		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA)) {
>  			dev_warn(dev, "Unable to turn on user SVA feature.\n");
> -		else
> +		} else {
>  			set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
>  
> -		if (idxd_enable_system_pasid(idxd))
> -			dev_warn(dev, "No in-kernel DMA with PASID.\n");
> -		else
> -			set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
> +			if (idxd_enable_system_pasid(idxd))
> +				dev_warn(dev, "No in-kernel DMA with PASID.\n");
> +			else
> +				set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
> +		}
>  	} else if (!sva) {
>  		dev_warn(dev, "User forced SVA off via module param.\n");
>  	}
> -- 
> 2.36.1
> 
Thanks.

-Fenghua

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

* [PATCH v3] dmaengine: idxd: Only call idxd_enable_system_pasid() if succeeded in enabling SVA feature
  2022-06-25 22:13 [PATCH] dmaengine: idxd: Only call idxd_enable_system_pasid if succeeded in enabling SVA feature Jerry Snitselaar
  2022-06-26  4:20 ` Fenghua Yu
  2022-06-26  4:51 ` [PATCH v2] dmaengine: idxd: Only call idxd_enable_system_pasid() " Jerry Snitselaar
@ 2022-06-26  5:16 ` Jerry Snitselaar
  2022-06-26  5:36   ` Fenghua Yu
  2022-07-05 13:00   ` Vinod Koul
  2 siblings, 2 replies; 8+ messages in thread
From: Jerry Snitselaar @ 2022-06-26  5:16 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul, linux-kernel, Dave Jiang, Fenghua Yu

On a Sapphire Rapids system if boot without intel_iommu=on, the IDXD
driver will crash during probe in iommu_sva_bind_device().

[   21.423729] BUG: kernel NULL pointer dereference, address: 0000000000000038
[   21.445108] #PF: supervisor read access in kernel mode
[   21.450912] #PF: error_code(0x0000) - not-present page
[   21.456706] PGD 0
[   21.459047] Oops: 0000 [#1] PREEMPT SMP NOPTI
[   21.464004] CPU: 0 PID: 1420 Comm: kworker/0:3 Not tainted 5.19.0-0.rc3.27.eln120.x86_64 #1
[   21.464011] Hardware name: Intel Corporation EAGLESTREAM/EAGLESTREAM, BIOS EGSDCRB1.SYS.0067.D12.2110190954 10/19/2021
[   21.464015] Workqueue: events work_for_cpu_fn
[   21.464030] RIP: 0010:iommu_sva_bind_device+0x1d/0xe0
[   21.464046] Code: c3 cc 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 57 41 56 49 89 d6 41 55 41 54 55 53 48 83 ec 08 48 8b 87 d8 02 00 00 <48> 8b 40 38 48 8b 50 10 48 83 7a 70 00 48 89 14 24 0f 84 91 00 00
[   21.464050] RSP: 0018:ff7245d9096b7db8 EFLAGS: 00010296
[   21.464054] RAX: 0000000000000000 RBX: ff1eadeec8a51000 RCX: 0000000000000000
[   21.464058] RDX: ff7245d9096b7e24 RSI: 0000000000000000 RDI: ff1eadeec8a510d0
[   21.464060] RBP: ff1eadeec8a51000 R08: ffffffffb1a12300 R09: ff1eadffbfce25b4
[   21.464062] R10: ffffffffffffffff R11: 0000000000000038 R12: ffffffffc09f8000
[   21.464065] R13: ff1eadeec8a510d0 R14: ff7245d9096b7e24 R15: ff1eaddf54429000
[   21.464067] FS:  0000000000000000(0000) GS:ff1eadee7f600000(0000) knlGS:0000000000000000
[   21.464070] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   21.464072] CR2: 0000000000000038 CR3: 00000008c0e10006 CR4: 0000000000771ef0
[   21.464074] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   21.464076] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
[   21.464078] PKRU: 55555554
[   21.464079] Call Trace:
[   21.464083]  <TASK>
[   21.464092]  idxd_pci_probe+0x259/0x1070 [idxd]
[   21.464121]  local_pci_probe+0x3e/0x80
[   21.464132]  work_for_cpu_fn+0x13/0x20
[   21.464136]  process_one_work+0x1c4/0x380
[   21.464143]  worker_thread+0x1ab/0x380
[   21.464147]  ? _raw_spin_lock_irqsave+0x23/0x50
[   21.464158]  ? process_one_work+0x380/0x380
[   21.464161]  kthread+0xe6/0x110
[   21.464168]  ? kthread_complete_and_exit+0x20/0x20
[   21.464172]  ret_from_fork+0x1f/0x30

iommu_sva_bind_device() requires SVA has been enabled successfully on
the IDXD device before it's called. Otherwise, iommu_sva_bind_device()
will access a NULL pointer. If Intel IOMMU is disabled, SVA cannot be
enabled and thus idxd_enable_system_pasid() and iommu_sva_bind_device()
should not be called.

Fixes: 42a1b73852c4 ("dmaengine: idxd: Separate user and kernel pasid enabling")
Cc: Vinod Koul <vkoul@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Link: https://lore.kernel.org/dmaengine/20220623170232.6whonfjuh3m5vcoy@cantor/
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
v3: Move changelog. Add lore link. Remove you.
v2: Balance braces on if else block. Fix up commit description.

 drivers/dma/idxd/init.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 355fb3ef4cbf..aa3478257ddb 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -512,15 +512,16 @@ static int idxd_probe(struct idxd_device *idxd)
 	dev_dbg(dev, "IDXD reset complete\n");
 
 	if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
-		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA))
+		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA)) {
 			dev_warn(dev, "Unable to turn on user SVA feature.\n");
-		else
+		} else {
 			set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
 
-		if (idxd_enable_system_pasid(idxd))
-			dev_warn(dev, "No in-kernel DMA with PASID.\n");
-		else
-			set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
+			if (idxd_enable_system_pasid(idxd))
+				dev_warn(dev, "No in-kernel DMA with PASID.\n");
+			else
+				set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
+		}
 	} else if (!sva) {
 		dev_warn(dev, "User forced SVA off via module param.\n");
 	}
-- 
2.36.1


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

* Re: [PATCH v3] dmaengine: idxd: Only call idxd_enable_system_pasid() if succeeded in enabling SVA feature
  2022-06-26  5:16 ` [PATCH v3] " Jerry Snitselaar
@ 2022-06-26  5:36   ` Fenghua Yu
  2022-07-01 17:52     ` Jerry Snitselaar
  2022-07-05 13:00   ` Vinod Koul
  1 sibling, 1 reply; 8+ messages in thread
From: Fenghua Yu @ 2022-06-26  5:36 UTC (permalink / raw)
  To: Jerry Snitselaar; +Cc: dmaengine, Vinod Koul, linux-kernel, Dave Jiang

On Sat, Jun 25, 2022 at 10:16:48PM -0700, Jerry Snitselaar wrote:
> On a Sapphire Rapids system if boot without intel_iommu=on, the IDXD
> driver will crash during probe in iommu_sva_bind_device().
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>

Acked-by: Fenghua Yu <fenghua.yu@intel.com>

Thanks.

-Fenghua

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

* Re: [PATCH v3] dmaengine: idxd: Only call idxd_enable_system_pasid() if succeeded in enabling SVA feature
  2022-06-26  5:36   ` Fenghua Yu
@ 2022-07-01 17:52     ` Jerry Snitselaar
  0 siblings, 0 replies; 8+ messages in thread
From: Jerry Snitselaar @ 2022-07-01 17:52 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dmaengine, Fenghua Yu, linux-kernel, Dave Jiang

On Sat, Jun 25, 2022 at 10:36:40PM -0700, Fenghua Yu wrote:
> On Sat, Jun 25, 2022 at 10:16:48PM -0700, Jerry Snitselaar wrote:
> > On a Sapphire Rapids system if boot without intel_iommu=on, the IDXD
> > driver will crash during probe in iommu_sva_bind_device().
> > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> 
> Acked-by: Fenghua Yu <fenghua.yu@intel.com>
> 
> Thanks.
> 
> -Fenghua

Hi Vinod,

Should this get pulled into your fixes branch?

Regards,
Jerry


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

* Re: [PATCH v3] dmaengine: idxd: Only call idxd_enable_system_pasid() if succeeded in enabling SVA feature
  2022-06-26  5:16 ` [PATCH v3] " Jerry Snitselaar
  2022-06-26  5:36   ` Fenghua Yu
@ 2022-07-05 13:00   ` Vinod Koul
  1 sibling, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2022-07-05 13:00 UTC (permalink / raw)
  To: Jerry Snitselaar; +Cc: dmaengine, linux-kernel, Dave Jiang, Fenghua Yu

On 25-06-22, 22:16, Jerry Snitselaar wrote:
> On a Sapphire Rapids system if boot without intel_iommu=on, the IDXD
> driver will crash during probe in iommu_sva_bind_device().

Applied, thanks

-- 
~Vinod

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

end of thread, other threads:[~2022-07-05 13:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-25 22:13 [PATCH] dmaengine: idxd: Only call idxd_enable_system_pasid if succeeded in enabling SVA feature Jerry Snitselaar
2022-06-26  4:20 ` Fenghua Yu
2022-06-26  4:51 ` [PATCH v2] dmaengine: idxd: Only call idxd_enable_system_pasid() " Jerry Snitselaar
2022-06-26  5:06   ` Fenghua Yu
2022-06-26  5:16 ` [PATCH v3] " Jerry Snitselaar
2022-06-26  5:36   ` Fenghua Yu
2022-07-01 17:52     ` Jerry Snitselaar
2022-07-05 13:00   ` Vinod Koul

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.