drm/i915/gvt: always return zero if read pvinfo failed

Submitted by Weinan Li on June 13, 2019, 3:05 a.m.

Details

Message ID 20190613030517.30539-1-weinan.z.li@intel.com
State New
Headers show
Series "drm/i915/gvt: always return zero if read pvinfo failed" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Weinan Li June 13, 2019, 3:05 a.m.
There is pvinfo reading come from vgpu might be failed, like reading
from one unknown address, now GVT-g returns the vreg which is one
uncertain value. To avoid misunderstanding, GVT-g will always return
zero if reading failed occurred.

Signed-off-by: Weinan Li <weinan.z.li@intel.com>
---
 drivers/gpu/drm/i915/gvt/handlers.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index a6ade66349bd..eab657d65276 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -1199,9 +1199,12 @@  static int pvinfo_mmio_read(struct intel_vgpu *vgpu, unsigned int offset,
 		invalid_read = true;
 		break;
 	}
-	if (invalid_read)
-		gvt_vgpu_err("invalid pvinfo read: [%x:%x] = %x\n",
+	if (invalid_read) {
+		gvt_vgpu_err("invalid pvinfo read: [0x%x:0x%x] = 0x0 instead of 0x%x\n",
 				offset, bytes, *(u32 *)p_data);
+		memset(p_data, 0, bytes);
+	}
+
 	vgpu->pv_notified = true;
 	return 0;
 }

Comments


> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Thursday, June 13, 2019 4:15 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/gvt: always return zero if read pvinfo failed
> 
> On 2019.06.13 11:05:17 +0800, Weinan Li wrote:
> > There is pvinfo reading come from vgpu might be failed, like reading
> > from one unknown address, now GVT-g returns the vreg which is one
> > uncertain value. To avoid misunderstanding, GVT-g will always return
> > zero if reading failed occurred.
> >
> > Signed-off-by: Weinan Li <weinan.z.li@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/handlers.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > b/drivers/gpu/drm/i915/gvt/handlers.c
> > index a6ade66349bd..eab657d65276 100644
> > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > @@ -1199,9 +1199,12 @@ static int pvinfo_mmio_read(struct intel_vgpu
> *vgpu, unsigned int offset,
> >  		invalid_read = true;
> >  		break;
> >  	}
> > -	if (invalid_read)
> > -		gvt_vgpu_err("invalid pvinfo read: [%x:%x] = %x\n",
> > +	if (invalid_read) {
> > +		gvt_vgpu_err("invalid pvinfo read: [0x%x:0x%x] = 0x0 instead of
> > +0x%x\n",
> >  				offset, bytes, *(u32 *)p_data);
> > +		memset(p_data, 0, bytes);
> > +	}
> > +
> 
> Shouldn't we make sure to set zero for undefined pvinfo memory?
> Instead of keep setting return value like this?
> 
There might be usage like this, write first then read back and check the return value.

> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Thursday, June 13, 2019 4:37 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/gvt: always return zero if read pvinfo failed
> 
> On 2019.06.13 08:34:37 +0000, Li, Weinan Z wrote:
> > > -----Original Message-----
> > > From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> > > Sent: Thursday, June 13, 2019 4:15 PM
> > > To: Li, Weinan Z <weinan.z.li@intel.com>
> > > Cc: intel-gvt-dev@lists.freedesktop.org
> > > Subject: Re: [PATCH] drm/i915/gvt: always return zero if read pvinfo
> > > failed
> > >
> > > On 2019.06.13 11:05:17 +0800, Weinan Li wrote:
> > > > There is pvinfo reading come from vgpu might be failed, like
> > > > reading from one unknown address, now GVT-g returns the vreg which
> > > > is one uncertain value. To avoid misunderstanding, GVT-g will
> > > > always return zero if reading failed occurred.
> > > >
> > > > Signed-off-by: Weinan Li <weinan.z.li@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/gvt/handlers.c | 7 +++++--
> > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > > > b/drivers/gpu/drm/i915/gvt/handlers.c
> > > > index a6ade66349bd..eab657d65276 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > > > @@ -1199,9 +1199,12 @@ static int pvinfo_mmio_read(struct
> > > > intel_vgpu
> > > *vgpu, unsigned int offset,
> > > >  		invalid_read = true;
> > > >  		break;
> > > >  	}
> > > > -	if (invalid_read)
> > > > -		gvt_vgpu_err("invalid pvinfo read: [%x:%x] = %x\n",
> > > > +	if (invalid_read) {
> > > > +		gvt_vgpu_err("invalid pvinfo read: [0x%x:0x%x] = 0x0 instead
> of
> > > > +0x%x\n",
> > > >  				offset, bytes, *(u32 *)p_data);
> > > > +		memset(p_data, 0, bytes);
> > > > +	}
> > > > +
> > >
> > > Shouldn't we make sure to set zero for undefined pvinfo memory?
> > > Instead of keep setting return value like this?
> > >
> > There might be usage like this, write first then read back and check the
> return value.
> >
> 
> yeah, better we can follow like reserved bit definition, discard write for
> undefined bits and return zero.
> 
Do you mean add write data verification in pvinfo_mmio_write? and let free read?

> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827