[PATCH 0/6] ARM: spectre-bhb fixes and tweaks

[PATCH 0/6] ARM: spectre-bhb fixes and tweaks

[Ard Biesheuvel]

A couple of bug fixes and tweaks for the ARM version of the Spectre BHB
mitigations. Patches #1 and #2 are bug fixes, the rest are minor
improvements for efficiency and maintainability.

Ard Biesheuvel (6):
  ARM: spectre-bhb: enable for Cortex-A15
  ARM: spectre-bhb: fix loop8 sequence for Thumb2
  ARM: spectre-bhb: simplify BPIALL vector macro
  ARM: spectre-bhb: use local DSB and elide ISB in loop8 sequence
  ARM: spectre-bhb: avoid cross-subsection jump using a numbered label
  ARM: spectre-bhb: rely on linker to emit cross-section literal loads

 arch/arm/kernel/entry-armv.S   | 53 ++++++++++----------
 arch/arm/kernel/entry-common.S |  2 +-
 arch/arm/mm/proc-v7-bugs.c     |  1 +
 3 files changed, 29 insertions(+), 27 deletions(-)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Ard Biesheuvel]

The Spectre-BHB mitigations were inadvertently left disabled for
Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
that case. So fix that.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/mm/proc-v7-bugs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
index 06dbfb968182..fb9f3eb6bf48 100644
--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
 {
 	if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
 		cpu_v7_spectre_v2_init();
+	cpu_v7_spectre_bhb_init();
 }
 
 void cpu_v7_bugs_init(void)
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Jon Hunter]

Hi Ard,

On 28/03/2022 14:47, Ard Biesheuvel wrote:
> The Spectre-BHB mitigations were inadvertently left disabled for
> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
> that case. So fix that.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>   arch/arm/mm/proc-v7-bugs.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> index 06dbfb968182..fb9f3eb6bf48 100644
> --- a/arch/arm/mm/proc-v7-bugs.c
> +++ b/arch/arm/mm/proc-v7-bugs.c
> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
>   {
>   	if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
>   		cpu_v7_spectre_v2_init();
> +	cpu_v7_spectre_bhb_init();
>   }
>   
>   void cpu_v7_bugs_init(void)


Since this patch has been merged, I am seeing a ton of messages when 
booting Linux on tegra124-jetson-tk1 ...

[ 1233.327547] CPU0: Spectre BHB: using loop workaround
[ 1233.327795] CPU1: Spectre BHB: using loop workaround
[ 1233.328270] CPU1: Spectre BHB: using loop workaround
[ 1233.328700] CPU1: Spectre BHB: using loop workaround
[ 1233.355477] CPU2: Spectre BHB: using loop workaround
** 7 printk messages dropped **
[ 1233.366271] CPU0: Spectre BHB: using loop workaround
[ 1233.366580] CPU0: Spectre BHB: using loop workaround
[ 1233.366815] CPU1: Spectre BHB: using loop workaround
[ 1233.405475] CPU1: Spectre BHB: using loop workaround
[ 1233.405874] CPU0: Spectre BHB: using loop workaround
[ 1233.406041] CPU1: Spectre BHB: using loop workaround
** 1 printk messages dropped **

This is significantly increasing the boot time and causing failures if 
the boot takes too long.

I see that Dmitry has posted a fix for one of these cases [0], but the 
above also needs to be fixed.

Thanks
Jon


[0] 
https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Ard Biesheuvel]

On Tue, 24 May 2022 at 16:50, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> Hi Ard,
>
> On 28/03/2022 14:47, Ard Biesheuvel wrote:
> > The Spectre-BHB mitigations were inadvertently left disabled for
> > Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
> > that case. So fix that.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >   arch/arm/mm/proc-v7-bugs.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> > index 06dbfb968182..fb9f3eb6bf48 100644
> > --- a/arch/arm/mm/proc-v7-bugs.c
> > +++ b/arch/arm/mm/proc-v7-bugs.c
> > @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
> >   {
> >       if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
> >               cpu_v7_spectre_v2_init();
> > +     cpu_v7_spectre_bhb_init();
> >   }
> >
> >   void cpu_v7_bugs_init(void)
>
>
> Since this patch has been merged, I am seeing a ton of messages when
> booting Linux on tegra124-jetson-tk1 ...
>

Thanks for the report. The good news is that the Spectre BHB
protection is finally taking effect on your system.

> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
> [ 1233.328700] CPU1: Spectre BHB: using loop workaround
> [ 1233.355477] CPU2: Spectre BHB: using loop workaround
> ** 7 printk messages dropped **
> [ 1233.366271] CPU0: Spectre BHB: using loop workaround
> [ 1233.366580] CPU0: Spectre BHB: using loop workaround
> [ 1233.366815] CPU1: Spectre BHB: using loop workaround
> [ 1233.405475] CPU1: Spectre BHB: using loop workaround
> [ 1233.405874] CPU0: Spectre BHB: using loop workaround
> [ 1233.406041] CPU1: Spectre BHB: using loop workaround
> ** 1 printk messages dropped **
>
> This is significantly increasing the boot time and causing failures if
> the boot takes too long.
>
> I see that Dmitry has posted a fix for one of these cases [0], but the
> above also needs to be fixed.
>

Agreed.

Do we have any idea why this init hook is being called so often? Is
this expected on systems that do agressive suspend/resume for power
management, or is there another existing issue here that is just
coming to light due to the newly added printk() ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Jon Hunter]

