[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:34:33 UTC 2019
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?
>>>> +
>>>> 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