[PATCH] drm/i915/gvt: Skip writing 0 to HWSP during D3 resume

[Colin Xu]

On 2020-09-08 17:15, Zhenyu Wang wrote:
> On 2020.08.19 09:09:53 +0800, Colin Xu wrote:
>> Guest driver may reset HWSP to 0 as init value during D3->D0:
>> The full sequence is:
>> - Boot ->D0
>> - Update HWSP
>> - D0->D3
>> - ...In D3 state...
>> - D3->D0
>> - DMLR reset.
>> - Set engine HWSP to 0.
>> - Set engine ring mode to 0.
>> - Set engine HWSP to correct value.
>> - Set engine ring mode to correct value.
>> Ring mode is masked register so set 0 won't take effect.
>> However HWPS addr 0 is considered as invalid GGTT address which will
>> report error like:
>> gvt: vgpu 1: write invalid HWSP address, reg:0x2080, value:0x0
>>
> So looks this is to handle that wrong error message but function is
> still same, right?
Yes that's right. Current logic can guarantee that only correct HWSP 
write from guest are accepted by GVT. Invalid HWSP address will be 
dropped and won't be submitted to HW. During resume, correct HWSP will 
be updated eventually updated. If pv_notified is set, the intermediate 
HWSP zero-out can be ignored until hit next reset.
>
>> During vGPU in D3, per-engine HWSP gpa remains valid so we can skip
>> update HWSP in this case.
>> Check both pv_notified and previous engine HWSP gpa, if pv already
>> notified and previous HWSP gpa is valid, we skip this HWSP init and
>> let later HWSP write update the correct value. We also need zero out
>> per-engine HWSP gpa on engine reset to make sure hws_pga is valid.
>>
>> Signed-off-by: Colin Xu <colin.xu at intel.com>
>> ---
>>   drivers/gpu/drm/i915/gvt/handlers.c | 30 ++++++++++++++++++++---------
>>   drivers/gpu/drm/i915/gvt/vgpu.c     |  7 +++++++
>>   2 files changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
>> index 840572add2d4..72860aaf1656 100644
>> --- a/drivers/gpu/drm/i915/gvt/handlers.c
>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
>> @@ -1489,12 +1489,6 @@ static int hws_pga_write(struct intel_vgpu *vgpu, unsigned int offset,
>>   	const struct intel_engine_cs *engine =
>>   		intel_gvt_render_mmio_to_engine(vgpu->gvt, offset);
>>   
>> -	if (!intel_gvt_ggtt_validate_range(vgpu, value, I915_GTT_PAGE_SIZE)) {
>> -		gvt_vgpu_err("write invalid HWSP address, reg:0x%x, value:0x%x\n",
>> -			      offset, value);
>> -		return -EINVAL;
>> -	}
>> -
>>   	/*
>>   	 * Need to emulate all the HWSP register write to ensure host can
>>   	 * update the VM CSB status correctly. Here listed registers can
>> @@ -1505,9 +1499,27 @@ static int hws_pga_write(struct intel_vgpu *vgpu, unsigned int offset,
>>   			     offset);
>>   		return -EINVAL;
>>   	}
>> -	vgpu->hws_pga[engine->id] = value;
>> -	gvt_dbg_mmio("VM(%d) write: 0x%x to HWSP: 0x%x\n",
>> -		     vgpu->id, value, offset);
>> +
>> +	if (!intel_gvt_ggtt_validate_range(vgpu, value, I915_GTT_PAGE_SIZE)) {
>> +		u32 old = vgpu->hws_pga[engine->id];
>> +
>> +		/* Skip zero out RING_HWS_PGA during D3 resume */
>> +		if (vgpu->pv_notified && value == 0 &&
>> +		    intel_gvt_ggtt_validate_range(vgpu, old,
>> +						  I915_GTT_PAGE_SIZE)) {
>> +			gvt_dbg_mmio("Skip zero out HWSP address, reg:0x%x, "
>> +				     "value:0x%x\n", offset, value);
>> +
>> +		} else {
>> +			gvt_vgpu_err("write invalid HWSP address, reg:0x%x, "
>> +				     "value:0x%x\n", offset, value);
>> +			return -EINVAL;
>> +		}
>> +	} else {
>> +		vgpu->hws_pga[engine->id] = value;
>> +		gvt_dbg_mmio("VM(%d) write: 0x%x to HWSP: 0x%x\n",
>> +			     vgpu->id, value, offset);
>> +	}
>>   
>>   	return intel_vgpu_default_mmio_write(vgpu, offset, &value, bytes);
>>   }
>> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
>> index 8fa9b31a2484..e0e073045d83 100644
>> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
>> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
>> @@ -558,6 +558,9 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
>>   	intel_vgpu_reset_submission(vgpu, resetting_eng);
>>   	/* full GPU reset or device model level reset */
>>   	if (engine_mask == ALL_ENGINES || dmlr) {
>> +		struct intel_engine_cs *engine;
>> +		intel_engine_mask_t tmp;
>> +
>>   		intel_vgpu_select_submission_ops(vgpu, ALL_ENGINES, 0);
>>   		if (engine_mask == ALL_ENGINES)
>>   			intel_vgpu_invalidate_ppgtt(vgpu);
>> @@ -588,6 +591,10 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
>>   			else
>>   				vgpu->pv_notified = false;
>>   		}
>> +
>> +		for_each_engine_masked(engine, gvt->gt, engine_mask, tmp) {
>> +			vgpu->hws_pga[engine->id] = 0;
>> +		}
>>   	}
>>   
>>   	vgpu->resetting_eng = 0;
>> -- 
>> 2.28.0
>>
>> _______________________________________________
>> intel-gvt-dev mailing list
>> intel-gvt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Best Regards,
Colin Xu