All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment
@ 2017-04-12  8:36 Weinan Li
  2017-04-12  8:53 ` Chris Wilson
  2017-04-12 10:13 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 7+ messages in thread
From: Weinan Li @ 2017-04-12  8:36 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev

I915_GEM_GET_APERTURE ioctl is used to probe aperture size from userspace.
Some applications like OpenCL use this information to know how much GM
resource can it use. In gvt environment, each vm only use the ballooned
part of aperture, so we should return the actual aperture size exclude
the reserved part by balloon.

I915_GEM_CONTEXT_GETPARAM ioctl query the I915_CONTEXT_PARAM_GTT_SIZE, we
also need to exclude the reserved part in GTT.

Signed-off-by: Weinan Li <weinan.z.li@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |  7 +++----
 drivers/gpu/drm/i915/i915_gem_context.c |  4 +++-
 drivers/gpu/drm/i915/i915_vgpu.c        | 18 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_vgpu.h        |  2 ++
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 84ea249..b3fb424 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -145,9 +145,8 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct drm_i915_gem_get_aperture *args = data;
 	struct i915_vma *vma;
-	size_t pinned;
+	size_t pinned = 0;
 
-	pinned = 0;
 	mutex_lock(&dev->struct_mutex);
 	list_for_each_entry(vma, &ggtt->base.active_list, vm_link)
 		if (i915_vma_is_pinned(vma))
@@ -157,9 +156,9 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 			pinned += vma->node.size;
 	mutex_unlock(&dev->struct_mutex);
 
-	args->aper_size = ggtt->base.total;
+	args->aper_size = ggtt->base.total -
+		 intel_vgt_reserved_size_by_balloon(dev_priv);
 	args->aper_available_size = args->aper_size - pinned;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8bd0c49..9f3280d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -88,6 +88,7 @@
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "i915_vgpu.h"
 #include "i915_trace.h"
 
 #define ALL_L3_SLICES(dev) (1 << NUM_L3_SLICES(dev)) - 1
@@ -1053,7 +1054,8 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 		else if (to_i915(dev)->mm.aliasing_ppgtt)
 			args->value = to_i915(dev)->mm.aliasing_ppgtt->base.total;
 		else
-			args->value = to_i915(dev)->ggtt.base.total;
+			args->value = to_i915(dev)->ggtt.base.total -
+			  intel_vgt_reserved_size_by_balloon(dev->dev_private);
 		break;
 	case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
 		args->value = i915_gem_context_no_error_capture(ctx);
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 4ab8a97..ce722d8 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -88,6 +88,11 @@ struct _balloon_info_ {
 	 * graphic memory, 2/3 for unmappable graphic memory.
 	 */
 	struct drm_mm_node space[4];
+	/*
+	 * Total space size exclude ballooned named reserved_total, it's
+	 * invisible for vGPU.
+	 */
+	size_t reserved_total;
 };
 
 static struct _balloon_info_ bl_info;
@@ -116,6 +121,14 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
 	memset(&bl_info, 0, sizeof(bl_info));
 }
 
+size_t intel_vgt_reserved_size_by_balloon(struct drm_i915_private *dev_priv)
+{
+	if (!intel_vgpu_active(dev_priv))
+		return 0;
+
+	return bl_info.reserved_total;
+}
+
 static int vgt_balloon_space(struct i915_ggtt *ggtt,
 			     struct drm_mm_node *node,
 			     unsigned long start, unsigned long end)
@@ -183,7 +196,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 	unsigned long mappable_base, mappable_size, mappable_end;
 	unsigned long unmappable_base, unmappable_size, unmappable_end;
-	int ret;
+	int ret, i;
 
 	if (!intel_vgpu_active(dev_priv))
 		return 0;
@@ -242,6 +255,9 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 			goto err;
 	}
 
+	for (i = 0; i < ARRAY_SIZE(bl_info.space); i++)
+		bl_info.reserved_total += bl_info.space[i].size;
+
 	DRM_INFO("VGT balloon successfully\n");
 	return 0;
 
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 3c3b2d2..e776580 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -29,5 +29,7 @@
 void i915_check_vgpu(struct drm_i915_private *dev_priv);
 int intel_vgt_balloon(struct drm_i915_private *dev_priv);
 void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
+size_t intel_vgt_reserved_size_by_balloon(
+		struct drm_i915_private *dev_priv);
 
 #endif /* _I915_VGPU_H_ */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment
  2017-04-12  8:36 [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment Weinan Li
@ 2017-04-12  8:53 ` Chris Wilson
  2017-04-12 10:19   ` Joonas Lahtinen
  2017-04-12 10:13 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2017-04-12  8:53 UTC (permalink / raw)
  To: Weinan Li; +Cc: intel-gfx, intel-gvt-dev

On Wed, Apr 12, 2017 at 04:36:57PM +0800, Weinan Li wrote:
> I915_GEM_GET_APERTURE ioctl is used to probe aperture size from userspace.
> Some applications like OpenCL use this information to know how much GM
> resource can it use.

That's a userspace bug.

> In gvt environment, each vm only use the ballooned
> part of aperture, so we should return the actual aperture size exclude
> the reserved part by balloon.
> 
> I915_GEM_CONTEXT_GETPARAM ioctl query the I915_CONTEXT_PARAM_GTT_SIZE, we
> also need to exclude the reserved part in GTT.
> 
> Signed-off-by: Weinan Li <weinan.z.li@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c         |  7 +++----
>  drivers/gpu/drm/i915/i915_gem_context.c |  4 +++-
>  drivers/gpu/drm/i915/i915_vgpu.c        | 18 +++++++++++++++++-
>  drivers/gpu/drm/i915/i915_vgpu.h        |  2 ++
>  4 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 84ea249..b3fb424 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -145,9 +145,8 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>  	struct drm_i915_gem_get_aperture *args = data;
>  	struct i915_vma *vma;
> -	size_t pinned;
> +	size_t pinned = 0;
>  
> -	pinned = 0;
>  	mutex_lock(&dev->struct_mutex);
>  	list_for_each_entry(vma, &ggtt->base.active_list, vm_link)
>  		if (i915_vma_is_pinned(vma))
> @@ -157,9 +156,9 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
>  			pinned += vma->node.size;
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	args->aper_size = ggtt->base.total;
> +	args->aper_size = ggtt->base.total -
> +		 intel_vgt_reserved_size_by_balloon(dev_priv);
>  	args->aper_available_size = args->aper_size - pinned;
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 8bd0c49..9f3280d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -88,6 +88,7 @@
>  #include <drm/drmP.h>
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include "i915_vgpu.h"
>  #include "i915_trace.h"
>  
>  #define ALL_L3_SLICES(dev) (1 << NUM_L3_SLICES(dev)) - 1
> @@ -1053,7 +1054,8 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
>  		else if (to_i915(dev)->mm.aliasing_ppgtt)
>  			args->value = to_i915(dev)->mm.aliasing_ppgtt->base.total;
>  		else
> -			args->value = to_i915(dev)->ggtt.base.total;
> +			args->value = to_i915(dev)->ggtt.base.total -
> +			  intel_vgt_reserved_size_by_balloon(dev->dev_private);
>  		break;
>  	case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
>  		args->value = i915_gem_context_no_error_capture(ctx);
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index 4ab8a97..ce722d8 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -88,6 +88,11 @@ struct _balloon_info_ {
>  	 * graphic memory, 2/3 for unmappable graphic memory.
>  	 */
>  	struct drm_mm_node space[4];
> +	/*
> +	 * Total space size exclude ballooned named reserved_total, it's
> +	 * invisible for vGPU.
> +	 */
> +	size_t reserved_total;

What is size_t?

>  };
>  
>  static struct _balloon_info_ bl_info;
> @@ -116,6 +121,14 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
>  	memset(&bl_info, 0, sizeof(bl_info));
>  }
>  
> +size_t intel_vgt_reserved_size_by_balloon(struct drm_i915_private *dev_priv)
> +{
> +	if (!intel_vgpu_active(dev_priv))
> +		return 0;
> +
> +	return bl_info.reserved_total;
> +}

Or just return bl_info.reserved_total.

Why is there a global here anyway?

Better would be to track dev_priv->ggtt.reserved

Then the core code becomes
	gtt_size = dev_priv->ggtt.total - dev_priv->ggtt.reserved;

and doesn't need to know the identity of every possible consumer.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/gvt: return the actual aperture size under gvt environment
  2017-04-12  8:36 [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment Weinan Li
  2017-04-12  8:53 ` Chris Wilson
