[PATCH v1 1/4] drm/i915/gvt: add one hypercall to clean vfio regions

[Zhenyu Wang]

On 2019.01.14 13:02:24 +0800, Hang Yuan wrote:
> On 1/14/19 11:56 AM, Zhenyu Wang wrote:
> > On 2019.01.10 19:04:45 +0800, hang.yuan at linux.intel.com wrote:
> > > From: Hang Yuan <hang.yuan at linux.intel.com>
> > > 
> > > Add one hypercall to free VFIO region memory. Also call it in vgpu
> > > destroy.
> > > 
> > > Signed-off-by: Hang Yuan <hang.yuan at linux.intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gvt/hypercall.h |  1 +
> > >   drivers/gpu/drm/i915/gvt/kvmgt.c     | 15 +++++++++++++++
> > >   drivers/gpu/drm/i915/gvt/mpt.h       | 14 ++++++++++++++
> > >   drivers/gpu/drm/i915/gvt/vgpu.c      |  1 +
> > >   4 files changed, 31 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
> > > index 5079886..2ab4138 100644
> > > --- a/drivers/gpu/drm/i915/gvt/hypercall.h
> > > +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> > > @@ -67,6 +67,7 @@ struct intel_gvt_mpt {
> > >   	int (*set_trap_area)(unsigned long handle, u64 start, u64 end,
> > >   			     bool map);
> > >   	int (*set_opregion)(void *vgpu);
> > > +	void (*clean_regions)(void *vgpu);
> > >   	int (*get_vfio_device)(void *vgpu);
> > >   	void (*put_vfio_device)(void *vgpu);
> > >   	bool (*is_valid_gfn)(unsigned long handle, unsigned long gfn);
> > 
> > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > index a19e684..8c30dc3 100644
> > > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > @@ -493,6 +493,20 @@ static int kvmgt_set_opregion(void *p_vgpu)
> > >   	return ret;
> > >   }
> > > +static void kvmgt_clean_regions(void *p_vgpu)
> > > +{
> > > +	int i;
> > > +	struct intel_vgpu *vgpu = (struct intel_vgpu *)p_vgpu;
> > > +
> > > +	for (i = 0; i < vgpu->vdev.num_regions; i++)
> > > +		if (vgpu->vdev.region[i].ops->release)
> > > +			vgpu->vdev.region[i].ops->release(vgpu,
> > > +					&vgpu->vdev.region[i]);
> > > +	vgpu->vdev.num_regions = 0;
> > > +	kfree(vgpu->vdev.region);
> > > +	vgpu->vdev.region = NULL;
> > > +}
> > 
> > It looks a little overkill to put this requiremnt on all hypervisor,
> > as VFIO region is KVMGT only thing, e.g might not be valid for other hypervisor.
> > And looks our region.ops.release is never actually useful, what we need
> > is to free up vgpu regions, maybe just at detach vgpu time to let hypervisor
> > do any cleanup required.
> > 
> Henry:Since there is already hypercall 'set_opregion', I use hypercall as
> well to do region clean for opregion and edid region to have consistent
> interface between gvt and hypervisor. 'clean_regions' will also be called if
> failure in vgpu create. So I didn't implement it in vgpu detach.
>

"set_opregion" hypercall means hypervisor might have different ways to
expose opregion, e.g through vfio region, etc. So that makes sense for a
mpt interface. But here only care about cleanup of VFIO region, and both
opregion and edid region's release function is noop..

> > > +
> > >   static void kvmgt_put_vfio_device(void *vgpu)
> > >   {
> > >   	if (WARN_ON(!((struct intel_vgpu *)vgpu)->vdev.vfio_device))
> > > @@ -1874,6 +1888,7 @@ static struct intel_gvt_mpt kvmgt_mpt = {
> > >   	.dma_map_guest_page = kvmgt_dma_map_guest_page,
> > >   	.dma_unmap_guest_page = kvmgt_dma_unmap_guest_page,
> > >   	.set_opregion = kvmgt_set_opregion,
> > > +	.clean_regions = kvmgt_clean_regions,
> > >   	.get_vfio_device = kvmgt_get_vfio_device,
> > >   	.put_vfio_device = kvmgt_put_vfio_device,
> > >   	.is_valid_gfn = kvmgt_is_valid_gfn,
> > > diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
> > > index 9b4225d..1a07994 100644
> > > --- a/drivers/gpu/drm/i915/gvt/mpt.h
> > > +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> > > @@ -314,6 +314,20 @@ static inline int intel_gvt_hypervisor_set_opregion(struct intel_vgpu *vgpu)
> > >   }
> > >   /**
> > > + * intel_gvt_hypervisor_clean_regions - Clean regions for guest
> > > + * @vgpu: a vGPU
> > > + *
> > > + */
> > > +static inline void intel_gvt_hypervisor_clean_regions(struct intel_vgpu *vgpu)
> > > +{
> > > +	if (!intel_gvt_host.mpt->clean_regions)
> > > +		return;
> > > +
> > > +	intel_gvt_host.mpt->clean_regions(vgpu);
> > > +}
> > > +
> > > +
> > > +/**
> > >    * intel_gvt_hypervisor_get_vfio_device - increase vfio device ref count
> > >    * @vgpu: a vGPU
> > >    *
> > > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> > > index e1c860f8..c5eb565 100644
> > > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > > @@ -276,6 +276,7 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
> > >   	WARN(vgpu->active, "vGPU is still active!\n");
> > > +	intel_gvt_hypervisor_clean_regions(vgpu);
> > >   	intel_gvt_debugfs_remove_vgpu(vgpu);
> > >   	intel_vgpu_clean_sched_policy(vgpu);
> > >   	intel_vgpu_clean_submission(vgpu);
> > > -- 
> > > 2.7.4
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20190114/9e4e58c4/attachment.sig>