[PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues

[PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues

[Vasant Hegde]

This patch series fixes the refcount warning and pasid state in free
path.

V1 : https://lore.kernel.org/linux-iommu/20230124104355.119166-1-vasant.hegde@amd.com/

@Jason, Joerg,
  During v1 series review Jason had suggested few things including
  moving to xarray to handle PASID. We are planning to implement
  xarray as separate series.

  This series fixes the kernel warning we are seeing because of
  commit 8bc54824da4e. I am keeping changes as minimal so that
  its easy to backport.

Base commit: d11370bf64c57a (iommu/next branch)

Daniel Marcovitch (1):
  iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind

Vasant Hegde (1):
  iommu/amd/iommu_v2: Clear pasid state in free path

 drivers/iommu/amd/iommu_v2.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.31.1

[PATCH v2 1/2] iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind

[Vasant Hegde]

From: Daniel Marcovitch <dmarcovitch@nvidia.com>

When unbinding pasid - a race condition exists vs outstanding page faults.

To prevent this, the pasid_state object contains a refcount.
    * set to 1 on pasid bind
    * incremented on each ppr notification start
    * decremented on each ppr notification done
    * decremented on pasid unbind

Since refcount_dec assumes that refcount will never reach 0:
  the current implementation causes the following to be invoked on
  pasid unbind:
        REFCOUNT_WARN("decrement hit 0; leaking memory")

Fix this issue by changing refcount_dec to refcount_dec_and_test
to explicitly handle refcount=1.

Fixes: 8bc54824da4e ("iommu/amd: Convert from atomic_t to refcount_t on pasid_state->count")
Signed-off-by: Daniel Marcovitch <dmarcovitch@nvidia.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/iommu_v2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
index 864e4ffb6aa9..858748baae0b 100644
--- a/drivers/iommu/amd/iommu_v2.c
+++ b/drivers/iommu/amd/iommu_v2.c
@@ -262,8 +262,8 @@ static void put_pasid_state(struct pasid_state *pasid_state)
 
 static void put_pasid_state_wait(struct pasid_state *pasid_state)
 {
-	refcount_dec(&pasid_state->count);
-	wait_event(pasid_state->wq, !refcount_read(&pasid_state->count));
+	if (!refcount_dec_and_test(&pasid_state->count))
+		wait_event(pasid_state->wq, !refcount_read(&pasid_state->count));
 	free_pasid_state(pasid_state);
 }
 
-- 
2.31.1

[PATCH v2 2/2] iommu/amd/iommu_v2: Clear pasid state in free path

[Vasant Hegde]

Clear pasid state in device amd_iommu_free_device() path. It will make
sure no new ppr notifier is registered in free path.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/iommu_v2.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
index 858748baae0b..1df1d227bd26 100644
--- a/drivers/iommu/amd/iommu_v2.c
+++ b/drivers/iommu/amd/iommu_v2.c
@@ -327,6 +327,9 @@ static void free_pasid_states(struct device_state *dev_state)
 
 		put_pasid_state(pasid_state);
 
+		/* Clear the pasid state so that the pasid can be re-used */
+		clear_pasid_state(dev_state, pasid_state->pasid);
+
 		/*
 		 * This will call the mn_release function and
 		 * unbind the PASID
-- 
2.31.1

Re: [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues

[Vasant Hegde]

HI Joerg,

On 6/9/2023 4:21 PM, Vasant Hegde wrote:
> This patch series fixes the refcount warning and pasid state in free
> path.
> 
> V1 : https://lore.kernel.org/linux-iommu/20230124104355.119166-1-vasant.hegde@amd.com/

Ping. Did you get a chance to look into this series?


-Vasant

> 
> @Jason, Joerg,
>   During v1 series review Jason had suggested few things including
>   moving to xarray to handle PASID. We are planning to implement
>   xarray as separate series.
> 
>   This series fixes the kernel warning we are seeing because of
>   commit 8bc54824da4e. I am keeping changes as minimal so that
>   its easy to backport.
> 
> Base commit: d11370bf64c57a (iommu/next branch)
> 
> Daniel Marcovitch (1):
>   iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind
> 
> Vasant Hegde (1):
>   iommu/amd/iommu_v2: Clear pasid state in free path
> 
>  drivers/iommu/amd/iommu_v2.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 

Re: [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues

[Joerg Roedel]

On Fri, Jun 09, 2023 at 10:51:44AM +0000, Vasant Hegde wrote:
> Daniel Marcovitch (1):
>   iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind
> 
> Vasant Hegde (1):
>   iommu/amd/iommu_v2: Clear pasid state in free path
> 
>  drivers/iommu/amd/iommu_v2.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Applied, thanks.