diff mbox series

[v10,12/25] arm64: irqflags: Use ICC_PMR_EL1 for interrupt masking

Message ID 1548946743-38979-13-git-send-email-julien.thierry@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: provide pseudo NMI with GICv3 | expand

Commit Message

Julien Thierry Jan. 31, 2019, 2:58 p.m. UTC 
Instead disabling interrupts by setting the PSR.I bit, use a priority
higher than the one used for interrupts to mask them via PMR.

When using PMR to disable interrupts, the value of PMR will be used
instead of PSR.[DAIF] for the irqflags.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
Suggested-by: Daniel Thompson <daniel.thompson@linaro.org>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Oleg Nesterov <oleg@redhat.com>
---
 arch/arm64/include/asm/efi.h      |  11 +++++
 arch/arm64/include/asm/irqflags.h | 100 +++++++++++++++++++++++++++-----------
 2 files changed, 83 insertions(+), 28 deletions(-)

Comments

Nathan Chancellor Feb. 8, 2019, 4:35 a.m. UTC | #1 
On Thu, Jan 31, 2019 at 02:58:50PM +0000, Julien Thierry wrote:
> Instead disabling interrupts by setting the PSR.I bit, use a priority
> higher than the one used for interrupts to mask them via PMR.
> 
> When using PMR to disable interrupts, the value of PMR will be used
> instead of PSR.[DAIF] for the irqflags.
> 
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> Suggested-by: Daniel Thompson <daniel.thompson@linaro.org>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> ---
>  arch/arm64/include/asm/efi.h      |  11 +++++
>  arch/arm64/include/asm/irqflags.h | 100 +++++++++++++++++++++++++++-----------
>  2 files changed, 83 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index 7ed3208..c9e9a69 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -44,6 +44,17 @@
>  
>  #define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
>  
> +/*
> + * Even when Linux uses IRQ priorities for IRQ disabling, EFI does not.
> + * And EFI shouldn't really play around with priority masking as it is not aware
> + * which priorities the OS has assigned to its interrupts.
> + */
> +#define arch_efi_save_flags(state_flags)		\
> +	((void)((state_flags) = read_sysreg(daif)))
> +
> +#define arch_efi_restore_flags(state_flags)	write_sysreg(state_flags, daif)
> +
> +
>  /* arch specific definitions used by the stub code */
>  
>  /*
> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
> index 24692ed..d4597b2 100644
> --- a/arch/arm64/include/asm/irqflags.h
> +++ b/arch/arm64/include/asm/irqflags.h
> @@ -18,7 +18,9 @@
>  
>  #ifdef __KERNEL__
>  
> +#include <asm/alternative.h>
>  #include <asm/ptrace.h>
> +#include <asm/sysreg.h>
>  
>  /*
>   * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
> @@ -36,33 +38,27 @@
>  /*
>   * CPU interrupt mask handling.
>   */
> -static inline unsigned long arch_local_irq_save(void)
> -{
> -	unsigned long flags;
> -	asm volatile(
> -		"mrs	%0, daif		// arch_local_irq_save\n"
> -		"msr	daifset, #2"
> -		: "=r" (flags)
> -		:
> -		: "memory");
> -	return flags;
> -}
> -
>  static inline void arch_local_irq_enable(void)
>  {
> -	asm volatile(
> -		"msr	daifclr, #2		// arch_local_irq_enable"
> -		:
> +	asm volatile(ALTERNATIVE(
> +		"msr	daifclr, #2		// arch_local_irq_enable\n"
> +		"nop",
> +		"msr_s  " __stringify(SYS_ICC_PMR_EL1) ",%0\n"
> +		"dsb	sy",
> +		ARM64_HAS_IRQ_PRIO_MASKING)
>  		:
> +		: "r" (GIC_PRIO_IRQON)
>  		: "memory");
>  }
>  
>  static inline void arch_local_irq_disable(void)
>  {
> -	asm volatile(
> -		"msr	daifset, #2		// arch_local_irq_disable"
> -		:
> +	asm volatile(ALTERNATIVE(
> +		"msr	daifset, #2		// arch_local_irq_disable",
> +		"msr_s  " __stringify(SYS_ICC_PMR_EL1) ", %0",
> +		ARM64_HAS_IRQ_PRIO_MASKING)
>  		:
> +		: "r" (GIC_PRIO_IRQOFF)
>  		: "memory");
>  }
>  
> @@ -71,12 +67,44 @@ static inline void arch_local_irq_disable(void)
>   */
>  static inline unsigned long arch_local_save_flags(void)
>  {
> +	unsigned long daif_bits;
>  	unsigned long flags;
> -	asm volatile(
> -		"mrs	%0, daif		// arch_local_save_flags"
> -		: "=r" (flags)
> -		:
> +
> +	daif_bits = read_sysreg(daif);
> +
> +	/*
> +	 * The asm is logically equivalent to:
> +	 *
> +	 * if (system_uses_irq_prio_masking())
> +	 *	flags = (daif_bits & PSR_I_BIT) ?
> +	 *			GIC_PRIO_IRQOFF :
> +	 *			read_sysreg_s(SYS_ICC_PMR_EL1);
> +	 * else
> +	 *	flags = daif_bits;
> +	 */
> +	asm volatile(ALTERNATIVE(
> +			"mov	%0, %1\n"
> +			"nop\n"
> +			"nop",
> +			"mrs_s	%0, " __stringify(SYS_ICC_PMR_EL1) "\n"
> +			"ands	%1, %1, " __stringify(PSR_I_BIT) "\n"
> +			"csel	%0, %0, %2, eq",
> +			ARM64_HAS_IRQ_PRIO_MASKING)
> +		: "=&r" (flags), "+r" (daif_bits)
> +		: "r" (GIC_PRIO_IRQOFF)
>  		: "memory");
> +
> +	return flags;
> +}
> +
> +static inline unsigned long arch_local_irq_save(void)
> +{
> +	unsigned long flags;
> +
> +	flags = arch_local_save_flags();
> +
> +	arch_local_irq_disable();
> +
>  	return flags;
>  }
>  
> @@ -85,16 +113,32 @@ static inline unsigned long arch_local_save_flags(void)
>   */
>  static inline void arch_local_irq_restore(unsigned long flags)
>  {
> -	asm volatile(
> -		"msr	daif, %0		// arch_local_irq_restore"
> -	:
> -	: "r" (flags)
> -	: "memory");
> +	asm volatile(ALTERNATIVE(
> +			"msr	daif, %0\n"
> +			"nop",
> +			"msr_s	" __stringify(SYS_ICC_PMR_EL1) ", %0\n"
> +			"dsb	sy",
> +			ARM64_HAS_IRQ_PRIO_MASKING)
> +		: "+r" (flags)
> +		:
> +		: "memory");
>  }
>  
>  static inline int arch_irqs_disabled_flags(unsigned long flags)
>  {
> -	return flags & PSR_I_BIT;
> +	int res;
> +
> +	asm volatile(ALTERNATIVE(
> +			"and	%w0, %w1, #" __stringify(PSR_I_BIT) "\n"
> +			"nop",
> +			"cmp	%w1, #" __stringify(GIC_PRIO_IRQOFF) "\n"
> +			"cset	%w0, ls",
> +			ARM64_HAS_IRQ_PRIO_MASKING)
> +		: "=&r" (res)
> +		: "r" ((int) flags)
> +		: "memory");
> +
> +	return res;
>  }
>  #endif
>  #endif
> -- 
> 1.9.1
> 

Hi Julien,

This patch introduced a slew of Clang warnings:

In file included from arch/arm64/kernel/signal.c:21:
In file included from include/linux/compat.h:10:
In file included from include/linux/time.h:6:
In file included from include/linux/seqlock.h:36:
In file included from include/linux/spinlock.h:54:
In file included from include/linux/irqflags.h:16:
arch/arm64/include/asm/irqflags.h:50:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                : "r" (GIC_PRIO_IRQON)
                       ^
arch/arm64/include/asm/ptrace.h:39:25: note: expanded from macro 'GIC_PRIO_IRQON'
#define GIC_PRIO_IRQON          0xf0
                                ^
arch/arm64/include/asm/irqflags.h:46:44: note: use constraint modifier "w"
                "msr_s  " __stringify(SYS_ICC_PMR_EL1) ",%0\n"
                                                         ^~
                                                         %w0
arch/arm64/include/asm/alternative.h:286:29: note: expanded from macro 'ALTERNATIVE'
        _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
                                   ^
arch/arm64/include/asm/alternative.h:88:30: note: expanded from macro '_ALTERNATIVE_CFG'
        __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
                                    ^
arch/arm64/include/asm/alternative.h:76:2: note: expanded from macro '__ALTERNATIVE_CFG'
        newinstr "\n"                                                   \
        ^
In file included from arch/arm64/kernel/signal.c:21:
In file included from include/linux/compat.h:10:
In file included from include/linux/time.h:6:
In file included from include/linux/seqlock.h:36:
In file included from include/linux/spinlock.h:54:
In file included from include/linux/irqflags.h:16:
arch/arm64/include/asm/irqflags.h:61:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                : "r" (GIC_PRIO_IRQOFF)
                       ^
arch/arm64/include/asm/ptrace.h:40:26: note: expanded from macro 'GIC_PRIO_IRQOFF'
#define GIC_PRIO_IRQOFF         (GIC_PRIO_IRQON & ~0x80)
                                ^
arch/arm64/include/asm/irqflags.h:58:45: note: use constraint modifier "w"
                "msr_s  " __stringify(SYS_ICC_PMR_EL1) ", %0",
                                                          ^
arch/arm64/include/asm/irqflags.h:94:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                : "r" (GIC_PRIO_IRQOFF)
                       ^
arch/arm64/include/asm/ptrace.h:40:26: note: expanded from macro 'GIC_PRIO_IRQOFF'
#define GIC_PRIO_IRQOFF         (GIC_PRIO_IRQON & ~0x80)
                                ^
arch/arm64/include/asm/irqflags.h:91:18: note: use constraint modifier "w"
                        "csel   %0, %0, %2, eq",
                                        ^~
                                        %w2
arch/arm64/include/asm/alternative.h:286:29: note: expanded from macro 'ALTERNATIVE'
        _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
                                   ^
arch/arm64/include/asm/alternative.h:88:30: note: expanded from macro '_ALTERNATIVE_CFG'
        __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
                                    ^
arch/arm64/include/asm/alternative.h:76:2: note: expanded from macro '__ALTERNATIVE_CFG'
        newinstr "\n"                                                   \
        ^
3 warnings generated.


I am not sure if they should be fixed with Clang's suggestion of a
constraint modifier or a cast like commit 1b57ec8c7527 ("arm64: io:
Ensure value passed to __iormb() is held in a 64-bit register"), hence
this message.

Thanks,
Nathan
Julien Thierry Feb. 8, 2019, 9:36 a.m. UTC | #2 
Hi Nathan,

On 08/02/2019 04:35, Nathan Chancellor wrote:
> On Thu, Jan 31, 2019 at 02:58:50PM +0000, Julien Thierry wrote:

[...]

> 
> Hi Julien,
> 
> This patch introduced a slew of Clang warnings:
> 
> In file included from arch/arm64/kernel/signal.c:21:
> In file included from include/linux/compat.h:10:
> In file included from include/linux/time.h:6:
> In file included from include/linux/seqlock.h:36:
> In file included from include/linux/spinlock.h:54:
> In file included from include/linux/irqflags.h:16:
> arch/arm64/include/asm/irqflags.h:50:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                 : "r" (GIC_PRIO_IRQON)
>                        ^
> arch/arm64/include/asm/ptrace.h:39:25: note: expanded from macro 'GIC_PRIO_IRQON'
> #define GIC_PRIO_IRQON          0xf0
>                                 ^
> arch/arm64/include/asm/irqflags.h:46:44: note: use constraint modifier "w"
>                 "msr_s  " __stringify(SYS_ICC_PMR_EL1) ",%0\n"
>                                                          ^~
>                                                          %w0

I'm not sure I get the relevance of this kind of warnings from Clang.
Had it been an output operand I could understand the concern of having a
variable too small to store the register value. But here it's an input
operand being place in a wider register...

> arch/arm64/include/asm/alternative.h:286:29: note: expanded from macro 'ALTERNATIVE'
>         _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
>                                    ^
> arch/arm64/include/asm/alternative.h:88:30: note: expanded from macro '_ALTERNATIVE_CFG'
>         __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
>                                     ^
> arch/arm64/include/asm/alternative.h:76:2: note: expanded from macro '__ALTERNATIVE_CFG'
>         newinstr "\n"                                                   \
>         ^
> In file included from arch/arm64/kernel/signal.c:21:
> In file included from include/linux/compat.h:10:
> In file included from include/linux/time.h:6:
> In file included from include/linux/seqlock.h:36:
> In file included from include/linux/spinlock.h:54:
> In file included from include/linux/irqflags.h:16:
> arch/arm64/include/asm/irqflags.h:61:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                 : "r" (GIC_PRIO_IRQOFF)
>                        ^
> arch/arm64/include/asm/ptrace.h:40:26: note: expanded from macro 'GIC_PRIO_IRQOFF'
> #define GIC_PRIO_IRQOFF         (GIC_PRIO_IRQON & ~0x80)
>                                 ^
> arch/arm64/include/asm/irqflags.h:58:45: note: use constraint modifier "w"
>                 "msr_s  " __stringify(SYS_ICC_PMR_EL1) ", %0",
>                                                           ^
> arch/arm64/include/asm/irqflags.h:94:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                 : "r" (GIC_PRIO_IRQOFF)
>                        ^
> arch/arm64/include/asm/ptrace.h:40:26: note: expanded from macro 'GIC_PRIO_IRQOFF'
> #define GIC_PRIO_IRQOFF         (GIC_PRIO_IRQON & ~0x80)
>                                 ^
> arch/arm64/include/asm/irqflags.h:91:18: note: use constraint modifier "w"
>                         "csel   %0, %0, %2, eq",
>                                         ^~
>                                         %w2
> arch/arm64/include/asm/alternative.h:286:29: note: expanded from macro 'ALTERNATIVE'
>         _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
>                                    ^
> arch/arm64/include/asm/alternative.h:88:30: note: expanded from macro '_ALTERNATIVE_CFG'
>         __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
>                                     ^
> arch/arm64/include/asm/alternative.h:76:2: note: expanded from macro '__ALTERNATIVE_CFG'
>         newinstr "\n"                                                   \
>         ^
> 3 warnings generated.
> 
> 
> I am not sure if they should be fixed with Clang's suggestion of a
> constraint modifier or a cast like commit 1b57ec8c7527 ("arm64: io:
> Ensure value passed to __iormb() is held in a 64-bit register"), hence
> this message.
> 

Clang's suggestion would not work as MSR instructions do not operate on
32-bit general purpose registers. Seeing that PMR is a 32-bit register,
I'd avoid adding UL for the GIC_PRIO_IRQ* constants.

So I'd recommend just casting the the asm inline operands to unsigned
long. This should only affect the 3 locations
arch/arm64/include/asm/irqflags.h.

Does the following patch work for you?

Thanks,
Nathan Chancellor Feb. 8, 2019, 4 p.m. UTC | #3 
On Fri, Feb 08, 2019 at 09:36:48AM +0000, Julien Thierry wrote:
> Hi Nathan,
> 
> On 08/02/2019 04:35, Nathan Chancellor wrote:
> > On Thu, Jan 31, 2019 at 02:58:50PM +0000, Julien Thierry wrote:
> 
> [...]
> 
> > 
> > Hi Julien,
> > 
> > This patch introduced a slew of Clang warnings:
> > 
> > In file included from arch/arm64/kernel/signal.c:21:
> > In file included from include/linux/compat.h:10:
> > In file included from include/linux/time.h:6:
> > In file included from include/linux/seqlock.h:36:
> > In file included from include/linux/spinlock.h:54:
> > In file included from include/linux/irqflags.h:16:
> > arch/arm64/include/asm/irqflags.h:50:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
> >                 : "r" (GIC_PRIO_IRQON)
> >                        ^
> > arch/arm64/include/asm/ptrace.h:39:25: note: expanded from macro 'GIC_PRIO_IRQON'
> > #define GIC_PRIO_IRQON          0xf0
> >                                 ^
> > arch/arm64/include/asm/irqflags.h:46:44: note: use constraint modifier "w"
> >                 "msr_s  " __stringify(SYS_ICC_PMR_EL1) ",%0\n"
> >                                                          ^~
> >                                                          %w0
> 
> I'm not sure I get the relevance of this kind of warnings from Clang.
> Had it been an output operand I could understand the concern of having a
> variable too small to store the register value. But here it's an input
> operand being place in a wider register...
> 
> > arch/arm64/include/asm/alternative.h:286:29: note: expanded from macro 'ALTERNATIVE'
> >         _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
> >                                    ^
> > arch/arm64/include/asm/alternative.h:88:30: note: expanded from macro '_ALTERNATIVE_CFG'
> >         __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
> >                                     ^
> > arch/arm64/include/asm/alternative.h:76:2: note: expanded from macro '__ALTERNATIVE_CFG'
> >         newinstr "\n"                                                   \
> >         ^
> > In file included from arch/arm64/kernel/signal.c:21:
> > In file included from include/linux/compat.h:10:
> > In file included from include/linux/time.h:6:
> > In file included from include/linux/seqlock.h:36:
> > In file included from include/linux/spinlock.h:54:
> > In file included from include/linux/irqflags.h:16:
> > arch/arm64/include/asm/irqflags.h:61:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
> >                 : "r" (GIC_PRIO_IRQOFF)
> >                        ^
> > arch/arm64/include/asm/ptrace.h:40:26: note: expanded from macro 'GIC_PRIO_IRQOFF'
> > #define GIC_PRIO_IRQOFF         (GIC_PRIO_IRQON & ~0x80)
> >                                 ^
> > arch/arm64/include/asm/irqflags.h:58:45: note: use constraint modifier "w"
> >                 "msr_s  " __stringify(SYS_ICC_PMR_EL1) ", %0",
> >                                                           ^
> > arch/arm64/include/asm/irqflags.h:94:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
> >                 : "r" (GIC_PRIO_IRQOFF)
> >                        ^
> > arch/arm64/include/asm/ptrace.h:40:26: note: expanded from macro 'GIC_PRIO_IRQOFF'
> > #define GIC_PRIO_IRQOFF         (GIC_PRIO_IRQON & ~0x80)
> >                                 ^
> > arch/arm64/include/asm/irqflags.h:91:18: note: use constraint modifier "w"
> >                         "csel   %0, %0, %2, eq",
> >                                         ^~
> >                                         %w2
> > arch/arm64/include/asm/alternative.h:286:29: note: expanded from macro 'ALTERNATIVE'
> >         _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
> >                                    ^
> > arch/arm64/include/asm/alternative.h:88:30: note: expanded from macro '_ALTERNATIVE_CFG'
> >         __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
> >                                     ^
> > arch/arm64/include/asm/alternative.h:76:2: note: expanded from macro '__ALTERNATIVE_CFG'
> >         newinstr "\n"                                                   \
> >         ^
> > 3 warnings generated.
> > 
> > 
> > I am not sure if they should be fixed with Clang's suggestion of a
> > constraint modifier or a cast like commit 1b57ec8c7527 ("arm64: io:
> > Ensure value passed to __iormb() is held in a 64-bit register"), hence
> > this message.
> > 
> 
> Clang's suggestion would not work as MSR instructions do not operate on
> 32-bit general purpose registers. Seeing that PMR is a 32-bit register,
> I'd avoid adding UL for the GIC_PRIO_IRQ* constants.
> 
> So I'd recommend just casting the the asm inline operands to unsigned
> long. This should only affect the 3 locations
> arch/arm64/include/asm/irqflags.h.
> 
> Does the following patch work for you?

Hi Julien,

Yes it does, thank you for the quick response and fix!

Nathan

> 
> Thanks,
> 
> -- 
> Julien Thierry
> 
> 
> -->
> 
> From e839dec632bbf440efe8314751138ba46324078c Mon Sep 17 00:00:00 2001
> From: Julien Thierry <julien.thierry@arm.com>
> Date: Fri, 8 Feb 2019 09:21:58 +0000
> Subject: [PATCH] arm64: irqflags: Fix clang build warnings
> 
> Clang complains when passing asm operands that are smaller than the
> registers they are mapped to:
> 
> arch/arm64/include/asm/irqflags.h:50:10: warning: value size does not
> 	match register size specified by the constraint and modifier
> 	[-Wasm-operand-widths]
>                 : "r" (GIC_PRIO_IRQON)
> 
> Fix it by casting the affected input operands to a type of the correct
> size.
> 
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>

Tested-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  arch/arm64/include/asm/irqflags.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
> index d4597b2..43d8366 100644
> --- a/arch/arm64/include/asm/irqflags.h
> +++ b/arch/arm64/include/asm/irqflags.h
> @@ -47,7 +47,7 @@ static inline void arch_local_irq_enable(void)
>  		"dsb	sy",
>  		ARM64_HAS_IRQ_PRIO_MASKING)
>  		:
> -		: "r" (GIC_PRIO_IRQON)
> +		: "r" ((unsigned long) GIC_PRIO_IRQON)
>  		: "memory");
>  }
>  
> @@ -58,7 +58,7 @@ static inline void arch_local_irq_disable(void)
>  		"msr_s  " __stringify(SYS_ICC_PMR_EL1) ", %0",
>  		ARM64_HAS_IRQ_PRIO_MASKING)
>  		:
> -		: "r" (GIC_PRIO_IRQOFF)
> +		: "r" ((unsigned long) GIC_PRIO_IRQOFF)
>  		: "memory");
>  }
>  
> @@ -91,7 +91,7 @@ static inline unsigned long arch_local_save_flags(void)
>  			"csel	%0, %0, %2, eq",
>  			ARM64_HAS_IRQ_PRIO_MASKING)
>  		: "=&r" (flags), "+r" (daif_bits)
> -		: "r" (GIC_PRIO_IRQOFF)
> +		: "r" ((unsigned long) GIC_PRIO_IRQOFF)
>  		: "memory");
>  
>  	return flags;
> -- 
> 1.9.1
> 
>
Catalin Marinas Feb. 8, 2019, 4:16 p.m. UTC | #4 
On Fri, Feb 08, 2019 at 09:36:48AM +0000, Julien Thierry wrote:
> From e839dec632bbf440efe8314751138ba46324078c Mon Sep 17 00:00:00 2001
> From: Julien Thierry <julien.thierry@arm.com>
> Date: Fri, 8 Feb 2019 09:21:58 +0000
> Subject: [PATCH] arm64: irqflags: Fix clang build warnings
> 
> Clang complains when passing asm operands that are smaller than the
> registers they are mapped to:
> 
> arch/arm64/include/asm/irqflags.h:50:10: warning: value size does not
> 	match register size specified by the constraint and modifier
> 	[-Wasm-operand-widths]
>                 : "r" (GIC_PRIO_IRQON)
> 
> Fix it by casting the affected input operands to a type of the correct
> size.
> 
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>

