[PATCH] drm/i915/gvt: Add the support of HUC_STATUS2 reg emulation for Guest VGPU
Zhao, Yakui
yakui.zhao at intel.com
Tue Mar 27 06:03:39 UTC 2018
On 2018年03月27日 13:07, Zhenyu Wang wrote:
> On 2018.03.27 13:07:51 +0800, yzhao3 wrote:
>> On 2018年03月27日 11:18, Zhenyu Wang wrote:
>>> On 2018.03.27 09:19:03 +0800, yakui.zhao at intel.com wrote:
>>>> From: Zhao Yakui <yakui.zhao at intel.com>
>>>>
>>>> The HUC_STATUS2 reg is used to indicate whether the Huc FW is loaded. Only
>>>> when it is loaded successfully, the user-space driver on guest can use the
>>>> Huc to do the expected operation. This provides the support of HUC_STATUS2
>>>> trap for guest VGPU.
>>>>
>>>
>>> As we don't support huc for guest now, what scenario does this fix?
>>> And for future huc support, I think we can have some short-cut in guest
>>> driver e.g skip fw load, get status from pvinfo, etc.
>>>
>> The user-space driver only uses the I915_PARAM_HUC_STATUS ioctl to check
>> whether the Huc can be used(I915_PARAM_HUC_STATUS will read the HUC_STATUS2
>> register). Only when one bit is enabled, it can submit the GPU commands
>> related with Huc operation.
>>
>> The current issue is that the HUC_STATUS2 reg on guest doesn't report the
>> required bits and then it can't continue the Huc operation although the Huc
>> is supported on gvt-g. If it is reported as expected, the guest can send the
>> GPU commands related with Huc operation.
>>
>
> ok, I was thinking huc commands require guc enabled, but seems not true,
> so we do can run huc command by execlist without guc. And given current way of
> i915 huc check, we do can expose huc capability. btw, besides HUC_STATUS2 are
> there other registers need to be handled for huc state?
Currently only the HUC_STATUS2 reg is needed for the querying Huc_state.
>
> Some code comment below.
>
>> For the future support of Huc/Guc, I agree with your point. The loading of
>> Guc/Huc FW should be skipped on guest VGPU. And based on the status of
>> Guc/Huc from gvt-g, the GPU commands related with Guc/Huc can be submitted
>> and then mediated to gvt-g.
>>
>>>> Signed-off-by: Zhao Yakui <yakui.zhao at intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/gvt/handlers.c | 1 +
>>>> drivers/gpu/drm/i915/gvt/mmio.c | 6 ++++++
>>>> 2 files changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
>>>> index a33c1c3e..465b7f6 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/handlers.c
>>>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
>>>> @@ -2867,6 +2867,7 @@ static int init_skl_mmio_info(struct intel_gvt *gvt)
>>>> MMIO_D(_MMIO(0x4ab8), D_KBL);
>>>> MMIO_D(_MMIO(0x2248), D_SKL_PLUS | D_KBL);
>>>> + MMIO_D(HUC_STATUS2, D_GEN9PLUS);
>
> We'd better have a real handler for that instead of only reading at reset time,
> as we shouldn't depend on any code sequence for gvt reset/init vs. huc loading,
> but should always check actual state.
>
It is also ok to add one real handler for HUC_STATUS2 reg.
But in fact as the Huc FW is initialized/loaded only once in course of
driver loading and the HUC_STATUS2 reg is read-only driver for the
driver, so it is initialized for vgpu reset/init.
If the handler is preferred, I will add it.
>
>>>> return 0;
>>>> }
>>>> diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c
>>>> index 11b71b3..7cf972d 100644
>>>> --- a/drivers/gpu/drm/i915/gvt/mmio.c
>>>> +++ b/drivers/gpu/drm/i915/gvt/mmio.c
>>>> @@ -235,6 +235,7 @@ void intel_vgpu_reset_mmio(struct intel_vgpu *vgpu, bool dmlr)
>>>> struct intel_gvt *gvt = vgpu->gvt;
>>>> const struct intel_gvt_device_info *info = &gvt->device_info;
>>>> void *mmio = gvt->firmware.mmio;
>>>> + struct drm_i915_private *dev_priv = gvt->dev_priv;
>>>> if (dmlr) {
>>>> memcpy(vgpu->mmio.vreg, mmio, info->mmio_size);
>>>> @@ -256,6 +257,11 @@ void intel_vgpu_reset_mmio(struct intel_vgpu *vgpu, bool dmlr)
>>>> memcpy(vgpu->mmio.sreg, mmio, GVT_GEN8_MMIO_RESET_OFFSET);
>>>> }
>>>> + if (HAS_HUC_UCODE(dev_priv)) {
>
> maybe use USES_HUC()? so only read hw value when huc is loaded, otherwise can just skip.
The status of Huc loading only depends on the value of HUC_STATUS2 reg.
(The dev_priv->huc_fw.load_status can't reflect the status of Huc as it
needs the Guc authentication).
So it is checked on the platforms that supports the Guc/Huc.
How do you think?
>
>>>> + mmio_hw_access_pre(dev_priv);
>>>> + vgpu_vreg(vgpu, HUC_STATUS2) = I915_READ(HUC_STATUS2);
>>>> + mmio_hw_access_post(dev_priv);
>>>> + }
>>>> }
>>>> /**
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> intel-gvt-dev mailing list
>>>> intel-gvt-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
>>>
>
>
>
> _______________________________________________
> 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