All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-09-29 17:48 ` Jason Gunthorpe
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-09-29 17:48 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang
  Cc: Alex Williamson, Christoph Hellwig, stable, Zhi Wang

When converting to directly create the vfio_device the mdev driver has to
put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
vfio_unregister_group_dev() in the remove.

This was missed for gvt, add it.

Cc: stable@vger.kernel.org
Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
 1 file changed, 1 insertion(+)

Should go through Alex's tree.

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 41bba40feef8f4..9003145adb5a93 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
 	if (WARN_ON_ONCE(vgpu->attached))
 		return;
 
+	vfio_unregister_group_dev(&vgpu->vfio_device);
 	vfio_put_device(&vgpu->vfio_device);
 }
 

base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c
-- 
2.37.3


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

* [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-09-29 17:48 ` Jason Gunthorpe
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-09-29 17:48 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang
  Cc: Alex Williamson, Zhi Wang, Christoph Hellwig, stable

When converting to directly create the vfio_device the mdev driver has to
put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
vfio_unregister_group_dev() in the remove.

This was missed for gvt, add it.

Cc: stable@vger.kernel.org
Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
 1 file changed, 1 insertion(+)

Should go through Alex's tree.

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 41bba40feef8f4..9003145adb5a93 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
 	if (WARN_ON_ONCE(vgpu->attached))
 		return;
 
+	vfio_unregister_group_dev(&vgpu->vfio_device);
 	vfio_put_device(&vgpu->vfio_device);
 }
 

base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c
-- 
2.37.3


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

* [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-09-29 17:48 ` Jason Gunthorpe
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-09-29 17:48 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang
  Cc: Christoph Hellwig, stable

When converting to directly create the vfio_device the mdev driver has to
put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
vfio_unregister_group_dev() in the remove.

This was missed for gvt, add it.

Cc: stable@vger.kernel.org
Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
 1 file changed, 1 insertion(+)

Should go through Alex's tree.

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 41bba40feef8f4..9003145adb5a93 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
 	if (WARN_ON_ONCE(vgpu->attached))
 		return;
 
+	vfio_unregister_group_dev(&vgpu->vfio_device);
 	vfio_put_device(&vgpu->vfio_device);
 }
 

base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c
-- 
2.37.3


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

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-09-29 17:48 ` Jason Gunthorpe
  (?)
  (?)
@ 2022-09-29 20:04 ` Patchwork
  -1 siblings, 0 replies; 38+ messages in thread
From: Patchwork @ 2022-09-29 20:04 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gvt: Add missing vfio_unregister_group_dev() call
URL   : https://patchwork.freedesktop.org/series/109264/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/109264/revisions/1/mbox/ not applied
Applying: drm/i915/gvt: Add missing vfio_unregister_group_dev() call
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/gvt/kvmgt.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/gvt/kvmgt.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/gvt/kvmgt.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915/gvt: Add missing vfio_unregister_group_dev() call
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



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

* RE: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-09-29 17:48 ` Jason Gunthorpe
  (?)
@ 2022-09-30  1:47   ` Tian, Kevin
  -1 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-09-30  1:47 UTC (permalink / raw)
  To: Jason Gunthorpe, David Airlie, Daniel Vetter, dri-devel,
	intel-gfx, intel-gvt-dev, Jani Nikula, Joonas Lahtinen, Vivi,
	Rodrigo, Tvrtko Ursulin, Zhenyu Wang
  Cc: Alex Williamson, Wang, Zhi A, Christoph Hellwig, stable

> From: Jason Gunthorpe
> Sent: Friday, September 30, 2022 1:49 AM
> 
> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use
> vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

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

* RE: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-09-30  1:47   ` Tian, Kevin
  0 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-09-30  1:47 UTC (permalink / raw)
  To: Jason Gunthorpe, David Airlie, Daniel Vetter, dri-devel,
	intel-gfx, intel-gvt-dev, Jani Nikula, Joonas Lahtinen, Vivi,
	Rodrigo, Tvrtko Ursulin, Zhenyu Wang
  Cc: stable, Alex Williamson, Christoph Hellwig, Wang, Zhi A

> From: Jason Gunthorpe
> Sent: Friday, September 30, 2022 1:49 AM
> 
> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use
> vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-09-30  1:47   ` Tian, Kevin
  0 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-09-30  1:47 UTC (permalink / raw)
  To: Jason Gunthorpe, David Airlie, Daniel Vetter, dri-devel,
	intel-gfx, intel-gvt-dev, Jani Nikula, Joonas Lahtinen, Vivi,
	Rodrigo, Tvrtko Ursulin, Zhenyu Wang
  Cc: stable, Christoph Hellwig

> From: Jason Gunthorpe
> Sent: Friday, September 30, 2022 1:49 AM
> 
> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use
> vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-09-29 17:48 ` Jason Gunthorpe
@ 2022-09-30  6:46   ` Christoph Hellwig
  -1 siblings, 0 replies; 38+ messages in thread
From: Christoph Hellwig @ 2022-09-30  6:46 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang, Alex Williamson, Christoph Hellwig, stable,
	Zhi Wang

Oops.  Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-09-30  6:46   ` Christoph Hellwig
  0 siblings, 0 replies; 38+ messages in thread
From: Christoph Hellwig @ 2022-09-30  6:46 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: David Airlie, intel-gfx, dri-devel, Daniel Vetter, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig

Oops.  Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-09-29 17:48 ` Jason Gunthorpe
  (?)
@ 2022-09-30 21:31   ` Alex Williamson
  -1 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-09-30 21:31 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang, Christoph Hellwig, stable, Zhi Wang

On Thu, 29 Sep 2022 14:48:35 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> Should go through Alex's tree.

Applied to vfio next branch for v6.1.  Thanks for the quick fix!

Alex
 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 41bba40feef8f4..9003145adb5a93 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
>  
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
>  	vfio_put_device(&vgpu->vfio_device);
>  }
>  
> 
> base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c


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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-09-30 21:31   ` Alex Williamson
  0 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-09-30 21:31 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: David Airlie, intel-gfx, dri-devel, Daniel Vetter, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig

On Thu, 29 Sep 2022 14:48:35 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> Should go through Alex's tree.

Applied to vfio next branch for v6.1.  Thanks for the quick fix!

Alex
 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 41bba40feef8f4..9003145adb5a93 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
>  
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
>  	vfio_put_device(&vgpu->vfio_device);
>  }
>  
> 
> base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c


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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-09-30 21:31   ` Alex Williamson
  0 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-09-30 21:31 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Tvrtko Ursulin, David Airlie, intel-gfx, dri-devel, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig, Zhi Wang