On 24/05/2022 16:21, Ard Biesheuvel wrote:
> On Tue, 24 May 2022 at 16:50, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> Hi Ard,
>>
>> On 28/03/2022 14:47, Ard Biesheuvel wrote:
>>> The Spectre-BHB mitigations were inadvertently left disabled for
>>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
>>> that case. So fix that.
>>>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>>    arch/arm/mm/proc-v7-bugs.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
>>> index 06dbfb968182..fb9f3eb6bf48 100644
>>> --- a/arch/arm/mm/proc-v7-bugs.c
>>> +++ b/arch/arm/mm/proc-v7-bugs.c
>>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
>>>    {
>>>        if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
>>>                cpu_v7_spectre_v2_init();
>>> +     cpu_v7_spectre_bhb_init();
>>>    }
>>>
>>>    void cpu_v7_bugs_init(void)
>>
>>
>> Since this patch has been merged, I am seeing a ton of messages when
>> booting Linux on tegra124-jetson-tk1 ...
>>
> 
> Thanks for the report. The good news is that the Spectre BHB
> protection is finally taking effect on your system.
> 
>> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
>> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
>> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
>> [ 1233.328700] CPU1: Spectre BHB: using loop workaround
>> [ 1233.355477] CPU2: Spectre BHB: using loop workaround
>> ** 7 printk messages dropped **
>> [ 1233.366271] CPU0: Spectre BHB: using loop workaround
>> [ 1233.366580] CPU0: Spectre BHB: using loop workaround
>> [ 1233.366815] CPU1: Spectre BHB: using loop workaround
>> [ 1233.405475] CPU1: Spectre BHB: using loop workaround
>> [ 1233.405874] CPU0: Spectre BHB: using loop workaround
>> [ 1233.406041] CPU1: Spectre BHB: using loop workaround
>> ** 1 printk messages dropped **
>>
>> This is significantly increasing the boot time and causing failures if
>> the boot takes too long.
>>
>> I see that Dmitry has posted a fix for one of these cases [0], but the
>> above also needs to be fixed.
>>
> 
> Agreed.
> 
> Do we have any idea why this init hook is being called so often? Is
> this expected on systems that do agressive suspend/resume for power
> management, or is there another existing issue here that is just
> coming to light due to the newly added printk() ?


So far I have not dug any further, but I do know it is not doing any 
aggressive suspend/resume. From what Dmitry indicated he saw this during 
CPU idle transitions and this platform would definitely by exercising 
CPU idle.

Jon

-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Russell King (Oracle)]

On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
> Hi Ard,
> 
> On 28/03/2022 14:47, Ard Biesheuvel wrote:
> > The Spectre-BHB mitigations were inadvertently left disabled for
> > Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
> > that case. So fix that.
> > 
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >   arch/arm/mm/proc-v7-bugs.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> > index 06dbfb968182..fb9f3eb6bf48 100644
> > --- a/arch/arm/mm/proc-v7-bugs.c
> > +++ b/arch/arm/mm/proc-v7-bugs.c
> > @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
> >   {
> >   	if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
> >   		cpu_v7_spectre_v2_init();
> > +	cpu_v7_spectre_bhb_init();
> >   }
> >   void cpu_v7_bugs_init(void)
> 
> 
> Since this patch has been merged, I am seeing a ton of messages when booting
> Linux on tegra124-jetson-tk1 ...
> 
> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
> [ 1233.328270] CPU1: Spectre BHB: using loop workaround

Now that you mention this, I vaguely remember some email on the list a
while ago about this being caused by something like cpuidle - but I'm
unable to find it now.

> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/

That was probably it.

We can't really do this for the other print, because the system status
can change as a result of CPUs being brought online. :(

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Jon Hunter]

On 24/05/2022 18:03, Russell King (Oracle) wrote:
> On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
>> Hi Ard,
>>
>> On 28/03/2022 14:47, Ard Biesheuvel wrote:
>>> The Spectre-BHB mitigations were inadvertently left disabled for
>>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
>>> that case. So fix that.
>>>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>>    arch/arm/mm/proc-v7-bugs.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
>>> index 06dbfb968182..fb9f3eb6bf48 100644
>>> --- a/arch/arm/mm/proc-v7-bugs.c
>>> +++ b/arch/arm/mm/proc-v7-bugs.c
>>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
>>>    {
>>>    	if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
>>>    		cpu_v7_spectre_v2_init();
>>> +	cpu_v7_spectre_bhb_init();
>>>    }
>>>    void cpu_v7_bugs_init(void)
>>
>>
>> Since this patch has been merged, I am seeing a ton of messages when booting
>> Linux on tegra124-jetson-tk1 ...
>>
>> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
>> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
>> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
> 
> Now that you mention this, I vaguely remember some email on the list a
> while ago about this being caused by something like cpuidle - but I'm
> unable to find it now.
> 
>> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
> 
> That was probably it.


I am seeing ...

[    4.415167] CPU0: Spectre BHB: using loop workaround
[    4.417621] [<c01109a0>] (unwind_backtrace) from [<c010b7ac>] (show_stack+0x10/0x14)
[    4.430291] [<c010b7ac>] (show_stack) from [<c09c2b38>] (dump_stack+0xc0/0xd4)
[    4.437512] [<c09c2b38>] (dump_stack) from [<c011a6c8>] (cpu_v7_spectre_bhb_init+0xd8/0x190)
[    4.445943] [<c011a6c8>] (cpu_v7_spectre_bhb_init) from [<c010dee8>] (cpu_suspend+0xac/0xc8)
[    4.454377] [<c010dee8>] (cpu_suspend) from [<c011e7e4>] (tegra114_idle_power_down+0x74/0x78)
[    4.462898] [<c011e7e4>] (tegra114_idle_power_down) from [<c06d3b44>] (cpuidle_enter_state+0x130/0x524)
[    4.472286] [<c06d3b44>] (cpuidle_enter_state) from [<c0164a30>] (do_idle+0x1b0/0x200)
[    4.480199] [<c0164a30>] (do_idle) from [<c0164d28>] (cpu_startup_entry+0x18/0x1c)
[    4.487762] [<c0164d28>] (cpu_startup_entry) from [<801018cc>] (0x801018cc)


So definitely CPU idle.
  
> We can't really do this for the other print, because the system status
> can change as a result of CPUs being brought online. :(


How about making this a pr_debug as opposed to pr_info?

Jon

-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Ard Biesheuvel]

On Tue, 24 May 2022 at 19:49, Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 24/05/2022 18:03, Russell King (Oracle) wrote:
> > On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
> >> Hi Ard,
> >>
> >> On 28/03/2022 14:47, Ard Biesheuvel wrote:
> >>> The Spectre-BHB mitigations were inadvertently left disabled for
> >>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
> >>> that case. So fix that.
> >>>
> >>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >>> ---
> >>>    arch/arm/mm/proc-v7-bugs.c | 1 +
> >>>    1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> >>> index 06dbfb968182..fb9f3eb6bf48 100644
> >>> --- a/arch/arm/mm/proc-v7-bugs.c
> >>> +++ b/arch/arm/mm/proc-v7-bugs.c
> >>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
> >>>    {
> >>>     if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
> >>>             cpu_v7_spectre_v2_init();
> >>> +   cpu_v7_spectre_bhb_init();
> >>>    }
> >>>    void cpu_v7_bugs_init(void)
> >>
> >>
> >> Since this patch has been merged, I am seeing a ton of messages when booting
> >> Linux on tegra124-jetson-tk1 ...
> >>
> >> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
> >> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
> >> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
> >
> > Now that you mention this, I vaguely remember some email on the list a
> > while ago about this being caused by something like cpuidle - but I'm
> > unable to find it now.
> >
> >> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
> >
> > That was probably it.
>
>
> I am seeing ...
>
> [    4.415167] CPU0: Spectre BHB: using loop workaround
> [    4.417621] [<c01109a0>] (unwind_backtrace) from [<c010b7ac>] (show_stack+0x10/0x14)
> [    4.430291] [<c010b7ac>] (show_stack) from [<c09c2b38>] (dump_stack+0xc0/0xd4)
> [    4.437512] [<c09c2b38>] (dump_stack) from [<c011a6c8>] (cpu_v7_spectre_bhb_init+0xd8/0x190)
> [    4.445943] [<c011a6c8>] (cpu_v7_spectre_bhb_init) from [<c010dee8>] (cpu_suspend+0xac/0xc8)
> [    4.454377] [<c010dee8>] (cpu_suspend) from [<c011e7e4>] (tegra114_idle_power_down+0x74/0x78)
> [    4.462898] [<c011e7e4>] (tegra114_idle_power_down) from [<c06d3b44>] (cpuidle_enter_state+0x130/0x524)
> [    4.472286] [<c06d3b44>] (cpuidle_enter_state) from [<c0164a30>] (do_idle+0x1b0/0x200)
> [    4.480199] [<c0164a30>] (do_idle) from [<c0164d28>] (cpu_startup_entry+0x18/0x1c)
> [    4.487762] [<c0164d28>] (cpu_startup_entry) from [<801018cc>] (0x801018cc)
>
>
> So definitely CPU idle.
>
> > We can't really do this for the other print, because the system status
> > can change as a result of CPUs being brought online. :(
>
>
> How about making this a pr_debug as opposed to pr_info?
>

We should pull the pr_info() into the conditional so that it only
executes the first time around:

--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -209,10 +209,10 @@ static int spectre_bhb_install_workaround(int method)
                        return SPECTRE_VULNERABLE;

                spectre_bhb_method = method;
-       }

-       pr_info("CPU%u: Spectre BHB: using %s workaround\n",
-               smp_processor_id(), spectre_bhb_method_name(method));
+               pr_info("CPU%u: Spectre BHB: using %s workaround\n",
+                       smp_processor_id(), spectre_bhb_method_name(method));
+       }

        return SPECTRE_MITIGATED;
 }

If you can confirm that this fixes the issue, I can send it out as a
proper patch.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Jon Hunter]

On 25/05/2022 08:09, Ard Biesheuvel wrote:
> On Tue, 24 May 2022 at 19:49, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 24/05/2022 18:03, Russell King (Oracle) wrote:
>>> On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
>>>> Hi Ard,
>>>>
>>>> On 28/03/2022 14:47, Ard Biesheuvel wrote:
>>>>> The Spectre-BHB mitigations were inadvertently left disabled for
>>>>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
>>>>> that case. So fix that.
>>>>>
>>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>>>> ---
>>>>>     arch/arm/mm/proc-v7-bugs.c | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
>>>>> index 06dbfb968182..fb9f3eb6bf48 100644
>>>>> --- a/arch/arm/mm/proc-v7-bugs.c
>>>>> +++ b/arch/arm/mm/proc-v7-bugs.c
>>>>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
>>>>>     {
>>>>>      if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
>>>>>              cpu_v7_spectre_v2_init();
>>>>> +   cpu_v7_spectre_bhb_init();
>>>>>     }
>>>>>     void cpu_v7_bugs_init(void)
>>>>
>>>>
>>>> Since this patch has been merged, I am seeing a ton of messages when booting
>>>> Linux on tegra124-jetson-tk1 ...
>>>>
>>>> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
>>>> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
>>>> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
>>>
>>> Now that you mention this, I vaguely remember some email on the list a
>>> while ago about this being caused by something like cpuidle - but I'm
>>> unable to find it now.
>>>
>>>> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
>>>
>>> That was probably it.
>>
>>
>> I am seeing ...
>>
>> [    4.415167] CPU0: Spectre BHB: using loop workaround
>> [    4.417621] [<c01109a0>] (unwind_backtrace) from [<c010b7ac>] (show_stack+0x10/0x14)
>> [    4.430291] [<c010b7ac>] (show_stack) from [<c09c2b38>] (dump_stack+0xc0/0xd4)
>> [    4.437512] [<c09c2b38>] (dump_stack) from [<c011a6c8>] (cpu_v7_spectre_bhb_init+0xd8/0x190)
>> [    4.445943] [<c011a6c8>] (cpu_v7_spectre_bhb_init) from [<c010dee8>] (cpu_suspend+0xac/0xc8)
>> [    4.454377] [<c010dee8>] (cpu_suspend) from [<c011e7e4>] (tegra114_idle_power_down+0x74/0x78)
>> [    4.462898] [<c011e7e4>] (tegra114_idle_power_down) from [<c06d3b44>] (cpuidle_enter_state+0x130/0x524)
>> [    4.472286] [<c06d3b44>] (cpuidle_enter_state) from [<c0164a30>] (do_idle+0x1b0/0x200)
>> [    4.480199] [<c0164a30>] (do_idle) from [<c0164d28>] (cpu_startup_entry+0x18/0x1c)
>> [    4.487762] [<c0164d28>] (cpu_startup_entry) from [<801018cc>] (0x801018cc)
>>
>>
>> So definitely CPU idle.
>>
>>> We can't really do this for the other print, because the system status
>>> can change as a result of CPUs being brought online. :(
>>
>>
>> How about making this a pr_debug as opposed to pr_info?
>>
> 
> We should pull the pr_info() into the conditional so that it only
> executes the first time around:
> 
> --- a/arch/arm/mm/proc-v7-bugs.c
> +++ b/arch/arm/mm/proc-v7-bugs.c
> @@ -209,10 +209,10 @@ static int spectre_bhb_install_workaround(int method)
>                          return SPECTRE_VULNERABLE;
> 
>                  spectre_bhb_method = method;
> -       }
> 
> -       pr_info("CPU%u: Spectre BHB: using %s workaround\n",
> -               smp_processor_id(), spectre_bhb_method_name(method));
> +               pr_info("CPU%u: Spectre BHB: using %s workaround\n",
> +                       smp_processor_id(), spectre_bhb_method_name(method));
> +       }
> 
>          return SPECTRE_MITIGATED;
>   }
> 
> If you can confirm that this fixes the issue, I can send it out as a
> proper patch.

That works for me.

Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks
Jon

-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Ard Biesheuvel]

On Wed, 25 May 2022 at 12:48, Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 25/05/2022 08:09, Ard Biesheuvel wrote:
> > On Tue, 24 May 2022 at 19:49, Jon Hunter <jonathanh@nvidia.com> wrote:
> >>
> >>
> >> On 24/05/2022 18:03, Russell King (Oracle) wrote:
> >>> On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
> >>>> Hi Ard,
> >>>>
> >>>> On 28/03/2022 14:47, Ard Biesheuvel wrote:
> >>>>> The Spectre-BHB mitigations were inadvertently left disabled for
> >>>>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
> >>>>> that case. So fix that.
> >>>>>
> >>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >>>>> ---
> >>>>>     arch/arm/mm/proc-v7-bugs.c | 1 +
> >>>>>     1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> >>>>> index 06dbfb968182..fb9f3eb6bf48 100644
> >>>>> --- a/arch/arm/mm/proc-v7-bugs.c
> >>>>> +++ b/arch/arm/mm/proc-v7-bugs.c
> >>>>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
> >>>>>     {
> >>>>>      if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
> >>>>>              cpu_v7_spectre_v2_init();
> >>>>> +   cpu_v7_spectre_bhb_init();
> >>>>>     }
> >>>>>     void cpu_v7_bugs_init(void)
> >>>>
> >>>>
> >>>> Since this patch has been merged, I am seeing a ton of messages when booting
> >>>> Linux on tegra124-jetson-tk1 ...
> >>>>
> >>>> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
> >>>> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
> >>>> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
> >>>
> >>> Now that you mention this, I vaguely remember some email on the list a
> >>> while ago about this being caused by something like cpuidle - but I'm
> >>> unable to find it now.
> >>>
> >>>> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
> >>>
> >>> That was probably it.
> >>
> >>
> >> I am seeing ...
> >>
> >> [    4.415167] CPU0: Spectre BHB: using loop workaround
> >> [    4.417621] [<c01109a0>] (unwind_backtrace) from [<c010b7ac>] (show_stack+0x10/0x14)
> >> [    4.430291] [<c010b7ac>] (show_stack) from [<c09c2b38>] (dump_stack+0xc0/0xd4)
> >> [    4.437512] [<c09c2b38>] (dump_stack) from [<c011a6c8>] (cpu_v7_spectre_bhb_init+0xd8/0x190)
> >> [    4.445943] [<c011a6c8>] (cpu_v7_spectre_bhb_init) from [<c010dee8>] (cpu_suspend+0xac/0xc8)
> >> [    4.454377] [<c010dee8>] (cpu_suspend) from [<c011e7e4>] (tegra114_idle_power_down+0x74/0x78)
> >> [    4.462898] [<c011e7e4>] (tegra114_idle_power_down) from [<c06d3b44>] (cpuidle_enter_state+0x130/0x524)
> >> [    4.472286] [<c06d3b44>] (cpuidle_enter_state) from [<c0164a30>] (do_idle+0x1b0/0x200)
> >> [    4.480199] [<c0164a30>] (do_idle) from [<c0164d28>] (cpu_startup_entry+0x18/0x1c)
> >> [    4.487762] [<c0164d28>] (cpu_startup_entry) from [<801018cc>] (0x801018cc)
> >>
> >>
> >> So definitely CPU idle.
> >>
> >>> We can't really do this for the other print, because the system status
> >>> can change as a result of CPUs being brought online. :(
> >>
> >>
> >> How about making this a pr_debug as opposed to pr_info?
> >>
> >
> > We should pull the pr_info() into the conditional so that it only
> > executes the first time around:
> >
> > --- a/arch/arm/mm/proc-v7-bugs.c
> > +++ b/arch/arm/mm/proc-v7-bugs.c
> > @@ -209,10 +209,10 @@ static int spectre_bhb_install_workaround(int method)
> >                          return SPECTRE_VULNERABLE;
> >
> >                  spectre_bhb_method = method;
> > -       }
> >
> > -       pr_info("CPU%u: Spectre BHB: using %s workaround\n",
> > -               smp_processor_id(), spectre_bhb_method_name(method));
> > +               pr_info("CPU%u: Spectre BHB: using %s workaround\n",
> > +                       smp_processor_id(), spectre_bhb_method_name(method));
> > +       }
> >
> >          return SPECTRE_MITIGATED;
> >   }
> >
> > If you can confirm that this fixes the issue, I can send it out as a
> > proper patch.
>
> That works for me.
>
> Tested-by: Jon Hunter <jonathanh@nvidia.com>
>

Thanks Jon.

The only downside here is that the message is only printed once for
all CPUs, unless we are enabling two different methods on ab
big.little system, in which case we print an error.

Personally, I think that is fine, but I'll need to tweak the message
to clarify this.

Russell, any thoughts?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Jon Hunter]

On 24/05/2022 18:03, Russell King (Oracle) wrote:
> On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
>> Hi Ard,
>>
>> On 28/03/2022 14:47, Ard Biesheuvel wrote:
>>> The Spectre-BHB mitigations were inadvertently left disabled for
>>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
>>> that case. So fix that.
>>>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>>    arch/arm/mm/proc-v7-bugs.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
>>> index 06dbfb968182..fb9f3eb6bf48 100644
>>> --- a/arch/arm/mm/proc-v7-bugs.c
>>> +++ b/arch/arm/mm/proc-v7-bugs.c
>>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
>>>    {
>>>    	if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
>>>    		cpu_v7_spectre_v2_init();
>>> +	cpu_v7_spectre_bhb_init();
>>>    }
>>>    void cpu_v7_bugs_init(void)
>>
>>
>> Since this patch has been merged, I am seeing a ton of messages when booting
>> Linux on tegra124-jetson-tk1 ...
>>
>> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
>> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
>> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
> 
> Now that you mention this, I vaguely remember some email on the list a
> while ago about this being caused by something like cpuidle - but I'm
> unable to find it now.
> 
>> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
> 
> That was probably it.
> 
> We can't really do this for the other print, because the system status
> can change as a result of CPUs being brought online. :(
> 

Does it make sense to only print the message if/when the method changes 
as opposed to every time the CPUs are brought online? That way, there 
would still be at least one print showing the current method. I believe 
that is what Ard had proposed.

Jon

-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Ard Biesheuvel]

On Tue, 7 Jun 2022 at 16:30, Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 24/05/2022 18:03, Russell King (Oracle) wrote:
> > On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
> >> Hi Ard,
> >>
> >> On 28/03/2022 14:47, Ard Biesheuvel wrote:
> >>> The Spectre-BHB mitigations were inadvertently left disabled for
> >>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
> >>> that case. So fix that.
> >>>
> >>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >>> ---
> >>>    arch/arm/mm/proc-v7-bugs.c | 1 +
> >>>    1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> >>> index 06dbfb968182..fb9f3eb6bf48 100644
> >>> --- a/arch/arm/mm/proc-v7-bugs.c
> >>> +++ b/arch/arm/mm/proc-v7-bugs.c
> >>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
> >>>    {
> >>>     if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
> >>>             cpu_v7_spectre_v2_init();
> >>> +   cpu_v7_spectre_bhb_init();
> >>>    }
> >>>    void cpu_v7_bugs_init(void)
> >>
> >>
> >> Since this patch has been merged, I am seeing a ton of messages when booting
> >> Linux on tegra124-jetson-tk1 ...
> >>
> >> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
> >> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
> >> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
> >
> > Now that you mention this, I vaguely remember some email on the list a
> > while ago about this being caused by something like cpuidle - but I'm
> > unable to find it now.
> >
> >> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
> >
> > That was probably it.
> >
> > We can't really do this for the other print, because the system status
> > can change as a result of CPUs being brought online. :(
> >
>
> Does it make sense to only print the message if/when the method changes
> as opposed to every time the CPUs are brought online? That way, there
> would still be at least one print showing the current method. I believe
> that is what Ard had proposed.
>

A fix for this issue is already in linux-next:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=bafa10435c4f34f4b9bda8fc7ee6e4330ebca3ea

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Jon Hunter]

On 07/06/2022 15:32, Ard Biesheuvel wrote:
> On Tue, 7 Jun 2022 at 16:30, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 24/05/2022 18:03, Russell King (Oracle) wrote:
>>> On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
>>>> Hi Ard,
>>>>
>>>> On 28/03/2022 14:47, Ard Biesheuvel wrote:
>>>>> The Spectre-BHB mitigations were inadvertently left disabled for
>>>>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
>>>>> that case. So fix that.
>>>>>
>>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>>>> ---
>>>>>     arch/arm/mm/proc-v7-bugs.c | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
>>>>> index 06dbfb968182..fb9f3eb6bf48 100644
>>>>> --- a/arch/arm/mm/proc-v7-bugs.c
>>>>> +++ b/arch/arm/mm/proc-v7-bugs.c
>>>>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
>>>>>     {
>>>>>      if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
>>>>>              cpu_v7_spectre_v2_init();
>>>>> +   cpu_v7_spectre_bhb_init();
>>>>>     }
>>>>>     void cpu_v7_bugs_init(void)
>>>>
>>>>
>>>> Since this patch has been merged, I am seeing a ton of messages when booting
>>>> Linux on tegra124-jetson-tk1 ...
>>>>
>>>> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
>>>> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
>>>> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
>>>
>>> Now that you mention this, I vaguely remember some email on the list a
>>> while ago about this being caused by something like cpuidle - but I'm
>>> unable to find it now.
>>>
>>>> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
>>>
>>> That was probably it.
>>>
>>> We can't really do this for the other print, because the system status
>>> can change as a result of CPUs being brought online. :(
>>>
>>
>> Does it make sense to only print the message if/when the method changes
>> as opposed to every time the CPUs are brought online? That way, there
>> would still be at least one print showing the current method. I believe
>> that is what Ard had proposed.
>>
> 
> A fix for this issue is already in linux-next:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=bafa10435c4f34f4b9bda8fc7ee6e4330ebca3ea


Ah wonderful! Sorry I had missed that. Once merged can we pull into 
stable as well?

Jon

-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] ARM: spectre-bhb: enable for Cortex-A15

[Ard Biesheuvel]

On Tue, 7 Jun 2022 at 16:35, Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 07/06/2022 15:32, Ard Biesheuvel wrote:
> > On Tue, 7 Jun 2022 at 16:30, Jon Hunter <jonathanh@nvidia.com> wrote:
> >>
> >>
> >> On 24/05/2022 18:03, Russell King (Oracle) wrote:
> >>> On Tue, May 24, 2022 at 03:50:17PM +0100, Jon Hunter wrote:
> >>>> Hi Ard,
> >>>>
> >>>> On 28/03/2022 14:47, Ard Biesheuvel wrote:
> >>>>> The Spectre-BHB mitigations were inadvertently left disabled for
> >>>>> Cortex-A15, due to the fact that cpu_v7_bugs_init() is not called in
> >>>>> that case. So fix that.
> >>>>>
> >>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >>>>> ---
> >>>>>     arch/arm/mm/proc-v7-bugs.c | 1 +
> >>>>>     1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> >>>>> index 06dbfb968182..fb9f3eb6bf48 100644
> >>>>> --- a/arch/arm/mm/proc-v7-bugs.c
> >>>>> +++ b/arch/arm/mm/proc-v7-bugs.c
> >>>>> @@ -288,6 +288,7 @@ void cpu_v7_ca15_ibe(void)
> >>>>>     {
> >>>>>      if (check_spectre_auxcr(this_cpu_ptr(&spectre_warned), BIT(0)))
> >>>>>              cpu_v7_spectre_v2_init();
> >>>>> +   cpu_v7_spectre_bhb_init();
> >>>>>     }
> >>>>>     void cpu_v7_bugs_init(void)
> >>>>
> >>>>
> >>>> Since this patch has been merged, I am seeing a ton of messages when booting
> >>>> Linux on tegra124-jetson-tk1 ...
> >>>>
> >>>> [ 1233.327547] CPU0: Spectre BHB: using loop workaround
> >>>> [ 1233.327795] CPU1: Spectre BHB: using loop workaround
> >>>> [ 1233.328270] CPU1: Spectre BHB: using loop workaround
> >>>
> >>> Now that you mention this, I vaguely remember some email on the list a
> >>> while ago about this being caused by something like cpuidle - but I'm
> >>> unable to find it now.
> >>>
> >>>> [0] https://lore.kernel.org/linux-arm-kernel/20220519161310.1489625-1-dmitry.osipenko@collabora.com/T/
> >>>
> >>> That was probably it.
> >>>
> >>> We can't really do this for the other print, because the system status
> >>> can change as a result of CPUs being brought online. :(
> >>>
> >>
> >> Does it make sense to only print the message if/when the method changes
> >> as opposed to every time the CPUs are brought online? That way, there
> >> would still be at least one print showing the current method. I believe
> >> that is what Ard had proposed.
> >>
> >
> > A fix for this issue is already in linux-next:
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=bafa10435c4f34f4b9bda8fc7ee6e4330ebca3ea
>
>
> Ah wonderful! Sorry I had missed that. Once merged can we pull into
> stable as well?
>

Yes. It has a fixes: tag so it will most likely get picked up
automatically, but feel free to remind the -stable maintainers once
this patch is merged by Linus.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] ARM: spectre-bhb: fix loop8 sequence for Thumb2

[Ard Biesheuvel]

In Thumb2, 'b . + 4' produces a branch instruction that uses a narrow
encoding, and so it does not jump to the following instruction as
expected. So use W(b) instead.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/kernel/entry-armv.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index ee3f7a599181..4bbd92d41031 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1040,7 +1040,7 @@ vector_bhb_loop8_\name:
 
 	@ bhb workaround
 	mov	r0, #8
-3:	b	. + 4
+3:	W(b)	. + 4
 	subs	r0, r0, #1
 	bne	3b
 	dsb
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] ARM: spectre-bhb: simplify BPIALL vector macro

[Ard Biesheuvel]

The BPIALL mitigation for Spectre-BHB adds a single instruction to the
handler sequence that doesn't clobber any registers. Given that these
sequences are 10 instructions long, they don't fit neatly into a
cacheline anyway, so we can simply move that single instruction to the
start of it, and rearrange the symbol names accordingly.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/kernel/entry-armv.S | 21 ++++++--------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 4bbd92d41031..a5725e82addc 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -996,6 +996,12 @@ __kuser_helper_end:
  */
 	.macro	vector_stub, name, mode, correction=0
 	.align	5
+#ifdef CONFIG_HARDEN_BRANCH_HISTORY
+vector_bhb_bpiall_\name:
+	mcr	p15, 0, r0, c7, c5, 6	@ BPIALL
+	@ isb not needed due to "movs pc, lr" in the vector stub
+	@ which gives a "context synchronisation".
+#endif
 
 vector_\name:
 	.if \correction
@@ -1047,21 +1053,6 @@ vector_bhb_loop8_\name:
 	isb
 	b	2b
 ENDPROC(vector_bhb_loop8_\name)
-
-vector_bhb_bpiall_\name:
-	.if \correction
-	sub	lr, lr, #\correction
-	.endif
-
-	@ Save r0, lr_<exception> (parent PC)
-	stmia	sp, {r0, lr}
-
-	@ bhb workaround
-	mcr	p15, 0, r0, c7, c5, 6	@ BPIALL
-	@ isb not needed due to "movs pc, lr" in the vector stub
-	@ which gives a "context synchronisation".
-	b	2b
-ENDPROC(vector_bhb_bpiall_\name)
 	.previous
 #endif
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] ARM: spectre-bhb: use local DSB and elide ISB in loop8 sequence

[Ard Biesheuvel]

The loop8 mitigation for Spectre-BHB only requires a CPU local DSB
rather than a systemwide one, which is much more costly. And by the same
reasoning as why it is justified to omit the ISB after BPIALL, we can
also elide the ISB and rely on the exception return for the context
synchronization.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/kernel/entry-armv.S   | 5 +++--
 arch/arm/kernel/entry-common.S | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index a5725e82addc..3a62ee790b5e 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1049,8 +1049,9 @@ vector_bhb_loop8_\name:
 3:	W(b)	. + 4
 	subs	r0, r0, #1
 	bne	3b
-	dsb
-	isb
+	dsb	nsh
+	@ isb not needed due to "movs pc, lr" in the vector stub
+	@ which gives a "context synchronisation".
 	b	2b
 ENDPROC(vector_bhb_loop8_\name)
 	.previous
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index dbc1913ee30b..96016a5ad72f 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -162,7 +162,7 @@ ENTRY(vector_bhb_loop8_swi)
 1:	b	2f
 2:	subs	r8, r8, #1
 	bne	1b
-	dsb
+	dsb	nsh
 	isb
 	b	3f
 ENDPROC(vector_bhb_loop8_swi)
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] ARM: spectre-bhb: avoid cross-subsection jump using a numbered label

[Ard Biesheuvel]

In order to minimize potential confusion regarding numbered labels
appearing in a different order in the assembler output due to the use of
subsections, use a named local label to jump back into the vector
handler code from the associated loop8 mitigation sequence.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/kernel/entry-armv.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 3a62ee790b5e..d08e7f62ae57 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1012,7 +1012,8 @@ vector_\name:
 	stmia	sp, {r0, lr}		@ save r0, lr
 
 	@ Save spsr_<exception> (parent CPSR)
-2:	mrs	lr, spsr
+.L\name\@:
+	mrs	lr, spsr
 	str	lr, [sp, #8]		@ save spsr
 
 	@
@@ -1052,7 +1053,7 @@ vector_bhb_loop8_\name:
 	dsb	nsh
 	@ isb not needed due to "movs pc, lr" in the vector stub
 	@ which gives a "context synchronisation".
-	b	2b
+	b	.L\name\@
 ENDPROC(vector_bhb_loop8_\name)
 	.previous
 #endif
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] ARM: spectre-bhb: avoid cross-subsection jump using a numbered label

[Russell King (Oracle)]

