diff mbox series

[03/10] drm/i915/uapi: expose the avail tracking

Message ID 20220525184337.491763-4-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series small BAR uapi bits | expand

Commit Message

Matthew Auld May 25, 2022, 6:43 p.m. UTC
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.

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(-)

Comments

kernel test robot May 26, 2022, 2:44 a.m. UTC | #1
Hi Matthew,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on v5.18 next-20220525]
[cannot apply to drm-intel/for-linux-next drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Matthew-Auld/small-BAR-uapi-bits/20220526-024641
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-rhel-8.3-kunit (https://download.01.org/0day-ci/archive/20220526/202205261034.CoXEwzSb-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/614521eb68cc1e72a489c1c796827329c98bf031
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Matthew-Auld/small-BAR-uapi-bits/20220526-024641
        git checkout 614521eb68cc1e72a489c1c796827329c98bf031
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/i915_ttm_buddy_manager.c:379: warning: expecting prototype for i915_ttm_buddy_man_visible_size(). Prototype was for i915_ttm_buddy_man_avail() instead


vim +379 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c

   367	
   368	/**
   369	 * i915_ttm_buddy_man_visible_size - Query the avail tracking for the manager.
   370	 *
   371	 * @man: The buddy allocator ttm manager
   372	 * @avail: The total available memory in pages for the entire manager.
   373	 * @visible_avail: The total available memory in pages for the CPU visible
   374	 * portion. Note that this will always give the same value as @avail on
   375	 * configurations that don't have a small BAR.
   376	 */
   377	void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
   378				     u64 *avail, u64 *visible_avail)
 > 379	{
   380		struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
   381	
   382		mutex_lock(&bman->lock);
   383		*avail = bman->mm.avail >> PAGE_SHIFT;
   384		*visible_avail = bman->visible_avail;
   385		mutex_unlock(&bman->lock);
   386	}
   387
Tvrtko Ursulin May 26, 2022, 7:58 a.m. UTC | #2
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;
>   		};
>   	};
>   };
Matthew Auld May 26, 2022, 8:10 a.m. UTC | #3
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@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;
>>           };
>>       };
>>   };
Tvrtko Ursulin May 26, 2022, 8:33 a.m. UTC | #4
On 26/05/2022 09:10, Matthew Auld wrote:
> 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 

So this query would provide 
VkPhysicalDeviceMemoryBudgetPropertiesEXT::heapBudget. Apart that it 
wouldn't since we thought to lie. And apart that it's text says:

"""
...how much total memory from each heap the current process can use at 
any given time, before allocations may start failing or causing 
performance degradation. The values may change based on other activity 
in the system that is outside the scope and control of the Vulkan 
implementation.
"""

It acknowledges itself in the second sentence that the first sentence is 
questionable.

And VkPhysicalDeviceMemoryBudgetPropertiesEXT::heapUsage would be still 
missing and would maybe come via fdinfo? Or you plan to add it to this 
same query later?

I like to source knowledge of best practices from the long established 
world of CPU scheduling and process memory management. Is anyone aware 
of this kind of techniques there - applications actively looking at free 
memory data from /proc/meminfo and dynamically adjusting their runtime 
behaviour based on it? And that they are not single application on a 
dedicated system type of things.

Or perhaps this Vk extension is envisaged for exactly those kind of 
scenarios? However if so then userspace can know all this data from 
probed size and the data set it created.

> Also note that the existing behaviour was to lie. I'm not sure what's 
> the best option here.

Uapi reserved -1 for unknown so we could do that?

Regards,

Tvrtko

>>
>> 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;
>>>           };
>>>       };
>>>   };
Matthew Auld May 30, 2022, 5:05 p.m. UTC | #5
On 26/05/2022 09:33, Tvrtko Ursulin wrote:
> 
> On 26/05/2022 09:10, Matthew Auld wrote:
>> 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 
> 
> 
> So this query would provide 
> VkPhysicalDeviceMemoryBudgetPropertiesEXT::heapBudget. Apart that it 
> wouldn't since we thought to lie. And apart that it's text says:
> 
> """
> ...how much total memory from each heap the current process can use at 
> any given time, before allocations may start failing or causing 
> performance degradation. The values may change based on other activity 
> in the system that is outside the scope and control of the Vulkan 
> implementation.
> """
> 
> It acknowledges itself in the second sentence that the first sentence is 
> questionable.
> 
> And VkPhysicalDeviceMemoryBudgetPropertiesEXT::heapUsage would be still 
> missing and would maybe come via fdinfo? Or you plan to add it to this 
> same query later?

IIUC the heapUsage is something like per app usage, which already looks 
to be tracked in anv, although I don't think it knows if stuff is 
actually resident or not. The heapBudget looks to then be roughly the 
heapUsage + info.unallocated.

> 
> I like to source knowledge of best practices from the long established 
> world of CPU scheduling and process memory management. Is anyone aware 
> of this kind of techniques there - applications actively looking at free 
> memory data from /proc/meminfo and dynamically adjusting their runtime 
> behaviour based on it? And that they are not single application on a 
> dedicated system type of things.
> 
> Or perhaps this Vk extension is envisaged for exactly those kind of 
> scenarios? However if so then userspace can know all this data from 
> probed size and the data set it created.
> 
>> Also note that the existing behaviour was to lie. I'm not sure what's 
>> the best option here.
> 
> Uapi reserved -1 for unknown so we could do that?

AFAICT we can do that for the info.unallocated_cpu_visible, but not for 
the existing info.unallocated without maybe breaking something?

> 
> Regards,
> 
> Tvrtko
> 
>>>
>>> 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;
>>>>           };
>>>>       };
>>>>   };
diff mbox series

Patch

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;
 		};
 	};
 };