[Mesa-dev] [PATCH] intel/genxml: Canonicalize addresses before writing them

[Chris Wilson]

On Thu, May 11, 2017 at 09:13:55AM -0700, Jason Ekstrand wrote:
> This doesn't actually fix any 48-bit bugs because the addresses provided
> to us from the kernel are already sign-extended.  However, it still
> seems like a good idea.

addr=1<<47-4096, delta=4096

> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  src/intel/genxml/gen_pack_header.py | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/src/intel/genxml/gen_pack_header.py b/src/intel/genxml/gen_pack_header.py
> index 020dbe4..df71689 100644
> --- a/src/intel/genxml/gen_pack_header.py
> +++ b/src/intel/genxml/gen_pack_header.py
> @@ -168,6 +168,31 @@ __gen_ufixed(float v, uint32_t start, uint32_t end, uint32_t fract_bits)
>  #error #define __gen_combine_address before including this file
>  #endif
>  
> +static inline uint64_t
> +__gen_address(__gen_user_data *data, void *location, __gen_address_type addr,
> +              uint32_t delta, uint32_t start, uint32_t end)
> +{
> +   __gen_validate_value(addr);
> +
> +   uint64_t offset = __gen_combine_address(data, location, addr, delta);
> +
> +   if (end >= 32) {
> +      /* From the Broadwell PRM Vol. 2a, MI_LOAD_REGISTER_MEM::MemoryAddress:
> +       *
> +       *    "This field specifies the address of the memory location where the
> +       *    register value specified in the DWord above will read from. The
> +       *    address specifies the DWord location of the data. Range =
> +       *    GraphicsVirtualAddress[63:2] for a DWord register GraphicsAddress
> +       *    [63:48] are ignored by the HW and assumed to be in correct
> +       *    canonical form [63:48] == [47]."
> +       */
> +      const int shift = 63 - 47;
> +      return (((int64_t)offset) << shift) >> shift;

I presume that start/end are known at compile time, so the branch here
is just a figment of my imagination.

There's no chance of having the shift + comments pulled in from a header?
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre