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

[Laurent Vivier <lvivier@redhat.com>]

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] 


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>

Thanks,
Laurent