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@xxxxxxxxxx>
>>>>>>>
>>>>>>> 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@xxxxxxxxxx>
>>>>>>
>>>>>> 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...
