[PATCH] drm/i915/gvt: add new mmio_info flag to avoid log flood

[Zhang, Pei]

Hey, Kevin:


> -----Original Message-----
> From: Tian, Kevin
> Sent: Thursday, February 16, 2017 09:22
> To: Zhang, Pei <pei.zhang at intel.com>; Zhenyu Wang <zhenyuw at linux.intel.com>
> Cc: intel-gvt-dev at lists.freedesktop.org
> Subject: RE: [PATCH] drm/i915/gvt: add new mmio_info flag to avoid log flood
> 
> > From: Zhang, Pei
> > Sent: Thursday, February 16, 2017 9:12 AM
> >
> >
> > > -----Original Message-----
> > > From: Zhenyu Wang [mailto:zhenyuw at linux.intel.com]
> > > Sent: Thursday, February 16, 2017 08:57
> > > To: Zhang, Pei <pei.zhang at intel.com>
> > > Cc: intel-gvt-dev at lists.freedesktop.org
> > > Subject: Re: [PATCH] drm/i915/gvt: add new mmio_info flag to avoid
> > > log flood
> > >
> > > On 2017.02.16 01:04:20 +0800, Pei Zhang wrote:
> > > > If guest is accessing an un-expected mmio through cmd, gvt will
> > > > print an error log. In some special case, the guest cmd access is
> > > > very huge which cause the error log flood. Add a special flag to avoid
> this condition.
> > > >
> > > > Signed-off-by: Pei Zhang <pei.zhang at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/gvt/cmd_parser.c |  5 ++++-
> > > >  drivers/gpu/drm/i915/gvt/handlers.c   | 16 ++++++++++++++++
> > > >  drivers/gpu/drm/i915/gvt/mmio.h       |  4 ++++
> > > >  3 files changed, 24 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > > b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > > index 7ee1db7..9dc55df 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > > @@ -830,8 +830,11 @@ static int cmd_reg_handler(struct
> parser_exec_state *s,
> > > >  	}
> > > >
> > > >  	if (!intel_gvt_mmio_is_cmd_access(gvt, offset)) {
> > > > -		gvt_err("vgpu%d: %s access to non-render register (%x)\n",
> > > > +		if (!intel_gvt_mmio_is_cmd_access_violate(gvt, offset)) {
> > > > +			intel_gvt_mmio_set_cmd_access_violate(gvt, offset);
> > > > +			gvt_err("vgpu%d: %s access to non-render register
> (%x)\n",
> > > >  				s->vgpu->id, cmd, offset);
> > > > +		}
> > > >  		return 0;
> > > >  	}
> > >
> > > So if we disable this for some vgpu, it will be kept disabled for
> > > all other vgpus, right? Seems not good.
> > [Pei] en, in fact, the mmio_info is shared for all vGPUs/guest OS. One
> > vGPU instance occurs mmio access violation, it would be same for all
> > other vGPU instances. Don't need to print the same error log for all
> > vGPU instances. So I think the global setting is fine. But it's better to
> remove the vgpu-id info from the print log, how do you think?
> >
> 
> It's not a sane design. Such error should be controlled per-vGPU.
[Pei] so, a best way is to define a base approach to support once print per vGPU instance for same logs. But this will be too complicated for this simple issue. So, I will remove this patch, and simply change gvt_err to gvt_dbg_mmio(xxx), based on zhengyu's suggestion.

> 
> Thanks
> Kevin