[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
Mon Dec 25 10:44:06 UTC 2017
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.
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