[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PULL 12/62] target/ppc/spapr: Enable mitiga
|
From: |
Philippe Mathieu-Daudé |
|
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PULL 12/62] target/ppc/spapr: Enable mitigations by default for pseries-4.0 machine type |
|
Date: |
Fri, 28 Jun 2019 16:28:26 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
On 6/28/19 4:14 PM, Laurent Vivier wrote:
> On 28/06/2019 13:27, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> On 3/12/19 9:54 AM, David Gibson wrote:
>>> From: Suraj Jitindar Singh <address@hidden>
>>>
>>> There are currently 3 mitigations the availability of which is controlled
>>> by the spapr-caps mechanism, cap-cfpc, cap-sbbc, and cap-ibs. Enable these
>>> mitigations by default for the pseries-4.0 machine type.
>>>
>>> By now machine firmware should have been upgraded to allow these
>>> settings.
>>>
>>> Signed-off-by: Suraj Jitindar Singh <address@hidden>
>>> Message-Id: <address@hidden>
>>> Signed-off-by: David Gibson <address@hidden>
>>> ---
>>> hw/ppc/spapr.c | 9 ++++++---
>>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 37fd7a1411..946bbcf9ee 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -4307,9 +4307,9 @@ static void spapr_machine_class_init(ObjectClass *oc,
>>> void *data)
>>> smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
>>> smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
>>> smc->default_caps.caps[SPAPR_CAP_DFP] = SPAPR_CAP_ON;
>>> - smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
>>> - smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
>>> - smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
>>> + smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_WORKAROUND;
>>> + smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_WORKAROUND;
>>> + smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_WORKAROUND;
>>> smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = 16; /* 64kiB */
>>> smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
>>> smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
>>> @@ -4389,6 +4389,9 @@ static void
>>> spapr_machine_3_1_class_options(MachineClass *mc)
>>> mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
>>> smc->update_dt_enabled = false;
>>> smc->dr_phb_enabled = false;
>>> + smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
>>> + smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
>>> + smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
>>> smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_OFF;
>>> }
>>
>> While trying auto-bisection for LP#1834613 [*] I found this commit break
>> clean bisection.
>>
>> ./configure --enable-debug
>>
>> $ qemu-system-ppc64 \
>> -kernel vmlinuz-vanilla \
>> -nographic -serial null
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-cfpc=workaround
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-sbbc=workaround
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-ibs=workaround
>> Opcode 13 10 10 00 (4c400420) leaked temporaries
>>
>> More verbose log:
>>
>> $ qemu-system-ppc64 \
>> -kernel vmlinuz-vanilla \
>> -nographic -append "console=hvc0" \
>> -d guest_errors,in_asm
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-cfpc=workaround
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-sbbc=workaround
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-ibs=workaround
>>
>>
>> SLOF **********************************************************************
>> QEMU Starting
>> Build Date = Jan 14 2019 18:00:39
>> FW Version = git-a5b428e1c1eae703
>> Press "s" to enter Open Firmware.
>> [...]
>> --------------
>> IN: __switch_to
>> 0xc00000000001aac0: 60000000 nop
>> 0xc00000000001aac4: 7f44d378 mr r4, r26
>> 0xc00000000001aac8: 7f23cb78 mr r3, r25
>> 0xc00000000001aacc: 4bff3235 bl 0xdd00
>
> The kernel logs are:
>
> [ 0.044473] Oops: Exception in kernel mode, sig: 4 [#1]
> [ 0.044899] BE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA
> pSeries
> [ 0.045191] Modules linked in:
> [ 0.045504] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 5.1.0-rc4-00058-g582549e3fbe1-dirty #11
> [ 0.045646] NIP: c00000000000be00 LR: c00000000000e168 CTR:
> 0000000000007fff
> [ 0.045747] REGS: c0000000011bb770 TRAP: 0700 Not tainted
> (5.1.0-rc4-00058-g582549e3fbe1-dirty)
> [ 0.045808] MSR: 8000000002089032 <SF,VEC,EE,ME,IR,DR,RI> CR: 24028822
> XER: 00000000
> [ 0.045971] CFAR: c00000000000bde4 IRQMASK: 1
> [ 0.045971] GPR00: c00000000001f390 c0000000011bba00 c0000000011bf800
> c0000000010db830
> [ 0.045971] GPR04: c00000001e4041b0 0000000000000000 0000000000000000
> 00000000028a5d7a
> [ 0.045971] GPR08: 0000000000000000 0000000000007fff 0000000000000000
> fffffffffffffffd
> [ 0.045971] GPR12: 0000000024028828 c0000000013b0000 000000001dc5ff00
> 00000000011d8e18
> [ 0.045971] GPR16: 00000000011d89e0 fffffffffffffffd 000000001dc5ff00
> 0000000000000014
> [ 0.045971] GPR20: 000000001daf0000 c0000000010da4e0 000000001eef0000
> 0000000024028822
> [ 0.045971] GPR24: c0000000010db830 c00000001e4041b0 000000001eef0000
> c000000000ff5598
> [ 0.045971] GPR28: c0000000010db830 c0000000010d9d00 c00000001e402680
> c0000000010d9d00
> [ 0.046505] NIP [c00000000000be00] flush_count_cache+0x120/0x2420
> [ 0.046561] LR [c00000000000e168] ._switch+0x68/0x180
> [ 0.046696] Call Trace:
> [ 0.046865] [c0000000011bba00] [c0000000011bba90] init_stack+0x3a90/0x4000
> (unreliable)
> [ 0.046970] [c0000000011bbbe0] [c00000000001f390] .__switch_to+0x280/0x490
> [ 0.047031] [c0000000011bbc90] [c000000000b62b5c] .__schedule+0x2bc/0xae0
> [ 0.047075] [c0000000011bbd80] [c000000000b633c8] .schedule+0x48/0xb0
> [ 0.047140] [c0000000011bbdf0] [c000000000b63918]
> .schedule_preempt_disabled+0x18/0x30
> [ 0.047187] [c0000000011bbe60] [c00000000001065c] .rest_init+0xcc/0xf0
> [ 0.047233] [c0000000011bbee0] [c000000000f04584] .start_kernel+0x604/0x648
> [ 0.047276] [c0000000011bbf90] [c00000000000b260]
> start_here_common+0x1c/0x53c
> [ 0.047409] Instruction dump:
> [ 0.047647] 48000005 48000005 48000005 48000005 48000005 4800001c 60000000
> 60000000
> [ 0.047744] 60000000 60000000 60000000 60000000 <7d2803a6> 39207fff
> 7d2903a6 4c400420
> [ 0.048410] ---[ end trace 523b05d3a02887f6 ]---
> [ 0.048523]
How do you got the klogs?
> This is fixed by:
>
> commit fa200c95f7f99ce14b8af25ea0be478c722d3cec
> Author: Greg Kurz <address@hidden>
> Date: Fri Mar 22 19:03:46 2019 +0100
>
> target/ppc: Enable "decrement and test CTR" version of bcctr
>
> Even if all ISAs up to v3 indeed mention:
>
> If the "decrement and test CTR" option is specified (BO2=0), the
> instruction form is invalid.
>
> The UMs of all existing 64-bit server class processors say:
>
> If BO[2] = 0, the contents of CTR (before any update) are used as the
> target address and for the test of the contents of CTR to resolve the
> branch. The contents of the CTR are then decremented and written back
> to the CTR.
>
> The linux kernel has spectre v2 mitigation code that relies on a
> BO[2] = 0 variant of bcctr, which is now activated by default on
> spapr, even with TCG. This causes linux guests to panic with
> the default machine type under TCG.
>
> Since any CPU model can provide its own behaviour for invalid forms,
> we could possibly introduce a new instruction flag to handle this.
> In practice, since the behaviour is shared by all 64-bit server
> processors starting with 970 up to POWER9, let's reuse the
> PPC_SEGMENT_64B flag. Caveat: this may have to be fixed later if
> POWER10 introduces a different behaviour.
>
> The existing behaviour of throwing a program interrupt is kept for
> all other CPU models.
>
> Signed-off-by: Greg Kurz <address@hidden>
> Message-Id: <address@hidden>
> Tested-by: Suraj Jitindar Singh <address@hidden>
> Signed-off-by: David Gibson <address@hidden>
So this commit misses:
Fixes: 2782ad4c4102d
This kind of hint is very helpful for post-merge reviews.
Thanks Laurent for your analysis :)