[PATCH] drm/i915/gvt: support inconsecutive partial PTE update
Hang Yuan
hang.yuan at linux.intel.com
Mon Sep 17 10:53:16 UTC 2018
On 09/17/2018 01:06 PM, intel-gvt-dev-bounces at lists.freedesktop.org wrote:
> On 09/17/2018 10:58 AM, Zhenyu Wang wrote:
>> On 2018.09.14 19:08:25 +0800, hang.yuan at linux.intel.com wrote:
>>> From: Hang Yuan <hang.yuan at linux.intel.com>
>>>
>>> Previously we assumed two 4-byte writes to the same PTE coming in
>>> sequence.
>>> But recently we observed inconsecutive partial write happening as
>>> well. So
>>> this patch enhances the previous solution. It now uses a list to save
>>> more
>>> partial writes. If one partial write can be combined with another one in
>>> the list to construct a full PTE, update its shadow entry. Otherwise,
>>> save
>>> the partial write in the list.
>>>
>>> This patch also update ggtt_mmio_write more to remove unnecessary
>>> virtual
>>> PTE read and add old shadow PTE invalidation on PTE update case.
>>>
>>> v2: invalidate old entry and flush ggtt (Zhenyu)
>>>
>>> Signed-off-by: Hang Yuan <hang.yuan at linux.intel.com>
>>> Cc: Yan Zhao <yan.y.zhao at intel.com>
>>> Cc: Xiaolin Zhang <xiaolin.zhang at intel.com>
>>> Cc: Zhenyu Wang <zhenyu.z.wang at intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gvt/gtt.c | 119
>>> +++++++++++++++++++++--------------------
>>> drivers/gpu/drm/i915/gvt/gtt.h | 9 +++-
>>> 2 files changed, 69 insertions(+), 59 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c
>>> b/drivers/gpu/drm/i915/gvt/gtt.c
>>> index 2402395..cf11f8f 100644
>>> --- a/drivers/gpu/drm/i915/gvt/gtt.c
>>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
>>> @@ -1905,7 +1905,6 @@ static struct intel_vgpu_mm
>>> *intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
>>> vgpu_free_mm(mm);
>>> return ERR_PTR(-ENOMEM);
>>> }
>>> - mm->ggtt_mm.last_partial_off = -1UL;
>>> return mm;
>>> }
>>> @@ -1930,7 +1929,6 @@ void _intel_vgpu_mm_release(struct kref *mm_ref)
>>> invalidate_ppgtt_mm(mm);
>>> } else {
>>> vfree(mm->ggtt_mm.virtual_ggtt);
>>> - mm->ggtt_mm.last_partial_off = -1UL;
>>> }
>>> vgpu_free_mm(mm);
>>> @@ -2168,6 +2166,8 @@ static int emulate_ggtt_mmio_write(struct
>>> intel_vgpu *vgpu, unsigned int off,
>>> struct intel_gvt_gtt_entry e, m;
>>> dma_addr_t dma_addr;
>>> int ret;
>>> + struct intel_gvt_partial_pte *partial_pte, *pos, *n;
>>> + bool partial_update = false;
>>> if (bytes != 4 && bytes != 8)
>>> return -EINVAL;
>>> @@ -2178,68 +2178,61 @@ static int emulate_ggtt_mmio_write(struct
>>> intel_vgpu *vgpu, unsigned int off,
>>> if (!vgpu_gmadr_is_valid(vgpu, gma))
>>> return 0;
>>> - ggtt_get_guest_entry(ggtt_mm, &e, g_gtt_index);
>>> -
>>> + e.type = GTT_TYPE_GGTT_PTE;
>>> memcpy((void *)&e.val64 + (off & (info->gtt_entry_size - 1)),
>>> p_data,
>>> bytes);
>>> /* If ggtt entry size is 8 bytes, and it's split into two 4 bytes
>>> - * write, we assume the two 4 bytes writes are consecutive.
>>> - * Otherwise, we abort and report error
>>> + * write, save the first 4 bytes in a list and update virtual
>>> + * PTE. Only update shadow PTE when the second 4 bytes comes.
>>> */
>>> if (bytes < info->gtt_entry_size) {
>>> - if (ggtt_mm->ggtt_mm.last_partial_off == -1UL) {
>>> - /* the first partial part*/
>>> - ggtt_mm->ggtt_mm.last_partial_off = off;
>>> - ggtt_mm->ggtt_mm.last_partial_data = e.val64;
>>> - return 0;
>>> - } else if ((g_gtt_index ==
>>> - (ggtt_mm->ggtt_mm.last_partial_off >>
>>> - info->gtt_entry_size_shift)) &&
>>> - (off != ggtt_mm->ggtt_mm.last_partial_off)) {
>>> - /* the second partial part */
>>> -
>>> - int last_off = ggtt_mm->ggtt_mm.last_partial_off &
>>> - (info->gtt_entry_size - 1);
>>> -
>>> - memcpy((void *)&e.val64 + last_off,
>>> - (void *)&ggtt_mm->ggtt_mm.last_partial_data +
>>> - last_off, bytes);
>>> -
>>> - ggtt_mm->ggtt_mm.last_partial_off = -1UL;
>>> - } else {
>>> - int last_offset;
>>> -
>>> - gvt_vgpu_err("failed to populate guest ggtt entry:
>>> abnormal ggtt entry write sequence, last_partial_off=%lx, offset=%x,
>>> bytes=%d, ggtt entry size=%d\n",
>>> - ggtt_mm->ggtt_mm.last_partial_off, off,
>>> - bytes, info->gtt_entry_size);
>>> -
>>> - /* set host ggtt entry to scratch page and clear
>>> - * virtual ggtt entry as not present for last
>>> - * partially write offset
>>> - */
>>> - last_offset = ggtt_mm->ggtt_mm.last_partial_off &
>>> - (~(info->gtt_entry_size - 1));
>>> -
>>> - ggtt_get_host_entry(ggtt_mm, &m, last_offset);
>>> - ggtt_invalidate_pte(vgpu, &m);
>>> - ops->set_pfn(&m, gvt->gtt.scratch_mfn);
>>> - ops->clear_present(&m);
>>> - ggtt_set_host_entry(ggtt_mm, &m, last_offset);
>>> - ggtt_invalidate(gvt->dev_priv);
>>> -
>>> - ggtt_get_guest_entry(ggtt_mm, &e, last_offset);
>>> - ops->clear_present(&e);
>>> - ggtt_set_guest_entry(ggtt_mm, &e, last_offset);
>>> -
>>> - ggtt_mm->ggtt_mm.last_partial_off = off;
>>> - ggtt_mm->ggtt_mm.last_partial_data = e.val64;
>>> + bool found = false;
>>> +
>>> + list_for_each_entry_safe(pos, n,
>>> + &ggtt_mm->ggtt_mm.partial_pte_list, list) {
>>> + if (g_gtt_index == pos->offset >>
>>> + info->gtt_entry_size_shift) {
>>> + if (off != pos->offset) {
>>> + /* the second partial part*/
>>> +
>>> + int last_off = pos->offset &
>>> + (info->gtt_entry_size - 1);
>>> +
>>> + memcpy((void *)&e.val64 + last_off,
>>> + (void *)&pos->data + last_off,
>>> + bytes);
>>> +
>>> + list_del(&pos->list);
>>> + kfree(pos);
>>> + } else {
>>> + /* update of the first partial part */
>>> +
>>> + pos->data = e.val64;
>>> + ggtt_set_guest_entry(ggtt_mm, &e,
>>> + g_gtt_index);
>>> + return 0;
>>> + }
>>> + found = true;
>>> + break;
>>
>> Move this two lines in above if (off != pos->offset) is easier to read.
> Ok.
Recalled there is coding style warning if move 'break' in 'if' branch
and follow by 'else' branch. So have the two lines after 'else' branch.
More information about the intel-gvt-dev
mailing list