[PATCH v3] drm/i915/gvt: update vreg on indirect context lri command
Zhenyu Wang
zhenyuw at linux.intel.com
Mon Jul 2 05:55:42 UTC 2018
On 2018.07.02 13:53:36 +0800, Hang Yuan wrote:
> 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.
Then make a warning in that case instead of hiding the situation so can be awared later.
Better with some comments for that too.
>
> > > > +
> > > > + 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
>
>
--
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/20180702/20d9bb44/attachment.sig>
More information about the intel-gvt-dev
mailing list