[Intel-gfx] [PATCH i-g-t] tests/gem_softpin: Use offset addresses in canonical form

[Michel Thierry]

On 1/6/2016 10:48 AM, Tvrtko Ursulin wrote:
>
> Hi,
>
> I've seen the RB but unfortunately I think we need another small tweak:
>
> On 11/12/15 14:14, Michel Thierry wrote:
>> i915 validates that requested offset is in canonical form, so tests need
>> to convert the offsets as required.
>>
>> Also add test to verify non-canonical 48-bit address will be rejected.
>>
>> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
>> ---
>>   tests/gem_softpin.c | 66
>> +++++++++++++++++++++++++++++++++++++----------------
>>   1 file changed, 46 insertions(+), 20 deletions(-)
>>
>> diff --git a/tests/gem_softpin.c b/tests/gem_softpin.c
>> index 7bee16b..2981b30 100644
>> --- a/tests/gem_softpin.c
>> +++ b/tests/gem_softpin.c
>> @@ -67,7 +67,7 @@ static void *create_mem_buffer(uint64_t size);
>>   static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr);
>>   static void gem_pin_userptr_test(void);
>>   static void gem_pin_bo_test(void);
>> -static void gem_pin_invalid_vma_test(bool test_decouple_flags);
>> +static void gem_pin_invalid_vma_test(bool test_decouple_flags, bool
>> test_canonical_offset);
>>   static void gem_pin_overlap_test(void);
>>   static void gem_pin_high_address_test(void);
>>
>> @@ -198,6 +198,15 @@ static void setup_exec_obj(struct
>> drm_i915_gem_exec_object2 *exec,
>>       exec->offset = offset;
>>   }
>>
>> +/* gen8_canonical_addr
>> + * Used to convert any address into canonical form, i.e. [63:48] ==
>> [47].
>> + * @address - a virtual address
>> +*/
>> +static uint64_t gen8_canonical_addr(uint64_t address)
>> +{
>> +    return ((int64_t)address << 16) >> 16;
>> +}
>> +
>
> This trick relies on implementation defined compiler behaviour and
> kernel patch was modified in the mean time to do it in a safer way. C&P
> of the relevant section:
>
> +/* Used to convert any address to canonical form.
> + * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
> + * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
> + * addresses to be in a canonical form:
> + * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in
> correct
> + * canonical form [63:48] == [47]."
> + */
> +#define GEN8_HIGH_ADDRESS_BIT 47
> +static inline uint64_t gen8_canonical_addr(uint64_t address)
> +{
> +    return sign_extend64(address, GEN8_HIGH_ADDRESS_BIT);
> +}
>
> Could you update the IGT to do the same?

Ok,
sign_extend64 is a kernel function, I'll add that same implementation in 
this test.