[v2] drm/i915/gvt: Deliver guest cursor hotspot info

Submitted by Zhang, Tina on March 28, 2018, 2:12 a.m.

Details

Message ID 1522203135-22175-1-git-send-email-tina.zhang@intel.com
State New
Headers show
Series "drm/i915/gvt: Deliver guest cursor hotspot info" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Zhang, Tina March 28, 2018, 2:12 a.m.
Guest OS driver uses PV info registers to deliver cursor hotspot info
to host. This patch is used to get cursor hotspot info from virtual
registers and deliver it to host userspace.

v1->v2:
- name as cursor_x_hot/cursor_y_hot. (Zhenyu)
- use i915_reg_t definition instead of magic numbers. (Zhenyu)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/i915/gvt/dmabuf.c     | 10 ++++------
 drivers/gpu/drm/i915/gvt/fb_decoder.c |  3 +++
 drivers/gpu/drm/i915/gvt/handlers.c   |  4 ++--
 drivers/gpu/drm/i915/gvt/vgpu.c       |  3 +++
 drivers/gpu/drm/i915/i915_pvinfo.h    |  5 ++++-
 5 files changed, 16 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c
index 9a471b0..ef1937b 100644
--- a/drivers/gpu/drm/i915/gvt/dmabuf.c
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
@@ -229,12 +229,10 @@  static int vgpu_get_plane_info(struct drm_device *dev,
 		info->x_pos = c.x_pos;
 		info->y_pos = c.y_pos;
 
-		/* The invalid cursor hotspot value is delivered to host
-		 * until we find a way to get the cursor hotspot info of
-		 * guest OS.
-		 */
-		info->x_hot = UINT_MAX;
-		info->y_hot = UINT_MAX;
+
+		info->x_hot = c.x_hot;
+		info->y_hot = c.y_hot;
+
 		info->size = (((info->stride * c.height * c.bpp) / 8)
 				+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
 	} else {
diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c
index 6b50fe7..14a7961 100644
--- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -36,6 +36,7 @@ 
 #include <uapi/drm/drm_fourcc.h>
 #include "i915_drv.h"
 #include "gvt.h"
+#include "i915_pvinfo.h"
 
 #define PRIMARY_FORMAT_NUM	16
 struct pixel_format {
@@ -390,6 +391,8 @@  int intel_vgpu_decode_cursor_plane(struct intel_vgpu *vgpu,
 	plane->y_pos = (val & _CURSOR_POS_Y_MASK) >> _CURSOR_POS_Y_SHIFT;
 	plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >> _CURSOR_SIGN_Y_SHIFT;
 
+	plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot));
+	plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot));
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index fbb908e..0d7560a 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -1198,8 +1198,8 @@  static int pvinfo_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
 		ret = handle_g2v_notification(vgpu, data);
 		break;
 	/* add xhot and yhot to handled list to avoid error log */
-	case 0x78830:
-	case 0x78834:
+	case _vgtif_reg(cursor_x_hot):
+	case _vgtif_reg(cursor_y_hot):
 	case _vgtif_reg(pdp[0].lo):
 	case _vgtif_reg(pdp[0].hi):
 	case _vgtif_reg(pdp[1].lo):
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 41f76e8..c56e634 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -58,6 +58,9 @@  void populate_pvinfo_page(struct intel_vgpu *vgpu)
 
 	vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) = vgpu_fence_sz(vgpu);
 
+	vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX;
+	vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX;
+
 	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));
diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
index 195203f..d61914a 100644
--- a/drivers/gpu/drm/i915/i915_pvinfo.h
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -93,7 +93,10 @@  struct vgt_if {
 	u32 rsv5[4];
 
 	u32 g2v_notify;
-	u32 rsv6[7];
+	u32 rsv6[5];
+
+	u32 cursor_x_hot;
+	u32 cursor_y_hot;
 
 	struct {
 		u32 lo;

Comments

On 2018.03.28 10:12:15 +0800, Tina Zhang wrote:
> Guest OS driver uses PV info registers to deliver cursor hotspot info
> to host. This patch is used to get cursor hotspot info from virtual
> registers and deliver it to host userspace.
>

As this also changes i915, need to include intel-gfx list too, comment below.

> v1->v2:
> - name as cursor_x_hot/cursor_y_hot. (Zhenyu)
> - use i915_reg_t definition instead of magic numbers. (Zhenyu)
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/gpu/drm/i915/gvt/dmabuf.c     | 10 ++++------
>  drivers/gpu/drm/i915/gvt/fb_decoder.c |  3 +++
>  drivers/gpu/drm/i915/gvt/handlers.c   |  4 ++--
>  drivers/gpu/drm/i915/gvt/vgpu.c       |  3 +++
>  drivers/gpu/drm/i915/i915_pvinfo.h    |  5 ++++-
>  5 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c
> index 9a471b0..ef1937b 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> @@ -229,12 +229,10 @@ static int vgpu_get_plane_info(struct drm_device *dev,
>  		info->x_pos = c.x_pos;
>  		info->y_pos = c.y_pos;
>  
> -		/* The invalid cursor hotspot value is delivered to host
> -		 * until we find a way to get the cursor hotspot info of
> -		 * guest OS.
> -		 */
> -		info->x_hot = UINT_MAX;
> -		info->y_hot = UINT_MAX;
> +
> +		info->x_hot = c.x_hot;
> +		info->y_hot = c.y_hot;

Do we need sanity check on guest setting? and fallback to invalid value
if check failed.

> +
>  		info->size = (((info->stride * c.height * c.bpp) / 8)
>  				+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
>  	} else {
> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> index 6b50fe7..14a7961 100644
> --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> @@ -36,6 +36,7 @@
>  #include <uapi/drm/drm_fourcc.h>
>  #include "i915_drv.h"
>  #include "gvt.h"
> +#include "i915_pvinfo.h"
>  
>  #define PRIMARY_FORMAT_NUM	16
>  struct pixel_format {
> @@ -390,6 +391,8 @@ int intel_vgpu_decode_cursor_plane(struct intel_vgpu *vgpu,
>  	plane->y_pos = (val & _CURSOR_POS_Y_MASK) >> _CURSOR_POS_Y_SHIFT;
>  	plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >> _CURSOR_SIGN_Y_SHIFT;
>  
> +	plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot));
> +	plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot));
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> index fbb908e..0d7560a 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -1198,8 +1198,8 @@ static int pvinfo_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
>  		ret = handle_g2v_notification(vgpu, data);
>  		break;
>  	/* add xhot and yhot to handled list to avoid error log */
> -	case 0x78830:
> -	case 0x78834:
> +	case _vgtif_reg(cursor_x_hot):
> +	case _vgtif_reg(cursor_y_hot):
>  	case _vgtif_reg(pdp[0].lo):
>  	case _vgtif_reg(pdp[0].hi):
>  	case _vgtif_reg(pdp[1].lo):
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 41f76e8..c56e634 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -58,6 +58,9 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
>  
>  	vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) = vgpu_fence_sz(vgpu);
>  
> +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX;
> +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX;
> +
>  	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));
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
> index 195203f..d61914a 100644
> --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -93,7 +93,10 @@ struct vgt_if {
>  	u32 rsv5[4];
>  
>  	u32 g2v_notify;
> -	u32 rsv6[7];
> +	u32 rsv6[5];
> +
> +	u32 cursor_x_hot;
> +	u32 cursor_y_hot;
>  
>  	struct {
>  		u32 lo;
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Wednesday, March 28, 2018 1:51 PM
> To: Zhang, Tina <tina.zhang@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>; intel-gvt-dev@lists.freedesktop.org;
> Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: Re: [PATCH v2] drm/i915/gvt: Deliver guest cursor hotspot info
> 
> On 2018.03.28 10:12:15 +0800, Tina Zhang wrote:
> > Guest OS driver uses PV info registers to deliver cursor hotspot info
> > to host. This patch is used to get cursor hotspot info from virtual
> > registers and deliver it to host userspace.
> >
> 
> As this also changes i915, need to include intel-gfx list too, comment below.
> 
> > v1->v2:
> > - name as cursor_x_hot/cursor_y_hot. (Zhenyu)
> > - use i915_reg_t definition instead of magic numbers. (Zhenyu)
> >
> > Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: Zhi Wang <zhi.a.wang@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/dmabuf.c     | 10 ++++------
> >  drivers/gpu/drm/i915/gvt/fb_decoder.c |  3 +++
> >  drivers/gpu/drm/i915/gvt/handlers.c   |  4 ++--
> >  drivers/gpu/drm/i915/gvt/vgpu.c       |  3 +++
> >  drivers/gpu/drm/i915/i915_pvinfo.h    |  5 ++++-
> >  5 files changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > index 9a471b0..ef1937b 100644
> > --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > @@ -229,12 +229,10 @@ static int vgpu_get_plane_info(struct drm_device
> *dev,
> >  		info->x_pos = c.x_pos;
> >  		info->y_pos = c.y_pos;
> >
> > -		/* The invalid cursor hotspot value is delivered to host
> > -		 * until we find a way to get the cursor hotspot info of
> > -		 * guest OS.
> > -		 */
> > -		info->x_hot = UINT_MAX;
> > -		info->y_hot = UINT_MAX;
> > +
> > +		info->x_hot = c.x_hot;
> > +		info->y_hot = c.y_hot;
> 
> Do we need sanity check on guest setting? and fallback to invalid value if check
> failed.
X_hot/y_hot are initialized with UINT_MAX, during "populate_pvinfo_page(...)", and the values
won't be changed until the related mmio in pv_info is updated by guest driver.
And here just report the current values to userspace, not matter they're updated with the new values
or still being the invalidate one (UINT_MAX).
So, a sanity checking might not be so necessary here, as they should respect to guest anyway, just like
the framebuffer does.
Thanks,

BR,
Tina

> 
> > +
> >  		info->size = (((info->stride * c.height * c.bpp) / 8)
> >  				+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
> >  	} else {
> > diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > index 6b50fe7..14a7961 100644
> > --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > @@ -36,6 +36,7 @@
> >  #include <uapi/drm/drm_fourcc.h>
> >  #include "i915_drv.h"
> >  #include "gvt.h"
> > +#include "i915_pvinfo.h"
> >
> >  #define PRIMARY_FORMAT_NUM	16
> >  struct pixel_format {
> > @@ -390,6 +391,8 @@ int intel_vgpu_decode_cursor_plane(struct
> intel_vgpu *vgpu,
> >  	plane->y_pos = (val & _CURSOR_POS_Y_MASK) >>
> _CURSOR_POS_Y_SHIFT;
> >  	plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >>
> _CURSOR_SIGN_Y_SHIFT;
> >
> > +	plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot));
> > +	plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot));
> >  	return 0;
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > b/drivers/gpu/drm/i915/gvt/handlers.c
> > index fbb908e..0d7560a 100644
> > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > @@ -1198,8 +1198,8 @@ static int pvinfo_mmio_write(struct intel_vgpu
> *vgpu, unsigned int offset,
> >  		ret = handle_g2v_notification(vgpu, data);
> >  		break;
> >  	/* add xhot and yhot to handled list to avoid error log */
> > -	case 0x78830:
> > -	case 0x78834:
> > +	case _vgtif_reg(cursor_x_hot):
> > +	case _vgtif_reg(cursor_y_hot):
> >  	case _vgtif_reg(pdp[0].lo):
> >  	case _vgtif_reg(pdp[0].hi):
> >  	case _vgtif_reg(pdp[1].lo):
> > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
> > b/drivers/gpu/drm/i915/gvt/vgpu.c index 41f76e8..c56e634 100644
> > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > @@ -58,6 +58,9 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
> >
> >  	vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) =
> > vgpu_fence_sz(vgpu);
> >
> > +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX;
> > +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX;
> > +
> >  	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));
> diff --git
> > a/drivers/gpu/drm/i915/i915_pvinfo.h
> > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > index 195203f..d61914a 100644
> > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > @@ -93,7 +93,10 @@ struct vgt_if {
> >  	u32 rsv5[4];
> >
> >  	u32 g2v_notify;
> > -	u32 rsv6[7];
> > +	u32 rsv6[5];
> > +
> > +	u32 cursor_x_hot;
> > +	u32 cursor_y_hot;
> >
> >  	struct {
> >  		u32 lo;
> > --
> > 2.7.4
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev@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
On 2018.03.28 06:10:33 +0000, Zhang, Tina wrote:
> > > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > index 9a471b0..ef1937b 100644
> > > --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > @@ -229,12 +229,10 @@ static int vgpu_get_plane_info(struct drm_device
> > *dev,
> > >  		info->x_pos = c.x_pos;
> > >  		info->y_pos = c.y_pos;
> > >
> > > -		/* The invalid cursor hotspot value is delivered to host
> > > -		 * until we find a way to get the cursor hotspot info of
> > > -		 * guest OS.
> > > -		 */
> > > -		info->x_hot = UINT_MAX;
> > > -		info->y_hot = UINT_MAX;
> > > +
> > > +		info->x_hot = c.x_hot;
> > > +		info->y_hot = c.y_hot;
> > 
> > Do we need sanity check on guest setting? and fallback to invalid value if check
> > failed.
> X_hot/y_hot are initialized with UINT_MAX, during "populate_pvinfo_page(...)", and the values
> won't be changed until the related mmio in pv_info is updated by guest driver.
> And here just report the current values to userspace, not matter they're updated with the new values
> or still being the invalidate one (UINT_MAX).
> So, a sanity checking might not be so necessary here, as they should respect to guest anyway, just like
> the framebuffer does.

As fb decoder has spec level definition to check, this hotspot value is
from guest input directly, what if it's set inconsistent like over cursor
plane size?

> 
> > 
> > > +
> > >  		info->size = (((info->stride * c.height * c.bpp) / 8)
> > >  				+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
> > >  	} else {
> > > diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > index 6b50fe7..14a7961 100644
> > > --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > @@ -36,6 +36,7 @@
> > >  #include <uapi/drm/drm_fourcc.h>
> > >  #include "i915_drv.h"
> > >  #include "gvt.h"
> > > +#include "i915_pvinfo.h"
> > >
> > >  #define PRIMARY_FORMAT_NUM	16
> > >  struct pixel_format {
> > > @@ -390,6 +391,8 @@ int intel_vgpu_decode_cursor_plane(struct
> > intel_vgpu *vgpu,
> > >  	plane->y_pos = (val & _CURSOR_POS_Y_MASK) >>
> > _CURSOR_POS_Y_SHIFT;
> > >  	plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >>
> > _CURSOR_SIGN_Y_SHIFT;
> > >
> > > +	plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot));
> > > +	plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot));
> > >  	return 0;
> > >  }
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > > b/drivers/gpu/drm/i915/gvt/handlers.c
> > > index fbb908e..0d7560a 100644
> > > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > > @@ -1198,8 +1198,8 @@ static int pvinfo_mmio_write(struct intel_vgpu
> > *vgpu, unsigned int offset,
> > >  		ret = handle_g2v_notification(vgpu, data);
> > >  		break;
> > >  	/* add xhot and yhot to handled list to avoid error log */
> > > -	case 0x78830:
> > > -	case 0x78834:
> > > +	case _vgtif_reg(cursor_x_hot):
> > > +	case _vgtif_reg(cursor_y_hot):
> > >  	case _vgtif_reg(pdp[0].lo):
> > >  	case _vgtif_reg(pdp[0].hi):
> > >  	case _vgtif_reg(pdp[1].lo):
> > > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > b/drivers/gpu/drm/i915/gvt/vgpu.c index 41f76e8..c56e634 100644
> > > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > > @@ -58,6 +58,9 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
> > >
> > >  	vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) =
> > > vgpu_fence_sz(vgpu);
> > >
> > > +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX;
> > > +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX;
> > > +
> > >  	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));
> > diff --git
> > > a/drivers/gpu/drm/i915/i915_pvinfo.h
> > > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > > index 195203f..d61914a 100644
> > > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > > @@ -93,7 +93,10 @@ struct vgt_if {
> > >  	u32 rsv5[4];
> > >
> > >  	u32 g2v_notify;
> > > -	u32 rsv6[7];
> > > +	u32 rsv6[5];
> > > +
> > > +	u32 cursor_x_hot;
> > > +	u32 cursor_y_hot;
> > >
> > >  	struct {
> > >  		u32 lo;
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > intel-gvt-dev mailing list
> > > intel-gvt-dev@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
> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Wednesday, March 28, 2018 2:18 PM
> To: Zhang, Tina <tina.zhang@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>; intel-gvt-dev@lists.freedesktop.org;
> Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: Re: [PATCH v2] drm/i915/gvt: Deliver guest cursor hotspot info
> 
> On 2018.03.28 06:10:33 +0000, Zhang, Tina wrote:
> > > > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > > b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > > index 9a471b0..ef1937b 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > > @@ -229,12 +229,10 @@ static int vgpu_get_plane_info(struct
> > > > drm_device
> > > *dev,
> > > >  		info->x_pos = c.x_pos;
> > > >  		info->y_pos = c.y_pos;
> > > >
> > > > -		/* The invalid cursor hotspot value is delivered to host
> > > > -		 * until we find a way to get the cursor hotspot info of
> > > > -		 * guest OS.
> > > > -		 */
> > > > -		info->x_hot = UINT_MAX;
> > > > -		info->y_hot = UINT_MAX;
> > > > +
> > > > +		info->x_hot = c.x_hot;
> > > > +		info->y_hot = c.y_hot;
> > >
> > > Do we need sanity check on guest setting? and fallback to invalid
> > > value if check failed.
> > X_hot/y_hot are initialized with UINT_MAX, during
> > "populate_pvinfo_page(...)", and the values won't be changed until the
> related mmio in pv_info is updated by guest driver.
> > And here just report the current values to userspace, not matter
> > they're updated with the new values or still being the invalidate one
> (UINT_MAX).
> > So, a sanity checking might not be so necessary here, as they should
> > respect to guest anyway, just like the framebuffer does.
> 
> As fb decoder has spec level definition to check, this hotspot value is from guest
> input directly, what if it's set inconsistent like over cursor plane size?
Then, it should be the userspace's responsibility to handle it, as only userspace knows
what the hotspots are.

BR,
Tina
> 
> >
> > >
> > > > +
> > > >  		info->size = (((info->stride * c.height * c.bpp) / 8)
> > > >  				+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
> > > >  	} else {
> > > > diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > > b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > > index 6b50fe7..14a7961 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > > @@ -36,6 +36,7 @@
> > > >  #include <uapi/drm/drm_fourcc.h>
> > > >  #include "i915_drv.h"
> > > >  #include "gvt.h"
> > > > +#include "i915_pvinfo.h"
> > > >
> > > >  #define PRIMARY_FORMAT_NUM	16
> > > >  struct pixel_format {
> > > > @@ -390,6 +391,8 @@ int intel_vgpu_decode_cursor_plane(struct
> > > intel_vgpu *vgpu,
> > > >  	plane->y_pos = (val & _CURSOR_POS_Y_MASK) >>
> > > _CURSOR_POS_Y_SHIFT;
> > > >  	plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >>
> > > _CURSOR_SIGN_Y_SHIFT;
> > > >
> > > > +	plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot));
> > > > +	plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot));
> > > >  	return 0;
> > > >  }
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > > > b/drivers/gpu/drm/i915/gvt/handlers.c
> > > > index fbb908e..0d7560a 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > > > @@ -1198,8 +1198,8 @@ static int pvinfo_mmio_write(struct
> > > > intel_vgpu
> > > *vgpu, unsigned int offset,
> > > >  		ret = handle_g2v_notification(vgpu, data);
> > > >  		break;
> > > >  	/* add xhot and yhot to handled list to avoid error log */
> > > > -	case 0x78830:
> > > > -	case 0x78834:
> > > > +	case _vgtif_reg(cursor_x_hot):
> > > > +	case _vgtif_reg(cursor_y_hot):
> > > >  	case _vgtif_reg(pdp[0].lo):
> > > >  	case _vgtif_reg(pdp[0].hi):
> > > >  	case _vgtif_reg(pdp[1].lo):
> > > > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > > b/drivers/gpu/drm/i915/gvt/vgpu.c index 41f76e8..c56e634 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > > > @@ -58,6 +58,9 @@ void populate_pvinfo_page(struct intel_vgpu
> > > > *vgpu)
> > > >
> > > >  	vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) =
> > > > vgpu_fence_sz(vgpu);
> > > >
> > > > +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX;
> > > > +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX;
> > > > +
> > > >  	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));
> > > diff --git
> > > > a/drivers/gpu/drm/i915/i915_pvinfo.h
> > > > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > > > index 195203f..d61914a 100644
> > > > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > > > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > > > @@ -93,7 +93,10 @@ struct vgt_if {
> > > >  	u32 rsv5[4];
> > > >
> > > >  	u32 g2v_notify;
> > > > -	u32 rsv6[7];
> > > > +	u32 rsv6[5];
> > > > +
> > > > +	u32 cursor_x_hot;
> > > > +	u32 cursor_y_hot;
> > > >
> > > >  	struct {
> > > >  		u32 lo;
> > > > --
> > > > 2.7.4
> > > >
> > > > _______________________________________________
> > > > intel-gvt-dev mailing list
> > > > intel-gvt-dev@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