Re: [Qemu-ppc] [RFC PATCH] spapr/irq: force XICS interrupt mode on non P9 machines

[Cédric Le Goater]

On 3/18/19 2:52 AM, David Gibson wrote:
> On Sun, Mar 17, 2019 at 09:33:42PM +0100, Cédric Le Goater wrote:
>> There is no need to propose the 'dual' interrupt mode interrupt device
>> on POWER7/8 machines and the XIVE mode will not operate. Simply force
>> XICS in this case.
>>
>> This makes the check in spapr_machine_init() redundant on XIVE-only
>> machines.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
> 
> This is not my preferred approach.  If the user explicitly selects
> xive or dual mode with a POWER8 cpu, we should hard error, rather than
> forcing a different mode from the one requested.

I though so.

> We do need to make sure we default to xics mode with POWER8, even on
> new machine types.

That's the problem. 

When parsing the options, in the machine instance_init(), we don't 
know the CPU type. 

C. 

>> ---
>>  hw/ppc/spapr_irq.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index f2ca1bb66c9d..d27ae68915a1 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -16,6 +16,7 @@
>>  #include "hw/ppc/spapr_xive.h"
>>  #include "hw/ppc/xics.h"
>>  #include "hw/ppc/xics_spapr.h"
>> +#include "cpu-models.h"
>>  #include "sysemu/kvm.h"
>>  
>>  #include "trace.h"
>> @@ -655,6 +656,7 @@ SpaprIrq spapr_irq_dual = {
>>  void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>>  {
>>      MachineState *machine = MACHINE(spapr);
>> +    Error *local_err = NULL;
>>  
>>      if (machine_kernel_irqchip_split(machine)) {
>>          error_setg(errp, "kernel_irqchip split mode not supported on 
>> pseries");
>> @@ -667,6 +669,14 @@ void spapr_irq_init(SpaprMachineState *spapr, Error 
>> **errp)
>>          return;
>>      }
>>  
>> +    /* Force XICS on non P9 machines */
>> +    if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
>> +                              0, spapr->max_compat_pvr)) {
>> +        error_setg(&local_err, "forcing XICS interrupt controller");
>> +        warn_report_err(local_err);
>> +        spapr->irq = &spapr_irq_xics;
>> +    }
>> +
>>      /* Initialize the MSI IRQ allocator. */
>>      if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
>>          spapr_irq_msi_init(spapr, spapr->irq->nr_msis);
>