[PATCH v3] drm/i915/gvt: update vreg on indirect context lri command
Hang Yuan
hang.yuan at linux.intel.com
Mon Jul 2 05:53:36 UTC 2018
On Mon, Jul 02, 2018 at 12:04:57PM +0800, Zhenyu Wang wrote:
> On 2018.07.02 11:11:22 +0800, Zhenyu Wang wrote:
> > On 2018.07.02 10:12:13 +0800, intel-gvt-dev-bounces at lists.freedesktop.org wrote:
> > > From: Hang Yuan <hang.yuan at linux.intel.com>
> > >
> > > Commit cd7e 61b9"init mmio by lri command in vgpu inhibit context"
> > > initializes registers saved/restored in context with its vreg value
> > > through lri command in ring buffer. It relies on vreg got updated
> > > on every guest access. There is a case found that Linux guest uses
> > > lri command in indirect-ctx to update the register. This patch adds
> > > vreg update on this case.
> > >
> > > v2: move mmio_attribute functions to gvt.h (Zhenyu Wang)
> > > v3: use mask_mmio_write in vreg update
> > >
> > > Fixes: cd7e61b9("drm/i915/gvt: init mmio by lri command in vgpu inhibit context")
> > > Signed-off-by: Hang Yuan <hang.yuan at linux.intel.com>
> > > Signed-off-by: Weinan Li <weinan.z.li at intel.com>
> > > ---
> > > drivers/gpu/drm/i915/gvt/cmd_parser.c | 13 +++++++++++++
> > > drivers/gpu/drm/i915/gvt/gvt.h | 29 +++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/gvt/handlers.c | 24 ++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/gvt/mmio.h | 2 ++
> > > drivers/gpu/drm/i915/gvt/mmio_context.c | 4 +++-
> > > 5 files changed, 71 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > index 0651e63..594f10b 100644
> > > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > @@ -863,6 +863,7 @@ static int cmd_reg_handler(struct parser_exec_state *s,
> > > {
> > > struct intel_vgpu *vgpu = s->vgpu;
> > > struct intel_gvt *gvt = vgpu->gvt;
> > > + u32 ctx_sr_ctl;
> > >
> > > if (offset + 4 > gvt->device_info.mmio_size) {
> > > gvt_vgpu_err("%s access to (%x) outside of MMIO range\n",
> > > @@ -895,6 +896,18 @@ static int cmd_reg_handler(struct parser_exec_state *s,
> > > patch_value(s, cmd_ptr(s, index), VGT_PVINFO_PAGE);
> > > }
> > >
> > > + intel_gvt_hypervisor_read_gpa(s->vgpu,
> > > + s->workload->ring_context_gpa + 12, &ctx_sr_ctl, 4);
> > > + if (IS_KABYLAKE(s->vgpu->gvt->dev_priv) &&
> > > + (ctx_sr_ctl & 1) &&
> > > + intel_gvt_mmio_is_in_ctx(gvt, offset) &&
> > > + !strncmp(cmd, "lri", 3)) {
> > > + u32 data = cmd_val(s, index + 1);
> >
> > We can first check 'lri' cmd, then might need to read ctx ctl state with
> > further checking, instead of always reading gpa. And why need to check for
> > KBL only? I know currently only KBL is applied by LRI cmd for context s/r,
> > but this checking for lri cmd update on vreg should still be applying for
> > supported hw, right?
Henry: I will move reading gpa after checking lri in next version. This patch
just wants to fix the issue introduced by the patch "init mmio by lri command
in vgpu inhibit context" which only works on KBL. So this patch's working scope
has many limitations to avoid regression to other good cases. For a general vreg
update solution for gpu command in context and ring buffer, right now don't see
any bugs by missing it and I understand needs to consider more situations like
lrm command and batch buffer. So it is not implemented in this patch.
> > > +
> > > + if (intel_gvt_mmio_has_mode_mask(s->vgpu->gvt, offset))
> > > + intel_vgpu_mask_mmio_write(vgpu, offset, &data, 4);
>
> And why only update for mmio with mode mask?
Henry: from the debugging, only the register at offset 0x7014 needs to be handled,
which is a mask-write register. But I agree it's better to consider other mmio as
well. I will update in next version.
>
> > > + }
> > > +
> > > /* TODO: Update the global mask if this MMIO is a masked-MMIO */
> > > intel_gvt_mmio_set_cmd_accessed(gvt, offset);
> > > return 0;
> > > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > > index de2a3a2..9a96715 100644
> > > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > > @@ -274,6 +274,8 @@ struct intel_gvt_mmio {
> > > #define F_CMD_ACCESSED (1 << 5)
> > > /* This reg could be accessed by unaligned address */
> > > #define F_UNALIGN (1 << 6)
> > > +/* This reg is saved/restored in context */
> > > +#define F_IN_CTX (1 << 7)
> > >
> > > struct gvt_mmio_block *mmio_block;
> > > unsigned int num_mmio_block;
> > > @@ -655,6 +657,33 @@ static inline bool intel_gvt_mmio_has_mode_mask(
> > > return gvt->mmio.mmio_attribute[offset >> 2] & F_MODE_MASK;
> > > }
> > >
> > > +/**
> > > + * intel_gvt_mmio_is_in_ctx - check if a MMIO has in-ctx mask
> > > + * @gvt: a GVT device
> > > + * @offset: register offset
> > > + *
> > > + * Returns:
> > > + * True if a MMIO has a in-context mask, false if it isn't.
> > > + *
> > > + */
> > > +static inline bool intel_gvt_mmio_is_in_ctx(
> > > + struct intel_gvt *gvt, unsigned int offset)
> > > +{
> > > + return gvt->mmio.mmio_attribute[offset >> 2] & F_IN_CTX;
> > > +}
> > > +
> > > +/**
> > > + * intel_gvt_mmio_set_in_ctx - mask a MMIO in logical context
> > > + * @gvt: a GVT device
> > > + * @offset: register offset
> > > + *
> > > + */
> > > +static inline void intel_gvt_mmio_set_in_ctx(
> > > + struct intel_gvt *gvt, unsigned int offset)
> > > +{
> > > + gvt->mmio.mmio_attribute[offset >> 2] |= F_IN_CTX;
> > > +}
> > > +
> > > int intel_gvt_debugfs_add_vgpu(struct intel_vgpu *vgpu);
> > > void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
> > > int intel_gvt_debugfs_init(struct intel_gvt *gvt);
> > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> > > index e39492a..0d8e15a 100644
> > > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > > @@ -3351,6 +3351,30 @@ int intel_vgpu_default_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> > > }
> > >
> > > /**
> > > + * intel_vgpu_mask_mmio_write - write mask register
> > > + * @vgpu: a vGPU
> > > + * @offset: access offset
> > > + * @p_data: write data buffer
> > > + * @bytes: access data length
> > > + *
> > > + * Returns:
> > > + * Zero on success, negative error code if failed.
> > > + */
> > > +int intel_vgpu_mask_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> > > + void *p_data, unsigned int bytes)
> > > +{
> > > + u32 mask, old_vreg;
> > > +
> > > + old_vreg = vgpu_vreg(vgpu, offset);
> > > + write_vreg(vgpu, offset, p_data, bytes);
> > > + mask = vgpu_vreg(vgpu, offset) >> 16;
> > > + vgpu_vreg(vgpu, offset) = (old_vreg & ~mask) |
> > > + (vgpu_vreg(vgpu, offset) & mask);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > * intel_gvt_in_force_nonpriv_whitelist - if a mmio is in whitelist to be
> > > * force-nopriv register
> > > *
> > > diff --git a/drivers/gpu/drm/i915/gvt/mmio.h b/drivers/gpu/drm/i915/gvt/mmio.h
> > > index e474188..1ffc69e 100644
> > > --- a/drivers/gpu/drm/i915/gvt/mmio.h
> > > +++ b/drivers/gpu/drm/i915/gvt/mmio.h
> > > @@ -99,4 +99,6 @@ bool intel_gvt_in_force_nonpriv_whitelist(struct intel_gvt *gvt,
> > > int intel_vgpu_mmio_reg_rw(struct intel_vgpu *vgpu, unsigned int offset,
> > > void *pdata, unsigned int bytes, bool is_read);
> > >
> > > +int intel_vgpu_mask_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> > > + void *p_data, unsigned int bytes);
> > > #endif
> > > diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c
> > > index 20be9a9..42e1e6b 100644
> > > --- a/drivers/gpu/drm/i915/gvt/mmio_context.c
> > > +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
> > > @@ -587,7 +587,9 @@ void intel_gvt_init_engine_mmio_context(struct intel_gvt *gvt)
> > >
> > > for (mmio = gvt->engine_mmio_list.mmio;
> > > i915_mmio_reg_valid(mmio->reg); mmio++) {
> > > - if (mmio->in_context)
> > > + if (mmio->in_context) {
> > > gvt->engine_mmio_list.ctx_mmio_count[mmio->ring_id]++;
> > > + intel_gvt_mmio_set_in_ctx(gvt, mmio->reg.reg);
> > > + }
> > > }
> > > }
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > intel-gvt-dev mailing list
> > > intel-gvt-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> >
> > --
> > Open Source Technology Center, Intel ltd.
> >
> > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
>
>
>
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
>
>
> --
> Open Source Technology Center, Intel ltd.
>
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
More information about the intel-gvt-dev
mailing list