@ 2017-04-12 10:13 ` Patchwork
  1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-04-12 10:13 UTC (permalink / raw)
  To: Li, Weinan Z; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gvt: return the actual aperture size under gvt environment
URL   : https://patchwork.freedesktop.org/series/22910/
State : failure

== Summary ==

Series 22910v1 drm/i915/gvt: return the actual aperture size under gvt environment
https://patchwork.freedesktop.org/api/1.0/series/22910/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                none       -> INCOMPLETE (fi-bxt-t5700)
Test pm_rpm:
        Subgroup basic-rte:
                pass       -> INCOMPLETE (fi-bsw-n3050)

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:430s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:428s
fi-bsw-n3050     total:242  pass:207  dwarn:0   dfail:0   fail:0   skip:34 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:504s
fi-bxt-t5700     total:226  pass:208  dwarn:0   dfail:0   fail:0   skip:17 
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:490s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:480s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:412s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:404s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:486s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:482s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:452s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:571s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:453s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:569s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:456s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:486s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:430s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:403s

02b830fd67a6a2681e7f64510c3256b51a85c21a drm-tip: 2017y-04m-12d-09h-19m-10s UTC integration manifest
046d56b drm/i915/gvt: return the actual aperture size under gvt environment

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4485/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment
  2017-04-12  8:53 ` Chris Wilson
@ 2017-04-12 10:19   ` Joonas Lahtinen
  2017-04-13  1:01     ` Li, Weinan Z
  0 siblings, 1 reply; 7+ messages in thread
From: Joonas Lahtinen @ 2017-04-12 10:19 UTC (permalink / raw)
  To: Chris Wilson, Weinan Li; +Cc: intel-gfx, intel-gvt-dev

On ke, 2017-04-12 at 09:53 +0100, Chris Wilson wrote:
> On Wed, Apr 12, 2017 at 04:36:57PM +0800, Weinan Li wrote:
> > 
> > I915_GEM_GET_APERTURE ioctl is used to probe aperture size from userspace.
> > Some applications like OpenCL use this information to know how much GM
> > resource can it use.
> 
> That's a userspace bug.

Yes, a new property might be in place. I don't think we can go and
change the meaning of a parameter just like that.

<SNIP>

> > @@ -116,6 +121,14 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
> >  	memset(&bl_info, 0, sizeof(bl_info));
> >  }
> >  
> > +size_t intel_vgt_reserved_size_by_balloon(struct drm_i915_private *dev_priv)
> > +{
> > +	if (!intel_vgpu_active(dev_priv))
> > +		return 0;
> > +
> > +	return bl_info.reserved_total;
> > +}
> 
> Or just return bl_info.reserved_total.
> 
> Why is there a global here anyway?
> 
> Better would be to track dev_priv->ggtt.reserved
> 
> Then the core code becomes
> 	gtt_size = dev_priv->ggtt.total - dev_priv->ggtt.reserved;
> 
> and doesn't need to know the identity of every possible consumer.

I was writing an e-mail about the same thing. So +1 on the idea.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment
  2017-04-12 10:19   ` Joonas Lahtinen
@ 2017-04-13  1:01     ` Li, Weinan Z
  2017-04-13 10:11       ` Joonas Lahtinen
  0 siblings, 1 reply; 7+ messages in thread
From: Li, Weinan Z @ 2017-04-13  1:01 UTC (permalink / raw)
  To: Joonas Lahtinen, Chris Wilson; +Cc: intel-gfx, intel-gvt-dev

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Wednesday, April 12, 2017 6:19 PM
> To: Chris Wilson <chris@chris-wilson.co.uk>; Li, Weinan Z
> <weinan.z.li@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: return the actual aperture size
> under gvt environment
> 
> On ke, 2017-04-12 at 09:53 +0100, Chris Wilson wrote:
> > On Wed, Apr 12, 2017 at 04:36:57PM +0800, Weinan Li wrote:
> > >
> > > I915_GEM_GET_APERTURE ioctl is used to probe aperture size from
> userspace.
> > > Some applications like OpenCL use this information to know how much
> > > GM resource can it use.
> >
> > That's a userspace bug.
> 
> Yes, a new property might be in place. I don't think we can go and change the
> meaning of a parameter just like that.
> 
> <SNIP>
> 
Here I don’t want to change the meaning of I915_GEM_GET_APERTURE, but for the ioctl,
We need to return the actual available aperture size exclude the reserved space by GVT balloon.

> > > @@ -116,6 +121,14 @@ void intel_vgt_deballoon(struct drm_i915_private
> *dev_priv)
> > >  	memset(&bl_info, 0, sizeof(bl_info));
> > >  }
> > >
> > > +size_t intel_vgt_reserved_size_by_balloon(struct drm_i915_private
> > > +*dev_priv) {
> > > +	if (!intel_vgpu_active(dev_priv))
> > > +		return 0;
> > > +
> > > +	return bl_info.reserved_total;
> > > +}
> >
> > Or just return bl_info.reserved_total.
> >
> > Why is there a global here anyway?
> >
> > Better would be to track dev_priv->ggtt.reserved
> >
> > Then the core code becomes
> > 	gtt_size = dev_priv->ggtt.total - dev_priv->ggtt.reserved;
> >
> > and doesn't need to know the identity of every possible consumer.
> 
> I was writing an e-mail about the same thing. So +1 on the idea.
> 
Agreed.
> Regards, Joonas
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment
  2017-04-13  1:01     ` Li, Weinan Z
@ 2017-04-13 10:11       ` Joonas Lahtinen
  2017-04-14  7:33         ` Li, Weinan Z
  0 siblings, 1 reply; 7+ messages in thread
From: Joonas Lahtinen @ 2017-04-13 10:11 UTC (permalink / raw)
  To: Li, Weinan Z, Chris Wilson; +Cc: intel-gfx, intel-gvt-dev

On to, 2017-04-13 at 01:01 +0000, Li, Weinan Z wrote:
> > 
> > -----Original Message-----
> > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > Sent: Wednesday, April 12, 2017 6:19 PM
> > To: Chris Wilson <chris@chris-wilson.co.uk>; Li, Weinan Z
> > <weinan.z.li@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: return the actual aperture size
> > under gvt environment
> > 
> > On ke, 2017-04-12 at 09:53 +0100, Chris Wilson wrote:
> > > 
> > > On Wed, Apr 12, 2017 at 04:36:57PM +0800, Weinan Li wrote:
> > > > 
> > > > 
> > > > I915_GEM_GET_APERTURE ioctl is used to probe aperture size from
> > userspace.
> > > 
> > > > 
> > > > Some applications like OpenCL use this information to know how much
> > > > GM resource can it use.
> > > 
> > > That's a userspace bug.
> > 
> > Yes, a new property might be in place. I don't think we can go and change the
> > meaning of a parameter just like that.
> > 
> > <SNIP>
> > 
> Here I don’t want to change the meaning of I915_GEM_GET_APERTURE, but for the ioctl,
> We need to return the actual available aperture size exclude the reserved space by GVT balloon.

IOCTLs represent the ABI contract we have with userspace. It has
previously returned size of the aperture, so we can't change it to be
something else (like the usable size of aperture as proposed here).

Somebody might be doing an assert that any address in aperture is below
I915_GEM_GET_APERTURE returned value, which has previously been
correct, but would be broken after this change. There are also
potentially other things consuming the aperture than VGT ballooning, so
the UMDs would still be misbehaving.

Shouldn't they rather be doing these decisions based on
aper_available_size?

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment
  2017-04-13 10:11       ` Joonas Lahtinen
@ 2017-04-14  7:33         ` Li, Weinan Z
  0 siblings, 0 replies; 7+ messages in thread
From: Li, Weinan Z @ 2017-04-14  7:33 UTC (permalink / raw)
  To: Joonas Lahtinen, Chris Wilson; +Cc: intel-gfx, intel-gvt-dev

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Thursday, April 13, 2017 6:11 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>; Chris Wilson <chris@chris-
> wilson.co.uk>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: return the actual aperture size
> under gvt environment
> 
> On to, 2017-04-13 at 01:01 +0000, Li, Weinan Z wrote:
> > >
> > > -----Original Message-----
> > > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > > Sent: Wednesday, April 12, 2017 6:19 PM
> > > To: Chris Wilson <chris@chris-wilson.co.uk>; Li, Weinan Z
> > > <weinan.z.li@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org;
> > > intel-gvt-dev@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: return the actual
> > > aperture size under gvt environment
> > >
> > > On ke, 2017-04-12 at 09:53 +0100, Chris Wilson wrote:
> > > >
> > > > On Wed, Apr 12, 2017 at 04:36:57PM +0800, Weinan Li wrote:
> > > > >
> > > > >
> > > > > I915_GEM_GET_APERTURE ioctl is used to probe aperture size from
> > > userspace.
> > > >
> > > > >
> > > > > Some applications like OpenCL use this information to know how
> > > > > much GM resource can it use.
> > > >
> > > > That's a userspace bug.
> > >
> > > Yes, a new property might be in place. I don't think we can go and
> > > change the meaning of a parameter just like that.
> > >
> > > <SNIP>
> > >
> > Here I don’t want to change the meaning of I915_GEM_GET_APERTURE, but
> > for the ioctl, We need to return the actual available aperture size exclude the
> reserved space by GVT balloon.
> 
> IOCTLs represent the ABI contract we have with userspace. It has previously
> returned size of the aperture, so we can't change it to be something else (like
> the usable size of aperture as proposed here).
> 
> Somebody might be doing an assert that any address in aperture is below
> I915_GEM_GET_APERTURE returned value, which has previously been correct,
> but would be broken after this change. There are also potentially other things
> consuming the aperture than VGT ballooning, so the UMDs would still be
> misbehaving.
> 
> Shouldn't they rather be doing these decisions based on aper_available_size?
> 
Known your mean, if we return the value as below:
-	args->aper_size = ggtt->base.total;
+	args->aper_size = ggtt->base.total - ggtt->base.reserved;
Then userspace may use 'args->aper_size' as the MAX aperture addr, it may cause other issues.
In GVT with balloon the aperture addr still be from 0 to ggtt->base.total.
If it's expected behavior, change the available aperture size may avoid this.
	args->aper_size = ggtt->base.total;
 	args->aper_available_size = args->aper_size - ggtt->base.reserved - pinned;
> Regards, Joonas
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-04-14  7:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-12  8:36 [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment Weinan Li
2017-04-12  8:53 ` Chris Wilson
2017-04-12 10:19   ` Joonas Lahtinen
2017-04-13  1:01     ` Li, Weinan Z
2017-04-13 10:11       ` Joonas Lahtinen
2017-04-14  7:33         ` Li, Weinan Z
2017-04-12 10:13 ` ✗ Fi.CI.BAT: failure for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.