From: Nick Hawkins <[email protected]>
Enable the workaround for the 764319 Cortex A-9 erratum.
CP14 read accesses to the DBGPRSR and DBGOSLSR registers generate an
unexpected Undefined Instruction exception when the DBGSWENABLE external
pin is set to 0, even when the CP14 accesses are performed from a
privileged mode. The work around catches the exception in a way
the kernel does not stop execution with the use of undef_hook. This
has been found to effect the HPE GXP SoC.
Signed-off-by: Nick Hawkins <[email protected]>
---
arch/arm/Kconfig | 11 +++++++++++
arch/arm/kernel/hw_breakpoint.c | 26 ++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 13f77eec7c40..6944adfb0fae 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -974,6 +974,17 @@ config ARM_ERRATA_764369
relevant cache maintenance functions and sets a specific bit
in the diagnostic control register of the SCU.
+config ARM_ERRATA_764319
+ bool "ARM errata: Read to DBGPRSR and DBGOSLSR may generate Undefined instruction"
+ depends on CPU_V7
+ help
+ This option enables the workaround for the 764319 Cortex A-9 erratum.
+ CP14 read accesses to the DBGPRSR and DBGOSLSR registers generate an
+ unexpected Undefined Instruction exception when the DBGSWENABLE
+ external pin is set to 0, even when the CP14 accesses are performed
+ from a privileged mode. This work around catches the exception in a
+ way the kernel does not stop execution.
+
config ARM_ERRATA_775420
bool "ARM errata: A data cache maintenance operation which aborts, might lead to deadlock"
depends on CPU_V7
diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index b1423fb130ea..c41a8436a796 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -941,6 +941,23 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
return ret;
}
+#ifdef CONFIG_ARM_ERRATA_764319
+int oslsr_fault;
+
+static int debug_oslsr_trap(struct pt_regs *regs, unsigned int instr)
+{
+ oslsr_fault = 1;
+ instruction_pointer(regs) += 4;
+ return 0;
+}
+
+static struct undef_hook debug_oslsr_hook = {
+ .instr_mask = 0xffffffff,
+ .instr_val = 0xee115e91,
+ .fn = debug_oslsr_trap,
+};
+#endif
+
/*
* One-time initialisation.
*/
@@ -974,7 +991,16 @@ static bool core_has_os_save_restore(void)
case ARM_DEBUG_ARCH_V7_1:
return true;
case ARM_DEBUG_ARCH_V7_ECP14:
+#ifdef CONFIG_ARM_ERRATA_764319
+ oslsr_fault = 0;
+ register_undef_hook(&debug_oslsr_hook);
ARM_DBG_READ(c1, c1, 4, oslsr);
+ unregister_undef_hook(&debug_oslsr_hook);
+ if (oslsr_fault)
+ return false;
+#else
+ ARM_DBG_READ(c1, c1, 4, oslsr);
+#endif
if (oslsr & ARM_OSLSR_OSLM0)
return true;
fallthrough;
--
2.17.1
On Fri, May 6, 2022 at 9:29 PM <[email protected]> wrote:
>
> From: Nick Hawkins <[email protected]>
>
> Enable the workaround for the 764319 Cortex A-9 erratum.
> CP14 read accesses to the DBGPRSR and DBGOSLSR registers generate an
> unexpected Undefined Instruction exception when the DBGSWENABLE external
> pin is set to 0, even when the CP14 accesses are performed from a
> privileged mode. The work around catches the exception in a way
> the kernel does not stop execution with the use of undef_hook. This
> has been found to effect the HPE GXP SoC.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> arch/arm/Kconfig | 11 +++++++++++
> arch/arm/kernel/hw_breakpoint.c | 26 ++++++++++++++++++++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 13f77eec7c40..6944adfb0fae 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -974,6 +974,17 @@ config ARM_ERRATA_764369
> relevant cache maintenance functions and sets a specific bit
> in the diagnostic control register of the SCU.
>
> +config ARM_ERRATA_764319
> + bool "ARM errata: Read to DBGPRSR and DBGOSLSR may generate Undefined instruction"
> + depends on CPU_V7
> + help
> + This option enables the workaround for the 764319 Cortex A-9 erratum.
> + CP14 read accesses to the DBGPRSR and DBGOSLSR registers generate an
> + unexpected Undefined Instruction exception when the DBGSWENABLE
> + external pin is set to 0, even when the CP14 accesses are performed
> + from a privileged mode. This work around catches the exception in a
> + way the kernel does not stop execution.
> +
> config ARM_ERRATA_775420
> bool "ARM errata: A data cache maintenance operation which aborts, might lead to deadlock"
> depends on CPU_V7
> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
> index b1423fb130ea..c41a8436a796 100644
> --- a/arch/arm/kernel/hw_breakpoint.c
> +++ b/arch/arm/kernel/hw_breakpoint.c
> @@ -941,6 +941,23 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
> return ret;
> }
>
> +#ifdef CONFIG_ARM_ERRATA_764319
> +int oslsr_fault;
> +
> +static int debug_oslsr_trap(struct pt_regs *regs, unsigned int instr)
> +{
> + oslsr_fault = 1;
> + instruction_pointer(regs) += 4;
> + return 0;
> +}
> +
> +static struct undef_hook debug_oslsr_hook = {
> + .instr_mask = 0xffffffff,
> + .instr_val = 0xee115e91,
> + .fn = debug_oslsr_trap,
> +};
> +#endif
> +
Hi Nick,
This seems a bit more complex than necessary. Can't you just use a custom
inline asm with an ex_table entry to catch the fault? Have a look at
__get_user_asm() for an example.
Arnd
Hi Arnd,
> Hi Nick,
> This seems a bit more complex than necessary. Can't you just use a custom
> inline asm with an ex_table entry to catch the fault? Have a look at
> __get_user_asm() for an example.
>
> Arnd
We got inspired from debug_reg_hook within the same source file ( ./arch/arm/kernel/hw_breakpoint.c ). We chose that path to keep coherency within the source code. We can implement the same fix by using an ex_table entry, but this will create two different ways at catching unknown instruction within the same source file. Will that be ok ?
vejmarie
On 2022-05-10 08:19, Arnd Bergmann wrote:
> On Fri, May 6, 2022 at 9:29 PM <[email protected]> wrote:
>>
>> From: Nick Hawkins <[email protected]>
>>
>> Enable the workaround for the 764319 Cortex A-9 erratum.
>> CP14 read accesses to the DBGPRSR and DBGOSLSR registers generate an
>> unexpected Undefined Instruction exception when the DBGSWENABLE external
>> pin is set to 0, even when the CP14 accesses are performed from a
>> privileged mode. The work around catches the exception in a way
>> the kernel does not stop execution with the use of undef_hook. This
>> has been found to effect the HPE GXP SoC.
>>
>> Signed-off-by: Nick Hawkins <[email protected]>
>> ---
>> arch/arm/Kconfig | 11 +++++++++++
>> arch/arm/kernel/hw_breakpoint.c | 26 ++++++++++++++++++++++++++
>> 2 files changed, 37 insertions(+)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 13f77eec7c40..6944adfb0fae 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -974,6 +974,17 @@ config ARM_ERRATA_764369
>> relevant cache maintenance functions and sets a specific bit
>> in the diagnostic control register of the SCU.
>>
>> +config ARM_ERRATA_764319
>> + bool "ARM errata: Read to DBGPRSR and DBGOSLSR may generate Undefined instruction"
>> + depends on CPU_V7
>> + help
>> + This option enables the workaround for the 764319 Cortex A-9 erratum.
>> + CP14 read accesses to the DBGPRSR and DBGOSLSR registers generate an
>> + unexpected Undefined Instruction exception when the DBGSWENABLE
>> + external pin is set to 0, even when the CP14 accesses are performed
>> + from a privileged mode. This work around catches the exception in a
>> + way the kernel does not stop execution.
>> +
>> config ARM_ERRATA_775420
>> bool "ARM errata: A data cache maintenance operation which aborts, might lead to deadlock"
>> depends on CPU_V7
>> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
>> index b1423fb130ea..c41a8436a796 100644
>> --- a/arch/arm/kernel/hw_breakpoint.c
>> +++ b/arch/arm/kernel/hw_breakpoint.c
>> @@ -941,6 +941,23 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
>> return ret;
>> }
>>
>> +#ifdef CONFIG_ARM_ERRATA_764319
>> +int oslsr_fault;
>> +
>> +static int debug_oslsr_trap(struct pt_regs *regs, unsigned int instr)
>> +{
>> + oslsr_fault = 1;
>> + instruction_pointer(regs) += 4;
>> + return 0;
>> +}
>> +
>> +static struct undef_hook debug_oslsr_hook = {
>> + .instr_mask = 0xffffffff,
>> + .instr_val = 0xee115e91,
>> + .fn = debug_oslsr_trap,
>> +};
>> +#endif
>> +
>
> Hi Nick,
>
> This seems a bit more complex than necessary. Can't you just use a custom
> inline asm with an ex_table entry to catch the fault? Have a look at
> __get_user_asm() for an example.
Indeed, according to the Cortex-A9 documentation the register should
always read as zero anyway, so all we should need to do is initialise
the local oslsr variable to 0 and have the fault handler just return to
the next instruction.
Robin.
On Tue, May 10, 2022 at 1:53 PM Verdun, Jean-Marie <[email protected]> wrote:
> >
> > Hi Arnd,
> >
> > > Hi Nick,
> >
> > > This seems a bit more complex than necessary. Can't you just use a custom
> > inline asm with an ex_table entry to catch the fault? Have a look at
> > __get_user_asm() for an example.
> >
> > Arnd
> >
> > We got inspired from debug_reg_hook within the same source file (
> >./arch/arm/kernel/hw_breakpoint.c ). We chose that path to keep
> >coherency within the source code. We can implement the same fix by
> >using an ex_table entry, but this will create two different ways at
> >catching unknown instruction within the same source file. Will that be ok ?
> I got a little lost trying to find where the breakpoint instruction comes from that gets trapped here, but I would guess that they had to do this using an undef_hook because the ex_table approach does not work there for some reason.
> I would still pick the ex_table method here if that works.
I will pursue the method you have recommended Arnd.
Thank you for the feedback as always.
-Nick Hawkins
On Tue, May 10, 2022 at 1:53 PM Verdun, Jean-Marie <[email protected]> wrote:
>
> Hi Arnd,
>
> > Hi Nick,
>
> > This seems a bit more complex than necessary. Can't you just use a custom
> > inline asm with an ex_table entry to catch the fault? Have a look at
> > __get_user_asm() for an example.
> >
> > Arnd
>
> We got inspired from debug_reg_hook within the same source file
>( ./arch/arm/kernel/hw_breakpoint.c ). We chose that path to keep coherency
> within the source code. We can implement the same fix by using an ex_table
> entry, but this will create two different ways at catching unknown instruction
> within the same source file. Will that be ok ?
I got a little lost trying to find where the breakpoint instruction comes
from that gets trapped here, but I would guess that they had to do this
using an undef_hook because the ex_table approach does not work
there for some reason.
I would still pick the ex_table method here if that works.
Arnd
On Tue, May 10, 2022 at 03:01:26PM +0200, Arnd Bergmann wrote:
> On Tue, May 10, 2022 at 1:53 PM Verdun, Jean-Marie <[email protected]> wrote:
> >
> > Hi Arnd,
> >
> > > Hi Nick,
> >
> > > This seems a bit more complex than necessary. Can't you just use a custom
> > > inline asm with an ex_table entry to catch the fault? Have a look at
> > > __get_user_asm() for an example.
> > >
> > > Arnd
> >
> > We got inspired from debug_reg_hook within the same source file
> >( ./arch/arm/kernel/hw_breakpoint.c ). We chose that path to keep coherency
> > within the source code. We can implement the same fix by using an ex_table
> > entry, but this will create two different ways at catching unknown instruction
> > within the same source file. Will that be ok ?
>
> I got a little lost trying to find where the breakpoint instruction comes
> from that gets trapped here, but I would guess that they had to do this
> using an undef_hook because the ex_table approach does not work
> there for some reason.
>
> I would still pick the ex_table method here if that works.
IIRC, the ex_table handlers are called only for data aborts and are intended
to be used to handle cases where we take a fault on a memory access (e.g.
translation fault). In this case, we're taking an undefined instruction
exception on a cp14 access and so the undef_hook is the right thing to use.
Will
Hi Arnd,
> If it doesn't work, then there is no point trying. You could try
> changing the exception
> handling so it searches the ex_table for Undefined Instruction
> exceptions as well,
> but that's probably more complicated.
I looked yesterday evening and share Will's point of view, I don't
really see how it could work and believe unfortunately that
we have to stay with the undef_hook approach. Let's us know if
you believe some improvements are required to the current patch.
Let's see the positive aspect we learnt something, and if we have
to face additional issue, we have that option open.
Side question: Does any of you will attend Open Source Summit North America ?
I will be there with Nick and HPE team to introduce our GXP Asic during a couple
of talks. Would be great if we could spend some time together.
vejmarie
> Arnd
On Wed, May 11, 2022 at 2:23 PM Hawkins, Nick <[email protected]> wrote:
> On Tue, May 10, 2022 at 03:01:26PM +0200, Arnd Bergmann wrote:
> > > On Tue, May 10, 2022 at 1:53 PM Verdun, Jean-Marie <[email protected]> wrote:
> > >
> > > I got a little lost trying to find where the breakpoint instruction
> > > comes from that gets trapped here, but I would guess that they had to
> > > do this using an undef_hook because the ex_table approach does not
> > > work there for some reason.
> > >
> > > I would still pick the ex_table method here if that works.
>
> > IIRC, the ex_table handlers are called only for data aborts and are intended to be used to handle cases where we take a fault on a memory access (e.g.
> translation fault). In this case, we're taking an undefined instruction exception on a cp14 access and so the undef_hook is the right thing to use.
>
> Given Will's input would you like me to still use the ex_table method?
If it doesn't work, then there is no point trying. You could try
changing the exception
handling so it searches the ex_table for Undefined Instruction
exceptions as well,
but that's probably more complicated.
Arnd
On Wed, May 11, 2022 at 02:45:27PM +0200, Arnd Bergmann wrote:
> If it doesn't work, then there is no point trying. You could try
> changing the exception handling so it searches the ex_table for
> Undefined Instruction exceptions as well, but that's probably more
> complicated.
What's the point when we have the undef hook?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Wed, May 11, 2022 at 4:28 PM Russell King (Oracle)
<[email protected]> wrote:
>
> On Wed, May 11, 2022 at 02:45:27PM +0200, Arnd Bergmann wrote:
> > If it doesn't work, then there is no point trying. You could try
> > changing the exception handling so it searches the ex_table for
> > Undefined Instruction exceptions as well, but that's probably more
> > complicated.
>
> What's the point when we have the undef hook?
It seemed a little easier to follow the code flow if things are in
one place.
Arnd
On Wed, May 11, 2022 at 2:59 PM Verdun, Jean-Marie <[email protected]> wrote:
>
> Hi Arnd,
>
> > If it doesn't work, then there is no point trying. You could try
> > changing the exception
> > handling so it searches the ex_table for Undefined Instruction
> > exceptions as well,
> > but that's probably more complicated.
>
> I looked yesterday evening and share Will's point of view, I don't
> really see how it could work and believe unfortunately that
> we have to stay with the undef_hook approach. Let's us know if
> you believe some improvements are required to the current patch.
>
> Let's see the positive aspect we learnt something, and if we have
> to face additional issue, we have that option open.
The current patch looks reaonable to me then, please add
Reviewed-by: Arnd Bergmann <[email protected]>
and submit it to Russell's patch tracker at
https://www.arm.linux.org.uk/developer/patches/info.php
with one small change applied: the 'oslsr_fault' variable should
be marked "static".
> Side question: Does any of you will attend Open Source Summit North America ?
> I will be there with Nick and HPE team to introduce our GXP Asic during a couple
> of talks. Would be great if we could spend some time together.
I have no plans to attend at the moment.
Arnd
On Tue, May 10, 2022 at 03:01:26PM +0200, Arnd Bergmann wrote:
> > On Tue, May 10, 2022 at 1:53 PM Verdun, Jean-Marie <[email protected]> wrote:
> > >
> > > Hi Arnd,
> > >
> > > > Hi Nick,
> > >
> > > > This seems a bit more complex than necessary. Can't you just use a custom
> > > > inline asm with an ex_table entry to catch the fault? Have a look at
> > > > __get_user_asm() for an example.
> > > >
> > > > Arnd
> > >
> > > We got inspired from debug_reg_hook within the same source file (
> > >./arch/arm/kernel/hw_breakpoint.c ). We chose that path to keep
> > >coherency within the source code. We can implement the same fix by
> > >using an ex_table entry, but this will create two different ways at
> > >catching unknown instruction within the same source file. Will that be ok ?
> >
> > I got a little lost trying to find where the breakpoint instruction
> > comes from that gets trapped here, but I would guess that they had to
> > do this using an undef_hook because the ex_table approach does not
> > work there for some reason.
> >
> > I would still pick the ex_table method here if that works.
> IIRC, the ex_table handlers are called only for data aborts and are intended to be used to handle cases where we take a fault on a memory access (e.g.
translation fault). In this case, we're taking an undefined instruction exception on a cp14 access and so the undef_hook is the right thing to use.
Hello Arnd,
Given Will's input would you like me to still use the ex_table method?
Thanks,
-Nick Hawkins