[PATCH v1 1/5] drm/i915/gvt: GVTg handle enable_pvmmio PVINFO register
Zhenyu Wang
zhenyuw at linux.intel.com
Tue Nov 6 06:06:21 UTC 2018
On 2018.11.06 05:20:19 +0000, Zhang, Xiaolin wrote:
> On 11/05/2018 05:59 PM, Zhenyu Wang wrote:
> > On 2018.11.05 17:20:45 +0800, Xiaolin Zhang wrote:
> >> implement enable_pvmmio PVINFO register handler in GVTg to
> >> control different level pvmmio optimization within guest.
> >>
> >> report VGT_CAPS_PVMMIO capability in pvinfo page for guest.
> >>
> >> v1: rebase
> >> v0: RFC
> >>
> >> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> >> Cc: Zhi Wang <zhi.a.wang at intel.com>
> >> Cc: Min He<min.he at intel.com>
> >> Cc: Fei Jiang <fei.jiang at intel.com>
> >> Cc: Zhipeng Gong <zhipeng.gong at intel.com>
> >> Cc: Hang Yuan <hang.yuan at intel.com>
> >> Cc: Zhiyuan Lv <zhiyuan.lv at intel.com>
> >> Signed-off-by: Xiaolin Zhang <xiaolin.zhang at intel.com>
> >> ---
> >> drivers/gpu/drm/i915/gvt/handlers.c | 10 ++++++++++
> >> drivers/gpu/drm/i915/gvt/vgpu.c | 7 +++++++
> >> 2 files changed, 17 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> >> index b5475c9..32c6687 100644
> >> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> >> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> >> @@ -1242,6 +1242,16 @@ static int pvinfo_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> >> case _vgtif_reg(g2v_notify):
> >> ret = handle_g2v_notification(vgpu, data);
> >> break;
> >> + case _vgtif_reg(enable_pvmmio):
> >> + if (vgpu->gvt->dev_priv->vgpu.pv_caps) {
> > I can't find where this is defined..And why this would reference host i915's definition?
> > Shouldn't this be just vgpu pv caps?
> below is the code code in i915_drv.h. pv caps is a part of vgpu
> structure as below.
> @@ -1348,6 +1349,7 @@ struct i915_workarounds {
> struct i915_virtual_gpu {
> bool active;
> u32 caps;
> + u32 pv_caps;
> };
struct i915_virtual_gpu is for guest state, but here you want
to check gvt host supported pvmmio caps, which seems weird. Don't
mix host state vs. guest state.
>
> >
> >> + vgpu_vreg(vgpu, offset) = data &
> >> + vgpu->gvt->dev_priv->vgpu.pv_caps;
> >> + DRM_INFO("vgpu id=%d pvmmio=0x%x\n",
> >> + vgpu->id, VGPU_PVMMIO(vgpu));
> >> + } else {
> >> + vgpu_vreg(vgpu, offset) = 0;
> >> + }
> >> + break;
> >> /* add xhot and yhot to handled list to avoid error log */
> >> case _vgtif_reg(cursor_x_hot):
> >> case _vgtif_reg(cursor_y_hot):
> >> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> >> index c628be0..d1674db 100644
> >> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> >> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> >> @@ -47,6 +47,7 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
> >> vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_48BIT_PPGTT;
> >> vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) |= VGT_CAPS_HWSP_EMULATION;
> >> vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) |= VGT_CAPS_HUGE_GTT;
> >> + vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) |= VGT_CAPS_PVMMIO;
> > This should be the last one when you enabled all caps.
> do you mean this line code change should be splitted out for a single patch?
> >
> >>
> >> vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.mappable_gmadr.base)) =
> >> vgpu_aperture_gmadr_base(vgpu);
> >> @@ -62,6 +63,8 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
> >> vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX;
> >> vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX;
> >>
> >> + vgpu_vreg_t(vgpu, vgtif_reg(enable_pvmmio)) = 0;
> > Could we rename this as 'pvmmio_caps' instead of enable_xxx which seems
> > like an interface to turn it on/off?
> no problem, rename is OK.
> >
> >> +
> >> gvt_dbg_core("Populate PVINFO PAGE for vGPU %d\n", vgpu->id);
> >> gvt_dbg_core("aperture base [GMADR] 0x%llx size 0x%llx\n",
> >> vgpu_aperture_gmadr_base(vgpu), vgpu_aperture_sz(vgpu));
> >> @@ -491,6 +494,8 @@ struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt,
> >> return vgpu;
> >> }
> >>
> >> +#define _vgtif_reg(x) \
> >> + (VGT_PVINFO_PAGE + offsetof(struct vgt_if, x))
> > If you need this, put it in some header.
> no problem.
> >
> >> /**
> >> * intel_gvt_reset_vgpu_locked - reset a virtual GPU by DMLR or GT reset
> >> * @vgpu: virtual GPU
> >> @@ -525,6 +530,7 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
> >> struct intel_gvt *gvt = vgpu->gvt;
> >> struct intel_gvt_workload_scheduler *scheduler = &gvt->scheduler;
> >> unsigned int resetting_eng = dmlr ? ALL_ENGINES : engine_mask;
> >> + int enable_pvmmio = vgpu_vreg(vgpu, _vgtif_reg(enable_pvmmio));
> >>
> >> gvt_dbg_core("------------------------------------------\n");
> >> gvt_dbg_core("resseting vgpu%d, dmlr %d, engine_mask %08x\n",
> >> @@ -556,6 +562,7 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
> >>
> >> intel_vgpu_reset_mmio(vgpu, dmlr);
> >> populate_pvinfo_page(vgpu);
> >> + vgpu_vreg(vgpu, _vgtif_reg(enable_pvmmio)) = enable_pvmmio;
> >> intel_vgpu_reset_display(vgpu);
> >>
> >> if (dmlr) {
> >> --
> >> 2.7.4
> >>
>
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20181106/c2057e4b/attachment.sig>
More information about the intel-gvt-dev
mailing list