Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 04:38:09PM +, Ben Dooks wrote: > On 10/02/14 15:21, Dave Martin wrote: [...] > >Does PCI have any way of finding out which parts of the configuration > >space are there before you are forced to go poking around in invalid > >address space? > > > >I'm guessing there may not be, otherwise this convsersation might not > >be happening ... but I don't know too much about PCI. > > IIRC for configuration accesses you have to wait for the PCIe core > to get a response from the other end. The systems I've seen either > poll for completion or hold the transaction until the pcie core has > finished working. So presumably if the other end isn't there, then it's up to the PCIe implementation to decide how to signal that back to the CPU, or are things more constrained than that? I sill don't understand whether the failing probe is triggered directly from the PCI common code in the kernel or whether it comes from the specific bus driver. If the latter, hacks for working round this at least won't touch the common code, which is the preferable outcome. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 04:38:09PM +, Ben Dooks wrote: On 10/02/14 15:21, Dave Martin wrote: [...] Does PCI have any way of finding out which parts of the configuration space are there before you are forced to go poking around in invalid address space? I'm guessing there may not be, otherwise this convsersation might not be happening ... but I don't know too much about PCI. IIRC for configuration accesses you have to wait for the PCIe core to get a response from the other end. The systems I've seen either poll for completion or hold the transaction until the pcie core has finished working. So presumably if the other end isn't there, then it's up to the PCIe implementation to decide how to signal that back to the CPU, or are things more constrained than that? I sill don't understand whether the failing probe is triggered directly from the PCI common code in the kernel or whether it comes from the specific bus driver. If the latter, hacks for working round this at least won't touch the common code, which is the preferable outcome. Cheers ---Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 04:37:00PM +, Russell King - ARM Linux wrote: > On Mon, Feb 10, 2014 at 04:28:22PM +, Dave Martin wrote: > > On Mon, Feb 10, 2014 at 03:19:34PM +, Russell King - ARM Linux wrote: > > > On Mon, Feb 10, 2014 at 02:42:28PM +, Dave Martin wrote: > > > > Should we require CPSR.A to me masked in Booting, for all CPUs that have > > > > it? > > > > > > If it's not masked at boot, then there can't be an imprecise exception > > > pending. > > > > Couldn't there still be a dangling abort condition triggered by the > > bootloader, which which doesn't raise the abort pin until after we > > entered the kernel? > > True, but the decompressor does disable them (see safe_svcmode_maskall), > so any raised abort is likely to hit the boot loader's vectors at that > time. They remain masked into the kernel from that point. > > If you're not using the decompressor then the A bit will be left as-is. > > Given that we've not yet had any failures, I'm inclined to just let the > status-quo be for the kernel entry - if it does cause problems then it's > clear that the right solution is that the A bit must be disabled. OK, that seems a reasonable view. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 10/02/14 15:21, Dave Martin wrote: On Mon, Feb 10, 2014 at 02:54:22PM +, Ben Dooks wrote: On 10/02/14 14:16, Dave Martin wrote: 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... The marvell pcie always had the option of delivering any bus errors as imprecise aborts. However it was /annoying/ and therefore You don't say ;) easier just to turn it off and rely on the hardware returning 0x for any configuration area it couldn't get to. Does PCI have any way of finding out which parts of the configuration space are there before you are forced to go poking around in invalid address space? I'm guessing there may not be, otherwise this convsersation might not be happening ... but I don't know too much about PCI. IIRC for configuration accesses you have to wait for the PCIe core to get a response from the other end. The systems I've seen either poll for completion or hold the transaction until the pcie core has finished working. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/10/2014 04:24 PM, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 03:12:47PM +, Dave Martin wrote: Firstly, blindly adding 4 to PC is obviouly not right, partly because we might be running an unrelated thread by the time the abort fires, and also because the affected instruction might not be 4 bytes in size in a Thumb kernel. Exactly. We ended up on some platforms having special accessors for PCI where we included a number of 'mov r0, r0' instructions after the accessor so we could properly cope with them - but this required knowledge that we were going to only receive an imprecise abort from these accessors and only for a few cycles after the instruction. However, that's not true with modern architectures. The point they're received will _not_ be the load/store which resulted in the abort, and in the case of a write, they could be many hundreds of cycles later, especially if the write has been buffered. What about putting a memory barrier after a load/store ? CPU should wait for the operation to complete right ? So adding four to the PC is definitely a very /bad/ thing to do. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 04:28:22PM +, Dave Martin wrote: > On Mon, Feb 10, 2014 at 03:19:34PM +, Russell King - ARM Linux wrote: > > On Mon, Feb 10, 2014 at 02:42:28PM +, Dave Martin wrote: > > > Should we require CPSR.A to me masked in Booting, for all CPUs that have > > > it? > > > > If it's not masked at boot, then there can't be an imprecise exception > > pending. > > Couldn't there still be a dangling abort condition triggered by the > bootloader, which which doesn't raise the abort pin until after we > entered the kernel? True, but the decompressor does disable them (see safe_svcmode_maskall), so any raised abort is likely to hit the boot loader's vectors at that time. They remain masked into the kernel from that point. If you're not using the decompressor then the A bit will be left as-is. Given that we've not yet had any failures, I'm inclined to just let the status-quo be for the kernel entry - if it does cause problems then it's clear that the right solution is that the A bit must be disabled. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 03:19:34PM +, Russell King - ARM Linux wrote: > On Mon, Feb 10, 2014 at 02:42:28PM +, Dave Martin wrote: > > Should we require CPSR.A to me masked in Booting, for all CPUs that have > > it? > > If it's not masked at boot, then there can't be an imprecise exception > pending. Couldn't there still be a dangling abort condition triggered by the bootloader, which which doesn't raise the abort pin until after we entered the kernel? > That's unlike interrupts, where a device could trigger an interrupt at > any moment (eg, a timer expiring.) List as with interrupts, there's no way to drain or cancel pending aborts that aren't asserted yet, but whose cause conditions are already established. It's possible that Strongly-Ordered memory is sufficient to guarantee that any D-side abort becomes synchronous in some implementations but I don't think the arhitecture guarantees it. It certainly won't be guaranteed for any other memory type. For these reasons, imprecise aborts seem a lot like interrupts: you can mask them or handle them; but controlling when and whether they occur involves platform-specific assumptions, at least in theory. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 03:12:47PM +, Dave Martin wrote: > Firstly, blindly adding 4 to PC is obviouly not right, partly because we > might be running an unrelated thread by the time the abort fires, and > also because the affected instruction might not be 4 bytes in size in a > Thumb kernel. Exactly. We ended up on some platforms having special accessors for PCI where we included a number of 'mov r0, r0' instructions after the accessor so we could properly cope with them - but this required knowledge that we were going to only receive an imprecise abort from these accessors and only for a few cycles after the instruction. However, that's not true with modern architectures. The point they're received will _not_ be the load/store which resulted in the abort, and in the case of a write, they could be many hundreds of cycles later, especially if the write has been buffered. So adding four to the PC is definitely a very /bad/ thing to do. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 02:54:22PM +, Ben Dooks wrote: > On 10/02/14 14:16, Dave Martin wrote: > >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... > > The marvell pcie always had the option of delivering any bus > errors as imprecise aborts. However it was /annoying/ and therefore You don't say ;) > easier just to turn it off and rely on the hardware returning 0x > for any configuration area it couldn't get to. Does PCI have any way of finding out which parts of the configuration space are there before you are forced to go poking around in invalid address space? I'm guessing there may not be, otherwise this convsersation might not be happening ... but I don't know too much about PCI. Maybe some driver-specific probing hook would make sense, so that we only need to use the russian roulette approach on hardware that forces us to. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 02:42:28PM +, Dave Martin wrote: > Should we require CPSR.A to me masked in Booting, for all CPUs that have > it? If it's not masked at boot, then there can't be an imprecise exception pending. That's unlike interrupts, where a device could trigger an interrupt at any moment (eg, a timer expiring.) -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 03:44:50PM +0100, Fabrice Gasnier wrote:
> On 02/10/2014 03:16 PM, Dave Martin wrote:
> >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...
> Hi,
>
> I'm currently re-basing to prepare upstream of such a driver so, no
> code for now on my side,
> but, I saw others that have similar behavior :
> http://www.spinics.net/lists/linux-pci/msg26124.html
> Basically, I think all PCI drivers using hook_fault_code(16+6, ...)
> use similar mechanism.
Hmmm. There seem to be a few problems here.
Firstly, blindly adding 4 to PC is obviouly not right, partly because we
might be running an unrelated thread by the time the abort fires, and
also because the affected instruction might not be 4 bytes in size in a
Thumb kernel.
Secondly, do we ever release the fault code hook? If not, it looks like
we just ignore all imprecise aborts from the moment that driver is
loaded, whatever causes them ... not ideal.
Are the aborts triggered by the PCI common code? Do you know which
precise code triggers the aborts?
I wonder whether it would be possible to arch hooks to do the
problematic probing another way.
Cheers
---Dave
> >
> >>Signed-off-by: Fabrice Gasnier
> >>---
> >> 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.
> Thanks,
> I'll prepare a new patch that way.
> >
> >>+ : "=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
> >>+++
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 10/02/14 14:16, Dave Martin wrote: 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... The marvell pcie always had the option of delivering any bus errors as imprecise aborts. However it was /annoying/ and therefore easier just to turn it off and rely on the hardware returning 0x for any configuration area it couldn't get to. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/10/2014 03:16 PM, Dave Martin wrote:
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...
Hi,
I'm currently re-basing to prepare upstream of such a driver so, no code
for now on my side,
but, I saw others that have similar behavior :
http://www.spinics.net/lists/linux-pci/msg26124.html
Basically, I think all PCI drivers using hook_fault_code(16+6, ...) use
similar mechanism.
Signed-off-by: Fabrice Gasnier
---
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.
Thanks,
I'll prepare a new patch that way.
+ : "=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.
I did a bit of analysis and summarized it in this thread. I've not been
further:
http://archive.arm.linux.org.uk/lurker/message/20140203.164322.edba427a.en.html
BR,
Fabrice
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 01:56:59PM +, Russell King - ARM Linux wrote: > On Mon, Feb 10, 2014 at 11:17:10AM +, Will Deacon wrote: > > On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: > > > On 02/07/2014 06:09 PM, Will Deacon wrote: > > > > On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: > > > >> 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(); > > > > Surely we want to enable this as early as possible? Now, putting this > > > > into > > > > head.S is ugly, as it duplicating it across all the proc*.S files, so > > > > why > > > > not setup_arch? > > > Sorry, I'm not sure to understand your last comment. > > > At least, I need it enabled before probing drivers (PCIe bus) > > > I've added imprecise abort enable code in traps.c, following Russel > > > King's advice, please see: > > > http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html > > > As abort bit is local to a cpu, i've also added it in smp.c, but this > > > may not be the right place ? > > > > > > Please elaborate, > > > > I was just suggesting that we move your local_abt_enable() call to > > setup_arch, since that's called before early_trap_init on the primary CPU. > > Why would we want to enable aborts before we've setup the vectors page > to handle an abort? That's akin to enabling interrupts and hoping there > isn't one pending... Should we require CPSR.A to me masked in Booting, for all CPUs that have it? By definition, we cannot dispatch those exceptions for a while, until some vectors have been set up; so it makes sense in the same way that requiring CPSR.[IF] to be set makes sense. The kernel should do its best to cope anyway, and immediately mask CPSR.A on entry if it's a v6 or later kernel. safe_svcmode_maskall was originally intended to do that, but it got watered down to cope with the Zaurus issues, so it now looks like most code paths don't mask CPSR.A there. I'm happy to take another look at it. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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
> ---
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 01:56:59PM +, Russell King - ARM Linux wrote: > On Mon, Feb 10, 2014 at 11:17:10AM +, Will Deacon wrote: > > On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: > > > On 02/07/2014 06:09 PM, Will Deacon wrote: > > > > On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: > > > >> 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(); > > > > Surely we want to enable this as early as possible? Now, putting this > > > > into > > > > head.S is ugly, as it duplicating it across all the proc*.S files, so > > > > why > > > > not setup_arch? > > > Sorry, I'm not sure to understand your last comment. > > > At least, I need it enabled before probing drivers (PCIe bus) > > > I've added imprecise abort enable code in traps.c, following Russel > > > King's advice, please see: > > > http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html > > > As abort bit is local to a cpu, i've also added it in smp.c, but this > > > may not be the right place ? > > > > > > Please elaborate, > > > > I was just suggesting that we move your local_abt_enable() call to > > setup_arch, since that's called before early_trap_init on the primary CPU. > > Why would we want to enable aborts before we've setup the vectors page > to handle an abort? That's akin to enabling interrupts and hoping there > isn't one pending... I figured we'd want to fall over as quickly as possible if the bootloader had left a pending exception for us, but you're right in that it's not very helpful if we can't print out a diagnostic. Fabrice, please ignore my suggestion and keep the unmasking where it is. Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Fri, Feb 07, 2014 at 05:09:03PM +, Will Deacon wrote:
> On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
> > +#define local_abt_enable() \
> > + ({ \
> > + unsigned long temp; \
> > + __asm__ __volatile__( \
> > + "mrs%0, cpsr@ sta\n"\
> > +" bic %0, %0, %1\n" \
> > +" msr cpsr_c, %0" \
> > + : "=r" (temp) \
> > + : "r" (PSR_A_BIT) \
>
> Can you use "i" instead of a register for this constant?
As the PSR A bit isn't in bits 7-0, cpsr_c isn't what's required here.
It needs something different... that's why my previous patch for you
didn't work.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 11:17:10AM +, Will Deacon wrote: > On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: > > On 02/07/2014 06:09 PM, Will Deacon wrote: > > > On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: > > >> 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(); > > > Surely we want to enable this as early as possible? Now, putting this into > > > head.S is ugly, as it duplicating it across all the proc*.S files, so why > > > not setup_arch? > > Sorry, I'm not sure to understand your last comment. > > At least, I need it enabled before probing drivers (PCIe bus) > > I've added imprecise abort enable code in traps.c, following Russel > > King's advice, please see: > > http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html > > As abort bit is local to a cpu, i've also added it in smp.c, but this > > may not be the right place ? > > > > Please elaborate, > > I was just suggesting that we move your local_abt_enable() call to > setup_arch, since that's called before early_trap_init on the primary CPU. Why would we want to enable aborts before we've setup the vectors page to handle an abort? That's akin to enabling interrupts and hoping there isn't one pending... -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/10/2014 12:17 PM, Will Deacon wrote: On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: On 02/07/2014 06:09 PM, Will Deacon wrote: On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: 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(); Surely we want to enable this as early as possible? Now, putting this into head.S is ugly, as it duplicating it across all the proc*.S files, so why not setup_arch? Sorry, I'm not sure to understand your last comment. At least, I need it enabled before probing drivers (PCIe bus) I've added imprecise abort enable code in traps.c, following Russel King's advice, please see: http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html As abort bit is local to a cpu, i've also added it in smp.c, but this may not be the right place ? Please elaborate, I was just suggesting that we move your local_abt_enable() call to setup_arch, since that's called before early_trap_init on the primary CPU. Thanks for the clarification. Yes, maybe others would like to have it enabled as you suggest. My point is, it's good enough for such use case to do it in early_trap_init. I'd prefer to follow Russel's first advice. Fabrice Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/10/2014 10:00 AM, Ben Dooks wrote:
On 10/02/14 08:50, Fabrice Gasnier wrote:
On 02/07/2014 06:09 PM, Will Deacon wrote:
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
This patch adds imprecise abort enable/disable macros.
It also enables imprecise aborts when starting kernel.
Signed-off-by: Fabrice Gasnier
---
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"\
+"msrcpsr_c, %0"\
+: "=r" (temp)\
+: "r" (PSR_A_BIT)\
Can you use "i" instead of a register for this constant?
Hi,
Sure, I will change it in a future patch.
+: "memory", "cc");\
You don't need the "cc" clobber.
That surprises me: I think "orr" and "bic" instruction might change N
and Z bits, depending on the result.
So shouldn't "cc" be placed here ?
I also see that it is used in local_fiq_enable/disable macros just
above, that are similar:
No, only if they have the S flag set on the instruction (ORRS,BICS)
Thank you for pointing that out!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: > On 02/07/2014 06:09 PM, Will Deacon wrote: > > On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: > >> 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(); > > Surely we want to enable this as early as possible? Now, putting this into > > head.S is ugly, as it duplicating it across all the proc*.S files, so why > > not setup_arch? > Sorry, I'm not sure to understand your last comment. > At least, I need it enabled before probing drivers (PCIe bus) > I've added imprecise abort enable code in traps.c, following Russel > King's advice, please see: > http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html > As abort bit is local to a cpu, i've also added it in smp.c, but this > may not be the right place ? > > Please elaborate, I was just suggesting that we move your local_abt_enable() call to setup_arch, since that's called before early_trap_init on the primary CPU. Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 10/02/14 08:50, Fabrice Gasnier wrote:
On 02/07/2014 06:09 PM, Will Deacon wrote:
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
This patch adds imprecise abort enable/disable macros.
It also enables imprecise aborts when starting kernel.
Signed-off-by: Fabrice Gasnier
---
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"\
+"msrcpsr_c, %0"\
+: "=r" (temp)\
+: "r" (PSR_A_BIT)\
Can you use "i" instead of a register for this constant?
Hi,
Sure, I will change it in a future patch.
+: "memory", "cc");\
You don't need the "cc" clobber.
That surprises me: I think "orr" and "bic" instruction might change N
and Z bits, depending on the result.
So shouldn't "cc" be placed here ?
I also see that it is used in local_fiq_enable/disable macros just
above, that are similar:
No, only if they have the S flag set on the instruction (ORRS,BICS)
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/07/2014 06:09 PM, Will Deacon wrote:
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
This patch adds imprecise abort enable/disable macros.
It also enables imprecise aborts when starting kernel.
Signed-off-by: Fabrice Gasnier
---
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"\
+ : "=r" (temp) \
+ : "r" (PSR_A_BIT) \
Can you use "i" instead of a register for this constant?
Hi,
Sure, I will change it in a future patch.
+ : "memory", "cc"); \
You don't need the "cc" clobber.
That surprises me: I think "orr" and "bic" instruction might change N
and Z bits, depending on the result.
So shouldn't "cc" be placed here ?
I also see that it is used in local_fiq_enable/disable macros just
above, that are similar:
#define local_fiq_enable()\
({\
unsigned long temp;\
__asm__ __volatile__(\
"mrs%0, cpsr@ stf\n"\
"bic%0, %0, #64\n"\
"msrcpsr_c, %0"\
: "=r" (temp)\
:\
: "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();
Surely we want to enable this as early as possible? Now, putting this into
head.S is ugly, as it duplicating it across all the proc*.S files, so why
not setup_arch?
Sorry, I'm not sure to understand your last comment.
At least, I need it enabled before probing drivers (PCIe bus)
I've added imprecise abort enable code in traps.c, following Russel
King's advice, please see:
http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html
As abort bit is local to a cpu, i've also added it in smp.c, but this
may not be the right place ?
Please elaborate,
Thanks,
Fabrice
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/07/2014 06:09 PM, Will Deacon wrote:
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
This patch adds imprecise abort enable/disable macros.
It also enables imprecise aborts when starting kernel.
Signed-off-by: Fabrice Gasnier fabrice.gasn...@st.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\
+ : =r (temp) \
+ : r (PSR_A_BIT) \
Can you use i instead of a register for this constant?
Hi,
Sure, I will change it in a future patch.
+ : memory, cc); \
You don't need the cc clobber.
That surprises me: I think orr and bic instruction might change N
and Z bits, depending on the result.
So shouldn't cc be placed here ?
I also see that it is used in local_fiq_enable/disable macros just
above, that are similar:
#define local_fiq_enable()\
({\
unsigned long temp;\
__asm__ __volatile__(\
mrs%0, cpsr@ stf\n\
bic%0, %0, #64\n\
msrcpsr_c, %0\
: =r (temp)\
:\
: 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();
Surely we want to enable this as early as possible? Now, putting this into
head.S is ugly, as it duplicating it across all the proc*.S files, so why
not setup_arch?
Sorry, I'm not sure to understand your last comment.
At least, I need it enabled before probing drivers (PCIe bus)
I've added imprecise abort enable code in traps.c, following Russel
King's advice, please see:
http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html
As abort bit is local to a cpu, i've also added it in smp.c, but this
may not be the right place ?
Please elaborate,
Thanks,
Fabrice
Will
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 10/02/14 08:50, Fabrice Gasnier wrote:
On 02/07/2014 06:09 PM, Will Deacon wrote:
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
This patch adds imprecise abort enable/disable macros.
It also enables imprecise aborts when starting kernel.
Signed-off-by: Fabrice Gasnier fabrice.gasn...@st.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\
+msrcpsr_c, %0\
+: =r (temp)\
+: r (PSR_A_BIT)\
Can you use i instead of a register for this constant?
Hi,
Sure, I will change it in a future patch.
+: memory, cc);\
You don't need the cc clobber.
That surprises me: I think orr and bic instruction might change N
and Z bits, depending on the result.
So shouldn't cc be placed here ?
I also see that it is used in local_fiq_enable/disable macros just
above, that are similar:
No, only if they have the S flag set on the instruction (ORRS,BICS)
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: On 02/07/2014 06:09 PM, Will Deacon wrote: On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: 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(); Surely we want to enable this as early as possible? Now, putting this into head.S is ugly, as it duplicating it across all the proc*.S files, so why not setup_arch? Sorry, I'm not sure to understand your last comment. At least, I need it enabled before probing drivers (PCIe bus) I've added imprecise abort enable code in traps.c, following Russel King's advice, please see: http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html As abort bit is local to a cpu, i've also added it in smp.c, but this may not be the right place ? Please elaborate, I was just suggesting that we move your local_abt_enable() call to setup_arch, since that's called before early_trap_init on the primary CPU. Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/10/2014 10:00 AM, Ben Dooks wrote:
On 10/02/14 08:50, Fabrice Gasnier wrote:
On 02/07/2014 06:09 PM, Will Deacon wrote:
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
This patch adds imprecise abort enable/disable macros.
It also enables imprecise aborts when starting kernel.
Signed-off-by: Fabrice Gasnier fabrice.gasn...@st.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\
+msrcpsr_c, %0\
+: =r (temp)\
+: r (PSR_A_BIT)\
Can you use i instead of a register for this constant?
Hi,
Sure, I will change it in a future patch.
+: memory, cc);\
You don't need the cc clobber.
That surprises me: I think orr and bic instruction might change N
and Z bits, depending on the result.
So shouldn't cc be placed here ?
I also see that it is used in local_fiq_enable/disable macros just
above, that are similar:
No, only if they have the S flag set on the instruction (ORRS,BICS)
Thank you for pointing that out!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/10/2014 12:17 PM, Will Deacon wrote: On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: On 02/07/2014 06:09 PM, Will Deacon wrote: On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: 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(); Surely we want to enable this as early as possible? Now, putting this into head.S is ugly, as it duplicating it across all the proc*.S files, so why not setup_arch? Sorry, I'm not sure to understand your last comment. At least, I need it enabled before probing drivers (PCIe bus) I've added imprecise abort enable code in traps.c, following Russel King's advice, please see: http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html As abort bit is local to a cpu, i've also added it in smp.c, but this may not be the right place ? Please elaborate, I was just suggesting that we move your local_abt_enable() call to setup_arch, since that's called before early_trap_init on the primary CPU. Thanks for the clarification. Yes, maybe others would like to have it enabled as you suggest. My point is, it's good enough for such use case to do it in early_trap_init. I'd prefer to follow Russel's first advice. Fabrice Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 11:17:10AM +, Will Deacon wrote: On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: On 02/07/2014 06:09 PM, Will Deacon wrote: On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: 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(); Surely we want to enable this as early as possible? Now, putting this into head.S is ugly, as it duplicating it across all the proc*.S files, so why not setup_arch? Sorry, I'm not sure to understand your last comment. At least, I need it enabled before probing drivers (PCIe bus) I've added imprecise abort enable code in traps.c, following Russel King's advice, please see: http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html As abort bit is local to a cpu, i've also added it in smp.c, but this may not be the right place ? Please elaborate, I was just suggesting that we move your local_abt_enable() call to setup_arch, since that's called before early_trap_init on the primary CPU. Why would we want to enable aborts before we've setup the vectors page to handle an abort? That's akin to enabling interrupts and hoping there isn't one pending... -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Fri, Feb 07, 2014 at 05:09:03PM +, Will Deacon wrote:
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
+#define local_abt_enable() \
+ ({ \
+ unsigned long temp; \
+ __asm__ __volatile__( \
+ mrs%0, cpsr@ sta\n\
+ bic %0, %0, %1\n \
+ msr cpsr_c, %0 \
+ : =r (temp) \
+ : r (PSR_A_BIT) \
Can you use i instead of a register for this constant?
As the PSR A bit isn't in bits 7-0, cpsr_c isn't what's required here.
It needs something different... that's why my previous patch for you
didn't work.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 01:56:59PM +, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 11:17:10AM +, Will Deacon wrote: On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: On 02/07/2014 06:09 PM, Will Deacon wrote: On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: 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(); Surely we want to enable this as early as possible? Now, putting this into head.S is ugly, as it duplicating it across all the proc*.S files, so why not setup_arch? Sorry, I'm not sure to understand your last comment. At least, I need it enabled before probing drivers (PCIe bus) I've added imprecise abort enable code in traps.c, following Russel King's advice, please see: http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html As abort bit is local to a cpu, i've also added it in smp.c, but this may not be the right place ? Please elaborate, I was just suggesting that we move your local_abt_enable() call to setup_arch, since that's called before early_trap_init on the primary CPU. Why would we want to enable aborts before we've setup the vectors page to handle an abort? That's akin to enabling interrupts and hoping there isn't one pending... I figured we'd want to fall over as quickly as possible if the bootloader had left a pending exception for us, but you're right in that it's not very helpful if we can't print out a diagnostic. Fabrice, please ignore my suggestion and keep the unmasking where it is. Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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.gasn...@st.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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 01:56:59PM +, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 11:17:10AM +, Will Deacon wrote: On Mon, Feb 10, 2014 at 08:50:16AM +, Fabrice Gasnier wrote: On 02/07/2014 06:09 PM, Will Deacon wrote: On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote: 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(); Surely we want to enable this as early as possible? Now, putting this into head.S is ugly, as it duplicating it across all the proc*.S files, so why not setup_arch? Sorry, I'm not sure to understand your last comment. At least, I need it enabled before probing drivers (PCIe bus) I've added imprecise abort enable code in traps.c, following Russel King's advice, please see: http://archive.arm.linux.org.uk/lurker/message/20140131.170827.d752a1cc.en.html As abort bit is local to a cpu, i've also added it in smp.c, but this may not be the right place ? Please elaborate, I was just suggesting that we move your local_abt_enable() call to setup_arch, since that's called before early_trap_init on the primary CPU. Why would we want to enable aborts before we've setup the vectors page to handle an abort? That's akin to enabling interrupts and hoping there isn't one pending... Should we require CPSR.A to me masked in Booting, for all CPUs that have it? By definition, we cannot dispatch those exceptions for a while, until some vectors have been set up; so it makes sense in the same way that requiring CPSR.[IF] to be set makes sense. The kernel should do its best to cope anyway, and immediately mask CPSR.A on entry if it's a v6 or later kernel. safe_svcmode_maskall was originally intended to do that, but it got watered down to cope with the Zaurus issues, so it now looks like most code paths don't mask CPSR.A there. I'm happy to take another look at it. Cheers ---Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/10/2014 03:16 PM, Dave Martin wrote:
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...
Hi,
I'm currently re-basing to prepare upstream of such a driver so, no code
for now on my side,
but, I saw others that have similar behavior :
http://www.spinics.net/lists/linux-pci/msg26124.html
Basically, I think all PCI drivers using hook_fault_code(16+6, ...) use
similar mechanism.
Signed-off-by: Fabrice Gasnier fabrice.gasn...@st.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.
Thanks,
I'll prepare a new patch that way.
+ : =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.
I did a bit of analysis and summarized it in this thread. I've not been
further:
http://archive.arm.linux.org.uk/lurker/message/20140203.164322.edba427a.en.html
BR,
Fabrice
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 10/02/14 14:16, Dave Martin wrote: 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... The marvell pcie always had the option of delivering any bus errors as imprecise aborts. However it was /annoying/ and therefore easier just to turn it off and rely on the hardware returning 0x for any configuration area it couldn't get to. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 03:44:50PM +0100, Fabrice Gasnier wrote:
On 02/10/2014 03:16 PM, Dave Martin wrote:
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...
Hi,
I'm currently re-basing to prepare upstream of such a driver so, no
code for now on my side,
but, I saw others that have similar behavior :
http://www.spinics.net/lists/linux-pci/msg26124.html
Basically, I think all PCI drivers using hook_fault_code(16+6, ...)
use similar mechanism.
Hmmm. There seem to be a few problems here.
Firstly, blindly adding 4 to PC is obviouly not right, partly because we
might be running an unrelated thread by the time the abort fires, and
also because the affected instruction might not be 4 bytes in size in a
Thumb kernel.
Secondly, do we ever release the fault code hook? If not, it looks like
we just ignore all imprecise aborts from the moment that driver is
loaded, whatever causes them ... not ideal.
Are the aborts triggered by the PCI common code? Do you know which
precise code triggers the aborts?
I wonder whether it would be possible to arch hooks to do the
problematic probing another way.
Cheers
---Dave
Signed-off-by: Fabrice Gasnier fabrice.gasn...@st.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.
Thanks,
I'll prepare a new patch that way.
+ : =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
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 02:42:28PM +, Dave Martin wrote: Should we require CPSR.A to me masked in Booting, for all CPUs that have it? If it's not masked at boot, then there can't be an imprecise exception pending. That's unlike interrupts, where a device could trigger an interrupt at any moment (eg, a timer expiring.) -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 02:54:22PM +, Ben Dooks wrote: On 10/02/14 14:16, Dave Martin wrote: 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... The marvell pcie always had the option of delivering any bus errors as imprecise aborts. However it was /annoying/ and therefore You don't say ;) easier just to turn it off and rely on the hardware returning 0x for any configuration area it couldn't get to. Does PCI have any way of finding out which parts of the configuration space are there before you are forced to go poking around in invalid address space? I'm guessing there may not be, otherwise this convsersation might not be happening ... but I don't know too much about PCI. Maybe some driver-specific probing hook would make sense, so that we only need to use the russian roulette approach on hardware that forces us to. Cheers ---Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 03:12:47PM +, Dave Martin wrote: Firstly, blindly adding 4 to PC is obviouly not right, partly because we might be running an unrelated thread by the time the abort fires, and also because the affected instruction might not be 4 bytes in size in a Thumb kernel. Exactly. We ended up on some platforms having special accessors for PCI where we included a number of 'mov r0, r0' instructions after the accessor so we could properly cope with them - but this required knowledge that we were going to only receive an imprecise abort from these accessors and only for a few cycles after the instruction. However, that's not true with modern architectures. The point they're received will _not_ be the load/store which resulted in the abort, and in the case of a write, they could be many hundreds of cycles later, especially if the write has been buffered. So adding four to the PC is definitely a very /bad/ thing to do. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 03:19:34PM +, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 02:42:28PM +, Dave Martin wrote: Should we require CPSR.A to me masked in Booting, for all CPUs that have it? If it's not masked at boot, then there can't be an imprecise exception pending. Couldn't there still be a dangling abort condition triggered by the bootloader, which which doesn't raise the abort pin until after we entered the kernel? That's unlike interrupts, where a device could trigger an interrupt at any moment (eg, a timer expiring.) List as with interrupts, there's no way to drain or cancel pending aborts that aren't asserted yet, but whose cause conditions are already established. It's possible that Strongly-Ordered memory is sufficient to guarantee that any D-side abort becomes synchronous in some implementations but I don't think the arhitecture guarantees it. It certainly won't be guaranteed for any other memory type. For these reasons, imprecise aborts seem a lot like interrupts: you can mask them or handle them; but controlling when and whether they occur involves platform-specific assumptions, at least in theory. Cheers ---Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 02/10/2014 04:24 PM, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 03:12:47PM +, Dave Martin wrote: Firstly, blindly adding 4 to PC is obviouly not right, partly because we might be running an unrelated thread by the time the abort fires, and also because the affected instruction might not be 4 bytes in size in a Thumb kernel. Exactly. We ended up on some platforms having special accessors for PCI where we included a number of 'mov r0, r0' instructions after the accessor so we could properly cope with them - but this required knowledge that we were going to only receive an imprecise abort from these accessors and only for a few cycles after the instruction. However, that's not true with modern architectures. The point they're received will _not_ be the load/store which resulted in the abort, and in the case of a write, they could be many hundreds of cycles later, especially if the write has been buffered. What about putting a memory barrier after a load/store ? CPU should wait for the operation to complete right ? So adding four to the PC is definitely a very /bad/ thing to do. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 04:28:22PM +, Dave Martin wrote: On Mon, Feb 10, 2014 at 03:19:34PM +, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 02:42:28PM +, Dave Martin wrote: Should we require CPSR.A to me masked in Booting, for all CPUs that have it? If it's not masked at boot, then there can't be an imprecise exception pending. Couldn't there still be a dangling abort condition triggered by the bootloader, which which doesn't raise the abort pin until after we entered the kernel? True, but the decompressor does disable them (see safe_svcmode_maskall), so any raised abort is likely to hit the boot loader's vectors at that time. They remain masked into the kernel from that point. If you're not using the decompressor then the A bit will be left as-is. Given that we've not yet had any failures, I'm inclined to just let the status-quo be for the kernel entry - if it does cause problems then it's clear that the right solution is that the A bit must be disabled. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On 10/02/14 15:21, Dave Martin wrote: On Mon, Feb 10, 2014 at 02:54:22PM +, Ben Dooks wrote: On 10/02/14 14:16, Dave Martin wrote: 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... The marvell pcie always had the option of delivering any bus errors as imprecise aborts. However it was /annoying/ and therefore You don't say ;) easier just to turn it off and rely on the hardware returning 0x for any configuration area it couldn't get to. Does PCI have any way of finding out which parts of the configuration space are there before you are forced to go poking around in invalid address space? I'm guessing there may not be, otherwise this convsersation might not be happening ... but I don't know too much about PCI. IIRC for configuration accesses you have to wait for the PCIe core to get a response from the other end. The systems I've seen either poll for completion or hold the transaction until the pcie core has finished working. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Mon, Feb 10, 2014 at 04:37:00PM +, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 04:28:22PM +, Dave Martin wrote: On Mon, Feb 10, 2014 at 03:19:34PM +, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 02:42:28PM +, Dave Martin wrote: Should we require CPSR.A to me masked in Booting, for all CPUs that have it? If it's not masked at boot, then there can't be an imprecise exception pending. Couldn't there still be a dangling abort condition triggered by the bootloader, which which doesn't raise the abort pin until after we entered the kernel? True, but the decompressor does disable them (see safe_svcmode_maskall), so any raised abort is likely to hit the boot loader's vectors at that time. They remain masked into the kernel from that point. If you're not using the decompressor then the A bit will be left as-is. Given that we've not yet had any failures, I'm inclined to just let the status-quo be for the kernel entry - if it does cause problems then it's clear that the right solution is that the A bit must be disabled. OK, that seems a reasonable view. Cheers ---Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
> This patch adds imprecise abort enable/disable macros.
> It also enables imprecise aborts when starting kernel.
>
> Signed-off-by: Fabrice Gasnier
> ---
> 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" \
> + : "=r" (temp) \
> + : "r" (PSR_A_BIT) \
Can you use "i" instead of a register for this constant?
> + : "memory", "cc"); \
You don't need the "cc" clobber.
> + })
> +
> +/*
> + * 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"); \
> + })
Same comments here.
> #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();
Surely we want to enable this as early as possible? Now, putting this into
head.S is ugly, as it duplicating it across all the proc*.S files, so why
not setup_arch?
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] ARM: Add imprecise abort enable/disable macro
On Fri, Feb 07, 2014 at 04:19:15PM +, Fabrice GASNIER wrote:
This patch adds imprecise abort enable/disable macros.
It also enables imprecise aborts when starting kernel.
Signed-off-by: Fabrice Gasnier fabrice.gasn...@st.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 \
+ : =r (temp) \
+ : r (PSR_A_BIT) \
Can you use i instead of a register for this constant?
+ : memory, cc); \
You don't need the cc clobber.
+ })
+
+/*
+ * 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); \
+ })
Same comments here.
#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();
Surely we want to enable this as early as possible? Now, putting this into
head.S is ugly, as it duplicating it across all the proc*.S files, so why
not setup_arch?
Will
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
