Re: Against removing aarch32 kvm host support

[Tony Lindgren]

* Michael Mrozek  [200428 14:27]:
> Am Dienstag, den 28.04.2020, 14:30 +0100 schrieb Marc Zyngier:
> I know we have to accept the decision, but so far, I've known Linux to support
> as many older devices as possible as well - removing KVM Host 32bit support
> would be a step back here.
> 
> Is there a specific reason for that?
> Is it too complex to maintain alongside the aarch64 KVM Host?

I don't know the details, but ideally things would be set up
in a way where folks interested in patching 32-bit arm kvm support
can do so without causing issues for 64-bit kvm development.

That being said, I don't know who might be interested in doing
all the work for that. It's unrealistic to expect Marc to do this
work if he's not using it.

Features that are used get more resources, and features that are
less used end up just bitrotting into a broken state in about
six weeks in the Linux kernel :)

Regards,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 00/15] ARM Spectre variant 2 fixes

[Tony Lindgren]

* Russell King - ARM Linux <li...@armlinux.org.uk> [180525 07:02]:
> Third version:
> - Remove "PSCI" from the SMC version of the workaround as well.
> - Avoid reporting active workaround if the IBE bit is not set.
> - Only probe for workaround_1 on Cortex A57 and A72, or non-ARM CPUs.
> - Require features probe for workaround_1 to return zero.
> - Validation that all CPUs in the system have the same workaround status.
> - Avoid corrupting r12 in workaround_1 KVM hypervisor implementation.

Looks good to me and things still work for me. I don't really
want to count that as Tested-by though :) But feel free to add:

Boot-tested-by: Tony Lindgren <t...@atomide.com>
Reviewed-by: Tony Lindgren <t...@atomide.com>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 08/15] ARM: spectre-v2: harden user aborts in kernel space

[Tony Lindgren]

* Tony Lindgren <t...@atomide.com> [180525 16:04]:
> * Russell King - ARM Linux <li...@armlinux.org.uk> [180525 15:54]:
> > On Fri, May 25, 2018 at 08:47:42AM -0700, Tony Lindgren wrote:
> > > We can now get this with the whole series applied:
> > > 
> > > CPU0: Spectre v2: firmware did not set auxiliary control register
> > > IBE bit, system vulnerable
> > > CPU: Spectre v2: using ICIALLU workaround
> > > 
> > > So maybe change the wording from "using %s workaround" to
> > > "chosen workaround %s"?
> > > 
> > > Or I guess disabling the pr_info when not functional can
> > > be done in the later patches based on some variable set
> > > by check_spectre_auxcr() would be doable too.
> > 
> > You should not be getting both messages.  It looks like you're using
> > an older series - I assumed you pulled my git tree rather than the
> > patches?  The public git tree isn't up to date with these changes.
> 
> Yes I just fetched your git tree because it allows using
> git mergetool because of the conflicts with next. I'll apply
> manually and test again.

And testing with the correct patches in the $subject series
makes the issue go away.

Regards,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 08/15] ARM: spectre-v2: harden user aborts in kernel space

[Tony Lindgren]

* Russell King - ARM Linux <li...@armlinux.org.uk> [180525 15:54]:
> On Fri, May 25, 2018 at 08:47:42AM -0700, Tony Lindgren wrote:
> > We can now get this with the whole series applied:
> > 
> > CPU0: Spectre v2: firmware did not set auxiliary control register
> > IBE bit, system vulnerable
> > CPU: Spectre v2: using ICIALLU workaround
> > 
> > So maybe change the wording from "using %s workaround" to
> > "chosen workaround %s"?
> > 
> > Or I guess disabling the pr_info when not functional can
> > be done in the later patches based on some variable set
> > by check_spectre_auxcr() would be doable too.
> 
> You should not be getting both messages.  It looks like you're using
> an older series - I assumed you pulled my git tree rather than the
> patches?  The public git tree isn't up to date with these changes.

Yes I just fetched your git tree because it allows using
git mergetool because of the conflicts with next. I'll apply
manually and test again.

Thanks,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 08/15] ARM: spectre-v2: harden user aborts in kernel space

[Tony Lindgren]

