[PATCH] drm/i915/gvt: introduced failsafe mode into vgpu

He, Min min.he at intel.com
Wed Feb 15 08:27:00 UTC 2017



在 2/15/2017 3:47 PM, Tian, Kevin 写道:
>> From: Min He
>> Sent: Wednesday, February 15, 2017 3:29 PM
>>
>> New failsafe mode is introduced, when we detect guest not supporting
>> GVTg.
>> In failsafe mode, we will ignore all the MMIO and cfg space read/write
>> from guest.
> instead of ignoring, is it better to make failsafe as a simple
> memory read/write emulation logic? Although we cannot
> guarantee guest behavior in such situation, returning a
> meaningful content (as previous programmed) looks more
> reasonable to cause less abnormal guest behaviors than
> returning all 1s.
Not tried other logic. Handling mmio registers is simple, but emulating 
GTT entries may
need to add more logic. To simplify the design, I used current logic.

>
>> Signed-off-by: Min He <min.he at intel.com>
>> Signed-off-by: Pei Zhang <pei.zhang at intel.com>
>> ---
>>   drivers/gpu/drm/i915/gvt/cfg_space.c |  2 ++
>>   drivers/gpu/drm/i915/gvt/gvt.h       |  2 ++
>>   drivers/gpu/drm/i915/gvt/handlers.c  | 26 ++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/gvt/mmio.c      |  8 ++++++++
>>   drivers/gpu/drm/i915/gvt/vgpu.c      |  8 +++++++-
>>   5 files changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c
>> b/drivers/gpu/drm/i915/gvt/cfg_space.c
>> index 4a6a2ed..d74410b 100644
>> --- a/drivers/gpu/drm/i915/gvt/cfg_space.c
>> +++ b/drivers/gpu/drm/i915/gvt/cfg_space.c
>> @@ -236,6 +236,8 @@ int intel_vgpu_emulate_cfg_write(struct intel_vgpu *vgpu,
>> unsigned int offset,
>>   	void *p_data, unsigned int bytes)
>>   {
>>   	int ret;
>> +	if (vgpu->failsafe)
>> +		return 0;
>>
>>   	if (WARN_ON(bytes > 4))
>>   		return -EINVAL;
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
>> index e227caf..93bd157 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> @@ -143,6 +143,8 @@ struct intel_vgpu {
>>   	int id;
>>   	unsigned long handle; /* vGPU handle used by hypervisor MPT modules */
>>   	bool active;
>> +	bool pv_notified;
>> +	bool failsafe;
>>   	bool resetting;
>>   	void *sched_data;
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
>> index 1d45062..6089a1e 100644
>> --- a/drivers/gpu/drm/i915/gvt/handlers.c
>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
>> @@ -154,6 +154,17 @@ static int sanitize_fence_mmio_access(struct intel_vgpu *vgpu,
>>   		unsigned int fence_num, void *p_data, unsigned int bytes)
>>   {
>>   	if (fence_num >= vgpu_fence_sz(vgpu)) {
>> +		if (!vgpu->pv_notified) {
>> +			/* When guest access oob fence regs without access
>> +			 * pv_info first, we treat guest not supporting GVT,
>> +			 * and we will let vgpu enter failsafe mode.
>> +			 */
>> +			vgpu->failsafe = true;
>> +			pr_err("***Detected guest not supporting GVT-g***\n");
>> +			pr_err("***Please update your guest driver*******\n");
>> +			pr_err("***Entering failsafe mode****************\n");
>> +			return -EINVAL;
>> +		}
> move above logic into a function to avoid duplication in multiple places.
Same as the answer to Changbin. We may have different entries to 
failsafe mode, so the error log is different.  It's hard to encapsule 
these logs into one function.

>
>>   		gvt_err("vgpu%d: found oob fence register access\n",
>>   				vgpu->id);
>>   		gvt_err("vgpu%d: total fence num %d access fence num %d\n",
>> @@ -1001,6 +1012,7 @@ static int pvinfo_mmio_read(struct intel_vgpu *vgpu, unsigned
>> int offset,
>>   	if (invalid_read)
>>   		gvt_err("invalid pvinfo read: [%x:%x] = %x\n",
>>   				offset, bytes, *(u32 *)p_data);
>> +	vgpu->pv_notified = true;
>>   	return 0;
>>   }
>>
>> @@ -1318,6 +1330,20 @@ static int ring_mode_mmio_write(struct intel_vgpu *vgpu,
>> unsigned int offset,
>>   	bool enable_execlist;
>>
>>   	write_vreg(vgpu, offset, p_data, bytes);
>> +
>> +	if (((data & _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE)) ||
>> +			(data & _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE)))
>> +			&& !vgpu->pv_notified) {
>> +		/* when PPGTT mode enabled, we will check if guest has called
>> +		 * pvinfo, if not, we will treat this guest as non-gvtg-aware
>> +		 * guest, and stop emulating its cfg space, mmio, gtt, etc.
>> +		 */
>> +		vgpu->failsafe = true;
>> +		pr_err("***Detected guest not supporting GVT-g***\n");
>> +		pr_err("***Please update your guest driver*******\n");
>> +		pr_err("***Entering failsafe mode****************\n");
>> +		return 0;
>> +	}
>>   	if ((data & _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE))
>>   			|| (data & _MASKED_BIT_DISABLE(GFX_RUN_LIST_ENABLE))) {
>>   		enable_execlist = !!(data & GFX_RUN_LIST_ENABLE);
>> diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c
>> index 4df078b..7f55b42 100644
>> --- a/drivers/gpu/drm/i915/gvt/mmio.c
>> +++ b/drivers/gpu/drm/i915/gvt/mmio.c
>> @@ -75,6 +75,11 @@ int intel_vgpu_emulate_mmio_read(struct intel_vgpu *vgpu,
>> uint64_t pa,
>>   	unsigned int offset = 0;
>>   	int ret = -EINVAL;
>>
>> +
>> +	if (vgpu->failsafe) {
>> +		memset(p_data, 0xff, bytes);
>> +		return 0;
>> +	}
>>   	mutex_lock(&gvt->lock);
>>
>>   	if (atomic_read(&vgpu->gtt.n_write_protected_guest_page)) {
>> @@ -188,6 +193,9 @@ int intel_vgpu_emulate_mmio_write(struct intel_vgpu *vgpu,
>> uint64_t pa,
>>   	u32 old_vreg = 0, old_sreg = 0;
>>   	int ret = -EINVAL;
>>
>> +	if (vgpu->failsafe)
>> +		return 0;
>> +
>>   	mutex_lock(&gvt->lock);
>>
>>   	if (atomic_read(&vgpu->gtt.n_write_protected_guest_page)) {
>> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
>> index 67d471c..0d1851a 100644
>> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
>> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
>> @@ -224,6 +224,8 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt
>> *gvt,
>>   		goto out_free_vgpu;
>>
>>   	vgpu->id = ret;
>> +	vgpu->failsafe = false;
>> +	vgpu->pv_notified = false;
>>   	vgpu->handle = param->handle;
>>   	vgpu->gvt = gvt;
>>   	bitmap_zero(vgpu->tlb_handle_pending, I915_NUM_ENGINES);
>> @@ -386,8 +388,12 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool
>> dmlr,
>>   		intel_vgpu_reset_mmio(vgpu);
>>   		populate_pvinfo_page(vgpu);
>>
>> -		if (dmlr)
>> +		if (dmlr) {
>>   			intel_vgpu_reset_cfg_space(vgpu);
>> +			/* only reset the failsafe mode when dmlr reset */
>> +			vgpu->failsafe = false;
>> +			vgpu->pv_notified = false;
>> +		}
>>   	}
>>
>>   	vgpu->resetting = false;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> 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