[PATCH] drm/i915/gvt: Activate/de-activate vGPU in mdev ops.

Xu, Terrence terrence.xu at intel.com
Wed Mar 29 11:58:22 UTC 2017


Got it.

>-----Original Message-----
>From: Wang, Zhi A
>Sent: Wednesday, March 29, 2017 4:53 PM
>To: Zhenyu Wang <zhenyuw at linux.intel.com>
>Cc: intel-gvt-dev at lists.freedesktop.org; Xu, Terrence <terrence.xu at intel.com>
>Subject: Re: [PATCH] drm/i915/gvt: Activate/de-activate vGPU in mdev ops.
>
>BTW: Tested on my SKL NUC. Terrence could test it also. :P
>
>于 03/29/17 16:52, Zhi Wang 写道:
>>
>>
>> 于 03/29/17 16:55, Zhenyu Wang 写道:
>>> On 2017.03.30 00:55:20 +0800, Zhi Wang wrote:
>>>> This patch introduces two functions for activating/de-activating
>>>> vGPU in mdev ops.
>>>>
>>>> A racing condition was found between virtual vblank emulation and
>>>> KVGMT mdev release path. To solve this problem, we factor out vGPU
>>>> activation /de-activation from vGPU creation/destruction path and
>>>> let KVMGT mdev release ops de-activate the vGPU before release
>>>> hypervisor handle. Once a vGPU is de-activated, GVT-g will not
>>>> emulate vblank for it or touch the hypervisor handle.
>>>>
>>>
>>> Could you elaborate what's the race?
>>>
>> vblank emulation will emulate and inject vblank for every active vGPU
>> with holding gvt->lock, while in mdev release path, it will directly
>> release hypervisor handle without change vGPU status or taking
>> gvt->lock, so a kernel panic is encountered when vblank emulation is
>> injecting a interrupt with a invalid hypervisor handle. (Reported by
>> Terrence)
>>
>> This patch is a part of refinement on KVM mdev release path. After a
>> discussion with Jike, there is another gap in KVM mdev release path
>> which also needs to be covered in vgpu_deactivate(). So better factor
>> out vgpu_deactive() in this patch.
>>
>>>> Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/gvt/gvt.c   |  2 ++
>>>>   drivers/gpu/drm/i915/gvt/gvt.h   |  5 ++++-
>>>>   drivers/gpu/drm/i915/gvt/kvmgt.c |  4 ++++
>>>>   drivers/gpu/drm/i915/gvt/vgpu.c  | 43
>>>> +++++++++++++++++++++++++++++++++++-----
>>>>   4 files changed, 48 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c
>>>> b/drivers/gpu/drm/i915/gvt/gvt.c index 3b9d59e..ef3baa0 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/gvt.c
>>>> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
>>>> @@ -52,6 +52,8 @@ static const struct intel_gvt_ops intel_gvt_ops = {
>>>>       .vgpu_create = intel_gvt_create_vgpu,
>>>>       .vgpu_destroy = intel_gvt_destroy_vgpu,
>>>>       .vgpu_reset = intel_gvt_reset_vgpu,
>>>> +    .vgpu_activate = intel_gvt_activate_vgpu,
>>>> +    .vgpu_deactivate = intel_gvt_deactivate_vgpu,
>>>>   };
>>>>
>>>>   /**
>>>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h
>>>> b/drivers/gpu/drm/i915/gvt/gvt.h index 2379192..2387eac 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>>>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>>>> @@ -382,7 +382,8 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu
>>>> *vgpu);
>>>>   void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
>>>>                    unsigned int engine_mask);
>>>>   void intel_gvt_reset_vgpu(struct intel_vgpu *vgpu);
>>>> -
>>>> +void intel_gvt_activate_vgpu(struct intel_vgpu *vgpu); void
>>>> +intel_gvt_deactivate_vgpu(struct intel_vgpu *vgpu);
>>>>
>>>>   /* validating GM functions */
>>>>   #define vgpu_gmadr_is_aperture(vgpu, gmadr) \ @@ -449,6 +450,8
>@@
>>>> struct intel_gvt_ops {
>>>>                   struct intel_vgpu_type *);
>>>>       void (*vgpu_destroy)(struct intel_vgpu *);
>>>>       void (*vgpu_reset)(struct intel_vgpu *);
>>>> +    void (*vgpu_activate)(struct intel_vgpu *);
>>>> +    void (*vgpu_deactivate)(struct intel_vgpu *);
>>>>   };
>>>>
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>>>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>>>> index 3c5682a..59c59e4 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>>>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>>>> @@ -544,6 +544,8 @@ static int intel_vgpu_open(struct mdev_device
>*mdev)
>>>>       if (ret)
>>>>           goto undo_group;
>>>>
>>>> +    intel_gvt_ops->vgpu_activate(vgpu);
>>>> +
>>>>       atomic_set(&vgpu->vdev.released, 0);
>>>>       return ret;
>>>>
>>>> @@ -569,6 +571,8 @@ static void __intel_vgpu_release(struct
>>>> intel_vgpu *vgpu)
>>>>       if (atomic_cmpxchg(&vgpu->vdev.released, 0, 1))
>>>>           return;
>>>>
>>>> +    intel_gvt_ops->vgpu_deactivate(vgpu);
>>>> +
>>>>       ret = vfio_unregister_notifier(mdev_dev(vgpu->vdev.mdev),
>>>> VFIO_IOMMU_NOTIFY,
>>>>                       &vgpu->vdev.iommu_notifier);
>>>>       WARN(ret, "vfio_unregister_notifier for iommu failed: %d\n",
>>>> ret); diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
>>>> b/drivers/gpu/drm/i915/gvt/vgpu.c index dfd2eff..fcd3c27 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
>>>> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
>>>> @@ -179,20 +179,34 @@ static void
>intel_gvt_update_vgpu_types(struct
>>>> intel_gvt *gvt)
>>>>   }
>>>>
>>>>   /**
>>>> - * intel_gvt_destroy_vgpu - destroy a virtual GPU
>>>> + * intel_gvt_active_vgpu - activate a virtual GPU
>>>>    * @vgpu: virtual GPU
>>>>    *
>>>> - * This function is called when user wants to destroy a virtual GPU.
>>>> + * This function is called when user wants to activate a virtual GPU.
>>>>    *
>>>>    */
>>>> -void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
>>>> +void intel_gvt_activate_vgpu(struct intel_vgpu *vgpu) {
>>>> +    mutex_lock(&vgpu->gvt->lock);
>>>> +    vgpu->active = true;
>>>> +    mutex_unlock(&vgpu->gvt->lock); }
>>>> +
>>>> +/**
>>>> + * intel_gvt_deactive_vgpu - deactivate a virtual GPU
>>>> + * @vgpu: virtual GPU
>>>> + *
>>>> + * This function is called when user wants to deactivate a virtual GPU.
>>>> + * All virtual GPU runtime information will be destroyed.
>>>> + *
>>>> + */
>>>> +void intel_gvt_deactivate_vgpu(struct intel_vgpu *vgpu)
>>>>   {
>>>>       struct intel_gvt *gvt = vgpu->gvt;
>>>>
>>>>       mutex_lock(&gvt->lock);
>>>>
>>>>       vgpu->active = false;
>>>> -    idr_remove(&gvt->vgpu_idr, vgpu->id);
>>>>
>>>>       if (atomic_read(&vgpu->running_workload_num)) {
>>>>           mutex_unlock(&gvt->lock);
>>>> @@ -201,6 +215,26 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu
>>>> *vgpu)
>>>>       }
>>>>
>>>>       intel_vgpu_stop_schedule(vgpu);
>>>> +
>>>> +    mutex_unlock(&gvt->lock);
>>>> +}
>>>> +
>>>> +/**
>>>> + * intel_gvt_destroy_vgpu - destroy a virtual GPU
>>>> + * @vgpu: virtual GPU
>>>> + *
>>>> + * This function is called when user wants to destroy a virtual GPU.
>>>> + *
>>>> + */
>>>> +void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu) {
>>>> +    struct intel_gvt *gvt = vgpu->gvt;
>>>> +
>>>> +    mutex_lock(&gvt->lock);
>>>> +
>>>> +    WARN(vgpu->active, "vGPU is still active!\n");
>>>> +
>>>> +    idr_remove(&gvt->vgpu_idr, vgpu->id);
>>>>       intel_vgpu_clean_sched_policy(vgpu);
>>>>       intel_vgpu_clean_gvt_context(vgpu);
>>>>       intel_vgpu_clean_execlist(vgpu); @@ -277,7 +311,6 @@ static
>>>> struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
>>>>       if (ret)
>>>>           goto out_clean_shadow_ctx;
>>>>
>>>> -    vgpu->active = true;
>>>>       mutex_unlock(&gvt->lock);
>>>>
>>>>       return 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
>>>


More information about the intel-gvt-dev mailing list