From b79c52aef3cdee903017c1e9834b53996d70010e Mon Sep 17 00:00:00 2001 From: Zhi Wang Date: Thu, 30 Mar 2017 01:48:39 +0800 Subject: [PATCH 1/3] drm/i915/gvt: Activate/de-activate vGPU in mdev ops. 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. V-blank emulation will emulate and inject V-blank interrupt for every active vGPU with holding gvt->lock, while in mdev release path, it will directly release hypervisor handle without changing vGPU status or taking gvt->lock, so a kernel oops is encountered when vblank emulation is injecting a interrupt with a invalid hypervisor handle. (Reported by Terrence) 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 v-blank for it or touch the hypervisor handle. Fixes: 659643f ("drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT") Signed-off-by: Zhi Wang Signed-off-by: Zhenyu Wang --- 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 | 53 ++++++++++++++++++++++++++------ 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c index 3b9d59e457ba..ef3baa0c4754 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 23791920ced1..2387eacf74bb 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 d641214578a7..9843d74056a8 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 41cfa5ccae84..2f5792a1ce38 100644 --- a/drivers/gpu/drm/i915/gvt/vgpu.c +++ b/drivers/gpu/drm/i915/gvt/vgpu.c @@ -178,6 +178,47 @@ static void intel_gvt_update_vgpu_types(struct intel_gvt *gvt) } } +/** + * intel_gvt_active_vgpu - activate a virtual GPU + * @vgpu: virtual GPU + * + * This function is called when user wants to activate a virtual GPU. + * + */ +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; + + if (atomic_read(&vgpu->running_workload_num)) { + mutex_unlock(&gvt->lock); + intel_gvt_wait_vgpu_idle(vgpu); + mutex_lock(&gvt->lock); + } + + intel_vgpu_stop_schedule(vgpu); + + mutex_unlock(&gvt->lock); +} + /** * intel_gvt_destroy_vgpu - destroy a virtual GPU * @vgpu: virtual GPU @@ -191,16 +232,9 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu) mutex_lock(&gvt->lock); - vgpu->active = false; + WARN(vgpu->active, "vGPU is still active!\n"); + idr_remove(&gvt->vgpu_idr, vgpu->id); - - if (atomic_read(&vgpu->running_workload_num)) { - mutex_unlock(&gvt->lock); - intel_gvt_wait_vgpu_idle(vgpu); - mutex_lock(&gvt->lock); - } - - intel_vgpu_stop_schedule(vgpu); 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; From f85726905745fb4f6e15c68e2ade9da5390f8d89 Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Thu, 30 Mar 2017 11:32:05 +0800 Subject: [PATCH 2/3] drm/i915/gvt: exclude cfg space from failsafe mode When test GVTg as below scenario: VM boot --> failsafe --> kill qemu --> VM boot. Qemu report error at the second boot: ERROR: PCI region size must be pow2 type=0x0, size=0x1fa1000 Qemu need access PCI_ROM_ADDRESS reg to determine the size of expansion PCI rom. The mechanism just like the BAR reg (write-read) and we should return the size 0 since we have no rom. If we reject the write to PCI_ROM_ADDRESS, Qemu cannot get the correct size of rom. Essentially, GVTg failsafe mode should not break PCI function. So we exclude cfg space from failsafe mode. This can fix above issue. v2: add Fixes and Bugzilla link. Fixes: fd64be636708d ("drm/i915/gvt: introduced failsafe mode into vgpu") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100296 Signed-off-by: Changbin Du Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cfg_space.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c b/drivers/gpu/drm/i915/gvt/cfg_space.c index b7d7721e72fa..40af17ec6312 100644 --- a/drivers/gpu/drm/i915/gvt/cfg_space.c +++ b/drivers/gpu/drm/i915/gvt/cfg_space.c @@ -285,9 +285,6 @@ int intel_vgpu_emulate_cfg_write(struct intel_vgpu *vgpu, unsigned int offset, { int ret; - if (vgpu->failsafe) - return 0; - if (WARN_ON(bytes > 4)) return -EINVAL; From aa4ce4493c88dc324911152d1ccd25469366dba3 Mon Sep 17 00:00:00 2001 From: Zhi Wang Date: Sat, 1 Apr 2017 00:00:53 +0800 Subject: [PATCH 3/3] drm/i915/gvt: Fix firmware loading interface for GVT-g golden HW state Firmware loading interface for GVT-g golden HW state has been broken before. This patch fixes GVT-g firmware loading interface. A user should apply this patch if he wants to load GVT-g golden HW state from firmware interface. Fixes: 579cea5 ("drm/i915/gvt: golden virtual HW state management") Cc: Zhenyu Wang Cc: drm-intel-fixes@lists.freedesktop.org Signed-off-by: Zhi Wang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/firmware.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c index 933a7c211a1c..dce8d15f706f 100644 --- a/drivers/gpu/drm/i915/gvt/firmware.c +++ b/drivers/gpu/drm/i915/gvt/firmware.c @@ -75,11 +75,11 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt) struct gvt_firmware_header *h; void *firmware; void *p; - unsigned long size; + unsigned long size, crc32_start; int i; int ret; - size = sizeof(*h) + info->mmio_size + info->cfg_space_size - 1; + size = sizeof(*h) + info->mmio_size + info->cfg_space_size; firmware = vzalloc(size); if (!firmware) return -ENOMEM; @@ -112,6 +112,9 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt) memcpy(gvt->firmware.mmio, p, info->mmio_size); + crc32_start = offsetof(struct gvt_firmware_header, crc32) + 4; + h->crc32 = crc32_le(0, firmware + crc32_start, size - crc32_start); + firmware_attr.size = size; firmware_attr.private = firmware; @@ -234,7 +237,7 @@ int intel_gvt_load_firmware(struct intel_gvt *gvt) firmware->mmio = mem; - sprintf(path, "%s/vid_0x%04x_did_0x%04x_rid_0x%04x.golden_hw_state", + sprintf(path, "%s/vid_0x%04x_did_0x%04x_rid_0x%02x.golden_hw_state", GVT_FIRMWARE_PATH, pdev->vendor, pdev->device, pdev->revision);