Re: [Qemu-devel] [PULL 12/62] target/ppc/spapr: Enable mitigations by default for pseries-4.0 machine type

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

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 <sjitindarsingh@gmail.com>
>>>
>>> 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 <sjitindarsingh@gmail.com>
>>> Message-Id: <20190301044609.9626-3-sjitindarsingh@gmail.com>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>> ---
>>>  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 <groug@kaod.org>
> 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 <groug@kaod.org>
>     Message-Id: <155327782604.1283071.10640596307206921951.stgit@bahia.lan>
>     Tested-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>     Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

So this commit misses:

Fixes: 2782ad4c4102d

This kind of hint is very helpful for post-merge reviews.

Thanks Laurent for your analysis :)