* Russell King  [180525 15:10]:
> +static void cpu_v7_spectre_init(void)
> +{
> + const char *spectre_v2_method = NULL;
> + int cpu = smp_processor_id();
> +
> + if (per_cpu(harden_branch_predictor_fn, cpu))
> + return;
> +
> + switch (read_cpuid_part()) {
> + case ARM_CPU_PART_CORTEX_A8:
> + case ARM_CPU_PART_CORTEX_A9:
> + case ARM_CPU_PART_CORTEX_A12:
> + case ARM_CPU_PART_CORTEX_A17:
> + case ARM_CPU_PART_CORTEX_A73:
> + case ARM_CPU_PART_CORTEX_A75:
> + per_cpu(harden_branch_predictor_fn, cpu) =
> + harden_branch_predictor_bpiall;
> + spectre_v2_method = "BPIALL";
> + break;
> +
> + case ARM_CPU_PART_CORTEX_A15:
> + case ARM_CPU_PART_BRAHMA_B15:
> + per_cpu(harden_branch_predictor_fn, cpu) =
> + harden_branch_predictor_iciallu;
> + spectre_v2_method = "ICIALLU";
> + break;
> + }
> + if (spectre_v2_method)
> + pr_info("CPU%u: Spectre v2: using %s workaround\n",
> + smp_processor_id(), spectre_v2_method);
> +}

We can now get this with the whole series applied:

CPU0: Spectre v2: firmware did not set auxiliary control register
IBE bit, system vulnerable
CPU: Spectre v2: using ICIALLU workaround

So maybe change the wording from "using %s workaround" to
"chosen workaround %s"?

Or I guess disabling the pr_info when not functional can
be done in the later patches based on some variable set
by check_spectre_auxcr() would be doable too.

Regards,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 07/14] ARM: spectre-v2: add Cortex A8 and A15 validation of the IBE bit

[Tony Lindgren]

* Russell King <rmk+ker...@armlinux.org.uk> [180521 12:09]:
> When the branch predictor hardening is enabled, firmware must have set
> the IBE bit in the auxiliary control register.  If this bit has not
> been set, the Spectre workarounds will not be functional.
> 
> Add validation that this bit is set, and print a warning at alert level
> if this is not the case.
> 
> Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
> Reviewed-by: Florian Fainelli <f.faine...@gmail.com>

Yup the alert is working:

Tested-by: Tony Lindgren <t...@atomide.com>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 06/14] ARM: spectre-v2: harden branch predictor on context switches

[Tony Lindgren]

* Russell King <rmk+ker...@armlinux.org.uk> [180521 12:06]:
> Harden the branch predictor against Spectre v2 attacks on context
> switches for ARMv7 and later CPUs.  We do this by:
> 
> Cortex A9, A12, A17, A73, A75: invalidating the BTB.
> Cortex A15, Brahma B15: invalidating the instruction cache.
> 
> Cortex A57 and Cortex A72 are not addressed in this patch.
> 
> Cortex R7 and Cortex R8 are also not addressed as we do not enforce
> memory protection on these cores.

Not seeing regressions so:

Tested-by: Tony Lindgren <t...@atomide.com>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 05/15] ARM: head: use PC-relative insn sequence for __smp_alt

[Tony Lindgren]

* Ard Biesheuvel <ard.biesheu...@linaro.org> [170811 12:37]:
> On 11 August 2017 at 16:13, Tony Lindgren <t...@atomide.com> wrote:
> > * Ard Biesheuvel <ard.biesheu...@linaro.org> [170805 13:54]:
> >> Replace the open coded PC relative offset calculations with a pair
> >> of adr_l invocations.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> >> ---
> >>  arch/arm/kernel/head.S | 12 ++--
> >>  1 file changed, 2 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> >> index 6e9df3663a57..aed341e0f530 100644
> >> --- a/arch/arm/kernel/head.S
> >> +++ b/arch/arm/kernel/head.S
> >> @@ -523,19 +523,11 @@ ARM_BE8(rev r0, r0) @ byteswap 
> >> if big endian
> >>   retne   lr
> >>
> >>  __fixup_smp_on_up:
> >> - adr r0, 1f
> >> - ldmia   r0, {r3 - r5}
> >> - sub r3, r0, r3
> >> - add r4, r4, r3
> >> - add r5, r5, r3
> >> + adr_l   r4, __smpalt_begin
> >> + adr_l   r5, __smpalt_end
> >>   b   __do_fixup_smp_on_up
> >>  ENDPROC(__fixup_smp)
> >>
> >> - .align
> >> -1:   .word   .
> >> - .word   __smpalt_begin
> >> - .word   __smpalt_end
> >> -
> >>   .pushsection .data
> >>   .globl  smp_on_up
> >>  smp_on_up:
> >
> > Ard, it's this one that cause boot to fail on omap3.
> > The rest of your set works for me with just this one
> > left out.
> >
> 
> I'm completely puzzled. The change is so simple that it is difficult
> to reduce it in parts, but if you still have the time to spare, mind
> trying the diff below?
> 
> 
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 6e9df3663a57..c0361e608210 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -524,17 +524,15 @@ ARM_BE8(rev   r0, r0) @
> byteswap if big endian
> 
>  __fixup_smp_on_up:
> adr r0, 1f
> -   ldmia   r0, {r3 - r5}
> -   sub r3, r0, r3
> -   add r4, r4, r3
> -   add r5, r5, r3
> +   ldmia   r0, {r4 - r5}
> +   add r4, r4, r0
> +   add r5, r5, r0
> b   __do_fixup_smp_on_up
>  ENDPROC(__fixup_smp)
> 
> .align
> -1: .word   .
> -   .word   __smpalt_begin
> -   .word   __smpalt_end
> +1: .word   __smpalt_begin - 1b
> +   .word   __smpalt_end - 1b
> 
> .pushsection .data
> .globl  smp_on_up
> 
> The main point of these changes is to eliminate absolute references,
> not to use the new macros, so if this does work, I will go with this
> instead.

For reference, I manually did this changes as it did not apply
after reverting your earlier version of this patch. No luck with
this change, so I'll test again when you you have updated patches
available.

Regards,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 05/15] ARM: head: use PC-relative insn sequence for __smp_alt

[Tony Lindgren]

* Ard Biesheuvel  [170805 13:54]:
> Replace the open coded PC relative offset calculations with a pair
> of adr_l invocations.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  arch/arm/kernel/head.S | 12 ++--
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 6e9df3663a57..aed341e0f530 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -523,19 +523,11 @@ ARM_BE8(rev r0, r0) @ byteswap if 
> big endian
>   retne   lr
>  
>  __fixup_smp_on_up:
> - adr r0, 1f
> - ldmia   r0, {r3 - r5}
> - sub r3, r0, r3
> - add r4, r4, r3
> - add r5, r5, r3
> + adr_l   r4, __smpalt_begin
> + adr_l   r5, __smpalt_end
>   b   __do_fixup_smp_on_up
>  ENDPROC(__fixup_smp)
>  
> - .align
> -1:   .word   .
> - .word   __smpalt_begin
> - .word   __smpalt_end
> -
>   .pushsection .data
>   .globl  smp_on_up
>  smp_on_up:

Ard, it's this one that cause boot to fail on omap3.
The rest of your set works for me with just this one
left out.

Regards,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 12/15] arm-soc: omap: replace open coded VA->PA calculations

[Tony Lindgren]

* Ard Biesheuvel <ard.biesheu...@linaro.org> [170810 02:23]:
> On 9 August 2017 at 22:05, Tony Lindgren <t...@atomide.com> wrote:
> > * Ard Biesheuvel <ard.biesheu...@linaro.org> [170809 12:24]:
> >> On 9 August 2017 at 20:05, Tony Lindgren <t...@atomide.com> wrote:
> >> > * Ard Biesheuvel <ard.biesheu...@linaro.org> [170805 13:54]:
> >> >> This replaces a couple of open coded calculations to obtain the
> >> >> physical address of a far symbol with calls to the new adr_l etc
> >> >> macros.
> >> >
> >> > I gave this series a quick test and omap3 no longer boots it seems.
> >> >
> >>
> >> Thanks Tony. I will investigate
> >
> > Thanks. Looks like omap4 still boots with all your patches, but
> > omap3 won't boot even with patch 12 left out.
> >
> 
> Are you using the same image on both? Which .config?

Yes it's the same image compiled with omap2plus_defconfig.

Regards,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 12/15] arm-soc: omap: replace open coded VA->PA calculations

[Tony Lindgren]

* Ard Biesheuvel <ard.biesheu...@linaro.org> [170809 12:24]:
> On 9 August 2017 at 20:05, Tony Lindgren <t...@atomide.com> wrote:
> > * Ard Biesheuvel <ard.biesheu...@linaro.org> [170805 13:54]:
> >> This replaces a couple of open coded calculations to obtain the
> >> physical address of a far symbol with calls to the new adr_l etc
> >> macros.
> >
> > I gave this series a quick test and omap3 no longer boots it seems.
> >
> 
> Thanks Tony. I will investigate

Thanks. Looks like omap4 still boots with all your patches, but
omap3 won't boot even with patch 12 left out.

Regards,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 12/15] arm-soc: omap: replace open coded VA->PA calculations

[Tony Lindgren]

* Ard Biesheuvel  [170805 13:54]:
> This replaces a couple of open coded calculations to obtain the
> physical address of a far symbol with calls to the new adr_l etc
> macros.

I gave this series a quick test and omap3 no longer boots it seems.

Regards,

Tony
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm