[PATCH 4/9] drm/i915/gvt: Factor out intel_vgpu_{get_or_create_ppgtt_mm, find_destroy_ppgtt_mm} interfaces

Zhi Wang zhi.a.wang at intel.com
Tue Dec 26 04:59:50 UTC 2017


OK. I see. Thanks for clarifying that.

On 12/26/17 11:15, Du, Changbin wrote:
> On Mon, Dec 25, 2017 at 06:44:06PM +0800, Zhi Wang wrote:
>> Please, "unreference" is different with "destroy". You can see the behavior
>> is not changed when only one guy is using the MM. But when the input is
>> changed, the destroy G2V call is called when a mm is being used. unreference
>> will keep the instance while destroy will destroy it immediately.
>>
> If a object is maintained by ref count, you cannot directly free it by not
> checking usage count. For your reference, you can check kernel page alloctor
> for good example.
> 
> In our case, the *destroy* api actually is an alias to *put*.
> 
>> Thanks,
>> Zhi.
>>
>> On 12/25/17 18:30, Du, Changbin wrote:
>>> On Mon, Dec 25, 2017 at 06:36:51PM +0800, Zhi Wang wrote:
>>>> I think in a re-factor patch, you shouldn't change a behavior without any
>>>> notice in the comments. Also I think you should consider add your comments
>>>> in this email to the comments of the patch so that people would know you
>>>> changed the behavior.
>>>>
>>> No, the behavior is not changed, just function name...
>>>
>>>> Thanks,
>>>> Zhi.
>>>>
>>>> On 12/25/17 18:25, Du, Changbin wrote:
>>>>> On Mon, Dec 25, 2017 at 06:19:08PM +0800, Zhi Wang wrote:
>>>>>> You changed the behavior a bit. Previously, in the G2V call, the vGPU mm is
>>>>>> "put" and now it's destroyed. Is there any reason why the behavior is
>>>>>> changed?
>>>>>>
>>>>> The request is to destroy the mm obj. So we should destroy it eventhough the
>>>>> under implementation is same - put the reference.
>>>>>
>>>>>> Thanks,
>>>>>> Zhi.
>>>>>>
>>>>>> On 12/25/17 17:11, changbin.du at intel.com wrote:
>>>>>>> From: Changbin Du <changbin.du at intel.com>
>>>>>>>
>>>>>>> Factor out these two interfaces so we can kill some duplicated code in
>>>>>>> scheduler.c.
>>>>>>>
>>>>>>> Signed-off-by: Changbin Du <changbin.du at intel.com>
>>>>>>> ---
>>>>>>>      drivers/gpu/drm/i915/gvt/gtt.c       | 28 ++++++++++++----------------
>>>>>>>      drivers/gpu/drm/i915/gvt/gtt.h       |  4 ++--
>>>>>>>      drivers/gpu/drm/i915/gvt/handlers.c  | 11 +++++++----
>>>>>>>      drivers/gpu/drm/i915/gvt/scheduler.c | 17 +++++------------
>>>>>>>      4 files changed, 26 insertions(+), 34 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
>>>>>>> index 9befbfa..fbb0aa2 100644
>>>>>>> --- a/drivers/gpu/drm/i915/gvt/gtt.c
>>>>>>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
>>>>>>> @@ -2268,20 +2268,19 @@ struct intel_vgpu_mm *intel_vgpu_find_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>>      }
>>>>>>>      /**
>>>>>>> - * intel_vgpu_g2v_create_ppgtt_mm - create a PPGTT mm object from
>>>>>>> - * g2v notification
>>>>>>> + * intel_vgpu_get_or_create_ppgtt_mm - find or create a PPGTT mm object.
>>>>>>>       * @vgpu: a vGPU
>>>>>>>       * @root_entry_type: ppgtt root entry type
>>>>>>>       * @pdps: guest pdps
>>>>>>>       *
>>>>>>> - * This function is used to create a PPGTT mm object from a guest to GVT-g
>>>>>>> - * notification.
>>>>>>> + * This function is used to find or create a PPGTT mm object from a guest.
>>>>>>>       *
>>>>>>>       * Returns:
>>>>>>>       * Zero on success, negative error code if failed.
>>>>>>>       */
>>>>>>> -int intel_vgpu_g2v_create_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>> -		intel_gvt_gtt_type_t root_entry_type, u64 pdps[GEN8_3LVL_PDPES])
>>>>>>> +struct intel_vgpu_mm *intel_vgpu_get_or_create_ppgtt_mm(
>>>>>>> +		struct intel_vgpu *vgpu, intel_gvt_gtt_type_t root_entry_type,
>>>>>>> +		u64 pdps[GEN8_3LVL_PDPES])
>>>>>>>      {
>>>>>>>      	struct intel_vgpu_mm *mm;
>>>>>>> @@ -2290,27 +2289,23 @@ int intel_vgpu_g2v_create_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>>      		intel_vgpu_mm_get(mm);
>>>>>>>      	} else {
>>>>>>>      		mm = intel_vgpu_create_ppgtt_mm(vgpu, root_entry_type, pdps);
>>>>>>> -		if (IS_ERR(mm)) {
>>>>>>> +		if (IS_ERR(mm))
>>>>>>>      			gvt_vgpu_err("fail to create mm\n");
>>>>>>> -			return PTR_ERR(mm);
>>>>>>> -		}
>>>>>>>      	}
>>>>>>> -	return 0;
>>>>>>> +	return mm;
>>>>>>>      }
>>>>>>>      /**
>>>>>>> - * intel_vgpu_g2v_destroy_ppgtt_mm - destroy a PPGTT mm object from
>>>>>>> - * g2v notification
>>>>>>> + * intel_vgpu_find_destroy_ppgtt_mm - find and destroy a PPGTT mm object.
>>>>>>>       * @vgpu: a vGPU
>>>>>>>       * @pdps: guest pdps
>>>>>>>       *
>>>>>>> - * This function is used to create a PPGTT mm object from a guest to GVT-g
>>>>>>> - * notification.
>>>>>>> + * This function is used to find a PPGTT mm object from a guest and destroy it.
>>>>>>>       *
>>>>>>>       * Returns:
>>>>>>>       * Zero on success, negative error code if failed.
>>>>>>>       */
>>>>>>> -int intel_vgpu_g2v_destroy_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>> +int intel_vgpu_find_destroy_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>>      		u64 pdps[GEN8_3LVL_PDPES])
>>>>>>>      {
>>>>>>>      	struct intel_vgpu_mm *mm;
>>>>>>> @@ -2320,7 +2315,8 @@ int intel_vgpu_g2v_destroy_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>>      		gvt_vgpu_err("fail to find ppgtt instance.\n");
>>>>>>>      		return -EINVAL;
>>>>>>>      	}
>>>>>>> -	intel_vgpu_mm_put(mm);
>>>>>>> +
>>>>>>> +	intel_vgpu_destroy_mm(mm);
>>>>>>>      	return 0;
>>>>>>>      }
>>>>>>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
>>>>>>> index fa94e97..4533e10 100644
>>>>>>> --- a/drivers/gpu/drm/i915/gvt/gtt.h
>>>>>>> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
>>>>>>> @@ -268,10 +268,10 @@ unsigned long intel_vgpu_gma_to_gpa(struct intel_vgpu_mm *mm,
>>>>>>>      struct intel_vgpu_mm *intel_vgpu_find_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>>      		u64 pdps[]);
>>>>>>> -int intel_vgpu_g2v_create_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>> +struct intel_vgpu_mm *intel_vgpu_get_or_create_ppgtt_mm(struct intel_vgpu *vgpu,
>>>>>>>      		intel_gvt_gtt_type_t root_entry_type, u64 pdps[]);
>>>>>>> -int intel_vgpu_g2v_destroy_ppgtt_mm(struct intel_vgpu *vgpu, u64 pdps[]);
>>>>>>> +int intel_vgpu_find_destroy_ppgtt_mm(struct intel_vgpu *vgpu, u64 pdps[]);
>>>>>>>      int intel_vgpu_emulate_ggtt_mmio_read(struct intel_vgpu *vgpu,
>>>>>>>      	unsigned int off, void *p_data, unsigned int bytes);
>>>>>>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
>>>>>>> index 2dff036..8c4c8c1 100644
>>>>>>> --- a/drivers/gpu/drm/i915/gvt/handlers.c
>>>>>>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
>>>>>>> @@ -1139,6 +1139,7 @@ static int pvinfo_mmio_read(struct intel_vgpu *vgpu, unsigned int offset,
>>>>>>>      static int handle_g2v_notification(struct intel_vgpu *vgpu, int notification)
>>>>>>>      {
>>>>>>> +	struct intel_vgpu_mm *mm;
>>>>>>>      	u64 *pdps;
>>>>>>>      	int ret = 0;
>>>>>>> @@ -1146,20 +1147,22 @@ static int handle_g2v_notification(struct intel_vgpu *vgpu, int notification)
>>>>>>>      	switch (notification) {
>>>>>>>      	case VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE:
>>>>>>> -		ret = intel_vgpu_g2v_create_ppgtt_mm(vgpu,
>>>>>>> +		mm = intel_vgpu_get_or_create_ppgtt_mm(vgpu,
>>>>>>>      				GTT_TYPE_PPGTT_ROOT_L3_ENTRY,
>>>>>>>      				pdps);
>>>>>>> +		ret = IS_ERR(mm) ? PTR_ERR(mm) : 0;
>>>>>>>      		break;
>>>>>>>      	case VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY:
>>>>>>> -		ret = intel_vgpu_g2v_destroy_ppgtt_mm(vgpu, pdps);
>>>>>>> +		ret = intel_vgpu_find_destroy_ppgtt_mm(vgpu, pdps);
>>>>>>>      		break;
>>>>>>>      	case VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE:
>>>>>>> -		ret = intel_vgpu_g2v_create_ppgtt_mm(vgpu,
>>>>>>> +		mm = intel_vgpu_get_or_create_ppgtt_mm(vgpu,
>>>>>>>      				GTT_TYPE_PPGTT_ROOT_L4_ENTRY,
>>>>>>>      				pdps);
>>>>>>> +		ret = IS_ERR(mm) ? PTR_ERR(mm) : 0;
>>>>>>>      		break;
>>>>>>>      	case VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY:
>>>>>>> -		ret = intel_vgpu_g2v_destroy_ppgtt_mm(vgpu, pdps);
>>>>>>> +		ret = intel_vgpu_find_destroy_ppgtt_mm(vgpu, pdps);
>>>>>>>      		break;
>>>>>>>      	case VGT_G2V_EXECLIST_CONTEXT_CREATE:
>>>>>>>      	case VGT_G2V_EXECLIST_CONTEXT_DESTROY:
>>>>>>> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
>>>>>>> index 23c33e6..3440ff2 100644
>>>>>>> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
>>>>>>> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
>>>>>>> @@ -1197,18 +1197,11 @@ static int prepare_mm(struct intel_vgpu_workload *workload)
>>>>>>>      	read_guest_pdps(workload->vgpu, workload->ring_context_gpa, pdps);
>>>>>>> -	mm = intel_vgpu_find_ppgtt_mm(workload->vgpu, pdps);
>>>>>>> -	if (mm) {
>>>>>>> -		intel_vgpu_mm_get(mm);
>>>>>>> -	} else {
>>>>>>> -
>>>>>>> -		mm = intel_vgpu_create_ppgtt_mm(workload->vgpu, root_entry_type,
>>>>>>> -						pdps);
>>>>>>> -		if (IS_ERR(mm)) {
>>>>>>> -			gvt_vgpu_err("fail to create mm object.\n");
>>>>>>> -			return PTR_ERR(mm);
>>>>>>> -		}
>>>>>>> -	}
>>>>>>> +	mm = intel_vgpu_get_or_create_ppgtt_mm(workload->vgpu, root_entry_type,
>>>>>>> +					       pdps);
>>>>>>> +	if (IS_ERR(mm))
>>>>>>> +		return PTR_ERR(mm);
>>>>>>> +
>>>>>>>      	workload->shadow_mm = mm;
>>>>>>>      	return 0;
>>>>>>>      }
>>>>>>>
>>>>>> _______________________________________________
>>>>>> 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