[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 17/19] target/ppc: Add POWER9 external interrupt m
|
From: |
Cédric Le Goater |
|
Subject: |
Re: [Qemu-ppc] [PATCH 17/19] target/ppc: Add POWER9 external interrupt model |
|
Date: |
Fri, 15 Feb 2019 16:43:30 +0100 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
On 2/13/19 6:16 AM, David Gibson wrote:
> On Mon, Jan 28, 2019 at 10:46:23AM +0100, Cédric Le Goater wrote:
>> From: Benjamin Herrenschmidt <address@hidden>
>>
>> Adds support for the Hypervisor directed interrupts in addition to the
>> OS ones.
>>
>> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>> include/hw/ppc/ppc.h | 2 ++
>> target/ppc/cpu-qom.h | 2 ++
>> target/ppc/cpu.h | 7 +++++++
>> hw/intc/xics.c | 1 +
>> hw/intc/xive.c | 2 +-
>> hw/ppc/ppc.c | 14 ++++++++++++++
>> target/ppc/translate_init.inc.c | 4 ++--
>> 7 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
>> index daaa04a22dbf..4bdcb8bacd4e 100644
>> --- a/include/hw/ppc/ppc.h
>> +++ b/include/hw/ppc/ppc.h
>> @@ -74,6 +74,7 @@ static inline void ppc40x_irq_init(PowerPCCPU *cpu) {}
>> static inline void ppc6xx_irq_init(PowerPCCPU *cpu) {}
>> static inline void ppc970_irq_init(PowerPCCPU *cpu) {}
>> static inline void ppcPOWER7_irq_init(PowerPCCPU *cpu) {}
>> +static inline void ppcPOWER9_irq_init(PowerPCCPU *cpu) {}
>> static inline void ppce500_irq_init(PowerPCCPU *cpu) {}
>> #else
>> void ppc40x_irq_init(PowerPCCPU *cpu);
>> @@ -81,6 +82,7 @@ void ppce500_irq_init(PowerPCCPU *cpu);
>> void ppc6xx_irq_init(PowerPCCPU *cpu);
>> void ppc970_irq_init(PowerPCCPU *cpu);
>> void ppcPOWER7_irq_init(PowerPCCPU *cpu);
>> +void ppcPOWER9_irq_init(PowerPCCPU *cpu);
>> #endif
>>
>> /* PPC machines for OpenBIOS */
>> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
>> index 7ff8b2d68632..079fbb9d8718 100644
>> --- a/target/ppc/cpu-qom.h
>> +++ b/target/ppc/cpu-qom.h
>> @@ -142,6 +142,8 @@ enum powerpc_input_t {
>> PPC_FLAGS_INPUT_970,
>> /* PowerPC POWER7 bus */
>> PPC_FLAGS_INPUT_POWER7,
>> + /* PowerPC POWER9 bus */
>> + PPC_FLAGS_INPUT_POWER9,
>> /* PowerPC 401 bus */
>> PPC_FLAGS_INPUT_401,
>> /* Freescale RCPU bus */
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index 385d33bd37ff..cc41ae6f3017 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -2322,6 +2322,13 @@ enum {
>> * them */
>> POWER7_INPUT_NB,
>> };
>> +
>> +enum {
>> + /* POWER9 input pins */
>> + POWER9_INPUT_INT = 0, /* Must match POWER7_INPUT_INT */
>
> Rather than having this vital comment here...
>
>> + POWER9_INPUT_HINT = 1,
>> + POWER9_INPUT_NB,
>> +};
>> #endif
>>
>> /* Hardware exceptions definitions */
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index 16e8ffa2aaf7..1f786175168f 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -342,6 +342,7 @@ static void icp_realize(DeviceState *dev, Error **errp)
>> env = &cpu->env;
>> switch (PPC_INPUT(env)) {
>> case PPC_FLAGS_INPUT_POWER7:
>> + case PPC_FLAGS_INPUT_POWER9: /* For SPAPR xics emulation */
>
> .. you could just split the cases here. It's a little more verbose,
> but it's more robust.
Yes clearly. This is much better.
Thanks,
C.
>> icp->output = env->irq_inputs[POWER7_INPUT_INT];
>> break;
>>
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 0e0e1dc9c1b7..119bb02d345d 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -553,7 +553,7 @@ static void xive_tctx_realize(DeviceState *dev, Error
>> **errp)
>>
>> env = &cpu->env;
>> switch (PPC_INPUT(env)) {
>> - case PPC_FLAGS_INPUT_POWER7:
>> + case PPC_FLAGS_INPUT_POWER9:
>
> And here, of course.
>
>> tctx->output = env->irq_inputs[POWER7_INPUT_INT];
>> break;
>>
>> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
>> index 608405f6f2ca..1a5e087c9b1e 100644
>> --- a/hw/ppc/ppc.c
>> +++ b/hw/ppc/ppc.c
>> @@ -289,6 +289,12 @@ static void power7_set_irq(void *opaque, int pin, int
>> level)
>> __func__, level);
>> ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>> break;
>> + case POWER9_INPUT_HINT:
>
> Having a POWER9 specific case in a function named for power7 is pretty
> odd. Best to split these as well, I think.
>
>> + /* Level sensitive - active high */
>> + LOG_IRQ("%s: set the external IRQ state to %d\n",
>> + __func__, level);
>> + ppc_set_irq(cpu, PPC_INTERRUPT_HVIRT, level);
>> + break;
>> default:
>> /* Unknown pin - do nothing */
>> LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
>> @@ -308,6 +314,14 @@ void ppcPOWER7_irq_init(PowerPCCPU *cpu)
>> env->irq_inputs = (void **)qemu_allocate_irqs(&power7_set_irq, cpu,
>> POWER7_INPUT_NB);
>> }
>> +
>> +void ppcPOWER9_irq_init(PowerPCCPU *cpu)
>> +{
>> + CPUPPCState *env = &cpu->env;
>> +
>> + env->irq_inputs = (void **)qemu_allocate_irqs(&power7_set_irq, cpu,
>> + POWER9_INPUT_NB);
>> +}
>> #endif /* defined(TARGET_PPC64) */
>>
>> /* PowerPC 40x internal IRQ controller */
>> diff --git a/target/ppc/translate_init.inc.c
>> b/target/ppc/translate_init.inc.c
>> index 6ffa4a8fe0fa..7f25215f0192 100644
>> --- a/target/ppc/translate_init.inc.c
>> +++ b/target/ppc/translate_init.inc.c
>> @@ -8793,7 +8793,7 @@ static void init_proc_POWER9(CPUPPCState *env)
>>
>> /* Allocate hardware IRQ controller */
>> init_excp_POWER9(env);
>> - ppcPOWER7_irq_init(ppc_env_get_cpu(env));
>> + ppcPOWER9_irq_init(ppc_env_get_cpu(env));
>> }
>>
>> static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr)
>> @@ -8920,7 +8920,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>> pcc->radix_page_info = &POWER9_radix_page_info;
>> #endif
>> pcc->excp_model = POWERPC_EXCP_POWER9;
>> - pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
>> + pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>> pcc->bfd_mach = bfd_mach_ppc64;
>> pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>> POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>