Applied. Thanks.
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 7ed3208..c9e9a69 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -44,6 +44,17 @@ 
 
 #define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
 
+/*
+ * Even when Linux uses IRQ priorities for IRQ disabling, EFI does not.
+ * And EFI shouldn't really play around with priority masking as it is not aware
+ * which priorities the OS has assigned to its interrupts.
+ */
+#define arch_efi_save_flags(state_flags)		\
+	((void)((state_flags) = read_sysreg(daif)))
+
+#define arch_efi_restore_flags(state_flags)	write_sysreg(state_flags, daif)
+
+
 /* arch specific definitions used by the stub code */
 
 /*
diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
index 24692ed..d4597b2 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -18,7 +18,9 @@ 
 
 #ifdef __KERNEL__
 
+#include <asm/alternative.h>
 #include <asm/ptrace.h>
+#include <asm/sysreg.h>
 
 /*
  * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
@@ -36,33 +38,27 @@ 
 /*
  * CPU interrupt mask handling.
  */
-static inline unsigned long arch_local_irq_save(void)
-{
-	unsigned long flags;
-	asm volatile(
-		"mrs	%0, daif		// arch_local_irq_save\n"
-		"msr	daifset, #2"
-		: "=r" (flags)
-		:
-		: "memory");
-	return flags;
-}
-
 static inline void arch_local_irq_enable(void)
 {
-	asm volatile(
-		"msr	daifclr, #2		// arch_local_irq_enable"
-		:
+	asm volatile(ALTERNATIVE(
+		"msr	daifclr, #2		// arch_local_irq_enable\n"
+		"nop",
+		"msr_s  " __stringify(SYS_ICC_PMR_EL1) ",%0\n"
+		"dsb	sy",
+		ARM64_HAS_IRQ_PRIO_MASKING)
 		:
+		: "r" (GIC_PRIO_IRQON)
 		: "memory");
 }
 
 static inline void arch_local_irq_disable(void)
 {
-	asm volatile(
-		"msr	daifset, #2		// arch_local_irq_disable"
-		:
+	asm volatile(ALTERNATIVE(
+		"msr	daifset, #2		// arch_local_irq_disable",
+		"msr_s  " __stringify(SYS_ICC_PMR_EL1) ", %0",
+		ARM64_HAS_IRQ_PRIO_MASKING)
 		:
+		: "r" (GIC_PRIO_IRQOFF)
 		: "memory");
 }
 
@@ -71,12 +67,44 @@  static inline void arch_local_irq_disable(void)
  */
 static inline unsigned long arch_local_save_flags(void)
 {
+	unsigned long daif_bits;
 	unsigned long flags;
-	asm volatile(
-		"mrs	%0, daif		// arch_local_save_flags"
-		: "=r" (flags)
-		:
+
+	daif_bits = read_sysreg(daif);
+
+	/*
+	 * The asm is logically equivalent to:
+	 *
+	 * if (system_uses_irq_prio_masking())
+	 *	flags = (daif_bits & PSR_I_BIT) ?
+	 *			GIC_PRIO_IRQOFF :
+	 *			read_sysreg_s(SYS_ICC_PMR_EL1);
+	 * else
+	 *	flags = daif_bits;
+	 */
+	asm volatile(ALTERNATIVE(
+			"mov	%0, %1\n"
+			"nop\n"
+			"nop",
+			"mrs_s	%0, " __stringify(SYS_ICC_PMR_EL1) "\n"
+			"ands	%1, %1, " __stringify(PSR_I_BIT) "\n"
+			"csel	%0, %0, %2, eq",
+			ARM64_HAS_IRQ_PRIO_MASKING)
+		: "=&r" (flags), "+r" (daif_bits)
+		: "r" (GIC_PRIO_IRQOFF)
 		: "memory");
+
+	return flags;
+}
+
+static inline unsigned long arch_local_irq_save(void)
+{
+	unsigned long flags;
+
+	flags = arch_local_save_flags();
+
+	arch_local_irq_disable();
+
 	return flags;
 }
 
@@ -85,16 +113,32 @@  static inline unsigned long arch_local_save_flags(void)
  */
 static inline void arch_local_irq_restore(unsigned long flags)
 {
-	asm volatile(
-		"msr	daif, %0		// arch_local_irq_restore"
-	:
-	: "r" (flags)
-	: "memory");
+	asm volatile(ALTERNATIVE(
+			"msr	daif, %0\n"
+			"nop",
+			"msr_s	" __stringify(SYS_ICC_PMR_EL1) ", %0\n"
+			"dsb	sy",
+			ARM64_HAS_IRQ_PRIO_MASKING)
+		: "+r" (flags)
+		:
+		: "memory");
 }
 
 static inline int arch_irqs_disabled_flags(unsigned long flags)
 {
-	return flags & PSR_I_BIT;
+	int res;
+
+	asm volatile(ALTERNATIVE(
+			"and	%w0, %w1, #" __stringify(PSR_I_BIT) "\n"
+			"nop",
+			"cmp	%w1, #" __stringify(GIC_PRIO_IRQOFF) "\n"
+			"cset	%w0, ls",
+			ARM64_HAS_IRQ_PRIO_MASKING)
+		: "=&r" (res)
+		: "r" ((int) flags)
+		: "memory");
+
+	return res;
 }
 #endif
 #endif