[Intel-gfx] [PATCH 03/10] drm/i915/uapi: expose the avail tracking
Matthew Auld
matthew.auld at intel.com
Thu May 26 08:10:35 UTC 2022
On 26/05/2022 08:58, Tvrtko Ursulin wrote:
>
> 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?
Lionel pointed out:
https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_EXT_memory_budget.html
Also note that the existing behaviour was to lie. I'm not sure what's
the best option here.
>
> Regards,
>
> Tvrtko
>
>> Testcase: igt at i915_query@query-regions-unallocated
>> Testcase: igt at i915_query@query-regions-sanity-check
>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>> Cc: Jon Bloomfield <jon.bloomfield at intel.com>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: Jordan Justen <jordan.l.justen at intel.com>
>> Cc: Kenneth Graunke <kenneth at whitecape.org>
>> Cc: Akeem G Abodunrin <akeem.g.abodunrin at 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;
>> };
>> };
>> };
More information about the Intel-gfx
mailing list