mbox series

[4.4,00/18] V4.4 backport of 32-bit arm spectre patches

Message ID 20181031140436.2964-1-dave.long@linaro.org
Headers show
Series V4.4 backport of 32-bit arm spectre patches | expand

Message

David Long Oct. 31, 2018, 2:04 p.m. UTC
From: "David A. Long" <dave.long@linaro.org>


V4.4 backport of spectre patches from Russell M. King's spectre branch.
Most KVM patches are excluded. Patches not yet in upstream are excluded.

Russell King (18):
  ARM: add more CPU part numbers for Cortex and Brahma B15 CPUs
  ARM: bugs: prepare processor bug infrastructure
  ARM: bugs: hook processor bug checking into SMP and suspend paths
  ARM: bugs: add support for per-processor bug checking
  ARM: spectre: add Kconfig symbol for CPUs vulnerable to Spectre
  ARM: spectre-v2: harden branch predictor on context switches
  ARM: spectre-v2: add Cortex A8 and A15 validation of the IBE bit
  ARM: spectre-v2: harden user aborts in kernel space
  ARM: spectre-v2: warn about incorrect context switching functions
  ARM: spectre-v1: add speculation barrier (csdb) macros
  ARM: spectre-v1: add array_index_mask_nospec() implementation
  ARM: spectre-v1: fix syscall entry
  ARM: signal: copy registers using __copy_from_user()
  ARM: vfp: use __copy_from_user() when restoring VFP state
  ARM: oabi-compat: copy semops using __copy_from_user()
  ARM: use __inttype() in get_user()
  ARM: spectre-v1: use get_user() for __get_user()
  ARM: spectre-v1: mitigate user accesses

 arch/arm/include/asm/assembler.h   |  12 +++
 arch/arm/include/asm/barrier.h     |  32 +++++++
 arch/arm/include/asm/bugs.h        |   6 +-
 arch/arm/include/asm/cp15.h        |  18 ++++
 arch/arm/include/asm/cputype.h     |   8 ++
 arch/arm/include/asm/proc-fns.h    |   4 +
 arch/arm/include/asm/system_misc.h |  15 ++++
 arch/arm/include/asm/thread_info.h |   4 +-
 arch/arm/include/asm/uaccess.h     |  25 ++++--
 arch/arm/kernel/Makefile           |   1 +
 arch/arm/kernel/bugs.c             |  18 ++++
 arch/arm/kernel/entry-common.S     |  18 ++--
 arch/arm/kernel/entry-header.S     |  25 ++++++
 arch/arm/kernel/signal.c           |  56 ++++++------
 arch/arm/kernel/smp.c              |   4 +
 arch/arm/kernel/suspend.c          |   2 +
 arch/arm/kernel/sys_oabi-compat.c  |   8 +-
 arch/arm/lib/copy_from_user.S      |   9 ++
 arch/arm/mm/Kconfig                |  23 +++++
 arch/arm/mm/Makefile               |   2 +-
 arch/arm/mm/fault.c                |   3 +
 arch/arm/mm/proc-macros.S          |   3 +-
 arch/arm/mm/proc-v7-2level.S       |   6 --
 arch/arm/mm/proc-v7-bugs.c         | 112 ++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              | 133 ++++++++++++++++++++++-------
 arch/arm/vfp/vfpmodule.c           |  17 ++--
 26 files changed, 462 insertions(+), 102 deletions(-)
 create mode 100644 arch/arm/kernel/bugs.c
 create mode 100644 arch/arm/mm/proc-v7-bugs.c

-- 
2.17.1

Comments

Hanjun Guo Nov. 23, 2018, 1:25 a.m. UTC | #1
On 2018/10/31 22:04, David Long wrote:
> From: "David A. Long" <dave.long@linaro.org>

> 

> V4.4 backport of spectre patches from Russell M. King's spectre branch.

> Most KVM patches are excluded. Patches not yet in upstream are excluded.


I tested this patch set on top of stable 4.4 kernel, running on boards with
A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function
regressions in our CI system,

Tested-by: Hanjun Guo <hanjun.guo@linaro.org>


Since this patch set didn't include PSCI based hardening for arm32, so
bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of
cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch
set is in a good shape I think. So what's the plan for this patch set?

