Re: [igt-dev] [RFC 5/5] igt/lib: If mappable aperture is missing return 0 size

Antonio Argenziano
2019-03-07T16:29:36Z

On 07/03/19 07:58, Chris Wilson wrote:
> Quoting Antonio Argenziano (2019-03-07 15:51:28)
>>
>>
>> On 21/02/19 13:51, Chris Wilson wrote:
>>> Quoting Antonio Argenziano (2019-02-21 21:45:46)
>>>>
>>>>
>>>> On 21/02/19 12:01, Chris Wilson wrote:
>>>>> Quoting Antonio Argenziano (2019-02-21 19:27:45)
>>>>>> So far the aperture size has been read directly from the bar,
>>>>>> in this patch we return zero if the mappable aperture is not available
>>>>>> as the value stored in the bar might not be accurate. The patch also adds a
>>>>>> 'require' when a call to gem_mappable_aperture_size() is made so that
>>>>>> the aperture is guaranteed to exist before checking the size.
>>>>>>
>>>>>> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
>>>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>>>>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>>>>>> ---
>>>>>> lib/ioctl_wrappers.c | 28 +++++++++++++++++++---------
>>>>>> lib/ioctl_wrappers.h | 2 +-
>>>>>> tests/i915/gem_concurrent_all.c | 12 ++++++------
>>>>>> tests/i915/gem_cpu_reloc.c | 14 ++++++++++----
>>>>>> tests/i915/gem_mmap.c | 4 ++--
>>>>>> tests/i915/gem_mmap_gtt.c | 10 +++++-----
>>>>>> tests/i915/gem_pwrite.c | 4 ++--
>>>>>> tests/i915/gem_shrink.c | 2 +-
>>>>>> tests/i915/i915_pm_rpm.c | 2 +-
>>>>>> tests/kms_flip.c | 2 +-
>>>>>> tests/prime_mmap.c | 7 +++++--
>>>>>> 11 files changed, 53 insertions(+), 34 deletions(-)
>>>>>>
>>>>>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
>>>>>> index 25dd8ad3..ab7c7a5e 100644
>>>>>> --- a/lib/ioctl_wrappers.c
>>>>>> +++ b/lib/ioctl_wrappers.c
>>>>>> @@ -1440,19 +1440,14 @@ uint64_t gem_aperture_size(int fd)
>>>>>> return aperture_size;
>>>>>> }
>>>>>>
>>>>>> -/**
>>>>>> - * gem_mappable_aperture_size:
>>>>>> - *
>>>>>> - * Feature test macro to query the kernel for the mappable gpu aperture size.
>>>>>> - * This is the area available for GTT memory mappings.
>>>>>> - *
>>>>>> - * Returns: The mappable gtt address space size.
>>>>>> - */
>>>>>> -uint64_t gem_mappable_aperture_size(void)
>>>>>> +static uint64_t __gem_mappable_aperture_size(int fd)
>>>>>> {
>>>>>> struct pci_device *pci_dev = intel_get_pci_device();
>>>>>> int bar;
>>>>>>
>>>>>> + if (!gem_mmap__has_gtt(fd))
>>>>>> + return 0; /* Aperture not available */
>>>>>> +
>>>>>> if (intel_gen(pci_dev->device_id) < 3)
>>>>>> bar = 0;
>>>>>
>>>>> I swear we made this accessible via fd so that we didn't need root to
>>>>> poke around libpciacess.
>>>>
>>>> mhh, couldn't find it. Is it in some IOCTL?
>>>
>>> No, we extended get_aperture_ioctl to provide it, once upon a time. I
>>> suggest we do something similar, or we use a param to get the actual
>>> value. (Even if it's legacy, we can still justify a usecase as I'm
>>> pretty sure 256MiB is hardcoded into several legacy userspaces.)
>>
>> Is something like: https://paste.debian.net/1072096/ acceptable?
>
> Yes.
>
> Kernel-> old new
> Userspace
> v
>
> old no change see old size, skips write
>
> new reads 0 legacy HW gets mappable size
> new HW, 0?
>
> So for new userspace, how do we determine 0 meaning old kernel, pick a
> value for ourselves, or 0 meaning no aperture at all. PCI-ID? Or is it
> simply that new userspace (that might run on no aperture devices)
> doesn't care about the aperture size anyway, so it's a meaningless
> question?

Good question, I'm afraid I don't know :). There is the option of saying
new userspace should check the gtt_version before using the aperture at
all but I can see the appeal of just getting the size to make that decision.

Antonio

> -Chris