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 <c...@kaod.org>
>
> 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);
>
