Re: [PATCH for-4.10] xen/arm: mm: Rework MAIR* definitions to handle 32-bit compilation environment

[Andre Przywara <andre.przywara@arm.com>]

Hi,

On 11/10/17 15:15, Julien Grall wrote:
> Commit a0543df403 "xen/arm: page: Clean-up the definition of MAIRVAL"
> combined the definition of MAIR0VAL and MAIR1VAL in MAIRVAL. Sadly, when
> building in 32-bit environment, the assembler is unable to compute
> 64-bit constant and will ignore the 32-bit most-significants bits. This
> will result of MAIR1 set 0.
> 
> Rather than fully reverting the offending commit, the code is reworked
> to still avoid hardcoded values but split the definition in 2.

Nasty issue, but given the circumstances the workaround seems not too
bad for me.

> Lastly, a comment is added to avoid trying to blindly combine the both
> definition again in the future.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> ---
>  xen/include/asm-arm/page.h | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index f558184e10..d948250a4a 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -52,18 +52,23 @@
>   *   ??               101
>   *   reserved         110
>   *   MT_NORMAL        111   1111 1111  -- Write-back write-allocate
> + *
> + * /!\ It is not possible to combine the definition in MAIRVAL and then
> + * split because it would result to a 64-bit value that some assembler
> + * doesn't understand.
>   */
> -#define MAIR(attr, mt) (_AC(attr, ULL) << ((mt) * 8))
> +#define _MAIR0(attr, mt) (_AC(attr, ULL) << ((mt) * 8))
> +#define _MAIR1(attr, mt) (_AC(attr, ULL) << (((mt) * 8) - 32))
> +
> +#define MAIR0VAL (_MAIR0(0x00, MT_DEVICE_nGnRnE)| \
> +                  _MAIR0(0x44, MT_NORMAL_NC)    | \
> +                  _MAIR0(0xaa, MT_NORMAL_WT)    | \
> +                  _MAIR0(0xee, MT_NORMAL_WB))
>  
> -#define MAIRVAL (MAIR(0x00, MT_DEVICE_nGnRnE)| \
> -                 MAIR(0x44, MT_NORMAL_NC)    | \
> -                 MAIR(0xaa, MT_NORMAL_WT)    | \
> -                 MAIR(0xee, MT_NORMAL_WB)    | \
> -                 MAIR(0x04, MT_DEVICE_nGnRE) | \
> -                 MAIR(0xff, MT_NORMAL))
> +#define MAIR1VAL (_MAIR1(0x04, MT_DEVICE_nGnRE) | \
> +                  _MAIR1(0xff, MT_NORMAL))
>  
> -#define MAIR0VAL (MAIRVAL & 0xffffffff)
> -#define MAIR1VAL (MAIRVAL >> 32)
> +#define MAIRVAL (MAIR1VAL << 32 | MAIR0VAL)
>  
>  /*
>   * Layout of the flags used for updating the hypervisor page tables
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel