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

[Jon Hunter <jonathanh@nvidia.com>]

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