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

[Zhenyu Wang]

On 2019.01.14 13:34:33 +0800, Hang Yuan wrote:
> On 1/14/19 1:05 PM, Zhenyu Wang wrote:
> > 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..
> > 
> Henry: So do you mean to expose the functions (clean_regions/set_edid) in
> kvmgt instead of hypercall if the function is only valid for KVM?
> 

For clean region here, for me it looks we just need to fix it by clean
vgpu regions in detach call, and for set_edid one mpt interface is ok
and region cleanup in detach would also work in error handling path.

-- 
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/e27b2e55/attachment.sig>