On Mon, Mar 28, 2022 at 03:47:13PM +0200, Ard Biesheuvel wrote:
> In order to minimize potential confusion regarding numbered labels
> appearing in a different order in the assembler output due to the use of
> subsections, use a named local label to jump back into the vector
> handler code from the associated loop8 mitigation sequence.

Using \@ doesn't buy us much as \name is already unique. Maybe using
some kind of prefix would be better, e.g. .Lfoo_\name would be more
descriptive?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] ARM: spectre-bhb: avoid cross-subsection jump using a numbered label

[Ard Biesheuvel]

On Thu, 31 Mar 2022 at 16:07, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Mar 28, 2022 at 03:47:13PM +0200, Ard Biesheuvel wrote:
> > In order to minimize potential confusion regarding numbered labels
> > appearing in a different order in the assembler output due to the use of
> > subsections, use a named local label to jump back into the vector
> > handler code from the associated loop8 mitigation sequence.
>
> Using \@ doesn't buy us much as \name is already unique. Maybe using
> some kind of prefix would be better, e.g. .Lfoo_\name would be more
> descriptive?
>

Fair point. I'll fix that.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] ARM: spectre-bhb: rely on linker to emit cross-section literal loads

[Ard Biesheuvel]

The assembler does not permit 'LDR PC, <sym>' when the symbol lives in a
different section, which is why we have been relying on rather fragile
open-coded arithmetic to load the address of the vector_swi routine into
the program counter using a single LDR instruction in the SWI slot in
the vector table. The literal was moved to a different section to in
commit 19accfd373847 ("ARM: move vector stubs") to ensure that the
vector stubs page does not need to be mapped readable for user space,
which is the case for the vector page itself, as it carries the kuser
helpers as well.

So the cross-section literal load is open-coded, and this relies on the
address of vector_swi to be at the very start of the vector stubs page,
and we won't notice if we got it wrong until booting the kernel and see
it break. Fortunately, it was guaranteed to break, so this was fragile
but not problematic.

Now that we have added two other variants of the vector table, we have 3
occurrences of the same trick, and so the size of our ISA/compiler/CPU
validation space has tripled, in a way that may cause regressions to only
be observed once booting the image in question on a CPU that exercises a
particular vector table.

So let's switch to true cross section references, and let the linker fix
them up like it fixes up all the other cross section references in the
vector page.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/kernel/entry-armv.S | 22 +++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index d08e7f62ae57..f17adc0b5b6a 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1064,10 +1064,15 @@ ENDPROC(vector_bhb_loop8_\name)
 	.endm
 
 	.section .stubs, "ax", %progbits
-	@ This must be the first word
+	@ These need to remain at the start of the section so that
+	@ they are in range of the 'SWI' entries in the vector tables
+	@ located 4k down.
+.L__vector_swi:
 	.word	vector_swi
 #ifdef CONFIG_HARDEN_BRANCH_HISTORY
+.L__vector_bhb_loop8_swi:
 	.word	vector_bhb_loop8_swi
+.L__vector_bhb_bpiall_swi:
 	.word	vector_bhb_bpiall_swi
 #endif
 
@@ -1210,10 +1215,11 @@ vector_addrexcptn:
 	.globl	vector_fiq
 
 	.section .vectors, "ax", %progbits
-.L__vectors_start:
 	W(b)	vector_rst
 	W(b)	vector_und
-	W(ldr)	pc, .L__vectors_start + 0x1000
+ARM(	.reloc	., R_ARM_LDR_PC_G0, .L__vector_swi		)
+THUMB(	.reloc	., R_ARM_THM_PC12, .L__vector_swi		)
+	W(ldr)	pc, .
 	W(b)	vector_pabt
 	W(b)	vector_dabt
 	W(b)	vector_addrexcptn
@@ -1222,10 +1228,11 @@ vector_addrexcptn:
 
 #ifdef CONFIG_HARDEN_BRANCH_HISTORY
 	.section .vectors.bhb.loop8, "ax", %progbits
-.L__vectors_bhb_loop8_start:
 	W(b)	vector_rst
 	W(b)	vector_bhb_loop8_und
-	W(ldr)	pc, .L__vectors_bhb_loop8_start + 0x1004
+ARM(	.reloc	., R_ARM_LDR_PC_G0, .L__vector_bhb_loop8_swi	)
+THUMB(	.reloc	., R_ARM_THM_PC12, .L__vector_bhb_loop8_swi	)
+	W(ldr)	pc, .
 	W(b)	vector_bhb_loop8_pabt
 	W(b)	vector_bhb_loop8_dabt
 	W(b)	vector_addrexcptn
@@ -1233,10 +1240,11 @@ vector_addrexcptn:
 	W(b)	vector_bhb_loop8_fiq
 
 	.section .vectors.bhb.bpiall, "ax", %progbits
-.L__vectors_bhb_bpiall_start:
 	W(b)	vector_rst
 	W(b)	vector_bhb_bpiall_und
-	W(ldr)	pc, .L__vectors_bhb_bpiall_start + 0x1008
+ARM(	.reloc	., R_ARM_LDR_PC_G0, .L__vector_bhb_bpiall_swi	)
+THUMB(	.reloc	., R_ARM_THM_PC12, .L__vector_bhb_bpiall_swi	)
+	W(ldr)	pc, .
 	W(b)	vector_bhb_bpiall_pabt
 	W(b)	vector_bhb_bpiall_dabt
 	W(b)	vector_addrexcptn
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel