[PATCH v1 1/4] drm/i915/gvt: add one hypercall to clean vfio regions
Hang Yuan
hang.yuan at linux.intel.com
Mon Jan 14 05:02:24 UTC 2019
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.
>> +
>> 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
More information about the intel-gvt-dev
mailing list