On 25/05/2022 19:43, Matthew Auld wrote:
Vulkan would like to have a rough measure of how much device memory can in theory be allocated. Also add unallocated_cpu_visible_size to track the visible portion, in case the device is using small BAR.
I have concerns here that it isn't useful and could even be counter-productive. If we give unprivileged access it may be considered a side channel, but if we "lie" (report total region size) to unprivileged clients (like in this patch), then they don't co-operate well and end trashing.
Is Vulkan really sure it wants this and why?
Regards,
Tvrtko
Testcase: igt@i915_query@query-regions-unallocated Testcase: igt@i915_query@query-regions-sanity-check Signed-off-by: Matthew Auld matthew.auld@intel.com Cc: Thomas Hellström thomas.hellstrom@linux.intel.com Cc: Lionel Landwerlin lionel.g.landwerlin@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Jon Bloomfield jon.bloomfield@intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Jordan Justen jordan.l.justen@intel.com Cc: Kenneth Graunke kenneth@whitecape.org Cc: Akeem G Abodunrin akeem.g.abodunrin@intel.com
drivers/gpu/drm/i915/i915_query.c | 10 +++++- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 20 ++++++++++++ drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 3 ++ drivers/gpu/drm/i915/intel_memory_region.c | 14 +++++++++ drivers/gpu/drm/i915/intel_memory_region.h | 3 ++ include/uapi/drm/i915_drm.h | 31 ++++++++++++++++++- 6 files changed, 79 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c index 9aa0b28aa6ee..e095c55f4d4b 100644 --- a/drivers/gpu/drm/i915/i915_query.c +++ b/drivers/gpu/drm/i915/i915_query.c @@ -502,7 +502,15 @@ static int query_memregion_info(struct drm_i915_private *i915, else info.probed_cpu_visible_size = mr->total;
info.unallocated_size = mr->avail;
if (perfmon_capable()) {intel_memory_region_avail(mr,&info.unallocated_size,&info.unallocated_cpu_visible_size);} else {info.unallocated_size = info.probed_size;info.unallocated_cpu_visible_size =info.probed_cpu_visible_size;}if (__copy_to_user(info_ptr, &info, sizeof(info))) return -EFAULT;
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index a5109548abc0..aa5c91e44438 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -365,6 +365,26 @@ u64 i915_ttm_buddy_man_visible_size(struct ttm_resource_manager *man) return bman->visible_size; }
+/**
- i915_ttm_buddy_man_visible_size - Query the avail tracking for the manager.
- @man: The buddy allocator ttm manager
- @avail: The total available memory in pages for the entire manager.
- @visible_avail: The total available memory in pages for the CPU visible
- portion. Note that this will always give the same value as @avail on
- configurations that don't have a small BAR.
- */
+void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
u64 *avail, u64 *visible_avail)+{
- struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
- mutex_lock(&bman->lock);
- *avail = bman->mm.avail >> PAGE_SHIFT;
- *visible_avail = bman->visible_avail;
- mutex_unlock(&bman->lock);
+}
- #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) void i915_ttm_buddy_man_force_visible_size(struct ttm_resource_manager *man, u64 size)
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h index 52d9586d242c..d64620712830 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h @@ -61,6 +61,9 @@ int i915_ttm_buddy_man_reserve(struct ttm_resource_manager *man,
u64 i915_ttm_buddy_man_visible_size(struct ttm_resource_manager *man);
+void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
u64 *avail, u64 *avail_visible);- #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) void i915_ttm_buddy_man_force_visible_size(struct ttm_resource_manager *man, u64 size);
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c index e38d2db1c3e3..94ee26e99549 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.c +++ b/drivers/gpu/drm/i915/intel_memory_region.c @@ -279,6 +279,20 @@ void intel_memory_region_set_name(struct intel_memory_region *mem, va_end(ap); }
+void intel_memory_region_avail(struct intel_memory_region *mr,
u64 *avail, u64 *visible_avail)+{
- if (mr->type == INTEL_MEMORY_LOCAL) {
i915_ttm_buddy_man_avail(mr->region_private,avail, visible_avail);*avail <<= PAGE_SHIFT;*visible_avail <<= PAGE_SHIFT;- } else {
*avail = mr->total;*visible_avail = mr->total;- }
+}
- void intel_memory_region_destroy(struct intel_memory_region *mem) { int ret = 0;
diff --git a/drivers/gpu/drm/i915/intel_memory_region.h b/drivers/gpu/drm/i915/intel_memory_region.h index 3d8378c1b447..2214f251bec3 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.h +++ b/drivers/gpu/drm/i915/intel_memory_region.h @@ -127,6 +127,9 @@ int intel_memory_region_reserve(struct intel_memory_region *mem, void intel_memory_region_debug(struct intel_memory_region *mr, struct drm_printer *printer);
+void intel_memory_region_avail(struct intel_memory_region *mr,
u64 *avail, u64 *visible_avail);- struct intel_memory_region * i915_gem_ttm_system_setup(struct drm_i915_private *i915, u16 type, u16 instance);
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 9df419a45244..e30f31a440b3 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -3228,7 +3228,15 @@ struct drm_i915_memory_region_info { */ __u64 probed_size;
- /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
/**
* @unallocated_size: Estimate of memory remaining (-1 = unknown)** Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable accounting.* Without this (or if this is an older kernel) the value here will* always equal the @probed_size. Note this is only currently tracked* for I915_MEMORY_CLASS_DEVICE regions (for other types the value here* will always equal the @probed_size).*/__u64 unallocated_size;
union {
@@ -3262,6 +3270,27 @@ struct drm_i915_memory_region_info { * @probed_size. */ __u64 probed_cpu_visible_size;
/*** @unallocated_cpu_visible_size: Estimate of CPU* visible memory remaining (-1 = unknown).** Note this is only tracked for* I915_MEMORY_CLASS_DEVICE regions (for other types the* value here will always equal the* @probed_cpu_visible_size).** Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable* accounting. Without this the value here will always* equal the @probed_cpu_visible_size. Note this is only* currently tracked for I915_MEMORY_CLASS_DEVICE* regions (for other types the value here will also* always equal the @probed_cpu_visible_size).** If this is an older kernel the value here will be* zero, see also @probed_cpu_visible_size.*/__u64 unallocated_cpu_visible_size;