On Thu, 29 Sep 2022 14:48:35 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> Should go through Alex's tree.

Applied to vfio next branch for v6.1.  Thanks for the quick fix!

Alex
 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 41bba40feef8f4..9003145adb5a93 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
>  
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
>  	vfio_put_device(&vgpu->vfio_device);
>  }
>  
> 
> base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c


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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-09-29 17:48 ` Jason Gunthorpe
  (?)
@ 2022-10-05 20:17   ` Alex Williamson
  -1 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-05 20:17 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Tvrtko Ursulin, David Airlie, intel-gfx, dri-devel, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig, Zhi Wang

On Thu, 29 Sep 2022 14:48:35 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> Should go through Alex's tree.
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 41bba40feef8f4..9003145adb5a93 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
>  
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
>  	vfio_put_device(&vgpu->vfio_device);
>  }
>  
> 
> base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c

This is marked for stable, but I think the stable backport for
existing kernels is actually:

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e3cd58946477..de89946c4817 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1595,6 +1595,9 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
 
 	if (WARN_ON_ONCE(vgpu->attached))
 		return;
+
+	vfio_unregister_group_dev(&vgpu->vfio_device);
+	vfio_uninit_group_dev(&vgpu->vfio_device);
 	intel_gvt_destroy_vgpu(vgpu);
 }


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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-05 20:17   ` Alex Williamson
  0 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-05 20:17 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: David Airlie, intel-gfx, dri-devel, Daniel Vetter, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig

On Thu, 29 Sep 2022 14:48:35 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> Should go through Alex's tree.
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 41bba40feef8f4..9003145adb5a93 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
>  
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
>  	vfio_put_device(&vgpu->vfio_device);
>  }
>  
> 
> base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c

This is marked for stable, but I think the stable backport for
existing kernels is actually:

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e3cd58946477..de89946c4817 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1595,6 +1595,9 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
 
 	if (WARN_ON_ONCE(vgpu->attached))
 		return;
+
+	vfio_unregister_group_dev(&vgpu->vfio_device);
+	vfio_uninit_group_dev(&vgpu->vfio_device);
 	intel_gvt_destroy_vgpu(vgpu);
 }


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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-05 20:17   ` Alex Williamson
  0 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-05 20:17 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang, Christoph Hellwig, stable, Zhi Wang

On Thu, 29 Sep 2022 14:48:35 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> Should go through Alex's tree.
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 41bba40feef8f4..9003145adb5a93 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
>  
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
>  	vfio_put_device(&vgpu->vfio_device);
>  }
>  
> 
> base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c

This is marked for stable, but I think the stable backport for
existing kernels is actually:

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e3cd58946477..de89946c4817 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1595,6 +1595,9 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
 
 	if (WARN_ON_ONCE(vgpu->attached))
 		return;
+
+	vfio_unregister_group_dev(&vgpu->vfio_device);
+	vfio_uninit_group_dev(&vgpu->vfio_device);
 	intel_gvt_destroy_vgpu(vgpu);
 }


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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: Add missing vfio_unregister_group_dev() call (rev2)
  2022-09-29 17:48 ` Jason Gunthorpe
                   ` (6 preceding siblings ...)
  (?)
@ 2022-10-05 21:16 ` Patchwork
  -1 siblings, 0 replies; 38+ messages in thread
From: Patchwork @ 2022-10-05 21:16 UTC (permalink / raw)
  To: Alex Williamson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gvt: Add missing vfio_unregister_group_dev() call (rev2)
URL   : https://patchwork.freedesktop.org/series/109264/
State : warning

== Summary ==

Error: dim checkpatch failed
d1cb20a63051 drm/i915/gvt: Add missing vfio_unregister_group_dev() call
-:16: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#16: 
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")

-:56: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)

total: 1 errors, 1 warnings, 0 checks, 9 lines checked



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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gvt: Add missing vfio_unregister_group_dev() call (rev2)
  2022-09-29 17:48 ` Jason Gunthorpe
                   ` (7 preceding siblings ...)
  (?)
@ 2022-10-05 21:38 ` Patchwork
  -1 siblings, 0 replies; 38+ messages in thread
From: Patchwork @ 2022-10-05 21:38 UTC (permalink / raw)
  To: Alex Williamson; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 11015 bytes --]

== Series Details ==

Series: drm/i915/gvt: Add missing vfio_unregister_group_dev() call (rev2)
URL   : https://patchwork.freedesktop.org/series/109264/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12219 -> Patchwork_109264v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_109264v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_109264v2, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/index.html

Participating hosts (42 -> 42)
------------------------------

  Additional (2): fi-hsw-4770 fi-rkl-11600 
  Missing    (2): fi-ctg-p8600 fi-hsw-4200u 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_109264v2:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_render_tiled_blits@basic:
    - fi-apl-guc:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/fi-apl-guc/igt@gem_render_tiled_blits@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-apl-guc/igt@gem_render_tiled_blits@basic.html

  
Known issues
------------

  Here are the changes found in Patchwork_109264v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-rkl-11600:       NOTRUN -> [SKIP][3] ([i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@gem_lmem_swapping@basic.html

  * igt@gem_softpin@allocator-basic-reserve:
    - fi-hsw-4770:        NOTRUN -> [SKIP][5] ([fdo#109271]) +9 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-hsw-4770/igt@gem_softpin@allocator-basic-reserve.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][6] ([i915#3282])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-hsw-4770:        NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#3012])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-hsw-4770/igt@i915_pm_backlight@basic-brightness.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][8] ([i915#3012])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rpm@module-reload:
    - fi-cfl-8109u:       [PASS][9] -> [DMESG-FAIL][10] ([i915#62])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-cfl-8109u:       [PASS][11] -> [DMESG-WARN][12] ([i915#5904]) +30 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - fi-cfl-8109u:       [PASS][13] -> [DMESG-WARN][14] ([i915#5904] / [i915#62])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       NOTRUN -> [INCOMPLETE][15] ([i915#5982])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-hsw-4770:        NOTRUN -> [SKIP][16] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-hsw-4770/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-rkl-11600:       NOTRUN -> [SKIP][17] ([fdo#111827]) +7 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
    - fi-rkl-11600:       NOTRUN -> [SKIP][18] ([i915#4103])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][19] ([fdo#109285] / [i915#4098])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cfl-8109u:       [PASS][20] -> [DMESG-WARN][21] ([i915#62]) +12 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_psr@primary_page_flip:
    - fi-rkl-11600:       NOTRUN -> [SKIP][22] ([i915#1072]) +3 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@kms_psr@primary_page_flip.html

  * igt@kms_psr@sprite_plane_onoff:
    - fi-hsw-4770:        NOTRUN -> [SKIP][23] ([fdo#109271] / [i915#1072]) +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-hsw-4770/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-rkl-11600:       NOTRUN -> [SKIP][24] ([i915#3555] / [i915#4098])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-read:
    - fi-rkl-11600:       NOTRUN -> [SKIP][25] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@prime_vgem@basic-read.html

  * igt@prime_vgem@basic-userptr:
    - fi-rkl-11600:       NOTRUN -> [SKIP][26] ([fdo#109295] / [i915#3301] / [i915#3708])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/fi-rkl-11600/igt@prime_vgem@basic-userptr.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0@smem:
    - {bat-rplp-1}:       [DMESG-WARN][27] ([i915#2867]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/bat-rplp-1/igt@gem_exec_suspend@basic-s0@smem.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/bat-rplp-1/igt@gem_exec_suspend@basic-s0@smem.html
    - {bat-adlm-1}:       [DMESG-WARN][29] ([i915#2867]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/bat-adlm-1/igt@gem_exec_suspend@basic-s0@smem.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/bat-adlm-1/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@i915_module_load@reload:
    - {bat-rpls-2}:       [DMESG-WARN][31] ([i915#5537]) -> [PASS][32] +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/bat-rpls-2/igt@i915_module_load@reload.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/bat-rpls-2/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@requests:
    - {bat-rpls-1}:       [INCOMPLETE][33] ([i915#6257]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/bat-rpls-1/igt@i915_selftest@live@requests.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/bat-rpls-1/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@reset:
    - {bat-rpls-2}:       [DMESG-FAIL][35] ([i915#4983]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12219/bat-rpls-2/igt@i915_selftest@live@reset.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/bat-rpls-2/igt@i915_selftest@live@reset.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5537]: https://gitlab.freedesktop.org/drm/intel/issues/5537
  [i915#5904]: https://gitlab.freedesktop.org/drm/intel/issues/5904
  [i915#5982]: https://gitlab.freedesktop.org/drm/intel/issues/5982
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6559]: https://gitlab.freedesktop.org/drm/intel/issues/6559
  [i915#6818]: https://gitlab.freedesktop.org/drm/intel/issues/6818


Build changes
-------------

  * Linux: CI_DRM_12219 -> Patchwork_109264v2

  CI-20190529: 20190529
  CI_DRM_12219: a6282eed0f613f3230a53a1826ac913edc0a65e4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6683: 5de4065922af981366357df5a94caf4e8d15c52a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_109264v2: a6282eed0f613f3230a53a1826ac913edc0a65e4 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

c959da3572d3 drm/i915/gvt: Add missing vfio_unregister_group_dev() call

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109264v2/index.html

[-- Attachment #2: Type: text/html, Size: 12867 bytes --]

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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-10-05 20:17   ` [Intel-gfx] " Alex Williamson
  (?)
@ 2022-10-05 22:03     ` Alex Williamson
  -1 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-05 22:03 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang, Christoph Hellwig, stable, Zhi Wang

On Wed, 5 Oct 2022 14:17:17 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Thu, 29 Sep 2022 14:48:35 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > When converting to directly create the vfio_device the mdev driver has to
> > put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> > vfio_unregister_group_dev() in the remove.
> > 
> > This was missed for gvt, add it.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> > Reported-by: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > Should go through Alex's tree.
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 41bba40feef8f4..9003145adb5a93 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
> >  	if (WARN_ON_ONCE(vgpu->attached))
> >  		return;

Actually, what's the purpose of this ^^^^ ?

We can't have a .remove callback that does nothing, this breaks
removing the device while it's in use.  Once we have the
vfio_unregister_group_dev() fix below, we'll block until the device is
unused, at which point vgpu->attached becomes false.  Unless I'm
missing something, I think we should also follow-up with a patch to
remove that bogus warn-on branch, right?  Thanks,

Alex

> >  
> > +	vfio_unregister_group_dev(&vgpu->vfio_device);
> >  	vfio_put_device(&vgpu->vfio_device);
> >  }
> >  
> > 
> > base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c  
> 
> This is marked for stable, but I think the stable backport for
> existing kernels is actually:
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index e3cd58946477..de89946c4817 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1595,6 +1595,9 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
> +
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
> +	vfio_uninit_group_dev(&vgpu->vfio_device);
>  	intel_gvt_destroy_vgpu(vgpu);
>  }


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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-05 22:03     ` Alex Williamson
  0 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-05 22:03 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Tvrtko Ursulin, David Airlie, intel-gfx, dri-devel, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig, Zhi Wang

On Wed, 5 Oct 2022 14:17:17 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Thu, 29 Sep 2022 14:48:35 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > When converting to directly create the vfio_device the mdev driver has to
> > put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> > vfio_unregister_group_dev() in the remove.
> > 
> > This was missed for gvt, add it.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> > Reported-by: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > Should go through Alex's tree.
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 41bba40feef8f4..9003145adb5a93 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
> >  	if (WARN_ON_ONCE(vgpu->attached))
> >  		return;

Actually, what's the purpose of this ^^^^ ?

We can't have a .remove callback that does nothing, this breaks
removing the device while it's in use.  Once we have the
vfio_unregister_group_dev() fix below, we'll block until the device is
unused, at which point vgpu->attached becomes false.  Unless I'm
missing something, I think we should also follow-up with a patch to
remove that bogus warn-on branch, right?  Thanks,

Alex

> >  
> > +	vfio_unregister_group_dev(&vgpu->vfio_device);
> >  	vfio_put_device(&vgpu->vfio_device);
> >  }
> >  
> > 
> > base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c  
> 
> This is marked for stable, but I think the stable backport for
> existing kernels is actually:
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index e3cd58946477..de89946c4817 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1595,6 +1595,9 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
> +
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
> +	vfio_uninit_group_dev(&vgpu->vfio_device);
>  	intel_gvt_destroy_vgpu(vgpu);
>  }


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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-05 22:03     ` Alex Williamson
  0 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-05 22:03 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: David Airlie, intel-gfx, dri-devel, Daniel Vetter, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig

On Wed, 5 Oct 2022 14:17:17 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Thu, 29 Sep 2022 14:48:35 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > When converting to directly create the vfio_device the mdev driver has to
> > put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> > vfio_unregister_group_dev() in the remove.
> > 
> > This was missed for gvt, add it.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev")
> > Reported-by: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > Should go through Alex's tree.
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 41bba40feef8f4..9003145adb5a93 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
> >  	if (WARN_ON_ONCE(vgpu->attached))
> >  		return;

Actually, what's the purpose of this ^^^^ ?

We can't have a .remove callback that does nothing, this breaks
removing the device while it's in use.  Once we have the
vfio_unregister_group_dev() fix below, we'll block until the device is
unused, at which point vgpu->attached becomes false.  Unless I'm
missing something, I think we should also follow-up with a patch to
remove that bogus warn-on branch, right?  Thanks,

Alex

> >  
> > +	vfio_unregister_group_dev(&vgpu->vfio_device);
> >  	vfio_put_device(&vgpu->vfio_device);
> >  }
> >  
> > 
> > base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c  
> 
> This is marked for stable, but I think the stable backport for
> existing kernels is actually:
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index e3cd58946477..de89946c4817 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1595,6 +1595,9 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  
>  	if (WARN_ON_ONCE(vgpu->attached))
>  		return;
> +
> +	vfio_unregister_group_dev(&vgpu->vfio_device);
> +	vfio_uninit_group_dev(&vgpu->vfio_device);
>  	intel_gvt_destroy_vgpu(vgpu);
>  }


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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-10-05 20:17   ` [Intel-gfx] " Alex Williamson
  (?)
@ 2022-10-06 11:35     ` Jason Gunthorpe
  -1 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-10-06 11:35 UTC (permalink / raw)
  To: Alex Williamson
  Cc: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang, Christoph Hellwig, stable, Zhi Wang

On Wed, Oct 05, 2022 at 02:17:17PM -0600, Alex Williamson wrote:

> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 41bba40feef8f4..9003145adb5a93 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
> >  	if (WARN_ON_ONCE(vgpu->attached))
> >  		return;
> >  
> > +	vfio_unregister_group_dev(&vgpu->vfio_device);
> >  	vfio_put_device(&vgpu->vfio_device);
> >  }
> >  
> > 
> > base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c
> 
> This is marked for stable, but I think the stable backport for
> existing kernels is actually:

Yes probably, this patch won't apply so if anyone wants to see it in
the stable series they need to follow the process to send the reworked
version.

Jason

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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-06 11:35     ` Jason Gunthorpe
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-10-06 11:35 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Tvrtko Ursulin, David Airlie, intel-gfx, dri-devel, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig, Zhi Wang

On Wed, Oct 05, 2022 at 02:17:17PM -0600, Alex Williamson wrote:

> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 41bba40feef8f4..9003145adb5a93 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
> >  	if (WARN_ON_ONCE(vgpu->attached))
> >  		return;
> >  
> > +	vfio_unregister_group_dev(&vgpu->vfio_device);
> >  	vfio_put_device(&vgpu->vfio_device);
> >  }
> >  
> > 
> > base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c
> 
> This is marked for stable, but I think the stable backport for
> existing kernels is actually:

Yes probably, this patch won't apply so if anyone wants to see it in
the stable series they need to follow the process to send the reworked
version.

Jason

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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-06 11:35     ` Jason Gunthorpe
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-10-06 11:35 UTC (permalink / raw)
  To: Alex Williamson
  Cc: David Airlie, intel-gfx, dri-devel, Daniel Vetter, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig

On Wed, Oct 05, 2022 at 02:17:17PM -0600, Alex Williamson wrote:

> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 41bba40feef8f4..9003145adb5a93 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
> >  	if (WARN_ON_ONCE(vgpu->attached))
> >  		return;
> >  
> > +	vfio_unregister_group_dev(&vgpu->vfio_device);
> >  	vfio_put_device(&vgpu->vfio_device);
> >  }
> >  
> > 
> > base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c
> 
> This is marked for stable, but I think the stable backport for
> existing kernels is actually:

Yes probably, this patch won't apply so if anyone wants to see it in
the stable series they need to follow the process to send the reworked
version.

Jason

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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-10-05 22:03     ` Alex Williamson
  (?)
@ 2022-10-06 11:37       ` Jason Gunthorpe
  -1 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-10-06 11:37 UTC (permalink / raw)
  To: Alex Williamson
  Cc: David Airlie, Daniel Vetter, dri-devel, intel-gfx, intel-gvt-dev,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Zhenyu Wang, Christoph Hellwig, stable, Zhi Wang

On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> We can't have a .remove callback that does nothing, this breaks
> removing the device while it's in use.  Once we have the
> vfio_unregister_group_dev() fix below, we'll block until the device is
> unused, at which point vgpu->attached becomes false.  Unless I'm
> missing something, I think we should also follow-up with a patch to
> remove that bogus warn-on branch, right?  Thanks,

Yes, looks right to me.

I question all the logical arround attached, where is the locking?

Jason

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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-06 11:37       ` Jason Gunthorpe
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-10-06 11:37 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Tvrtko Ursulin, David Airlie, intel-gfx, dri-devel, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig, Zhi Wang

On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> We can't have a .remove callback that does nothing, this breaks
> removing the device while it's in use.  Once we have the
> vfio_unregister_group_dev() fix below, we'll block until the device is
> unused, at which point vgpu->attached becomes false.  Unless I'm
> missing something, I think we should also follow-up with a patch to
> remove that bogus warn-on branch, right?  Thanks,

Yes, looks right to me.

I question all the logical arround attached, where is the locking?

Jason

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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-06 11:37       ` Jason Gunthorpe
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Gunthorpe @ 2022-10-06 11:37 UTC (permalink / raw)
  To: Alex Williamson
  Cc: David Airlie, intel-gfx, dri-devel, Daniel Vetter, Rodrigo Vivi,
	stable, intel-gvt-dev, Christoph Hellwig

On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> We can't have a .remove callback that does nothing, this breaks
> removing the device while it's in use.  Once we have the
> vfio_unregister_group_dev() fix below, we'll block until the device is
> unused, at which point vgpu->attached becomes false.  Unless I'm
> missing something, I think we should also follow-up with a patch to
> remove that bogus warn-on branch, right?  Thanks,

Yes, looks right to me.

I question all the logical arround attached, where is the locking?

Jason

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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-10-06 11:37       ` Jason Gunthorpe
  (?)
@ 2022-10-06 18:31         ` Alex Williamson
  -1 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-06 18:31 UTC (permalink / raw)
  To: Zhenyu Wang, Zhi Wang, Tian, Kevin
  Cc: Jason Gunthorpe, David Airlie, Daniel Vetter, dri-devel,
	intel-gfx, intel-gvt-dev, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Christoph Hellwig, stable

On Thu, 6 Oct 2022 08:37:09 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> > We can't have a .remove callback that does nothing, this breaks
> > removing the device while it's in use.  Once we have the
> > vfio_unregister_group_dev() fix below, we'll block until the device is
> > unused, at which point vgpu->attached becomes false.  Unless I'm
> > missing something, I think we should also follow-up with a patch to
> > remove that bogus warn-on branch, right?  Thanks,  
> 
> Yes, looks right to me.
> 
> I question all the logical arround attached, where is the locking?

Zhenyu, Zhi, Kevin,

Could someone please take a look at use of vgpu->attached in the GVT-g
driver?  It's use in intel_vgpu_remove() is bogus, the .release
callback needs to use vfio_unregister_group_dev() to wait for the
device to be unused.  The WARN_ON/return here breaks all future use of
the device.  I assume @attached has something to do with the page table
interface with KVM, but it all looks racy anyway.

Also, whatever purpose vgpu->released served looks unnecessary now.
Thanks,

Alex


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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-06 18:31         ` Alex Williamson
  0 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-06 18:31 UTC (permalink / raw)
  To: Zhenyu Wang, Zhi Wang, Tian, Kevin
  Cc: Tvrtko Ursulin, Jason Gunthorpe, David Airlie, intel-gfx,
	dri-devel, Rodrigo Vivi, stable, intel-gvt-dev,
	Christoph Hellwig

On Thu, 6 Oct 2022 08:37:09 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> > We can't have a .remove callback that does nothing, this breaks
> > removing the device while it's in use.  Once we have the
> > vfio_unregister_group_dev() fix below, we'll block until the device is
> > unused, at which point vgpu->attached becomes false.  Unless I'm
> > missing something, I think we should also follow-up with a patch to
> > remove that bogus warn-on branch, right?  Thanks,  
> 
> Yes, looks right to me.
> 
> I question all the logical arround attached, where is the locking?

Zhenyu, Zhi, Kevin,

Could someone please take a look at use of vgpu->attached in the GVT-g
driver?  It's use in intel_vgpu_remove() is bogus, the .release
callback needs to use vfio_unregister_group_dev() to wait for the
device to be unused.  The WARN_ON/return here breaks all future use of
the device.  I assume @attached has something to do with the page table
interface with KVM, but it all looks racy anyway.

Also, whatever purpose vgpu->released served looks unnecessary now.
Thanks,

Alex


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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-06 18:31         ` Alex Williamson
  0 siblings, 0 replies; 38+ messages in thread
From: Alex Williamson @ 2022-10-06 18:31 UTC (permalink / raw)
  To: Zhenyu Wang, Zhi Wang, Tian, Kevin
  Cc: Jason Gunthorpe, David Airlie, intel-gfx, dri-devel,
	Daniel Vetter, Rodrigo Vivi, stable, intel-gvt-dev,
	Christoph Hellwig

On Thu, 6 Oct 2022 08:37:09 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> > We can't have a .remove callback that does nothing, this breaks
> > removing the device while it's in use.  Once we have the
> > vfio_unregister_group_dev() fix below, we'll block until the device is
> > unused, at which point vgpu->attached becomes false.  Unless I'm
> > missing something, I think we should also follow-up with a patch to
> > remove that bogus warn-on branch, right?  Thanks,  
> 
> Yes, looks right to me.
> 
> I question all the logical arround attached, where is the locking?

Zhenyu, Zhi, Kevin,

Could someone please take a look at use of vgpu->attached in the GVT-g
driver?  It's use in intel_vgpu_remove() is bogus, the .release
callback needs to use vfio_unregister_group_dev() to wait for the
device to be unused.  The WARN_ON/return here breaks all future use of
the device.  I assume @attached has something to do with the page table
interface with KVM, but it all looks racy anyway.

Also, whatever purpose vgpu->released served looks unnecessary now.
Thanks,

Alex


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

* RE: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-10-06 18:31         ` Alex Williamson
  (?)
@ 2022-10-10 23:14           ` Tian, Kevin
  -1 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-10-10 23:14 UTC (permalink / raw)
  To: Alex Williamson, Zhenyu Wang, Wang, Zhi A
  Cc: Tvrtko Ursulin, Jason Gunthorpe, David Airlie, intel-gfx,
	dri-devel, Vivi, Rodrigo, stable, intel-gvt-dev,
	Christoph Hellwig

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Friday, October 7, 2022 2:31 AM
> 
> On Thu, 6 Oct 2022 08:37:09 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> > > We can't have a .remove callback that does nothing, this breaks
> > > removing the device while it's in use.  Once we have the
> > > vfio_unregister_group_dev() fix below, we'll block until the device is
> > > unused, at which point vgpu->attached becomes false.  Unless I'm
> > > missing something, I think we should also follow-up with a patch to
> > > remove that bogus warn-on branch, right?  Thanks,
> >
> > Yes, looks right to me.
> >
> > I question all the logical arround attached, where is the locking?
> 
> Zhenyu, Zhi, Kevin,
> 
> Could someone please take a look at use of vgpu->attached in the GVT-g
> driver?  It's use in intel_vgpu_remove() is bogus, the .release
> callback needs to use vfio_unregister_group_dev() to wait for the
> device to be unused.  The WARN_ON/return here breaks all future use of
> the device.  I assume @attached has something to do with the page table
> interface with KVM, but it all looks racy anyway.
> 
> Also, whatever purpose vgpu->released served looks unnecessary now.
> Thanks,
> 

Zhi is looking at it.

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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-10 23:14           ` Tian, Kevin
  0 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-10-10 23:14 UTC (permalink / raw)
  To: Alex Williamson, Zhenyu Wang, Wang, Zhi A
  Cc: Jason Gunthorpe, David Airlie, intel-gfx, dri-devel,
	Daniel Vetter, Vivi, Rodrigo, stable, intel-gvt-dev,
	Christoph Hellwig

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Friday, October 7, 2022 2:31 AM
> 
> On Thu, 6 Oct 2022 08:37:09 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> > > We can't have a .remove callback that does nothing, this breaks
> > > removing the device while it's in use.  Once we have the
> > > vfio_unregister_group_dev() fix below, we'll block until the device is
> > > unused, at which point vgpu->attached becomes false.  Unless I'm
> > > missing something, I think we should also follow-up with a patch to
> > > remove that bogus warn-on branch, right?  Thanks,
> >
> > Yes, looks right to me.
> >
> > I question all the logical arround attached, where is the locking?
> 
> Zhenyu, Zhi, Kevin,
> 
> Could someone please take a look at use of vgpu->attached in the GVT-g
> driver?  It's use in intel_vgpu_remove() is bogus, the .release
> callback needs to use vfio_unregister_group_dev() to wait for the
> device to be unused.  The WARN_ON/return here breaks all future use of
> the device.  I assume @attached has something to do with the page table
> interface with KVM, but it all looks racy anyway.
> 
> Also, whatever purpose vgpu->released served looks unnecessary now.
> Thanks,
> 

Zhi is looking at it.

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

* RE: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-10 23:14           ` Tian, Kevin
  0 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-10-10 23:14 UTC (permalink / raw)
  To: Alex Williamson, Zhenyu Wang, Wang, Zhi A
  Cc: Jason Gunthorpe, David Airlie, Daniel Vetter, dri-devel,
	intel-gfx, intel-gvt-dev, Jani Nikula, Joonas Lahtinen, Vivi,
	Rodrigo, Tvrtko Ursulin, Christoph Hellwig, stable

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Friday, October 7, 2022 2:31 AM
> 
> On Thu, 6 Oct 2022 08:37:09 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> > > We can't have a .remove callback that does nothing, this breaks
> > > removing the device while it's in use.  Once we have the
> > > vfio_unregister_group_dev() fix below, we'll block until the device is
> > > unused, at which point vgpu->attached becomes false.  Unless I'm
> > > missing something, I think we should also follow-up with a patch to
> > > remove that bogus warn-on branch, right?  Thanks,
> >
> > Yes, looks right to me.
> >
> > I question all the logical arround attached, where is the locking?
> 
> Zhenyu, Zhi, Kevin,
> 
> Could someone please take a look at use of vgpu->attached in the GVT-g
> driver?  It's use in intel_vgpu_remove() is bogus, the .release
> callback needs to use vfio_unregister_group_dev() to wait for the
> device to be unused.  The WARN_ON/return here breaks all future use of
> the device.  I assume @attached has something to do with the page table
> interface with KVM, but it all looks racy anyway.
> 
> Also, whatever purpose vgpu->released served looks unnecessary now.
> Thanks,
> 

Zhi is looking at it.

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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-10-06 18:31         ` Alex Williamson
  (?)
@ 2022-10-19  9:40           ` Wang, Zhi A
  -1 siblings, 0 replies; 38+ messages in thread
From: Wang, Zhi A @ 2022-10-19  9:40 UTC (permalink / raw)
  To: Alex Williamson, Zhenyu Wang, Tian, Kevin
  Cc: Tvrtko Ursulin, Jason Gunthorpe, David Airlie, intel-gfx,
	dri-devel, Vivi, Rodrigo, stable, intel-gvt-dev,
	Christoph Hellwig

On 10/6/22 18:31, Alex Williamson wrote:
> On Thu, 6 Oct 2022 08:37:09 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
>> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
>>> We can't have a .remove callback that does nothing, this breaks
>>> removing the device while it's in use.  Once we have the
>>> vfio_unregister_group_dev() fix below, we'll block until the device is
>>> unused, at which point vgpu->attached becomes false.  Unless I'm
>>> missing something, I think we should also follow-up with a patch to
>>> remove that bogus warn-on branch, right?  Thanks,  
>>
>> Yes, looks right to me.
>>
>> I question all the logical arround attached, where is the locking?
> 
> Zhenyu, Zhi, Kevin,
> 
> Could someone please take a look at use of vgpu->attached in the GVT-g
> driver?  It's use in intel_vgpu_remove() is bogus, the .release
> callback needs to use vfio_unregister_group_dev() to wait for the
> device to be unused.  The WARN_ON/return here breaks all future use of
> the device.  I assume @attached has something to do with the page table
> interface with KVM, but it all looks racy anyway.
> 
Thanks for pointing this out.

It was introduced in the GVT-g refactor patch series and Christoph might
not want to touch the vgpu->released while he needed a new state.

I dig it a bit. vgpu->attached would be used for preventing multiple open
on a single vGPU and indicate the kvm_get_kvm() has been done.
vgpu->released was to prevent the release before close, which is now
handled by the vfio_device_*.

What I would like to do are: 
1) Remove the vgpu->released. 2) Use alock to protect vgpu->attached.

After those were solved, the WARN_ON/return in the intel_vgpu_remove()
should be safely removed as the .release will be called after .close_device
deceases the vfio_device->refcnt to zero.

Thanks,
Zhi.

> Also, whatever purpose vgpu->released served looks unnecessary now.
> Thanks,
> 
> Alex
> 


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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-19  9:40           ` Wang, Zhi A
  0 siblings, 0 replies; 38+ messages in thread
From: Wang, Zhi A @ 2022-10-19  9:40 UTC (permalink / raw)
  To: Alex Williamson, Zhenyu Wang, Tian, Kevin
  Cc: Jason Gunthorpe, David Airlie, intel-gfx, dri-devel,
	Daniel Vetter, Vivi, Rodrigo, stable, intel-gvt-dev,
	Christoph Hellwig

On 10/6/22 18:31, Alex Williamson wrote:
> On Thu, 6 Oct 2022 08:37:09 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
>> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
>>> We can't have a .remove callback that does nothing, this breaks
>>> removing the device while it's in use.  Once we have the
>>> vfio_unregister_group_dev() fix below, we'll block until the device is
>>> unused, at which point vgpu->attached becomes false.  Unless I'm
>>> missing something, I think we should also follow-up with a patch to
>>> remove that bogus warn-on branch, right?  Thanks,  
>>
>> Yes, looks right to me.
>>
>> I question all the logical arround attached, where is the locking?
> 
> Zhenyu, Zhi, Kevin,
> 
> Could someone please take a look at use of vgpu->attached in the GVT-g
> driver?  It's use in intel_vgpu_remove() is bogus, the .release
> callback needs to use vfio_unregister_group_dev() to wait for the
> device to be unused.  The WARN_ON/return here breaks all future use of
> the device.  I assume @attached has something to do with the page table
> interface with KVM, but it all looks racy anyway.
> 
Thanks for pointing this out.

It was introduced in the GVT-g refactor patch series and Christoph might
not want to touch the vgpu->released while he needed a new state.

I dig it a bit. vgpu->attached would be used for preventing multiple open
on a single vGPU and indicate the kvm_get_kvm() has been done.
vgpu->released was to prevent the release before close, which is now
handled by the vfio_device_*.

What I would like to do are: 
1) Remove the vgpu->released. 2) Use alock to protect vgpu->attached.

After those were solved, the WARN_ON/return in the intel_vgpu_remove()
should be safely removed as the .release will be called after .close_device
deceases the vfio_device->refcnt to zero.

Thanks,
Zhi.

> Also, whatever purpose vgpu->released served looks unnecessary now.
> Thanks,
> 
> Alex
> 


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

* Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-19  9:40           ` Wang, Zhi A
  0 siblings, 0 replies; 38+ messages in thread
From: Wang, Zhi A @ 2022-10-19  9:40 UTC (permalink / raw)
  To: Alex Williamson, Zhenyu Wang, Tian, Kevin
  Cc: Jason Gunthorpe, David Airlie, Daniel Vetter, dri-devel,
	intel-gfx, intel-gvt-dev, Jani Nikula, Joonas Lahtinen, Vivi,
	Rodrigo, Tvrtko Ursulin, Christoph Hellwig, stable

On 10/6/22 18:31, Alex Williamson wrote:
> On Thu, 6 Oct 2022 08:37:09 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
>> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
>>> We can't have a .remove callback that does nothing, this breaks
>>> removing the device while it's in use.  Once we have the
>>> vfio_unregister_group_dev() fix below, we'll block until the device is
>>> unused, at which point vgpu->attached becomes false.  Unless I'm
>>> missing something, I think we should also follow-up with a patch to
>>> remove that bogus warn-on branch, right?  Thanks,  
>>
>> Yes, looks right to me.
>>
>> I question all the logical arround attached, where is the locking?
> 
> Zhenyu, Zhi, Kevin,
> 
> Could someone please take a look at use of vgpu->attached in the GVT-g
> driver?  It's use in intel_vgpu_remove() is bogus, the .release
> callback needs to use vfio_unregister_group_dev() to wait for the
> device to be unused.  The WARN_ON/return here breaks all future use of
> the device.  I assume @attached has something to do with the page table
> interface with KVM, but it all looks racy anyway.
> 
Thanks for pointing this out.

It was introduced in the GVT-g refactor patch series and Christoph might
not want to touch the vgpu->released while he needed a new state.

I dig it a bit. vgpu->attached would be used for preventing multiple open
on a single vGPU and indicate the kvm_get_kvm() has been done.
vgpu->released was to prevent the release before close, which is now
handled by the vfio_device_*.

What I would like to do are: 
1) Remove the vgpu->released. 2) Use alock to protect vgpu->attached.

After those were solved, the WARN_ON/return in the intel_vgpu_remove()
should be safely removed as the .release will be called after .close_device
deceases the vfio_device->refcnt to zero.

Thanks,
Zhi.

> Also, whatever purpose vgpu->released served looks unnecessary now.
> Thanks,
> 
> Alex
> 


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

* RE: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
  2022-10-19  9:40           ` [Intel-gfx] " Wang, Zhi A
  (?)
@ 2022-10-19 10:13             ` Tian, Kevin
  -1 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-10-19 10:13 UTC (permalink / raw)
  To: Wang, Zhi A, Alex Williamson, Zhenyu Wang
  Cc: Tvrtko Ursulin, Jason Gunthorpe, David Airlie, intel-gfx,
	dri-devel, Vivi, Rodrigo, stable, intel-gvt-dev,
	Christoph Hellwig

> From: Wang, Zhi A <zhi.a.wang@intel.com>
> Sent: Wednesday, October 19, 2022 5:41 PM
> 
> On 10/6/22 18:31, Alex Williamson wrote:
> > On Thu, 6 Oct 2022 08:37:09 -0300
> > Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> >> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> >>> We can't have a .remove callback that does nothing, this breaks
> >>> removing the device while it's in use.  Once we have the
> >>> vfio_unregister_group_dev() fix below, we'll block until the device is
> >>> unused, at which point vgpu->attached becomes false.  Unless I'm
> >>> missing something, I think we should also follow-up with a patch to
> >>> remove that bogus warn-on branch, right?  Thanks,
> >>
> >> Yes, looks right to me.
> >>
> >> I question all the logical arround attached, where is the locking?
> >
> > Zhenyu, Zhi, Kevin,
> >
> > Could someone please take a look at use of vgpu->attached in the GVT-g
> > driver?  It's use in intel_vgpu_remove() is bogus, the .release
> > callback needs to use vfio_unregister_group_dev() to wait for the
> > device to be unused.  The WARN_ON/return here breaks all future use of
> > the device.  I assume @attached has something to do with the page table
> > interface with KVM, but it all looks racy anyway.
> >
> Thanks for pointing this out.
> 
> It was introduced in the GVT-g refactor patch series and Christoph might
> not want to touch the vgpu->released while he needed a new state.
> 
> I dig it a bit. vgpu->attached would be used for preventing multiple open
> on a single vGPU and indicate the kvm_get_kvm() has been done.

vfio core already ensures that .open_device() is called only once:

vfio_device_open()
{
	...
	mutex_lock(&device->dev_set->lock);
	device->open_count++;
	if (device->open_count == 1) {
		...
		if (device->ops->open_device) {
			ret = device->ops->open_device(device);
			...
}

> vgpu->released was to prevent the release before close, which is now
> handled by the vfio_device_*.
> 
> What I would like to do are:
> 1) Remove the vgpu->released. 2) Use alock to protect vgpu->attached.
> 
> After those were solved, the WARN_ON/return in the intel_vgpu_remove()
> should be safely removed as the .release will be called after .close_device
> deceases the vfio_device->refcnt to zero.
> 
> Thanks,
> Zhi.
> 
> > Also, whatever purpose vgpu->released served looks unnecessary now.
> > Thanks,
> >
> > Alex
> >


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

* Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-19 10:13             ` Tian, Kevin
  0 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-10-19 10:13 UTC (permalink / raw)
  To: Wang, Zhi A, Alex Williamson, Zhenyu Wang
  Cc: Jason Gunthorpe, David Airlie, intel-gfx, dri-devel,
	Daniel Vetter, Vivi, Rodrigo, stable, intel-gvt-dev,
	Christoph Hellwig

> From: Wang, Zhi A <zhi.a.wang@intel.com>
> Sent: Wednesday, October 19, 2022 5:41 PM
> 
> On 10/6/22 18:31, Alex Williamson wrote:
> > On Thu, 6 Oct 2022 08:37:09 -0300
> > Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> >> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> >>> We can't have a .remove callback that does nothing, this breaks
> >>> removing the device while it's in use.  Once we have the
> >>> vfio_unregister_group_dev() fix below, we'll block until the device is
> >>> unused, at which point vgpu->attached becomes false.  Unless I'm
> >>> missing something, I think we should also follow-up with a patch to
> >>> remove that bogus warn-on branch, right?  Thanks,
> >>
> >> Yes, looks right to me.
> >>
> >> I question all the logical arround attached, where is the locking?
> >
> > Zhenyu, Zhi, Kevin,
> >
> > Could someone please take a look at use of vgpu->attached in the GVT-g
> > driver?  It's use in intel_vgpu_remove() is bogus, the .release
> > callback needs to use vfio_unregister_group_dev() to wait for the
> > device to be unused.  The WARN_ON/return here breaks all future use of
> > the device.  I assume @attached has something to do with the page table
> > interface with KVM, but it all looks racy anyway.
> >
> Thanks for pointing this out.
> 
> It was introduced in the GVT-g refactor patch series and Christoph might
> not want to touch the vgpu->released while he needed a new state.
> 
> I dig it a bit. vgpu->attached would be used for preventing multiple open
> on a single vGPU and indicate the kvm_get_kvm() has been done.

vfio core already ensures that .open_device() is called only once:

vfio_device_open()
{
	...
	mutex_lock(&device->dev_set->lock);
	device->open_count++;
	if (device->open_count == 1) {
		...
		if (device->ops->open_device) {
			ret = device->ops->open_device(device);
			...
}

> vgpu->released was to prevent the release before close, which is now
> handled by the vfio_device_*.
> 
> What I would like to do are:
> 1) Remove the vgpu->released. 2) Use alock to protect vgpu->attached.
> 
> After those were solved, the WARN_ON/return in the intel_vgpu_remove()
> should be safely removed as the .release will be called after .close_device
> deceases the vfio_device->refcnt to zero.
> 
> Thanks,
> Zhi.
> 
> > Also, whatever purpose vgpu->released served looks unnecessary now.
> > Thanks,
> >
> > Alex
> >


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

* RE: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
@ 2022-10-19 10:13             ` Tian, Kevin
  0 siblings, 0 replies; 38+ messages in thread
From: Tian, Kevin @ 2022-10-19 10:13 UTC (permalink / raw)
  To: Wang, Zhi A, Alex Williamson, Zhenyu Wang
  Cc: Jason Gunthorpe, David Airlie, Daniel Vetter, dri-devel,
	intel-gfx, intel-gvt-dev, Jani Nikula, Joonas Lahtinen, Vivi,
	Rodrigo, Tvrtko Ursulin, Christoph Hellwig, stable

> From: Wang, Zhi A <zhi.a.wang@intel.com>
> Sent: Wednesday, October 19, 2022 5:41 PM
> 
> On 10/6/22 18:31, Alex Williamson wrote:
> > On Thu, 6 Oct 2022 08:37:09 -0300
> > Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> >> On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote:
> >>> We can't have a .remove callback that does nothing, this breaks
> >>> removing the device while it's in use.  Once we have the
> >>> vfio_unregister_group_dev() fix below, we'll block until the device is
> >>> unused, at which point vgpu->attached becomes false.  Unless I'm
> >>> missing something, I think we should also follow-up with a patch to
> >>> remove that bogus warn-on branch, right?  Thanks,
> >>
> >> Yes, looks right to me.
> >>
> >> I question all the logical arround attached, where is the locking?
> >
> > Zhenyu, Zhi, Kevin,
> >
> > Could someone please take a look at use of vgpu->attached in the GVT-g
> > driver?  It's use in intel_vgpu_remove() is bogus, the .release
> > callback needs to use vfio_unregister_group_dev() to wait for the
> > device to be unused.  The WARN_ON/return here breaks all future use of
> > the device.  I assume @attached has something to do with the page table
> > interface with KVM, but it all looks racy anyway.
> >
> Thanks for pointing this out.
> 
> It was introduced in the GVT-g refactor patch series and Christoph might
> not want to touch the vgpu->released while he needed a new state.
> 
> I dig it a bit. vgpu->attached would be used for preventing multiple open
> on a single vGPU and indicate the kvm_get_kvm() has been done.

vfio core already ensures that .open_device() is called only once:

vfio_device_open()
{
	...
	mutex_lock(&device->dev_set->lock);
	device->open_count++;
	if (device->open_count == 1) {
		...
		if (device->ops->open_device) {
			ret = device->ops->open_device(device);
			...
}

> vgpu->released was to prevent the release before close, which is now
> handled by the vfio_device_*.
> 
> What I would like to do are:
> 1) Remove the vgpu->released. 2) Use alock to protect vgpu->attached.
> 
> After those were solved, the WARN_ON/return in the intel_vgpu_remove()
> should be safely removed as the .release will be called after .close_device
> deceases the vfio_device->refcnt to zero.
> 
> Thanks,
> Zhi.
> 
> > Also, whatever purpose vgpu->released served looks unnecessary now.
> > Thanks,
> >
> > Alex
> >


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

end of thread, other threads:[~2022-10-19 10:52 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 17:48 [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call Jason Gunthorpe
2022-09-29 17:48 ` [Intel-gfx] " Jason Gunthorpe
2022-09-29 17:48 ` Jason Gunthorpe
2022-09-29 20:04 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork
2022-09-30  1:47 ` [PATCH] " Tian, Kevin
2022-09-30  1:47   ` [Intel-gfx] " Tian, Kevin
2022-09-30  1:47   ` Tian, Kevin
2022-09-30  6:46 ` Christoph Hellwig
2022-09-30  6:46   ` [Intel-gfx] " Christoph Hellwig
2022-09-30 21:31 ` Alex Williamson
2022-09-30 21:31   ` Alex Williamson
2022-09-30 21:31   ` [Intel-gfx] " Alex Williamson
2022-10-05 20:17 ` Alex Williamson
2022-10-05 20:17   ` Alex Williamson
2022-10-05 20:17   ` [Intel-gfx] " Alex Williamson
2022-10-05 22:03   ` Alex Williamson
2022-10-05 22:03     ` [Intel-gfx] " Alex Williamson
2022-10-05 22:03     ` Alex Williamson
2022-10-06 11:37     ` Jason Gunthorpe
2022-10-06 11:37       ` [Intel-gfx] " Jason Gunthorpe
2022-10-06 11:37       ` Jason Gunthorpe
2022-10-06 18:31       ` Alex Williamson
2022-10-06 18:31         ` [Intel-gfx] " Alex Williamson
2022-10-06 18:31         ` Alex Williamson
2022-10-10 23:14         ` Tian, Kevin
2022-10-10 23:14           ` Tian, Kevin
2022-10-10 23:14           ` [Intel-gfx] " Tian, Kevin
2022-10-19  9:40         ` Wang, Zhi A
2022-10-19  9:40           ` Wang, Zhi A
2022-10-19  9:40           ` [Intel-gfx] " Wang, Zhi A
2022-10-19 10:13           ` Tian, Kevin
2022-10-19 10:13             ` Tian, Kevin
2022-10-19 10:13             ` [Intel-gfx] " Tian, Kevin
2022-10-06 11:35   ` Jason Gunthorpe
2022-10-06 11:35     ` [Intel-gfx] " Jason Gunthorpe
2022-10-06 11:35     ` Jason Gunthorpe
2022-10-05 21:16 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: Add missing vfio_unregister_group_dev() call (rev2) Patchwork
2022-10-05 21:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

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.