Thanks
Hanjun
Marc Zyngier Nov. 23, 2018, 9:10 a.m. UTC | #2
On 23/11/2018 01:25, Hanjun Guo wrote:
> On 2018/10/31 22:04, David Long wrote:

>> From: "David A. Long" <dave.long@linaro.org>

>>

>> V4.4 backport of spectre patches from Russell M. King's spectre branch.

>> Most KVM patches are excluded. Patches not yet in upstream are excluded.

> 

> I tested this patch set on top of stable 4.4 kernel, running on boards with

> A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

> regressions in our CI system,

> 

> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

> 

> Since this patch set didn't include PSCI based hardening for arm32, so

> bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

> cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

> set is in a good shape I think. So what's the plan for this patch set?


Well, not having these patches means that a 32bit kernel won't be get
any Spectre-v2 mitigation when run as a guest on an arm64 platform. It
turns out that this is a pretty common setup among people building large
pieces of SW, such as distributions.

Not having KVM host mitigation on 32bit ARM is probably OK (let's face
it, I'm the only user), but not mitigating it as a guest doesn't seem
completely OK to me.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
Hanjun Guo Nov. 23, 2018, 9:40 a.m. UTC | #3
Hi Marc,

On 2018/11/23 17:10, Marc Zyngier wrote:
> On 23/11/2018 01:25, Hanjun Guo wrote:

>> On 2018/10/31 22:04, David Long wrote:

>>> From: "David A. Long" <dave.long@linaro.org>

>>>

>>> V4.4 backport of spectre patches from Russell M. King's spectre branch.

>>> Most KVM patches are excluded. Patches not yet in upstream are excluded.

>>

>> I tested this patch set on top of stable 4.4 kernel, running on boards with

>> A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

>> regressions in our CI system,

>>

>> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

>>

>> Since this patch set didn't include PSCI based hardening for arm32, so

>> bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

>> cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

>> set is in a good shape I think. So what's the plan for this patch set?

> 

> Well, not having these patches means that a 32bit kernel won't be get

> any Spectre-v2 mitigation when run as a guest on an arm64 platform. It

> turns out that this is a pretty common setup among people building large

> pieces of SW, such as distributions.


I almost miss this point, that makes sense to me :)

> 

> Not having KVM host mitigation on 32bit ARM is probably OK (let's face

> it, I'm the only user), but not mitigating it as a guest doesn't seem

> completely OK to me.


We are working on a patch set which is backported from mainline to fix
ARM64 spectre-v1, spectre-v2 and SSBD for stable 4.4 kernel, and that
patch set (almost done) has PSCI patches which is needed by 32bit ARM,
so how about posting those ARM64 spectre fixes then backport all those
kvm patches for 32bit ARM spectre fix as well?

Thanks
Hanjun
Marc Zyngier Nov. 23, 2018, 11:09 a.m. UTC | #4
Hi Hanjun,

On 23/11/2018 09:40, Hanjun Guo wrote:
> Hi Marc,

> 

> On 2018/11/23 17:10, Marc Zyngier wrote:

>> On 23/11/2018 01:25, Hanjun Guo wrote:

>>> On 2018/10/31 22:04, David Long wrote:

>>>> From: "David A. Long" <dave.long@linaro.org>

>>>>

>>>> V4.4 backport of spectre patches from Russell M. King's spectre branch.

>>>> Most KVM patches are excluded. Patches not yet in upstream are excluded.

>>>

>>> I tested this patch set on top of stable 4.4 kernel, running on boards with

>>> A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

>>> regressions in our CI system,

>>>

>>> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

>>>

>>> Since this patch set didn't include PSCI based hardening for arm32, so

>>> bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

>>> cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

>>> set is in a good shape I think. So what's the plan for this patch set?

>>

>> Well, not having these patches means that a 32bit kernel won't be get

>> any Spectre-v2 mitigation when run as a guest on an arm64 platform. It

>> turns out that this is a pretty common setup among people building large

>> pieces of SW, such as distributions.

> 

> I almost miss this point, that makes sense to me :)

> 

>>

>> Not having KVM host mitigation on 32bit ARM is probably OK (let's face

>> it, I'm the only user), but not mitigating it as a guest doesn't seem

>> completely OK to me.

> 

> We are working on a patch set which is backported from mainline to fix

> ARM64 spectre-v1, spectre-v2 and SSBD for stable 4.4 kernel, and that

> patch set (almost done) has PSCI patches which is needed by 32bit ARM,

> so how about posting those ARM64 spectre fixes then backport all those

> kvm patches for 32bit ARM spectre fix as well?

I'm not sure I get what you mean by PSCI. PSCI is not involved in the
Spectre-v2 mitigation, as we use a specially designed SMC call, relying
on the SMCCC 1.1 infrastructure. Maybe it is what you're referring to here?

Again, I don't think it is worth the hassle backporting the KVM patches.
What I'd like to see is the guest (and bare metal) support code that
uses the ARCH_WORKAROUND_1 SMCCC 1.1 infrastructure.

I also don't think it is worth creating an artificial dependency between
the two architectures. Yes, some patches are common (the SMCCC
infrastructure), but that can be easily be solved at merge time. My vote
would be for David to carry the relevant patches in this series.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
Hanjun Guo Nov. 24, 2018, 1:51 a.m. UTC | #5
Hi Marc, David,

On 2018/11/23 19:09, Marc Zyngier wrote:
> Hi Hanjun,

> 

> On 23/11/2018 09:40, Hanjun Guo wrote:

>> Hi Marc,

>>

>> On 2018/11/23 17:10, Marc Zyngier wrote:

>>> On 23/11/2018 01:25, Hanjun Guo wrote:

>>>> On 2018/10/31 22:04, David Long wrote:

>>>>> From: "David A. Long" <dave.long@linaro.org>

>>>>>

>>>>> V4.4 backport of spectre patches from Russell M. King's spectre branch.

>>>>> Most KVM patches are excluded. Patches not yet in upstream are excluded.

>>>>

>>>> I tested this patch set on top of stable 4.4 kernel, running on boards with

>>>> A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

>>>> regressions in our CI system,

>>>>

>>>> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

>>>>

>>>> Since this patch set didn't include PSCI based hardening for arm32, so

>>>> bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

>>>> cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

>>>> set is in a good shape I think. So what's the plan for this patch set?

>>>

>>> Well, not having these patches means that a 32bit kernel won't be get

>>> any Spectre-v2 mitigation when run as a guest on an arm64 platform. It

>>> turns out that this is a pretty common setup among people building large

>>> pieces of SW, such as distributions.

>>

>> I almost miss this point, that makes sense to me :)

>>

>>>

>>> Not having KVM host mitigation on 32bit ARM is probably OK (let's face

>>> it, I'm the only user), but not mitigating it as a guest doesn't seem

>>> completely OK to me.

>>

>> We are working on a patch set which is backported from mainline to fix

>> ARM64 spectre-v1, spectre-v2 and SSBD for stable 4.4 kernel, and that

>> patch set (almost done) has PSCI patches which is needed by 32bit ARM,

>> so how about posting those ARM64 spectre fixes then backport all those

>> kvm patches for 32bit ARM spectre fix as well?

> I'm not sure I get what you mean by PSCI. PSCI is not involved in the

> Spectre-v2 mitigation, as we use a specially designed SMC call, relying

> on the SMCCC 1.1 infrastructure. Maybe it is what you're referring to here?


Sorry for the inexplicit, yes, it's the SMCCC 1.1 I'm referring here.

> 

> Again, I don't think it is worth the hassle backporting the KVM patches.

> What I'd like to see is the guest (and bare metal) support code that

> uses the ARCH_WORKAROUND_1 SMCCC 1.1 infrastructure.

> 

> I also don't think it is worth creating an artificial dependency between

> the two architectures. Yes, some patches are common (the SMCCC

> infrastructure), but that can be easily be solved at merge time. My vote

> would be for David to carry the relevant patches in this series.


Both are OK to me. David, could you please update this patch set
as Marc suggested?

Thanks
Hanjun
David Long Nov. 27, 2018, 2:16 a.m. UTC | #6
On 11/23/18 6:09 AM, Marc Zyngier wrote:
> Hi Hanjun,

> 

> On 23/11/2018 09:40, Hanjun Guo wrote:

>> Hi Marc,

>>

>> On 2018/11/23 17:10, Marc Zyngier wrote:

>>> On 23/11/2018 01:25, Hanjun Guo wrote:

>>>> On 2018/10/31 22:04, David Long wrote:

>>>>> From: "David A. Long" <dave.long@linaro.org>

>>>>>

>>>>> V4.4 backport of spectre patches from Russell M. King's spectre branch.

>>>>> Most KVM patches are excluded. Patches not yet in upstream are excluded.

>>>>

>>>> I tested this patch set on top of stable 4.4 kernel, running on boards with

>>>> A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

>>>> regressions in our CI system,

>>>>

>>>> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

>>>>

>>>> Since this patch set didn't include PSCI based hardening for arm32, so

>>>> bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

>>>> cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

>>>> set is in a good shape I think. So what's the plan for this patch set?

>>>

>>> Well, not having these patches means that a 32bit kernel won't be get

>>> any Spectre-v2 mitigation when run as a guest on an arm64 platform. It

>>> turns out that this is a pretty common setup among people building large

>>> pieces of SW, such as distributions.

>>

>> I almost miss this point, that makes sense to me :)

>>


I've been watching arm32 spectre patches appear since September and I 
have a work item to backport these too in the near future.  I've been 
trying to focus on backporting 64-bit security patches to v4.4 in the 
short term though.

>>>

>>> Not having KVM host mitigation on 32bit ARM is probably OK (let's face

>>> it, I'm the only user), but not mitigating it as a guest doesn't seem

>>> completely OK to me.

>>

>> We are working on a patch set which is backported from mainline to fix

>> ARM64 spectre-v1, spectre-v2 and SSBD for stable 4.4 kernel, and that

>> patch set (almost done) has PSCI patches which is needed by 32bit ARM,

>> so how about posting those ARM64 spectre fixes then backport all those

>> kvm patches for 32bit ARM spectre fix as well?

> I'm not sure I get what you mean by PSCI. PSCI is not involved in the

> Spectre-v2 mitigation, as we use a specially designed SMC call, relying

> on the SMCCC 1.1 infrastructure. Maybe it is what you're referring to here?

> 

> Again, I don't think it is worth the hassle backporting the KVM patches.

> What I'd like to see is the guest (and bare metal) support code that

> uses the ARCH_WORKAROUND_1 SMCCC 1.1 infrastructure.

> 

> I also don't think it is worth creating an artificial dependency between

> the two architectures. Yes, some patches are common (the SMCCC

> infrastructure), but that can be easily be solved at merge time. My vote

> would be for David to carry the relevant patches in this series.

> 

> Thanks,

> 

> 	M.

> 


I will look at what this means for a new patch set. I'm concerned about 
how much infrastructure this might mean backporting to v4.4, but I don't 
have enough familiarity with the code yet to know if that's a valid concern.

-dl
Hanjun Guo Dec. 3, 2018, 12:24 p.m. UTC | #7
On 2018/11/27 10:16, David Long wrote:
> On 11/23/18 6:09 AM, Marc Zyngier wrote:

>> Hi Hanjun,

>>

>> On 23/11/2018 09:40, Hanjun Guo wrote:

>>> Hi Marc,

>>>

>>> On 2018/11/23 17:10, Marc Zyngier wrote:

>>>> On 23/11/2018 01:25, Hanjun Guo wrote:

>>>>> On 2018/10/31 22:04, David Long wrote:

>>>>>> From: "David A. Long" <dave.long@linaro.org>

>>>>>>

>>>>>> V4.4 backport of spectre patches from Russell M. King's spectre branch.

>>>>>> Most KVM patches are excluded. Patches not yet in upstream are excluded.

>>>>>

>>>>> I tested this patch set on top of stable 4.4 kernel, running on boards with

>>>>> A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

>>>>> regressions in our CI system,

>>>>>

>>>>> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

>>>>>

>>>>> Since this patch set didn't include PSCI based hardening for arm32, so

>>>>> bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

>>>>> cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

>>>>> set is in a good shape I think. So what's the plan for this patch set?

>>>>

>>>> Well, not having these patches means that a 32bit kernel won't be get

>>>> any Spectre-v2 mitigation when run as a guest on an arm64 platform. It

>>>> turns out that this is a pretty common setup among people building large

>>>> pieces of SW, such as distributions.

>>>

>>> I almost miss this point, that makes sense to me :)

>>>

> 

> I've been watching arm32 spectre patches appear since September and I have

> a work item to backport these too in the near future.  I've been trying

> to focus on backporting 64-bit security patches to v4.4 in the shortterm though.

It's great, I'm happy to test your patches, please cc me for next version.

Thanks
Hanjun
David Long Dec. 14, 2018, 5:23 a.m. UTC | #8
On 11/23/18 6:09 AM, Marc Zyngier wrote:
> Hi Hanjun,

> 

> On 23/11/2018 09:40, Hanjun Guo wrote:

>> Hi Marc,

>>

>> On 2018/11/23 17:10, Marc Zyngier wrote:

>>> On 23/11/2018 01:25, Hanjun Guo wrote:

>>>> On 2018/10/31 22:04, David Long wrote:

>>>>> From: "David A. Long" <dave.long@linaro.org>

>>>>>

>>>>> V4.4 backport of spectre patches from Russell M. King's spectre branch.

>>>>> Most KVM patches are excluded. Patches not yet in upstream are excluded.

>>>>

>>>> I tested this patch set on top of stable 4.4 kernel, running on boards with

>>>> A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

>>>> regressions in our CI system,

>>>>

>>>> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

>>>>

>>>> Since this patch set didn't include PSCI based hardening for arm32, so

>>>> bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

>>>> cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

>>>> set is in a good shape I think. So what's the plan for this patch set?

>>>

>>> Well, not having these patches means that a 32bit kernel won't be get

>>> any Spectre-v2 mitigation when run as a guest on an arm64 platform. It

>>> turns out that this is a pretty common setup among people building large

>>> pieces of SW, such as distributions.

>>

>> I almost miss this point, that makes sense to me :)

>>

>>>

>>> Not having KVM host mitigation on 32bit ARM is probably OK (let's face

>>> it, I'm the only user), but not mitigating it as a guest doesn't seem

>>> completely OK to me.

>>

>> We are working on a patch set which is backported from mainline to fix

>> ARM64 spectre-v1, spectre-v2 and SSBD for stable 4.4 kernel, and that

>> patch set (almost done) has PSCI patches which is needed by 32bit ARM,

>> so how about posting those ARM64 spectre fixes then backport all those

>> kvm patches for 32bit ARM spectre fix as well?

> I'm not sure I get what you mean by PSCI. PSCI is not involved in the

> Spectre-v2 mitigation, as we use a specially designed SMC call, relying

> on the SMCCC 1.1 infrastructure. Maybe it is what you're referring to here?

> 

> Again, I don't think it is worth the hassle backporting the KVM patches.

> What I'd like to see is the guest (and bare metal) support code that

> uses the ARCH_WORKAROUND_1 SMCCC 1.1 infrastructure.

> 

> I also don't think it is worth creating an artificial dependency between

> the two architectures. Yes, some patches are common (the SMCCC

> infrastructure), but that can be easily be solved at merge time. My vote

> would be for David to carry the relevant patches in this series.

> 

> Thanks,

> 

> 	M.

> 


Marc,

Sorry to be slow in getting back to you on this.

As I've been looking at the six or so virtualization-related patches I 
excluded from the backports for less ancient release streams, for the 
v4.4 stream, I'm having a hard time believing you want the "KVM" patches 
left out.  Just their subject lines sure make them sound like they would 
have the guest impact you are worried about.  Here's the ones that worry 
me from the v4.9 backport:

[PATCH 4.9 11/24] ARM: KVM: invalidate BTB on guest exit for Cortex-A12/A17
[PATCH 4.9 12/24] ARM: KVM: invalidate icache on guest exit for Cortex-A15
[PATCH 4.9 13/24] ARM: spectre-v2: KVM: invalidate icache on guest exit 
for Brahma B15

Are these really not interesting for v4.4, or am I misunderstanding 
which patches you meant?

Thanks,
-dl
Russell King (Oracle) Dec. 14, 2018, 4:37 p.m. UTC | #9
On Fri, Dec 14, 2018 at 12:23:48AM -0500, David Long wrote:
> On 11/23/18 6:09 AM, Marc Zyngier wrote:

> >Hi Hanjun,

> >

> >On 23/11/2018 09:40, Hanjun Guo wrote:

> >>Hi Marc,

> >>

> >>On 2018/11/23 17:10, Marc Zyngier wrote:

> >>>On 23/11/2018 01:25, Hanjun Guo wrote:

> >>>>On 2018/10/31 22:04, David Long wrote:

> >>>>>From: "David A. Long" <dave.long@linaro.org>

> >>>>>

> >>>>>V4.4 backport of spectre patches from Russell M. King's spectre branch.

> >>>>>Most KVM patches are excluded. Patches not yet in upstream are excluded.

> >>>>

> >>>>I tested this patch set on top of stable 4.4 kernel, running on boards with

> >>>>A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

> >>>>regressions in our CI system,

> >>>>

> >>>>Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

> >>>>

> >>>>Since this patch set didn't include PSCI based hardening for arm32, so

> >>>>bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

> >>>>cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

> >>>>set is in a good shape I think. So what's the plan for this patch set?

> >>>

> >>>Well, not having these patches means that a 32bit kernel won't be get

> >>>any Spectre-v2 mitigation when run as a guest on an arm64 platform. It

> >>>turns out that this is a pretty common setup among people building large

> >>>pieces of SW, such as distributions.

> >>

> >>I almost miss this point, that makes sense to me :)

> >>

> >>>

> >>>Not having KVM host mitigation on 32bit ARM is probably OK (let's face

> >>>it, I'm the only user), but not mitigating it as a guest doesn't seem

> >>>completely OK to me.

> >>

> >>We are working on a patch set which is backported from mainline to fix

> >>ARM64 spectre-v1, spectre-v2 and SSBD for stable 4.4 kernel, and that

> >>patch set (almost done) has PSCI patches which is needed by 32bit ARM,

> >>so how about posting those ARM64 spectre fixes then backport all those

> >>kvm patches for 32bit ARM spectre fix as well?

> >I'm not sure I get what you mean by PSCI. PSCI is not involved in the

> >Spectre-v2 mitigation, as we use a specially designed SMC call, relying

> >on the SMCCC 1.1 infrastructure. Maybe it is what you're referring to here?

> >

> >Again, I don't think it is worth the hassle backporting the KVM patches.

> >What I'd like to see is the guest (and bare metal) support code that

> >uses the ARCH_WORKAROUND_1 SMCCC 1.1 infrastructure.

> >

> >I also don't think it is worth creating an artificial dependency between

> >the two architectures. Yes, some patches are common (the SMCCC

> >infrastructure), but that can be easily be solved at merge time. My vote

> >would be for David to carry the relevant patches in this series.

> >

> >Thanks,

> >

> >	M.

> >

> 

> Marc,

> 

> Sorry to be slow in getting back to you on this.

> 

> As I've been looking at the six or so virtualization-related patches I

> excluded from the backports for less ancient release streams, for the v4.4

> stream, I'm having a hard time believing you want the "KVM" patches left

> out.  Just their subject lines sure make them sound like they would have the

> guest impact you are worried about.  Here's the ones that worry me from the

> v4.9 backport:

> 

> [PATCH 4.9 11/24] ARM: KVM: invalidate BTB on guest exit for Cortex-A12/A17

> [PATCH 4.9 12/24] ARM: KVM: invalidate icache on guest exit for Cortex-A15

> [PATCH 4.9 13/24] ARM: spectre-v2: KVM: invalidate icache on guest exit for

> Brahma B15

> 

> Are these really not interesting for v4.4, or am I misunderstanding which

> patches you meant?


I'm getting the impression that this has completely de-railed the
backporting effort.

I'm also wondering if this is actually a good idea.  The 4.4 and 4.9
32-bit kernels implement a particularly simple hypervisor, where the
hypervisor is nothing more than:

__hyp_stub_do_trap:
        cmp     r0, #-1
        mrceq   p15, 4, r0, c12, c0, 0  @ get HVBAR
        mcrne   p15, 4, r0, c12, c0, 0  @ set HVBAR
        __ERET

Would it not be sane to assume that a v4.4 host kernel would support
a v4.4 guest kernel under virtualisation?  Since this same code is in
v4.9, the same is true there (it got changed in v4.12).

If we backport the SMCCC_ARCH_WORKAROUND_1 bits, we end up with an
incompatibility between the hypervisor code in v4.4 and the guest
kernel code - the guest kernel will attempt to make a hypervisor call
with r0=SMCCC_ARCH_WORKAROUND_1, which is 0x80008000.  This will end
up setting the HVBAR to that value, which is clearly not intended,
which will end up pointing the hypervisor vectors to that address.

Of course, the same will be true if we run a pre-4.12 host kernel
under a post-4.12 kernel with the SMCCC_ARCH_WORKAROUND_1 hack.

I forget whether the stable kernels picked up on the changes for this
hypervisor or not - if not, it isn't a trivial "just make the guest
use the SMCCC_ARCH_WORKAROUND_1 call."

I think Marc's of the opinion that he's the only one who runs kernels
under a 32-bit host kernel - how sure can we be that no one out there
other than Marc does this?

Apart from that, I'm getting very concerned about the amount of time
the backporting is taking - there are about 40 patches in all, and I
believe only around half that have so far been backported to any of
the stable kernels.  We seem to be hung-up on dealing with v4.4 that
other stable kernels aren't getting the other fixes backported.

We've seen the stable people attempt to pick up patches from the series
that make no sense on their own, because the real Spectre fixes don't
apply because of previous Spectre patches that are missing.

All the time, we have people using the stable kernels without Spectre
mitigation in place - and Spectre will have been known about for a
year next month.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
Marc Zyngier Dec. 17, 2018, 10:23 a.m. UTC | #10
On 14/12/2018 16:37, Russell King - ARM Linux wrote:
> On Fri, Dec 14, 2018 at 12:23:48AM -0500, David Long wrote:

>> On 11/23/18 6:09 AM, Marc Zyngier wrote:

>>> Hi Hanjun,

>>>

>>> On 23/11/2018 09:40, Hanjun Guo wrote:

>>>> Hi Marc,

>>>>

>>>> On 2018/11/23 17:10, Marc Zyngier wrote:

>>>>> On 23/11/2018 01:25, Hanjun Guo wrote:

>>>>>> On 2018/10/31 22:04, David Long wrote:

>>>>>>> From: "David A. Long" <dave.long@linaro.org>

>>>>>>>

>>>>>>> V4.4 backport of spectre patches from Russell M. King's spectre branch.

>>>>>>> Most KVM patches are excluded. Patches not yet in upstream are excluded.

>>>>>>

>>>>>> I tested this patch set on top of stable 4.4 kernel, running on boards with

>>>>>> A9 and A15 based Hisilicon SoCs, didn't see boot regression and other function

>>>>>> regressions in our CI system,

>>>>>>

>>>>>> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

>>>>>>

>>>>>> Since this patch set didn't include PSCI based hardening for arm32, so

>>>>>> bugfix 6282e916f774 ("ARM: 8809/1: proc-v7: fix Thumb annotation of

>>>>>> cpu_v7_hvc_switch_mm") is not needed for this patch set and this patch

>>>>>> set is in a good shape I think. So what's the plan for this patch set?

>>>>>

>>>>> Well, not having these patches means that a 32bit kernel won't be get

>>>>> any Spectre-v2 mitigation when run as a guest on an arm64 platform. It

>>>>> turns out that this is a pretty common setup among people building large

>>>>> pieces of SW, such as distributions.

>>>>

>>>> I almost miss this point, that makes sense to me :)

>>>>

>>>>>

>>>>> Not having KVM host mitigation on 32bit ARM is probably OK (let's face

>>>>> it, I'm the only user), but not mitigating it as a guest doesn't seem

>>>>> completely OK to me.

>>>>

>>>> We are working on a patch set which is backported from mainline to fix

>>>> ARM64 spectre-v1, spectre-v2 and SSBD for stable 4.4 kernel, and that

>>>> patch set (almost done) has PSCI patches which is needed by 32bit ARM,

>>>> so how about posting those ARM64 spectre fixes then backport all those

>>>> kvm patches for 32bit ARM spectre fix as well?

>>> I'm not sure I get what you mean by PSCI. PSCI is not involved in the

>>> Spectre-v2 mitigation, as we use a specially designed SMC call, relying

>>> on the SMCCC 1.1 infrastructure. Maybe it is what you're referring to here?

>>>

>>> Again, I don't think it is worth the hassle backporting the KVM patches.

>>> What I'd like to see is the guest (and bare metal) support code that

>>> uses the ARCH_WORKAROUND_1 SMCCC 1.1 infrastructure.

>>>

>>> I also don't think it is worth creating an artificial dependency between

>>> the two architectures. Yes, some patches are common (the SMCCC

>>> infrastructure), but that can be easily be solved at merge time. My vote

>>> would be for David to carry the relevant patches in this series.

>>>

>>> Thanks,

>>>

>>> 	M.

>>>

>>

>> Marc,

>>

>> Sorry to be slow in getting back to you on this.

>>

>> As I've been looking at the six or so virtualization-related patches I

>> excluded from the backports for less ancient release streams, for the v4.4

>> stream, I'm having a hard time believing you want the "KVM" patches left

>> out.  Just their subject lines sure make them sound like they would have the

>> guest impact you are worried about.  Here's the ones that worry me from the

>> v4.9 backport:

>>

>> [PATCH 4.9 11/24] ARM: KVM: invalidate BTB on guest exit for Cortex-A12/A17

>> [PATCH 4.9 12/24] ARM: KVM: invalidate icache on guest exit for Cortex-A15

>> [PATCH 4.9 13/24] ARM: spectre-v2: KVM: invalidate icache on guest exit for

>> Brahma B15

>>

>> Are these really not interesting for v4.4, or am I misunderstanding which

>> patches you meant?

> 

> I'm getting the impression that this has completely de-railed the

> backporting effort.

> 

> I'm also wondering if this is actually a good idea.  The 4.4 and 4.9

> 32-bit kernels implement a particularly simple hypervisor, where the

> hypervisor is nothing more than:

> 

> __hyp_stub_do_trap:

>         cmp     r0, #-1

>         mrceq   p15, 4, r0, c12, c0, 0  @ get HVBAR

>         mcrne   p15, 4, r0, c12, c0, 0  @ set HVBAR

>         __ERET


Hypervisor? More like a glorified exception handler, and not something
that can be reached by a guest.

> Would it not be sane to assume that a v4.4 host kernel would support

> a v4.4 guest kernel under virtualisation?  Since this same code is in

> v4.9, the same is true there (it got changed in v4.12).

> 

> If we backport the SMCCC_ARCH_WORKAROUND_1 bits, we end up with an

> incompatibility between the hypervisor code in v4.4 and the guest

> kernel code - the guest kernel will attempt to make a hypervisor call

> with r0=SMCCC_ARCH_WORKAROUND_1, which is 0x80008000.  This will end

> up setting the HVBAR to that value, which is clearly not intended,

> which will end up pointing the hypervisor vectors to that address.


I don't get it. How does the guest issues an HVC if you don't have KVM
installed and running? How does it reach it when KVM is in use?

> Of course, the same will be true if we run a pre-4.12 host kernel

> under a post-4.12 kernel with the SMCCC_ARCH_WORKAROUND_1 hack.

> 

> I forget whether the stable kernels picked up on the changes for this

> hypervisor or not - if not, it isn't a trivial "just make the guest

> use the SMCCC_ARCH_WORKAROUND_1 call."

> 

> I think Marc's of the opinion that he's the only one who runs kernels

> under a 32-bit host kernel - how sure can we be that no one out there

> other than Marc does this?


I'm quite curious to find out. During the 4.19 dev cycle, a KVM-enabled
32bit host kernel would die at boot-time. This was only caught at -rc6.
At that stage, I'm wondering whether it is worth the effort.

> Apart from that, I'm getting very concerned about the amount of time

> the backporting is taking - there are about 40 patches in all, and I

> believe only around half that have so far been backported to any of

> the stable kernels.  We seem to be hung-up on dealing with v4.4 that

> other stable kernels aren't getting the other fixes backported.

> 

> We've seen the stable people attempt to pick up patches from the series

> that make no sense on their own, because the real Spectre fixes don't

> apply because of previous Spectre patches that are missing.

> 

> All the time, we have people using the stable kernels without Spectre

> mitigation in place - and Spectre will have been known about for a

> year next month.


If you want a reduced scope to the mitigation on older kernels, this is
your call.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...