Re: [Qemu-ppc] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[Cédric Le Goater]

On 5/16/19 3:24 AM, David Gibson wrote:
> On Wed, May 15, 2019 at 07:04:24PM +0200, Greg Kurz wrote:
>> If a machine is started with ic-mode=xive but the guest only knows
>> about XICS, eg. an RHEL 7.6 guest, the kernel panics. This is
>> expected but a bit unfortunate since the crash doesn't provide
>> much information for the end user to guess what's happening.
>>
>> Detect that during CAS and exit QEMU with a proper error message
>> instead, like it is already done for the MMU.
>>
>> Even if this is less likely to happen, the opposite case of a guest
>> that only knows about XIVE would certainly fail all the same if the
>> machine is started with ic-mode=xics.
>>
>> Also, the only valid values a guest can pass in byte 23 of OV5 during
>> CAS are 0b00 (XIVE legacy mode) and 0b01 (XIVE exploitation mode). Any
>> other value is a bug, at least with the current spec. Again, it does
>> not seem right to let the guest go on without a precise idea of the
>> interrupt mode it asked for.
>>
>> Handle these cases as well.
>>
>> Reported-by: Satheesh Rajendran <address@hidden>
>> Signed-off-by: Greg Kurz <address@hidden>
> 
> Seems sensible to me, applied.

yes. I would have splitted the patch though. But this is minor.

Thanks,

C. 


>> ---
>>  hw/ppc/spapr_hcall.c |   24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 6c16d2b12040..63a55614b83d 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1513,6 +1513,7 @@ static target_ulong 
>> h_client_architecture_support(PowerPCCPU *cpu,
>>      bool guest_radix;
>>      Error *local_err = NULL;
>>      bool raw_mode_supported = false;
>> +    bool guest_xive;
>>  
>>      cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, 
>> &local_err);
>>      if (local_err) {
>> @@ -1545,10 +1546,17 @@ static target_ulong 
>> h_client_architecture_support(PowerPCCPU *cpu,
>>          error_report("guest requested hash and radix MMU, which is 
>> invalid.");
>>          exit(EXIT_FAILURE);
>>      }
>> +    if (spapr_ovec_test(ov5_guest, OV5_XIVE_BOTH)) {
>> +        error_report("guest requested an invalid interrupt mode");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>>      /* The radix/hash bit in byte 24 requires special handling: */
>>      guest_radix = spapr_ovec_test(ov5_guest, OV5_MMU_RADIX_300);
>>      spapr_ovec_clear(ov5_guest, OV5_MMU_RADIX_300);
>>  
>> +    guest_xive = spapr_ovec_test(ov5_guest, OV5_XIVE_EXPLOIT);
>> +
>>      /*
>>       * HPT resizing is a bit of a special case, because when enabled
>>       * we assume an HPT guest will support it until it says it
>> @@ -1632,6 +1640,22 @@ static target_ulong 
>> h_client_architecture_support(PowerPCCPU *cpu,
>>                                            ov5_updates) != 0);
>>      }
>>  
>> +    /*
>> +     * Ensure the guest asks for an interrupt mode we support; otherwise
>> +     * terminate the boot.
>> +     */
>> +    if (guest_xive) {
>> +        if (spapr->irq->ov5 == SPAPR_OV5_XIVE_LEGACY) {
>> +            error_report("Guest requested unavailable interrupt mode 
>> (XIVE)");
>> +            exit(EXIT_FAILURE);
>> +        }
>> +    } else {
>> +        if (spapr->irq->ov5 == SPAPR_OV5_XIVE_EXPLOIT) {
>> +            error_report("Guest requested unavailable interrupt mode 
>> (XICS)");
>> +            exit(EXIT_FAILURE);
>> +        }
>> +    }
>> +
>>      /*
>>       * Generate a machine reset when we have an update of the
>>       * interrupt mode. Only required when the machine supports both
>>
>