| Message ID | 1497964374-31518-1-git-send-email-tina.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
This patch should be merged to intel-gvt-dev. Here just for the review. Thanks. BR, Tina > -----Original Message----- > From: Patchwork [mailto:patchwork@emeril.freedesktop.org] > Sent: Tuesday, June 20, 2017 10:06 PM > To: Zhang, Tina <tina.zhang@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: ✗ Fi.CI.BAT: failure for drm/i915/gvt: Fix guest i915 48bit full ppgtt > blocking issue > > == Series Details == > > Series: drm/i915/gvt: Fix guest i915 48bit full ppgtt blocking issue > URL : https://patchwork.freedesktop.org/series/26049/ > State : failure > > == Summary == > > CHK include/config/kernel.release > CHK include/generated/uapi/linux/version.h > CHK include/generated/utsrelease.h > CHK include/generated/bounds.h > CHK include/generated/timeconst.h > CHK include/generated/asm-offsets.h > CALL scripts/checksyscalls.sh > CHK scripts/mod/devicetable-offsets.h > CHK include/generated/compile.h > CHK kernel/config_data.h > CC [M] drivers/gpu/drm/i915/gvt/vgpu.o In file included from > drivers/gpu/drm/i915/gvt/gvt.h:38:0, > from drivers/gpu/drm/i915/gvt/vgpu.c:35: > drivers/gpu/drm/i915/gvt/vgpu.c: In function ‘populate_pvinfo_page’: > ./include/linux/compiler-gcc.h:161:2: error: ‘struct vgt_if’ has no member > named ‘vgt_caps’ > __builtin_offsetof(a, b) > ^ > drivers/gpu/drm/i915/gvt/mmio.h:75:9: note: in definition of macro > ‘INTEL_GVT_MMIO_OFFSET’ > typeof(reg) __reg = reg; \ > ^ > drivers/gpu/drm/i915/gvt/vgpu.c:46:2: note: in expansion of macro ‘vgpu_vreg’ > vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT; > ^ > drivers/gpu/drm/i915/i915_pvinfo.h:103:2: note: in expansion of macro > ‘_MMIO’ > _MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x))) > ^ > ./include/linux/stddef.h:16:32: note: in expansion of macro > ‘__compiler_offsetof’ > #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER) > ^ > drivers/gpu/drm/i915/i915_pvinfo.h:103:27: note: in expansion of macro > ‘offsetof’ > _MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x))) > ^ > drivers/gpu/drm/i915/gvt/vgpu.c:46:18: note: in expansion of macro ‘vgtif_reg’ > vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT; > ^ > ./include/linux/compiler-gcc.h:161:2: error: ‘struct vgt_if’ has no member > named ‘vgt_caps’ > __builtin_offsetof(a, b) > ^ > drivers/gpu/drm/i915/gvt/mmio.h:75:22: note: in definition of macro > ‘INTEL_GVT_MMIO_OFFSET’ > typeof(reg) __reg = reg; \ > ^ > drivers/gpu/drm/i915/gvt/vgpu.c:46:2: note: in expansion of macro ‘vgpu_vreg’ > vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT; > ^ > drivers/gpu/drm/i915/i915_pvinfo.h:103:2: note: in expansion of macro > ‘_MMIO’ > _MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x))) > ^ > ./include/linux/stddef.h:16:32: note: in expansion of macro > ‘__compiler_offsetof’ > #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER) > ^ > drivers/gpu/drm/i915/i915_pvinfo.h:103:27: note: in expansion of macro > ‘offsetof’ > _MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x))) > ^ > drivers/gpu/drm/i915/gvt/vgpu.c:46:18: note: in expansion of macro ‘vgtif_reg’ > vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT; > ^ > drivers/gpu/drm/i915/gvt/vgpu.c:46:41: error: > ‘VGT_CAPS_FULL_PPGTT_48BIT’ undeclared (first use in this function) > vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT; > ^ > drivers/gpu/drm/i915/gvt/vgpu.c:46:41: note: each undeclared identifier is > reported only once for each function it appears in > scripts/Makefile.build:302: recipe for target 'drivers/gpu/drm/i915/gvt/vgpu.o' > failed > make[4]: *** [drivers/gpu/drm/i915/gvt/vgpu.o] Error 1 > scripts/Makefile.build:561: recipe for target 'drivers/gpu/drm/i915' failed > make[3]: *** [drivers/gpu/drm/i915] Error 2 > scripts/Makefile.build:561: recipe for target 'drivers/gpu/drm' failed > make[2]: *** [drivers/gpu/drm] Error 2 > scripts/Makefile.build:561: recipe for target 'drivers/gpu' failed > make[1]: *** [drivers/gpu] Error 2 > Makefile:1016: recipe for target 'drivers' failed > make: *** [drivers] Error 2
Hi Tina,
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.12-rc6 next-20170621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tina-Zhang/drm-i915-gvt-Fix-guest-i915-48bit-full-ppgtt-blocking-issue/20170621-233658
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a0-06220349 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
In file included from drivers/gpu/drm/i915/gvt/gvt.h:38:0,
from drivers/gpu/drm/i915/gvt/vgpu.c:35:
drivers/gpu/drm/i915/gvt/vgpu.c: In function 'populate_pvinfo_page':
include/linux/compiler-gcc.h:161:2: error: 'struct vgt_if' has no member named 'vgt_caps'; did you mean 'vgt_id'?
__builtin_offsetof(a, b)
^
drivers/gpu/drm/i915/gvt/mmio.h:75:9: note: in definition of macro 'INTEL_GVT_MMIO_OFFSET'
typeof(reg) __reg = reg; \
^~~
drivers/gpu/drm/i915/gvt/vgpu.c:46:2: note: in expansion of macro 'vgpu_vreg'
vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT;
^~~~~~~~~
drivers/gpu/drm/i915/i915_pvinfo.h:103:2: note: in expansion of macro '_MMIO'
_MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x)))
^~~~~
include/linux/stddef.h:16:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/i915_pvinfo.h:103:27: note: in expansion of macro 'offsetof'
_MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x)))
^~~~~~~~
drivers/gpu/drm/i915/gvt/vgpu.c:46:18: note: in expansion of macro 'vgtif_reg'
vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT;
^~~~~~~~~
include/linux/compiler-gcc.h:161:2: error: 'struct vgt_if' has no member named 'vgt_caps'; did you mean 'vgt_id'?
__builtin_offsetof(a, b)
^
drivers/gpu/drm/i915/gvt/mmio.h:75:22: note: in definition of macro 'INTEL_GVT_MMIO_OFFSET'
typeof(reg) __reg = reg; \
^~~
drivers/gpu/drm/i915/gvt/vgpu.c:46:2: note: in expansion of macro 'vgpu_vreg'
vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT;
^~~~~~~~~
drivers/gpu/drm/i915/i915_pvinfo.h:103:2: note: in expansion of macro '_MMIO'
_MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x)))
^~~~~
include/linux/stddef.h:16:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/i915_pvinfo.h:103:27: note: in expansion of macro 'offsetof'
_MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x)))
^~~~~~~~
drivers/gpu/drm/i915/gvt/vgpu.c:46:18: note: in expansion of macro 'vgtif_reg'
vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT;
^~~~~~~~~
>> drivers/gpu/drm/i915/gvt/vgpu.c:46:41: error: 'VGT_CAPS_FULL_PPGTT_48BIT' undeclared (first use in this function)
vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT;
^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gvt/vgpu.c:46:41: note: each undeclared identifier is reported only once for each function it appears in
vim +/VGT_CAPS_FULL_PPGTT_48BIT +46 drivers/gpu/drm/i915/gvt/vgpu.c
40 /* setup the ballooning information */
41 vgpu_vreg64(vgpu, vgtif_reg(magic)) = VGT_MAGIC;
42 vgpu_vreg(vgpu, vgtif_reg(version_major)) = 1;
43 vgpu_vreg(vgpu, vgtif_reg(version_minor)) = 0;
44 vgpu_vreg(vgpu, vgtif_reg(display_ready)) = 0;
45 vgpu_vreg(vgpu, vgtif_reg(vgt_id)) = vgpu->id;
> 46 vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT;
47 vgpu_vreg(vgpu, vgtif_reg(avail_rs.mappable_gmadr.base)) =
48 vgpu_aperture_gmadr_base(vgpu);
49 vgpu_vreg(vgpu, vgtif_reg(avail_rs.mappable_gmadr.size)) =
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 66374db..5edcfaa 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -979,29 +979,26 @@ static int ppgtt_populate_shadow_page(struct intel_vgpu_ppgtt_spt *spt) } static int ppgtt_handle_guest_entry_removal(struct intel_vgpu_guest_page *gpt, - unsigned long index) + struct intel_gvt_gtt_entry *se, unsigned long index) { struct intel_vgpu_ppgtt_spt *spt = guest_page_to_ppgtt_spt(gpt); struct intel_vgpu_shadow_page *sp = &spt->shadow_page; struct intel_vgpu *vgpu = spt->vgpu; struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops; - struct intel_gvt_gtt_entry e; int ret; - ppgtt_get_shadow_entry(spt, &e, index); - - trace_gpt_change(spt->vgpu->id, "remove", spt, sp->type, e.val64, + trace_gpt_change(spt->vgpu->id, "remove", spt, sp->type, se->val64, index); - if (!ops->test_present(&e)) + if (!ops->test_present(se)) return 0; - if (ops->get_pfn(&e) == vgpu->gtt.scratch_pt[sp->type].page_mfn) + if (ops->get_pfn(se) == vgpu->gtt.scratch_pt[sp->type].page_mfn) return 0; - if (gtt_type_is_pt(get_next_pt_type(e.type))) { + if (gtt_type_is_pt(get_next_pt_type(se->type))) { struct intel_vgpu_ppgtt_spt *s = - ppgtt_find_shadow_page(vgpu, ops->get_pfn(&e)); + ppgtt_find_shadow_page(vgpu, ops->get_pfn(se)); if (!s) { gvt_vgpu_err("fail to find guest page\n"); ret = -ENXIO; @@ -1011,12 +1008,10 @@ static int ppgtt_handle_guest_entry_removal(struct intel_vgpu_guest_page *gpt, if (ret) goto fail; } - ops->set_pfn(&e, vgpu->gtt.scratch_pt[sp->type].page_mfn); - ppgtt_set_shadow_entry(spt, &e, index); return 0; fail: gvt_vgpu_err("fail: shadow page %p guest entry 0x%llx type %d\n", - spt, e.val64, e.type); + spt, se->val64, se->type); return ret; } @@ -1236,22 +1231,37 @@ static int ppgtt_handle_guest_write_page_table( { struct intel_vgpu_ppgtt_spt *spt = guest_page_to_ppgtt_spt(gpt); struct intel_vgpu *vgpu = spt->vgpu; + int type = spt->shadow_page.type; struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops; + struct intel_gvt_gtt_entry se; int ret; int new_present; new_present = ops->test_present(we); - ret = ppgtt_handle_guest_entry_removal(gpt, index); - if (ret) - goto fail; + /* + * Adding the new entry first and then removing the old one, that can + * guarantee the ppgtt table is validated during the window between + * adding and removal. + */ + ppgtt_get_shadow_entry(spt, &se, index); if (new_present) { ret = ppgtt_handle_guest_entry_add(gpt, we, index); if (ret) goto fail; } + + ret = ppgtt_handle_guest_entry_removal(gpt, &se, index); + if (ret) + goto fail; + + if (!new_present) { + ops->set_pfn(&se, vgpu->gtt.scratch_pt[type].page_mfn); + ppgtt_set_shadow_entry(spt, &se, index); + } + return 0; fail: gvt_vgpu_err("fail: shadow page %p guest entry 0x%llx type %d.\n", @@ -1323,7 +1333,7 @@ static int ppgtt_handle_guest_write_page_table_bytes(void *gp, struct intel_vgpu *vgpu = spt->vgpu; struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops; const struct intel_gvt_device_info *info = &vgpu->gvt->device_info; - struct intel_gvt_gtt_entry we; + struct intel_gvt_gtt_entry we, se; unsigned long index; int ret; @@ -1339,7 +1349,8 @@ static int ppgtt_handle_guest_write_page_table_bytes(void *gp, return ret; } else { if (!test_bit(index, spt->post_shadow_bitmap)) { - ret = ppgtt_handle_guest_entry_removal(gpt, index); + ppgtt_get_shadow_entry(spt, &se, index); + ret = ppgtt_handle_guest_entry_removal(gpt, &se, index); if (ret) return ret; } diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c index 90c14e6..73f47c1 100644 --- a/drivers/gpu/drm/i915/gvt/vgpu.c +++ b/drivers/gpu/drm/i915/gvt/vgpu.c @@ -43,6 +43,7 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu) vgpu_vreg(vgpu, vgtif_reg(version_minor)) = 0; vgpu_vreg(vgpu, vgtif_reg(display_ready)) = 0; vgpu_vreg(vgpu, vgtif_reg(vgt_id)) = vgpu->id; + vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT_48BIT; vgpu_vreg(vgpu, vgtif_reg(avail_rs.mappable_gmadr.base)) = vgpu_aperture_gmadr_base(vgpu); vgpu_vreg(vgpu, vgtif_reg(avail_rs.mappable_gmadr.size)) =
Guest i915 48bit full ppgtt functionality was blocking by an issue, which would lead to gpu hardware hang. Guest i915 driver may update the ppgtt table just before this workload is going to be submitted to the hardware by device model. This case wasn't handled well by device model before, due to the small time window between removing old ppgtt entry and adding the new one. Errors occur when the workload is executed by hardware during that small time window. This patch is to remove this time window by adding the new ppgtt entry first and then remove the old one. Changes in v2: - Move VGT_CAPS_FULL_PPGTT introduction to patch 2/4. (Joonas) Changes since v2: - Divide the whole patch set into two separate patch series, with one patch in i915 side to check guest i915 full ppgtt capability and enable it when this capability is supported by the device model, and the other one in gvt side which fixs the blocking issue and enables the device model to provide the capability to guest. And this patch focuses on gvt side. (Joonas) - Change the title from "reorder the shadow ppgtt update process by adding entry first" to "Fix guest i915 full ppgtt blocking issue". (Tina) Changes since v3: - Rebase to the latest branch. Changes since v4: - Tested by Tina Zhang. Changes since v5: - Rebase to the latest branch. Signed-off-by: Tina Zhang <tina.zhang@intel.com> --- drivers/gpu/drm/i915/gvt/gtt.c | 45 +++++++++++++++++++++++++---------------- drivers/gpu/drm/i915/gvt/vgpu.c | 1 + 2 files changed, 29 insertions(+), 17 deletions(-)