linux-kernel - Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140210141634.GA2794@e103592.cambridge.arm.com>
Date:	Mon, 10 Feb 2014 14:16:40 +0000
From:	Dave Martin <Dave.Martin@....com>
To:	Fabrice GASNIER <fabrice.gasnier@...com>
Cc:	linux@....linux.org.uk, u.kleine-koenig@...gutronix.de,
	jonathan.austin@....com, catalin.marinas@....com,
	will.deacon@....com, nico@...aro.org, sboyd@...eaurora.org,
	marc.zyngier@....com, ben.dooks@...ethink.co.uk,
	vgupta@...opsys.com, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, maxime.coquelin@...com
Subject: Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro

On Fri, Feb 07, 2014 at 05:19:15PM +0100, Fabrice GASNIER wrote:
> This patch adds imprecise abort enable/disable macros.
> It also enables imprecise aborts when starting kernel.

Relying on imprecise aborts for hardware probing would be considered bad
hardware and/or software design for ARM-specific stuff.

PCI is more generic though, so we may have to put up with this to some
extent.  Can you point me to the affected probing code?  I'm not very
familiar with that stuff...

> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@...com>
> ---
>  arch/arm/include/asm/irqflags.h |   33 +++++++++++++++++++++++++++++++++
>  arch/arm/kernel/smp.c           |    1 +
>  arch/arm/kernel/traps.c         |    4 ++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/arch/arm/include/asm/irqflags.h b/arch/arm/include/asm/irqflags.h
> index 3b763d6..82e3834 100644
> --- a/arch/arm/include/asm/irqflags.h
> +++ b/arch/arm/include/asm/irqflags.h
> @@ -51,6 +51,9 @@ static inline void arch_local_irq_disable(void)
>  
>  #define local_fiq_enable()  __asm__("cpsie f	@ __stf" : : : "memory", "cc")
>  #define local_fiq_disable() __asm__("cpsid f	@ __clf" : : : "memory", "cc")
> +
> +#define local_abt_enable()  __asm__("cpsie a	@ __sta" : : : "memory", "cc")
> +#define local_abt_disable() __asm__("cpsid a	@ __cla" : : : "memory", "cc")
>  #else
>  
>  /*
> @@ -130,6 +133,36 @@ static inline void arch_local_irq_disable(void)
>  	: "memory", "cc");					\
>  	})
>  
> +/*
> + * Enable Aborts
> + */
> +#define local_abt_enable()					\
> +	({							\
> +		unsigned long temp;				\
> +	__asm__ __volatile__(					\
> +	"mrs	%0, cpsr		@ sta\n"		\
> +"	bic	%0, %0, %1\n"					\
> +"	msr	cpsr_c, %0"					\

I suggest you use "cpsie/cpsid a" instead.  This requires ARMv6, but the
CPSR.A bit only exists on ARMv6 and later anyway.  Poking that bit
on earlier CPUs may cause unpredictable behaviour, so these macros
should be no-ops for v5 and earlier.

> +	: "=r" (temp)						\
> +	: "r" (PSR_A_BIT)					\
> +	: "memory", "cc");					\
> +	})
> +
> +/*
> + * Disable Aborts
> + */
> +#define local_abt_disable()					\
> +	({							\
> +		unsigned long temp;				\
> +	__asm__ __volatile__(					\
> +	"mrs	%0, cpsr		@ cla\n"		\
> +"	orr	%0, %0, %1\n"					\
> +"	msr	cpsr_c, %0"					\
> +	: "=r" (temp)						\
> +	: "r" (PSR_A_BIT)					\
> +	: "memory", "cc");					\
> +	})
> +
>  #endif
>  
>  /*
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index dc894ab..c2093cb 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -377,6 +377,7 @@ asmlinkage void secondary_start_kernel(void)
>  
>  	local_irq_enable();
>  	local_fiq_enable();
> +	local_abt_enable();
>  
>  	/*
>  	 * OK, it's off to the idle thread for us
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 4636d56..ef15709 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -900,6 +900,10 @@ void __init early_trap_init(void *vectors_base)
>  
>  	flush_icache_range(vectors, vectors + PAGE_SIZE * 2);
>  	modify_domain(DOMAIN_USER, DOMAIN_CLIENT);
> +
> +	/* Enable imprecise aborts */
> +	local_abt_enable();

It would be good to clean up why aborts are not being consistently
enabled on boot.

Really, they should be enabled, except for a brief window during
boot when the vectors are not mapped and the abort can't be dispatched.

Cheers
---Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/