diff mbox

[v2] ARM: tlb: ASID macro should give 32bit result for BE correct operation

Message ID 1381160903-1248-2-git-send-email-victor.kamensky@linaro.org
State Accepted
Commit a1af3474487cc3b8731b990dceac6b6aad7f3ed8
Headers show

Commit Message

vkamensky Oct. 7, 2013, 3:48 p.m. UTC
In order for ASID macro to be used as expression passed to
inline asm as 'r' operand it needs to give 32 bit unsigned result,
not unsigned 64bit expression.

Otherwise when 64bit ASID is passed to inline assembler statement
as 'r' operand (32bit) compiler behavior is not well specified.
For example when __flush_tlb_mm function compiled in big endian
case, and ASID is passed to tlb_op macro directly, 0 will be passed
as 'mcr	15, 0, r4, cr8, cr3, {2}' argument in r4, unless ASID
macro changed to produce 32 bit result.

Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
 arch/arm/include/asm/mmu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Will Deacon Oct. 7, 2013, 3:50 p.m. UTC | #1
On Mon, Oct 07, 2013 at 04:48:23PM +0100, Victor Kamensky wrote:
> In order for ASID macro to be used as expression passed to
> inline asm as 'r' operand it needs to give 32 bit unsigned result,
> not unsigned 64bit expression.
> 
> Otherwise when 64bit ASID is passed to inline assembler statement
> as 'r' operand (32bit) compiler behavior is not well specified.
> For example when __flush_tlb_mm function compiled in big endian
> case, and ASID is passed to tlb_op macro directly, 0 will be passed
> as 'mcr	15, 0, r4, cr8, cr3, {2}' argument in r4, unless ASID
> macro changed to produce 32 bit result.
> 
> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
> ---
>  arch/arm/include/asm/mmu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
> index 6f18da0..64fd151 100644
> --- a/arch/arm/include/asm/mmu.h
> +++ b/arch/arm/include/asm/mmu.h
> @@ -16,7 +16,7 @@ typedef struct {
>  #ifdef CONFIG_CPU_HAS_ASID
>  #define ASID_BITS	8
>  #define ASID_MASK	((~0ULL) << ASID_BITS)
> -#define ASID(mm)	((mm)->context.id.counter & ~ASID_MASK)
> +#define ASID(mm)	((unsigned int)((mm)->context.id.counter & ~ASID_MASK))
>  #else
>  #define ASID(mm)	(0)
>  #endif

  Acked-by: Will Deacon <will.deacon@arm.com>

Cheers,

Will
Russell King - ARM Linux Oct. 7, 2013, 3:52 p.m. UTC | #2
On Mon, Oct 07, 2013 at 08:48:23AM -0700, Victor Kamensky wrote:
> -#define ASID(mm)	((mm)->context.id.counter & ~ASID_MASK)
> +#define ASID(mm)	((unsigned int)((mm)->context.id.counter & ~ASID_MASK))

Not a big problem, but "unsigned" is sufficient as per my suggestion.
See:

	http://en.wikipedia.org/wiki/C_data_types

or look it up in the C standards. :)  I'll take it either way.
vkamensky Oct. 7, 2013, 4:19 p.m. UTC | #3
On 7 October 2013 08:52, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Oct 07, 2013 at 08:48:23AM -0700, Victor Kamensky wrote:
>> -#define ASID(mm)     ((mm)->context.id.counter & ~ASID_MASK)
>> +#define ASID(mm)     ((unsigned int)((mm)->context.id.counter & ~ASID_MASK))
>
> Not a big problem, but "unsigned" is sufficient as per my suggestion.
> See:
>
>         http://en.wikipedia.org/wiki/C_data_types
>
> or look it up in the C standards. :)  I'll take it either way.

Understood, I just tried to be consistent with the rest of file -
'unsigned int' was
used just few lines above place that got changed.

Thanks,
Victor
diff mbox

Patch

diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
index 6f18da0..64fd151 100644
--- a/arch/arm/include/asm/mmu.h
+++ b/arch/arm/include/asm/mmu.h
@@ -16,7 +16,7 @@  typedef struct {
 #ifdef CONFIG_CPU_HAS_ASID
 #define ASID_BITS	8
 #define ASID_MASK	((~0ULL) << ASID_BITS)
-#define ASID(mm)	((mm)->context.id.counter & ~ASID_MASK)
+#define ASID(mm)	((unsigned int)((mm)->context.id.counter & ~ASID_MASK))
 #else
 #define ASID(mm)	(0)
 #endif