On 01/06/2021 18:46, Fabiano Rosas wrote:
This patch introduces a new way to dispatch the emulated interrupts in
powerpc_excp. It leverages the QEMU object model to store the
implementations for each interrupt and link them to their identifier
from POWERPC_EXCP enum. The processor-specific code then uses this
identifier to register which interrupts it supports.
Interrupts now come out of the big switch in powerpc_excp into their
own functions:
static void ppc_intr_system_reset(<args>)
{
/*
* Interrupt code. Sets any specific registers and MSR bits.
*/
}
PPC_DEFINE_INTR(POWERPC_EXCP_RESET, system_reset, "System reset");
^This line registers the interrupt with QOM.
When we initialize the emulated processor, the correct set of
interrupts is instantiated (pretty much like we already do):
static void init_excp_POWER9(CPUPPCState *env)
{
ppc_intr_add(env, 0x00000100, POWERPC_EXCP_RESET);
(...)
}
When it comes the time to inject the interrupt:
static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
{
(...)
intr = &env->entry_points[excp];
intr->setup_regs(<args>); <-- ppc_intr_system_reset function
(...)
env->spr[srr0] = nip;
env->spr[srr1] = msr;
env->nip = intr->addr;
env->msr = new_msr;
}
Some points to notice:
- The structure for the new PPCInterrupt class object is stored
directly inside of CPUPPCState (env) so the translation code can
still access it linearly at an offset.
- Some interrupts were being registered for P7/8/9/10 but were never
implemented (i.e. not in the powerpc_excp switch statement). They
are likely never triggered. We now get the benefit of QOM warning in
such cases:
qemu-system-ppc64: missing object type 'POWERPC_EXCP_SDOOR'
qemu-system-ppc64: missing object type 'POWERPC_EXCP_HV_MAINT'
- The code currently allows for Program interrupts to be ignored and
System call interrupts to be directed to the vhyp hypercall code. I
have added an 'ignore' flag to deal with these two cases and return
early from powerpc_excp.
Signed-off-by: Fabiano Rosas <faro...@linux.ibm.com>
---
I don't see anything really wrong with the code itself, but this patch
should probably be broken up into at least 2, one for the code motion
and another for the ppc_intr'ification of the exception model.
target/ppc/cpu.h | 29 +-
target/ppc/cpu_init.c | 640 +++++++++++++++++++--------------------
target/ppc/excp_helper.c | 545 ++-------------------------------
target/ppc/interrupts.c | 638 ++++++++++++++++++++++++++++++++++++++
target/ppc/machine.c | 2 +-
target/ppc/meson.build | 1 +
target/ppc/ppc_intr.h | 55 ++++
target/ppc/translate.c | 3 +-
8 files changed, 1066 insertions(+), 847 deletions(-)
create mode 100644 target/ppc/interrupts.c
create mode 100644 target/ppc/ppc_intr.h
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b0934d9be4..012677965f 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -174,6 +174,33 @@ enum {
POWERPC_EXCP_TRAP = 0x40,
};
+typedef struct PPCInterrupt PPCInterrupt;
+typedef struct ppc_intr_args ppc_intr_args;
+typedef void (*ppc_intr_fn_t)(PowerPCCPU *cpu, PPCInterrupt *intr,
+ int excp_model, ppc_intr_args *regs,
+ bool *ignore);
+
+struct ppc_intr_args {
+ target_ulong nip;
+ target_ulong msr;
+ target_ulong new_nip;
+ target_ulong new_msr;
+ int sprn_srr0;
+ int sprn_srr1;
+ int sprn_asrr0;
+ int sprn_asrr1;
+ int lev;
+};
+
This part also has me a bit confused. Why define it first in
excp_helper.c in the last patch just to move it to here now?
+struct PPCInterrupt {
+ Object parent;
+
+ int id;
+ const char *name;
+ target_ulong addr;
+ ppc_intr_fn_t setup_regs;
+};
+
#define PPC_INPUT(env) ((env)->bus_model)
/*****************************************************************************/
@@ -1115,7 +1142,7 @@ struct CPUPPCState {
uint32_t irq_input_state;
void **irq_inputs;
- target_ulong excp_vectors[POWERPC_EXCP_NB]; /* Exception vectors */
+ PPCInterrupt entry_points[POWERPC_EXCP_NB];
target_ulong excp_prefix;
target_ulong ivor_mask;
target_ulong ivpr_mask;
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d0411e7302..d91183357d 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -46,6 +46,7 @@
#include "helper_regs.h"
#include "internal.h"
#include "spr_tcg.h"
+#include "ppc_intr.h"
/* #define PPC_DEBUG_SPR */
/* #define USE_APPLE_GDB */
@@ -2132,16 +2133,16 @@ static void register_8xx_sprs(CPUPPCState *env)
static void init_excp_4xx_real(CPUPPCState *env)
{
#if !defined(CONFIG_USER_ONLY)
- env->excp_vectors[POWERPC_EXCP_CRITICAL] = 0x00000100;
- env->excp_vectors[POWERPC_EXCP_MCHECK] = 0x00000200;
- env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
- env->excp_vectors[POWERPC_EXCP_ALIGN] = 0x00000600;
- env->excp_vectors[POWERPC_EXCP_PROGRAM] = 0x00000700;
- env->excp_vectors[POWERPC_EXCP_SYSCALL] = 0x00000C00;
- env->excp_vectors[POWERPC_EXCP_PIT] = 0x00001000;
- env->excp_vectors[POWERPC_EXCP_FIT] = 0x00001010;
- env->excp_vectors[POWERPC_EXCP_WDT] = 0x00001020;
- env->excp_vectors[POWERPC_EXCP_DEBUG] = 0x00002000;
+ ppc_intr_add(env, 0x00000100, POWERPC_EXCP_CRITICAL);
+ ppc_intr_add(env, 0x00000200, POWERPC_EXCP_MCHECK);
+ ppc_intr_add(env, 0x00000500, POWERPC_EXCP_EXTERNAL);
+ ppc_intr_add(env, 0x00000600, POWERPC_EXCP_ALIGN);
+ ppc_intr_add(env, 0x00000700, POWERPC_EXCP_PROGRAM);
+ ppc_intr_add(env, 0x00000C00, POWERPC_EXCP_SYSCALL);
+ ppc_intr_add(env, 0x00001000, POWERPC_EXCP_PIT);
+ ppc_intr_add(env, 0x00001010, POWERPC_EXCP_FIT);
+ ppc_intr_add(env, 0x00001020, POWERPC_EXCP_WDT);
+ ppc_intr_add(env, 0x00002000, POWERPC_EXCP_DEBUG);
env->ivor_mask = 0x0000FFF0UL;
env->ivpr_mask = 0xFFFF0000UL;
/* Hardware reset vector */
<snip>
@@ -2624,8 +2625,8 @@ static void init_excp_POWER9(CPUPPCState *env)
init_excp_POWER8(env);
#if !defined(CONFIG_USER_ONLY)
- env->excp_vectors[POWERPC_EXCP_HVIRT] = 0x00000EA0;
- env->excp_vectors[POWERPC_EXCP_SYSCALL_VECTORED] = 0x00017000;
+ ppc_intr_add(env, 0x00000EA0, POWERPC_EXCP_HVIRT);
+ ppc_intr_add(env, 0x00017000, POWERPC_EXCP_SYSCALL_VECTORED);
#endif
}
Not sure if this is possible, but if this bit can be done separately as
an earlier patch, it would make reviewing a lot easier.
@@ -8375,13 +8376,8 @@ static void init_ppc_proc(PowerPCCPU *cpu)
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
CPUPPCState *env = &cpu->env;
#if !defined(CONFIG_USER_ONLY)
- int i;
env->irq_inputs = NULL;
- /* Set all exception vectors to an invalid address */
- for (i = 0; i < POWERPC_EXCP_NB; i++) {
- env->excp_vectors[i] = (target_ulong)(-1ULL);
- }
We don't need to use this to set invalid values?
env->ivor_mask = 0x00000000;
env->ivpr_mask = 0x00000000;
/* Default MMU definitions */
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 12bf829c8f..26cbfab923 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -29,14 +29,6 @@
#endif
/* #define DEBUG_OP */
-/* #define DEBUG_SOFTWARE_TLB */
-/* #define DEBUG_EXCEPTIONS */
-
-#ifdef DEBUG_EXCEPTIONS
-# define LOG_EXCP(...) qemu_log(__VA_ARGS__)
-#else
-# define LOG_EXCP(...) do { } while (0)
-#endif
/*****************************************************************************/
/* Exception processing */
<snip>
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index e16a2721e2..2c82bda8cc 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -951,7 +951,8 @@ void spr_write_excp_vector(DisasContext *ctx, int sprn, int
gprn)
TCGv t0 = tcg_temp_new();
tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUPPCState, ivor_mask));
tcg_gen_and_tl(t0, t0, cpu_gpr[gprn]);
- tcg_gen_st_tl(t0, cpu_env, offsetof(CPUPPCState, excp_vectors[sprn_offs]));
+ tcg_gen_st_tl(t0, cpu_env, offsetof(CPUPPCState, entry_points[sprn_offs]) +
+ offsetof(PPCInterrupt, addr));
gen_store_spr(sprn, t0);
tcg_temp_free(t0);
}
Other than that, from what I can see, looks ok
--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
