* [RFC PATCH 00/13] PowerPC interrupt rework
@ 2022-08-15 16:20 Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 01/13] target/ppc: define PPC_INTERRUPT_* values directly Matheus Ferst
` (13 more replies)
0 siblings, 14 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Currently, PowerPC interrupts are handled as follows:
1) The CPU_INTERRUPT_HARD bit of cs->interrupt_request gates all
interrupts;
2) The bits of env->pending_interrupts identify which particular
interrupt is raised;
3) ppc_set_irq can be used to set/clear env->pending_interrupt bit and
CPU_INTERRUPT_HARD, but some places access env->pending_interrupt
directly;
4) ppc_cpu_exec_interrupt is called by cpu_handle_interrupt when
cs->interrupt_request indicates that there is some interrupt pending.
This method checks CPU_INTERRUPT_HARD and calls ppc_hw_interrupt. If
env->pending_interrupt is zero after this call, CPU_INTERRUPT_HARD
will be cleared.
5) ppc_hw_interrupt checks if there is any unmasked interrupt and calls
powerpc_excp with the appropriate POWERPC_EXCP_* value. The method
will also reset the corresponding bit in env->pending_interrupt for
interrupts that clear on delivery.
If all pending interrupts are masked, CPU_INTERRUPT_HARD will be set,
but ppc_hw_interrupt will not deliver or clear the interrupt, so
CPU_INTERRUPT_HARD will not be reset by ppc_cpu_exec_interrupt. With
that, cs->has_work keeps returning true, creating a loop that acquires
and release qemu_mutex_lock_iothread, causing the poor performance
reported in [1].
This patch series attempts to rework the PowerPC interrupt code to set
CPU_INTERRUPT_HARD only when there are unmasked interrupts. Then
cs->has_work can be simplified to a check of CPU_INTERRUPT_HARD, so it
also only returns true when at least one interrupt can be delivered.
To achieve that, we are basically following Alex Bannée's suggestion[2]
in the original thread: the interrupt masking logic will be factored
out of ppc_hw_interrupt in a new method, ppc_pending_interrupts. This
method is then used to decide if CPU_INTERRUPT_HARD should be set or
cleared after changes to MSR, LPCR, env->pending_interrupts, and
power-management instructions.
We used [3] to check for regressions at each patch in this series. After
patch 12, booting a powernv machine with a newer skiboot with "-smp 4"
goes from 1m09s to 20.79s.
[1] https://lists.gnu.org/archive/html/qemu-ppc/2022-06/msg00336.html
[2] https://lists.gnu.org/archive/html/qemu-ppc/2022-06/msg00372.html
[3] https://github.com/legoater/qemu-ppc-boot
Matheus Ferst (13):
target/ppc: define PPC_INTERRUPT_* values directly
target/ppc: always use ppc_set_irq to set env->pending_interrupts
target/ppc: move interrupt masking out of ppc_hw_interrupt
target/ppc: prepare to split ppc_interrupt_pending by excp_model
target/ppc: create a interrupt masking method for POWER9/POWER10
target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9
target/ppc: create a interrupt masking method for POWER8
target/ppc: remove unused interrupts from ppc_pending_interrupt_p8
target/ppc: create a interrupt masking method for POWER7
target/ppc: remove unused interrupts from ppc_pending_interrupt_p7
target/ppc: remove ppc_store_lpcr from CONFIG_USER_ONLY builds
target/ppc: introduce ppc_maybe_interrupt
target/ppc: unify cpu->has_work based on cs->interrupt_request
hw/ppc/ppc.c | 17 +-
hw/ppc/trace-events | 2 +-
target/ppc/cpu.c | 2 +
target/ppc/cpu.h | 43 +--
target/ppc/cpu_init.c | 212 +------------
target/ppc/excp_helper.c | 651 ++++++++++++++++++++++++++++++++-------
target/ppc/helper_regs.c | 2 +
target/ppc/misc_helper.c | 11 +-
target/ppc/translate.c | 8 +-
9 files changed, 580 insertions(+), 368 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [RFC PATCH 01/13] target/ppc: define PPC_INTERRUPT_* values directly
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-18 2:18 ` David Gibson
2022-08-15 16:20 ` [RFC PATCH 02/13] target/ppc: always use ppc_set_irq to set env->pending_interrupts Matheus Ferst
` (12 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
This enum defines the bit positions in env->pending_interrupts for each
interrupt. However, except for the comparison in kvmppc_set_interrupt,
the values are always used as (1 << PPC_INTERRUPT_*). Define them
directly like that to save some clutter. No functional change intended.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
hw/ppc/ppc.c | 10 +++---
hw/ppc/trace-events | 2 +-
target/ppc/cpu.h | 40 +++++++++++-----------
target/ppc/cpu_init.c | 56 +++++++++++++++---------------
target/ppc/excp_helper.c | 74 ++++++++++++++++++++--------------------
target/ppc/misc_helper.c | 6 ++--
6 files changed, 94 insertions(+), 94 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 690f448cb9..77e611e81c 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -40,7 +40,7 @@
static void cpu_ppc_tb_stop (CPUPPCState *env);
static void cpu_ppc_tb_start (CPUPPCState *env);
-void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
+void ppc_set_irq(PowerPCCPU *cpu, int irq, int level)
{
CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
@@ -56,21 +56,21 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
old_pending = env->pending_interrupts;
if (level) {
- env->pending_interrupts |= 1 << n_IRQ;
+ env->pending_interrupts |= irq;
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
} else {
- env->pending_interrupts &= ~(1 << n_IRQ);
+ env->pending_interrupts &= ~irq;
if (env->pending_interrupts == 0) {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
}
}
if (old_pending != env->pending_interrupts) {
- kvmppc_set_interrupt(cpu, n_IRQ, level);
+ kvmppc_set_interrupt(cpu, irq, level);
}
- trace_ppc_irq_set_exit(env, n_IRQ, level, env->pending_interrupts,
+ trace_ppc_irq_set_exit(env, irq, level, env->pending_interrupts,
CPU(cpu)->interrupt_request);
if (locked) {
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index 5c0a215cad..c9ee1285b8 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -116,7 +116,7 @@ ppc40x_set_tb_clk(uint32_t value) "new frequency %" PRIu32
ppc40x_timers_init(uint32_t value) "frequency %" PRIu32
ppc_irq_set(void *env, uint32_t pin, uint32_t level) "env [%p] pin %d level %d"
-ppc_irq_set_exit(void *env, uint32_t n_IRQ, uint32_t level, uint32_t pending, uint32_t request) "env [%p] n_IRQ %d level %d => pending 0x%08" PRIx32 " req 0x%08" PRIx32
+ppc_irq_set_exit(void *env, uint32_t irq, uint32_t level, uint32_t pending, uint32_t request) "env [%p] irq 0x%05" PRIx32 " level %d => pending 0x%08" PRIx32 " req 0x%08" PRIx32
ppc_irq_set_state(const char *name, uint32_t level) "\"%s\" level %d"
ppc_irq_reset(const char *name) "%s"
ppc_irq_cpu(const char *action) "%s"
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index a4c893cfad..c7864bb3b1 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2418,27 +2418,27 @@ enum {
/* Hardware exceptions definitions */
enum {
/* External hardware exception sources */
- PPC_INTERRUPT_RESET = 0, /* Reset exception */
- PPC_INTERRUPT_WAKEUP, /* Wakeup exception */
- PPC_INTERRUPT_MCK, /* Machine check exception */
- PPC_INTERRUPT_EXT, /* External interrupt */
- PPC_INTERRUPT_SMI, /* System management interrupt */
- PPC_INTERRUPT_CEXT, /* Critical external interrupt */
- PPC_INTERRUPT_DEBUG, /* External debug exception */
- PPC_INTERRUPT_THERM, /* Thermal exception */
+ PPC_INTERRUPT_RESET = 0x00001, /* Reset exception */
+ PPC_INTERRUPT_WAKEUP = 0x00002, /* Wakeup exception */
+ PPC_INTERRUPT_MCK = 0x00004, /* Machine check exception */
+ PPC_INTERRUPT_EXT = 0x00008, /* External interrupt */
+ PPC_INTERRUPT_SMI = 0x00010, /* System management interrupt */
+ PPC_INTERRUPT_CEXT = 0x00020, /* Critical external interrupt */
+ PPC_INTERRUPT_DEBUG = 0x00040, /* External debug exception */
+ PPC_INTERRUPT_THERM = 0x00080, /* Thermal exception */
/* Internal hardware exception sources */
- PPC_INTERRUPT_DECR, /* Decrementer exception */
- PPC_INTERRUPT_HDECR, /* Hypervisor decrementer exception */
- PPC_INTERRUPT_PIT, /* Programmable interval timer interrupt */
- PPC_INTERRUPT_FIT, /* Fixed interval timer interrupt */
- PPC_INTERRUPT_WDT, /* Watchdog timer interrupt */
- PPC_INTERRUPT_CDOORBELL, /* Critical doorbell interrupt */
- PPC_INTERRUPT_DOORBELL, /* Doorbell interrupt */
- PPC_INTERRUPT_PERFM, /* Performance monitor interrupt */
- PPC_INTERRUPT_HMI, /* Hypervisor Maintenance interrupt */
- PPC_INTERRUPT_HDOORBELL, /* Hypervisor Doorbell interrupt */
- PPC_INTERRUPT_HVIRT, /* Hypervisor virtualization interrupt */
- PPC_INTERRUPT_EBB, /* Event-based Branch exception */
+ PPC_INTERRUPT_DECR = 0x00100, /* Decrementer exception */
+ PPC_INTERRUPT_HDECR = 0x00200, /* Hypervisor decrementer exception */
+ PPC_INTERRUPT_PIT = 0x00400, /* Programmable interval timer int. */
+ PPC_INTERRUPT_FIT = 0x00800, /* Fixed interval timer interrupt */
+ PPC_INTERRUPT_WDT = 0x01000, /* Watchdog timer interrupt */
+ PPC_INTERRUPT_CDOORBELL = 0x02000, /* Critical doorbell interrupt */
+ PPC_INTERRUPT_DOORBELL = 0x04000, /* Doorbell interrupt */
+ PPC_INTERRUPT_PERFM = 0x08000, /* Performance monitor interrupt */
+ PPC_INTERRUPT_HMI = 0x10000, /* Hypervisor Maintenance interrupt */
+ PPC_INTERRUPT_HDOORBELL = 0x20000, /* Hypervisor Doorbell interrupt */
+ PPC_INTERRUPT_HVIRT = 0x40000, /* Hypervisor virtualization interrupt */
+ PPC_INTERRUPT_EBB = 0x80000, /* Event-based Branch exception */
};
/* Processor Compatibility mask (PCR) */
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d1493a660c..850334545a 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5932,23 +5932,23 @@ static bool cpu_has_work_POWER7(CPUState *cs)
if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
return false;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
(env->spr[SPR_LPCR] & LPCR_P7_PECE0)) {
return true;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DECR)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
(env->spr[SPR_LPCR] & LPCR_P7_PECE1)) {
return true;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_MCK)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_MCK) &&
(env->spr[SPR_LPCR] & LPCR_P7_PECE2)) {
return true;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HMI)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_HMI) &&
(env->spr[SPR_LPCR] & LPCR_P7_PECE2)) {
return true;
}
- if (env->pending_interrupts & (1u << PPC_INTERRUPT_RESET)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
return true;
}
return false;
@@ -6096,31 +6096,31 @@ static bool cpu_has_work_POWER8(CPUState *cs)
if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
return false;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
(env->spr[SPR_LPCR] & LPCR_P8_PECE2)) {
return true;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DECR)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
(env->spr[SPR_LPCR] & LPCR_P8_PECE3)) {
return true;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_MCK)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_MCK) &&
(env->spr[SPR_LPCR] & LPCR_P8_PECE4)) {
return true;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HMI)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_HMI) &&
(env->spr[SPR_LPCR] & LPCR_P8_PECE4)) {
return true;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DOORBELL)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
(env->spr[SPR_LPCR] & LPCR_P8_PECE0)) {
return true;
}
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HDOORBELL)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
(env->spr[SPR_LPCR] & LPCR_P8_PECE1)) {
return true;
}
- if (env->pending_interrupts & (1u << PPC_INTERRUPT_RESET)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
return true;
}
return false;
@@ -6307,7 +6307,7 @@ static bool cpu_has_work_POWER9(CPUState *cs)
return true;
}
/* External Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
(env->spr[SPR_LPCR] & LPCR_EEE)) {
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
if (!heic || !FIELD_EX64_HV(env->msr) ||
@@ -6316,31 +6316,31 @@ static bool cpu_has_work_POWER9(CPUState *cs)
}
}
/* Decrementer Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DECR)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
(env->spr[SPR_LPCR] & LPCR_DEE)) {
return true;
}
/* Machine Check or Hypervisor Maintenance Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_MCK |
- 1u << PPC_INTERRUPT_HMI)) && (env->spr[SPR_LPCR] & LPCR_OEE)) {
+ if ((env->pending_interrupts & (PPC_INTERRUPT_MCK | PPC_INTERRUPT_HMI))
+ && (env->spr[SPR_LPCR] & LPCR_OEE)) {
return true;
}
/* Privileged Doorbell Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DOORBELL)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
(env->spr[SPR_LPCR] & LPCR_PDEE)) {
return true;
}
/* Hypervisor Doorbell Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HDOORBELL)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
(env->spr[SPR_LPCR] & LPCR_HDEE)) {
return true;
}
/* Hypervisor virtualization exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HVIRT)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
(env->spr[SPR_LPCR] & LPCR_HVEE)) {
return true;
}
- if (env->pending_interrupts & (1u << PPC_INTERRUPT_RESET)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
return true;
}
return false;
@@ -6524,7 +6524,7 @@ static bool cpu_has_work_POWER10(CPUState *cs)
return true;
}
/* External Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
(env->spr[SPR_LPCR] & LPCR_EEE)) {
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
if (!heic || !FIELD_EX64_HV(env->msr) ||
@@ -6533,31 +6533,31 @@ static bool cpu_has_work_POWER10(CPUState *cs)
}
}
/* Decrementer Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DECR)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
(env->spr[SPR_LPCR] & LPCR_DEE)) {
return true;
}
/* Machine Check or Hypervisor Maintenance Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_MCK |
- 1u << PPC_INTERRUPT_HMI)) && (env->spr[SPR_LPCR] & LPCR_OEE)) {
+ if ((env->pending_interrupts & (PPC_INTERRUPT_MCK | PPC_INTERRUPT_HMI))
+ && (env->spr[SPR_LPCR] & LPCR_OEE)) {
return true;
}
/* Privileged Doorbell Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DOORBELL)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
(env->spr[SPR_LPCR] & LPCR_PDEE)) {
return true;
}
/* Hypervisor Doorbell Exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HDOORBELL)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
(env->spr[SPR_LPCR] & LPCR_HDEE)) {
return true;
}
/* Hypervisor virtualization exception */
- if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HVIRT)) &&
+ if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
(env->spr[SPR_LPCR] & LPCR_HVEE)) {
return true;
}
- if (env->pending_interrupts & (1u << PPC_INTERRUPT_RESET)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
return true;
}
return false;
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 7550aafed6..b9476b5d03 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1683,21 +1683,21 @@ static void ppc_hw_interrupt(CPUPPCState *env)
bool async_deliver;
/* External reset */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_RESET)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_RESET);
+ if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_RESET;
powerpc_excp(cpu, POWERPC_EXCP_RESET);
return;
}
/* Machine check exception */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_MCK)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_MCK);
+ if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_MCK;
powerpc_excp(cpu, POWERPC_EXCP_MCHECK);
return;
}
#if 0 /* TODO */
/* External debug exception */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_DEBUG)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DEBUG);
+ if (env->pending_interrupts & PPC_INTERRUPT_DEBUG) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_DEBUG;
powerpc_excp(cpu, POWERPC_EXCP_DEBUG);
return;
}
@@ -1712,19 +1712,19 @@ static void ppc_hw_interrupt(CPUPPCState *env)
async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
/* Hypervisor decrementer exception */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
/* LPCR will be clear when not supported so this will work */
bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
/* HDEC clears on delivery */
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDECR);
+ env->pending_interrupts &= ~PPC_INTERRUPT_HDECR;
powerpc_excp(cpu, POWERPC_EXCP_HDECR);
return;
}
}
/* Hypervisor virtualization interrupt */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_HVIRT)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
/* LPCR will be clear when not supported so this will work */
bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
@@ -1734,7 +1734,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
}
/* External interrupt can ignore MSR:EE under some circumstances */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_EXT)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
/* HEIC blocks delivery to the hypervisor */
@@ -1751,45 +1751,45 @@ static void ppc_hw_interrupt(CPUPPCState *env)
}
if (FIELD_EX64(env->msr, MSR, CE)) {
/* External critical interrupt */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_CEXT)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
powerpc_excp(cpu, POWERPC_EXCP_CRITICAL);
return;
}
}
if (async_deliver != 0) {
/* Watchdog timer on embedded PowerPC */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_WDT)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_WDT);
+ if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_WDT;
powerpc_excp(cpu, POWERPC_EXCP_WDT);
return;
}
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_CDOORBELL)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_CDOORBELL);
+ if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_CDOORBELL;
powerpc_excp(cpu, POWERPC_EXCP_DOORCI);
return;
}
/* Fixed interval timer on embedded PowerPC */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_FIT)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_FIT);
+ if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_FIT;
powerpc_excp(cpu, POWERPC_EXCP_FIT);
return;
}
/* Programmable interval timer on embedded PowerPC */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_PIT)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PIT);
+ if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_PIT;
powerpc_excp(cpu, POWERPC_EXCP_PIT);
return;
}
/* Decrementer exception */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_DECR)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
if (ppc_decr_clear_on_delivery(env)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DECR);
+ env->pending_interrupts &= ~PPC_INTERRUPT_DECR;
}
powerpc_excp(cpu, POWERPC_EXCP_DECR);
return;
}
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL);
+ if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
if (is_book3s_arch2x(env)) {
powerpc_excp(cpu, POWERPC_EXCP_SDOOR);
} else {
@@ -1797,31 +1797,31 @@ static void ppc_hw_interrupt(CPUPPCState *env)
}
return;
}
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDOORBELL);
+ if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_HDOORBELL;
powerpc_excp(cpu, POWERPC_EXCP_SDOOR_HV);
return;
}
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM);
+ if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_PERFM;
powerpc_excp(cpu, POWERPC_EXCP_PERFM);
return;
}
/* Thermal interrupt */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_THERM)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_THERM);
+ if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_THERM;
powerpc_excp(cpu, POWERPC_EXCP_THERM);
return;
}
/* EBB exception */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_EBB)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
/*
* EBB exception must be taken in problem state and
* with BESCR_GE set.
*/
if (FIELD_EX64(env->msr, MSR, PR) &&
(env->spr[SPR_BESCR] & BESCR_GE)) {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
+ env->pending_interrupts &= ~PPC_INTERRUPT_EBB;
if (env->spr[SPR_BESCR] & BESCR_PMEO) {
powerpc_excp(cpu, POWERPC_EXCP_PERFM_EBB);
@@ -2098,7 +2098,7 @@ static void do_ebb(CPUPPCState *env, int ebb_excp)
if (FIELD_EX64(env->msr, MSR, PR)) {
powerpc_excp(cpu, ebb_excp);
} else {
- env->pending_interrupts |= 1 << PPC_INTERRUPT_EBB;
+ env->pending_interrupts |= PPC_INTERRUPT_EBB;
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
}
}
@@ -2209,7 +2209,7 @@ void helper_msgclr(CPUPPCState *env, target_ulong rb)
return;
}
- env->pending_interrupts &= ~(1 << irq);
+ env->pending_interrupts &= ~irq;
}
void helper_msgsnd(target_ulong rb)
@@ -2228,7 +2228,7 @@ void helper_msgsnd(target_ulong rb)
CPUPPCState *cenv = &cpu->env;
if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
- cenv->pending_interrupts |= 1 << irq;
+ cenv->pending_interrupts |= irq;
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
}
}
@@ -2253,7 +2253,7 @@ void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb)
return;
}
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDOORBELL);
+ env->pending_interrupts &= ~PPC_INTERRUPT_HDOORBELL;
}
static void book3s_msgsnd_common(int pir, int irq)
@@ -2267,7 +2267,7 @@ static void book3s_msgsnd_common(int pir, int irq)
/* TODO: broadcast message to all threads of the same processor */
if (cenv->spr_cb[SPR_PIR].default_value == pir) {
- cenv->pending_interrupts |= 1 << irq;
+ cenv->pending_interrupts |= irq;
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
}
}
@@ -2294,7 +2294,7 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb)
return;
}
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL);
+ env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
}
/*
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index b0a5e7ce76..05e35572bc 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -163,7 +163,7 @@ target_ulong helper_load_dpdes(CPUPPCState *env)
helper_hfscr_facility_check(env, HFSCR_MSGP, "load DPDES", HFSCR_IC_MSGP);
/* TODO: TCG supports only one thread */
- if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) {
+ if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
dpdes = 1;
}
@@ -185,10 +185,10 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val)
}
if (val & 0x1) {
- env->pending_interrupts |= 1 << PPC_INTERRUPT_DOORBELL;
+ env->pending_interrupts |= PPC_INTERRUPT_DOORBELL;
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
} else {
- env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL);
+ env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
}
}
#endif /* defined(TARGET_PPC64) */
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 02/13] target/ppc: always use ppc_set_irq to set env->pending_interrupts
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 01/13] target/ppc: define PPC_INTERRUPT_* values directly Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 03/13] target/ppc: move interrupt masking out of ppc_hw_interrupt Matheus Ferst
` (11 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Use ppc_set_irq to raise/clear interrupts to ensure CPU_INTERRUPT_HARD
will be set/reset accordingly.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 17 +++++++----------
target/ppc/misc_helper.c | 9 ++-------
2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index b9476b5d03..ae9576546f 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -23,6 +23,7 @@
#include "exec/exec-all.h"
#include "internal.h"
#include "helper_regs.h"
+#include "hw/ppc/ppc.h"
#include "trace.h"
@@ -2080,7 +2081,6 @@ void helper_rfebb(CPUPPCState *env, target_ulong s)
static void do_ebb(CPUPPCState *env, int ebb_excp)
{
PowerPCCPU *cpu = env_archcpu(env);
- CPUState *cs = CPU(cpu);
/*
* FSCR_EBB and FSCR_IC_EBB are the same bits used with
@@ -2098,8 +2098,7 @@ static void do_ebb(CPUPPCState *env, int ebb_excp)
if (FIELD_EX64(env->msr, MSR, PR)) {
powerpc_excp(cpu, ebb_excp);
} else {
- env->pending_interrupts |= PPC_INTERRUPT_EBB;
- cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+ ppc_set_irq(cpu, PPC_INTERRUPT_EBB, 1);
}
}
@@ -2209,7 +2208,7 @@ void helper_msgclr(CPUPPCState *env, target_ulong rb)
return;
}
- env->pending_interrupts &= ~irq;
+ ppc_set_irq(env_archcpu(env), irq, 0);
}
void helper_msgsnd(target_ulong rb)
@@ -2228,8 +2227,7 @@ void helper_msgsnd(target_ulong rb)
CPUPPCState *cenv = &cpu->env;
if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
- cenv->pending_interrupts |= irq;
- cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+ ppc_set_irq(cpu, irq, 1);
}
}
qemu_mutex_unlock_iothread();
@@ -2253,7 +2251,7 @@ void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb)
return;
}
- env->pending_interrupts &= ~PPC_INTERRUPT_HDOORBELL;
+ ppc_set_irq(env_archcpu(env), PPC_INTERRUPT_HDOORBELL, 0);
}
static void book3s_msgsnd_common(int pir, int irq)
@@ -2267,8 +2265,7 @@ static void book3s_msgsnd_common(int pir, int irq)
/* TODO: broadcast message to all threads of the same processor */
if (cenv->spr_cb[SPR_PIR].default_value == pir) {
- cenv->pending_interrupts |= irq;
- cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+ ppc_set_irq(cpu, irq, 1);
}
}
qemu_mutex_unlock_iothread();
@@ -2294,7 +2291,7 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb)
return;
}
- env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
+ ppc_set_irq(env_archcpu(env), PPC_INTERRUPT_HDOORBELL, 0);
}
/*
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index 05e35572bc..a9bc1522e2 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -25,6 +25,7 @@
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
#include "mmu-book3s-v3.h"
+#include "hw/ppc/ppc.h"
#include "helper_regs.h"
@@ -173,7 +174,6 @@ target_ulong helper_load_dpdes(CPUPPCState *env)
void helper_store_dpdes(CPUPPCState *env, target_ulong val)
{
PowerPCCPU *cpu = env_archcpu(env);
- CPUState *cs = CPU(cpu);
helper_hfscr_facility_check(env, HFSCR_MSGP, "store DPDES", HFSCR_IC_MSGP);
@@ -184,12 +184,7 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val)
return;
}
- if (val & 0x1) {
- env->pending_interrupts |= PPC_INTERRUPT_DOORBELL;
- cpu_interrupt(cs, CPU_INTERRUPT_HARD);
- } else {
- env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
- }
+ ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, val & 0x1);
}
#endif /* defined(TARGET_PPC64) */
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 03/13] target/ppc: move interrupt masking out of ppc_hw_interrupt
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 01/13] target/ppc: define PPC_INTERRUPT_* values directly Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 02/13] target/ppc: always use ppc_set_irq to set env->pending_interrupts Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 20:09 ` Fabiano Rosas
2022-08-15 16:20 ` [RFC PATCH 04/13] target/ppc: prepare to split ppc_interrupt_pending by excp_model Matheus Ferst
` (10 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Move the interrupt masking logic to a new method, ppc_pending_interrupt,
and only handle the interrupt processing in ppc_hw_interrupt.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 228 ++++++++++++++++++++++++---------------
1 file changed, 141 insertions(+), 87 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index ae9576546f..8690017c70 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1678,29 +1678,22 @@ void ppc_cpu_do_interrupt(CPUState *cs)
powerpc_excp(cpu, cs->exception_index);
}
-static void ppc_hw_interrupt(CPUPPCState *env)
+static int ppc_pending_interrupt(CPUPPCState *env)
{
- PowerPCCPU *cpu = env_archcpu(env);
bool async_deliver;
/* External reset */
if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
- env->pending_interrupts &= ~PPC_INTERRUPT_RESET;
- powerpc_excp(cpu, POWERPC_EXCP_RESET);
- return;
+ return PPC_INTERRUPT_RESET;
}
/* Machine check exception */
if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
- env->pending_interrupts &= ~PPC_INTERRUPT_MCK;
- powerpc_excp(cpu, POWERPC_EXCP_MCHECK);
- return;
+ return PPC_INTERRUPT_MCK;
}
#if 0 /* TODO */
/* External debug exception */
if (env->pending_interrupts & PPC_INTERRUPT_DEBUG) {
- env->pending_interrupts &= ~PPC_INTERRUPT_DEBUG;
- powerpc_excp(cpu, POWERPC_EXCP_DEBUG);
- return;
+ return PPC_INTERRUPT_DEBUG;
}
#endif
@@ -1718,9 +1711,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
/* HDEC clears on delivery */
- env->pending_interrupts &= ~PPC_INTERRUPT_HDECR;
- powerpc_excp(cpu, POWERPC_EXCP_HDECR);
- return;
+ return PPC_INTERRUPT_HDECR;
}
}
@@ -1729,8 +1720,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
/* LPCR will be clear when not supported so this will work */
bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
- powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
- return;
+ return PPC_INTERRUPT_HVIRT;
}
}
@@ -1742,77 +1732,47 @@ static void ppc_hw_interrupt(CPUPPCState *env)
if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
!FIELD_EX64(env->msr, MSR, PR))) ||
(env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
- if (books_vhyp_promotes_external_to_hvirt(cpu)) {
- powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
- } else {
- powerpc_excp(cpu, POWERPC_EXCP_EXTERNAL);
- }
- return;
+ return PPC_INTERRUPT_EXT;
}
}
if (FIELD_EX64(env->msr, MSR, CE)) {
/* External critical interrupt */
if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
- powerpc_excp(cpu, POWERPC_EXCP_CRITICAL);
- return;
+ return PPC_INTERRUPT_CEXT;
}
}
if (async_deliver != 0) {
/* Watchdog timer on embedded PowerPC */
if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
- env->pending_interrupts &= ~PPC_INTERRUPT_WDT;
- powerpc_excp(cpu, POWERPC_EXCP_WDT);
- return;
+ return PPC_INTERRUPT_WDT;
}
if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
- env->pending_interrupts &= ~PPC_INTERRUPT_CDOORBELL;
- powerpc_excp(cpu, POWERPC_EXCP_DOORCI);
- return;
+ return PPC_INTERRUPT_CDOORBELL;
}
/* Fixed interval timer on embedded PowerPC */
if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
- env->pending_interrupts &= ~PPC_INTERRUPT_FIT;
- powerpc_excp(cpu, POWERPC_EXCP_FIT);
- return;
+ return PPC_INTERRUPT_FIT;
}
/* Programmable interval timer on embedded PowerPC */
if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
- env->pending_interrupts &= ~PPC_INTERRUPT_PIT;
- powerpc_excp(cpu, POWERPC_EXCP_PIT);
- return;
+ return PPC_INTERRUPT_PIT;
}
/* Decrementer exception */
if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
- if (ppc_decr_clear_on_delivery(env)) {
- env->pending_interrupts &= ~PPC_INTERRUPT_DECR;
- }
- powerpc_excp(cpu, POWERPC_EXCP_DECR);
- return;
+ return PPC_INTERRUPT_DECR;
}
if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
- env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
- if (is_book3s_arch2x(env)) {
- powerpc_excp(cpu, POWERPC_EXCP_SDOOR);
- } else {
- powerpc_excp(cpu, POWERPC_EXCP_DOORI);
- }
- return;
+ return PPC_INTERRUPT_DOORBELL;
}
if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
- env->pending_interrupts &= ~PPC_INTERRUPT_HDOORBELL;
- powerpc_excp(cpu, POWERPC_EXCP_SDOOR_HV);
- return;
+ return PPC_INTERRUPT_HDOORBELL;
}
if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
- env->pending_interrupts &= ~PPC_INTERRUPT_PERFM;
- powerpc_excp(cpu, POWERPC_EXCP_PERFM);
- return;
+ return PPC_INTERRUPT_PERFM;
}
/* Thermal interrupt */
if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
- env->pending_interrupts &= ~PPC_INTERRUPT_THERM;
- powerpc_excp(cpu, POWERPC_EXCP_THERM);
- return;
+ return PPC_INTERRUPT_THERM;
}
/* EBB exception */
if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
@@ -1822,33 +1782,104 @@ static void ppc_hw_interrupt(CPUPPCState *env)
*/
if (FIELD_EX64(env->msr, MSR, PR) &&
(env->spr[SPR_BESCR] & BESCR_GE)) {
- env->pending_interrupts &= ~PPC_INTERRUPT_EBB;
-
- if (env->spr[SPR_BESCR] & BESCR_PMEO) {
- powerpc_excp(cpu, POWERPC_EXCP_PERFM_EBB);
- } else if (env->spr[SPR_BESCR] & BESCR_EEO) {
- powerpc_excp(cpu, POWERPC_EXCP_EXTERNAL_EBB);
- }
-
- return;
+ return PPC_INTERRUPT_EBB;
}
}
}
- if (env->resume_as_sreset) {
- /*
- * This is a bug ! It means that has_work took us out of halt without
- * anything to deliver while in a PM state that requires getting
- * out via a 0x100
- *
- * This means we will incorrectly execute past the power management
- * instruction instead of triggering a reset.
- *
- * It generally means a discrepancy between the wakeup conditions in the
- * processor has_work implementation and the logic in this function.
- */
- cpu_abort(env_cpu(env),
- "Wakeup from PM state but interrupt Undelivered");
+ return 0;
+}
+
+static void ppc_hw_interrupt(CPUPPCState *env, int pending_interrupt)
+{
+ PowerPCCPU *cpu = env_archcpu(env);
+
+ switch (pending_interrupt) {
+ case PPC_INTERRUPT_RESET: /* External reset */
+ env->pending_interrupts &= ~PPC_INTERRUPT_RESET;
+ powerpc_excp(cpu, POWERPC_EXCP_RESET);
+ break;
+ case PPC_INTERRUPT_MCK: /* Machine check exception */
+ env->pending_interrupts &= ~PPC_INTERRUPT_MCK;
+ powerpc_excp(cpu, POWERPC_EXCP_MCHECK);
+ break;
+#if 0 /* TODO */
+ case PPC_INTERRUPT_DEBUG: /* External debug exception */
+ env->pending_interrupts &= ~PPC_INTERRUPT_DEBUG;
+ powerpc_excp(cpu, POWERPC_EXCP_DEBUG);
+ break;
+#endif
+
+ case PPC_INTERRUPT_HDECR: /* Hypervisor decrementer exception */
+ /* HDEC clears on delivery */
+ env->pending_interrupts &= ~PPC_INTERRUPT_HDECR;
+ powerpc_excp(cpu, POWERPC_EXCP_HDECR);
+ break;
+ case PPC_INTERRUPT_HVIRT: /* Hypervisor virtualization interrupt */
+ powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
+ break;
+
+ case PPC_INTERRUPT_EXT:
+ if (books_vhyp_promotes_external_to_hvirt(cpu)) {
+ powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
+ } else {
+ powerpc_excp(cpu, POWERPC_EXCP_EXTERNAL);
+ }
+ break;
+ case PPC_INTERRUPT_CEXT: /* External critical interrupt */
+ powerpc_excp(cpu, POWERPC_EXCP_CRITICAL);
+ break;
+
+ case PPC_INTERRUPT_WDT: /* Watchdog timer on embedded PowerPC */
+ env->pending_interrupts &= ~PPC_INTERRUPT_WDT;
+ powerpc_excp(cpu, POWERPC_EXCP_WDT);
+ break;
+ case PPC_INTERRUPT_CDOORBELL:
+ env->pending_interrupts &= ~PPC_INTERRUPT_CDOORBELL;
+ powerpc_excp(cpu, POWERPC_EXCP_DOORCI);
+ break;
+ case PPC_INTERRUPT_FIT: /* Fixed interval timer on embedded PowerPC */
+ env->pending_interrupts &= ~PPC_INTERRUPT_FIT;
+ powerpc_excp(cpu, POWERPC_EXCP_FIT);
+ break;
+ case PPC_INTERRUPT_PIT: /* Programmable interval timer on embedded PowerPC */
+ env->pending_interrupts &= ~PPC_INTERRUPT_PIT;
+ powerpc_excp(cpu, POWERPC_EXCP_PIT);
+ break;
+ case PPC_INTERRUPT_DECR: /* Decrementer exception */
+ if (ppc_decr_clear_on_delivery(env)) {
+ env->pending_interrupts &= ~PPC_INTERRUPT_DECR;
+ }
+ powerpc_excp(cpu, POWERPC_EXCP_DECR);
+ break;
+ case PPC_INTERRUPT_DOORBELL:
+ env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
+ if (is_book3s_arch2x(env)) {
+ powerpc_excp(cpu, POWERPC_EXCP_SDOOR);
+ } else {
+ powerpc_excp(cpu, POWERPC_EXCP_DOORI);
+ }
+ break;
+ case PPC_INTERRUPT_HDOORBELL:
+ env->pending_interrupts &= ~PPC_INTERRUPT_HDOORBELL;
+ powerpc_excp(cpu, POWERPC_EXCP_SDOOR_HV);
+ break;
+ case PPC_INTERRUPT_PERFM:
+ env->pending_interrupts &= ~PPC_INTERRUPT_PERFM;
+ powerpc_excp(cpu, POWERPC_EXCP_PERFM);
+ break;
+ case PPC_INTERRUPT_THERM: /* Thermal interrupt */
+ env->pending_interrupts &= ~PPC_INTERRUPT_THERM;
+ powerpc_excp(cpu, POWERPC_EXCP_THERM);
+ break;
+ case PPC_INTERRUPT_EBB: /* EBB exception */
+ env->pending_interrupts &= ~PPC_INTERRUPT_EBB;
+ if (env->spr[SPR_BESCR] & BESCR_PMEO) {
+ powerpc_excp(cpu, POWERPC_EXCP_PERFM_EBB);
+ } else if (env->spr[SPR_BESCR] & BESCR_EEO) {
+ powerpc_excp(cpu, POWERPC_EXCP_EXTERNAL_EBB);
+ }
+ break;
}
}
@@ -1884,15 +1915,38 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = &cpu->env;
+ int pending_interrupt;
- if (interrupt_request & CPU_INTERRUPT_HARD) {
- ppc_hw_interrupt(env);
- if (env->pending_interrupts == 0) {
- cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
- }
- return true;
+ if ((interrupt_request & CPU_INTERRUPT_HARD) == 0) {
+ return false;
}
- return false;
+
+ pending_interrupt = ppc_pending_interrupt(env);
+ if (pending_interrupt == 0) {
+ if (env->resume_as_sreset) {
+ /*
+ * This is a bug ! It means that has_work took us out of halt
+ * without anything to deliver while in a PM state that requires
+ * getting out via a 0x100
+ *
+ * This means we will incorrectly execute past the power management
+ * instruction instead of triggering a reset.
+ *
+ * It generally means a discrepancy between the wakeup conditions in
+ * the processor has_work implementation and the logic in this
+ * function.
+ */
+ cpu_abort(env_cpu(env),
+ "Wakeup from PM state but interrupt Undelivered");
+ }
+ return false;
+ }
+
+ ppc_hw_interrupt(env, pending_interrupt);
+ if (env->pending_interrupts == 0) {
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+ }
+ return true;
}
#endif /* !CONFIG_USER_ONLY */
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 04/13] target/ppc: prepare to split ppc_interrupt_pending by excp_model
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (2 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 03/13] target/ppc: move interrupt masking out of ppc_hw_interrupt Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 20:25 ` Fabiano Rosas
2022-08-15 16:20 ` [RFC PATCH 05/13] target/ppc: create an interrupt masking method for POWER9/POWER10 Matheus Ferst
` (9 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Rename the method to ppc_interrupt_pending_legacy and create a new
ppc_interrupt_pending that will call the appropriate interrupt masking
method based on env->excp_model.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 8690017c70..59981efd16 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1678,7 +1678,7 @@ void ppc_cpu_do_interrupt(CPUState *cs)
powerpc_excp(cpu, cs->exception_index);
}
-static int ppc_pending_interrupt(CPUPPCState *env)
+static int ppc_pending_interrupt_legacy(CPUPPCState *env)
{
bool async_deliver;
@@ -1790,6 +1790,14 @@ static int ppc_pending_interrupt(CPUPPCState *env)
return 0;
}
+static int ppc_pending_interrupt(CPUPPCState *env)
+{
+ switch (env->excp_model) {
+ default:
+ return ppc_pending_interrupt_legacy(env);
+ }
+}
+
static void ppc_hw_interrupt(CPUPPCState *env, int pending_interrupt)
{
PowerPCCPU *cpu = env_archcpu(env);
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 05/13] target/ppc: create an interrupt masking method for POWER9/POWER10
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (3 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 04/13] target/ppc: prepare to split ppc_interrupt_pending by excp_model Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 22:39 ` Fabiano Rosas
2022-08-15 16:20 ` [RFC PATCH 06/13] target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9 Matheus Ferst
` (8 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Create an interrupt masking method for the POWER9 and POWER10
processors. The new method is based on cpu_has_work_POWER{9,10} and
ppc_pending_interrupt_legacy.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 160 +++++++++++++++++++++++++++++++++++++++
1 file changed, 160 insertions(+)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 59981efd16..2ca6a917b2 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1678,6 +1678,163 @@ void ppc_cpu_do_interrupt(CPUState *cs)
powerpc_excp(cpu, cs->exception_index);
}
+static int ppc_pending_interrupt_p9(CPUPPCState *env)
+{
+ CPUState *cs = env_cpu(env);
+ bool async_deliver = false;
+
+ /* External reset */
+ if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
+ return PPC_INTERRUPT_RESET;
+ }
+
+ if (cs->halted) {
+ uint64_t psscr = env->spr[SPR_PSSCR];
+
+ if (!(psscr & PSSCR_EC)) {
+ /* If EC is clear, return any system-caused interrupt */
+ async_deliver = true;
+ } else {
+ /* External Exception */
+ if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
+ (env->spr[SPR_LPCR] & LPCR_EEE)) {
+ bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
+ if (!heic || !FIELD_EX64_HV(env->msr) ||
+ FIELD_EX64(env->msr, MSR, PR)) {
+ return PPC_INTERRUPT_EXT;
+ }
+ }
+ /* Decrementer Exception */
+ if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
+ (env->spr[SPR_LPCR] & LPCR_DEE)) {
+ return PPC_INTERRUPT_DECR;
+ }
+ /* Machine Check or Hypervisor Maintenance Exception */
+ if (env->spr[SPR_LPCR] & LPCR_OEE) {
+ if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
+ return PPC_INTERRUPT_MCK;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_HMI) {
+ return PPC_INTERRUPT_HMI;
+ }
+ }
+ /* Privileged Doorbell Exception */
+ if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
+ (env->spr[SPR_LPCR] & LPCR_PDEE)) {
+ return PPC_INTERRUPT_DOORBELL;
+ }
+ /* Hypervisor Doorbell Exception */
+ if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
+ (env->spr[SPR_LPCR] & LPCR_HDEE)) {
+ return PPC_INTERRUPT_HDOORBELL;
+ }
+ /* Hypervisor virtualization exception */
+ if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
+ (env->spr[SPR_LPCR] & LPCR_HVEE)) {
+ return PPC_INTERRUPT_HVIRT;
+ }
+ return 0;
+ }
+ }
+
+ /* Machine check exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
+ return PPC_INTERRUPT_MCK;
+ }
+
+ /*
+ * For interrupts that gate on MSR:EE, we need to do something a
+ * bit more subtle, as we need to let them through even when EE is
+ * clear when coming out of some power management states (in order
+ * for them to become a 0x100).
+ */
+ async_deliver |= FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
+
+ /* Hypervisor decrementer exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
+ /* LPCR will be clear when not supported so this will work */
+ bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
+ if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
+ /* HDEC clears on delivery */
+ return PPC_INTERRUPT_HDECR;
+ }
+ }
+
+ /* Hypervisor virtualization interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
+ /* LPCR will be clear when not supported so this will work */
+ bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
+ if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
+ return PPC_INTERRUPT_HVIRT;
+ }
+ }
+
+ /* External interrupt can ignore MSR:EE under some circumstances */
+ if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
+ bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
+ bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
+ /* HEIC blocks delivery to the hypervisor */
+ if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
+ !FIELD_EX64(env->msr, MSR, PR))) ||
+ (env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
+ return PPC_INTERRUPT_EXT;
+ }
+ }
+ if (FIELD_EX64(env->msr, MSR, CE)) {
+ /* External critical interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
+ return PPC_INTERRUPT_CEXT;
+ }
+ }
+ if (async_deliver != 0) {
+ /* Watchdog timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
+ return PPC_INTERRUPT_WDT;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
+ return PPC_INTERRUPT_CDOORBELL;
+ }
+ /* Fixed interval timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
+ return PPC_INTERRUPT_FIT;
+ }
+ /* Programmable interval timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
+ return PPC_INTERRUPT_PIT;
+ }
+ /* Decrementer exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
+ return PPC_INTERRUPT_DECR;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
+ return PPC_INTERRUPT_DOORBELL;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
+ return PPC_INTERRUPT_HDOORBELL;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
+ return PPC_INTERRUPT_PERFM;
+ }
+ /* Thermal interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
+ return PPC_INTERRUPT_THERM;
+ }
+ /* EBB exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
+ /*
+ * EBB exception must be taken in problem state and
+ * with BESCR_GE set.
+ */
+ if (FIELD_EX64(env->msr, MSR, PR) &&
+ (env->spr[SPR_BESCR] & BESCR_GE)) {
+ return PPC_INTERRUPT_EBB;
+ }
+ }
+ }
+
+ return 0;
+}
+
static int ppc_pending_interrupt_legacy(CPUPPCState *env)
{
bool async_deliver;
@@ -1793,6 +1950,9 @@ static int ppc_pending_interrupt_legacy(CPUPPCState *env)
static int ppc_pending_interrupt(CPUPPCState *env)
{
switch (env->excp_model) {
+ case POWERPC_EXCP_POWER9:
+ case POWERPC_EXCP_POWER10:
+ return ppc_pending_interrupt_p9(env);
default:
return ppc_pending_interrupt_legacy(env);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 06/13] target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (4 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 05/13] target/ppc: create an interrupt masking method for POWER9/POWER10 Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 21:23 ` Fabiano Rosas
2022-08-15 16:20 ` [RFC PATCH 07/13] target/ppc: create an interrupt masking method for POWER8 Matheus Ferst
` (7 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Critical Input, Watchdog Timer, and Fixed Interval Timer are only
defined for embedded CPUs. The Programmable Interval Timer is 40x-only.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 2ca6a917b2..42b57019ba 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1780,28 +1780,10 @@ static int ppc_pending_interrupt_p9(CPUPPCState *env)
return PPC_INTERRUPT_EXT;
}
}
- if (FIELD_EX64(env->msr, MSR, CE)) {
- /* External critical interrupt */
- if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
- return PPC_INTERRUPT_CEXT;
- }
- }
if (async_deliver != 0) {
- /* Watchdog timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
- return PPC_INTERRUPT_WDT;
- }
if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
return PPC_INTERRUPT_CDOORBELL;
}
- /* Fixed interval timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
- return PPC_INTERRUPT_FIT;
- }
- /* Programmable interval timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
- return PPC_INTERRUPT_PIT;
- }
/* Decrementer exception */
if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
return PPC_INTERRUPT_DECR;
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 07/13] target/ppc: create an interrupt masking method for POWER8
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (5 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 06/13] target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9 Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 08/13] target/ppc: remove unused interrupts from ppc_pending_interrupt_p8 Matheus Ferst
` (6 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Create an interrupt masking method for POWER8. The new method is based
on cpu_has_work_POWER8 and ppc_pending_interrupt_legacy.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 138 +++++++++++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 42b57019ba..13c2d5e2ce 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1678,6 +1678,142 @@ void ppc_cpu_do_interrupt(CPUState *cs)
powerpc_excp(cpu, cs->exception_index);
}
+static int ppc_pending_interrupt_p8(CPUPPCState *env)
+{
+ CPUState *cs = env_cpu(env);
+ bool async_deliver = false;
+
+ /* External reset */
+ if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
+ return PPC_INTERRUPT_RESET;
+ }
+
+ if (cs->halted) {
+ if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
+ (env->spr[SPR_LPCR] & LPCR_P8_PECE2)) {
+ return PPC_INTERRUPT_EXT;
+ }
+ if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
+ (env->spr[SPR_LPCR] & LPCR_P8_PECE3)) {
+ return PPC_INTERRUPT_DECR;
+ }
+ if ((env->pending_interrupts & PPC_INTERRUPT_MCK) &&
+ (env->spr[SPR_LPCR] & LPCR_P8_PECE4)) {
+ return PPC_INTERRUPT_MCK;
+ }
+ if ((env->pending_interrupts & PPC_INTERRUPT_HMI) &&
+ (env->spr[SPR_LPCR] & LPCR_P8_PECE4)) {
+ return PPC_INTERRUPT_HMI;
+ }
+ if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
+ (env->spr[SPR_LPCR] & LPCR_P8_PECE0)) {
+ return PPC_INTERRUPT_DOORBELL;
+ }
+ if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
+ (env->spr[SPR_LPCR] & LPCR_P8_PECE1)) {
+ return PPC_INTERRUPT_HDOORBELL;
+ }
+ return 0;
+ }
+
+ /* Machine check exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
+ return PPC_INTERRUPT_MCK;
+ }
+
+ /*
+ * For interrupts that gate on MSR:EE, we need to do something a
+ * bit more subtle, as we need to let them through even when EE is
+ * clear when coming out of some power management states (in order
+ * for them to become a 0x100).
+ */
+ async_deliver |= FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
+
+ /* Hypervisor decrementer exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
+ /* LPCR will be clear when not supported so this will work */
+ bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
+ if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
+ /* HDEC clears on delivery */
+ return PPC_INTERRUPT_HDECR;
+ }
+ }
+
+ /* Hypervisor virtualization interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
+ /* LPCR will be clear when not supported so this will work */
+ bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
+ if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
+ return PPC_INTERRUPT_HVIRT;
+ }
+ }
+
+ /* External interrupt can ignore MSR:EE under some circumstances */
+ if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
+ bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
+ bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
+ /* HEIC blocks delivery to the hypervisor */
+ if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
+ !FIELD_EX64(env->msr, MSR, PR))) ||
+ (env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
+ return PPC_INTERRUPT_EXT;
+ }
+ }
+ if (FIELD_EX64(env->msr, MSR, CE)) {
+ /* External critical interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
+ return PPC_INTERRUPT_CEXT;
+ }
+ }
+ if (async_deliver != 0) {
+ /* Watchdog timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
+ return PPC_INTERRUPT_WDT;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
+ return PPC_INTERRUPT_CDOORBELL;
+ }
+ /* Fixed interval timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
+ return PPC_INTERRUPT_FIT;
+ }
+ /* Programmable interval timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
+ return PPC_INTERRUPT_PIT;
+ }
+ /* Decrementer exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
+ return PPC_INTERRUPT_DECR;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
+ return PPC_INTERRUPT_DOORBELL;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
+ return PPC_INTERRUPT_HDOORBELL;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
+ return PPC_INTERRUPT_PERFM;
+ }
+ /* Thermal interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
+ return PPC_INTERRUPT_THERM;
+ }
+ /* EBB exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
+ /*
+ * EBB exception must be taken in problem state and
+ * with BESCR_GE set.
+ */
+ if (FIELD_EX64(env->msr, MSR, PR) &&
+ (env->spr[SPR_BESCR] & BESCR_GE)) {
+ return PPC_INTERRUPT_EBB;
+ }
+ }
+ }
+
+ return 0;
+}
+
static int ppc_pending_interrupt_p9(CPUPPCState *env)
{
CPUState *cs = env_cpu(env);
@@ -1932,6 +2068,8 @@ static int ppc_pending_interrupt_legacy(CPUPPCState *env)
static int ppc_pending_interrupt(CPUPPCState *env)
{
switch (env->excp_model) {
+ case POWERPC_EXCP_POWER8:
+ return ppc_pending_interrupt_p8(env);
case POWERPC_EXCP_POWER9:
case POWERPC_EXCP_POWER10:
return ppc_pending_interrupt_p9(env);
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 08/13] target/ppc: remove unused interrupts from ppc_pending_interrupt_p8
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (6 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 07/13] target/ppc: create an interrupt masking method for POWER8 Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 09/13] target/ppc: create an interrupt masking method for POWER7 Matheus Ferst
` (5 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
The Hypervisor Virtualization Interrupt was introduced in PowerISA v3.0.
Critical Input, Watchdog Timer, and Fixed Interval Timer are only
defined for embedded CPUs. The Programmable Interval Timer is 40x-only.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 27 ---------------------------
1 file changed, 27 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 13c2d5e2ce..0dbd385bf0 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1739,15 +1739,6 @@ static int ppc_pending_interrupt_p8(CPUPPCState *env)
}
}
- /* Hypervisor virtualization interrupt */
- if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
- /* LPCR will be clear when not supported so this will work */
- bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
- if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
- return PPC_INTERRUPT_HVIRT;
- }
- }
-
/* External interrupt can ignore MSR:EE under some circumstances */
if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
@@ -1759,28 +1750,10 @@ static int ppc_pending_interrupt_p8(CPUPPCState *env)
return PPC_INTERRUPT_EXT;
}
}
- if (FIELD_EX64(env->msr, MSR, CE)) {
- /* External critical interrupt */
- if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
- return PPC_INTERRUPT_CEXT;
- }
- }
if (async_deliver != 0) {
- /* Watchdog timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
- return PPC_INTERRUPT_WDT;
- }
if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
return PPC_INTERRUPT_CDOORBELL;
}
- /* Fixed interval timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
- return PPC_INTERRUPT_FIT;
- }
- /* Programmable interval timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
- return PPC_INTERRUPT_PIT;
- }
/* Decrementer exception */
if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
return PPC_INTERRUPT_DECR;
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 09/13] target/ppc: create an interrupt masking method for POWER7
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (7 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 08/13] target/ppc: remove unused interrupts from ppc_pending_interrupt_p8 Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 10/13] target/ppc: remove unused interrupts from ppc_pending_interrupt_p7 Matheus Ferst
` (4 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
The new method is based on cpu_has_work_POWER7 and
ppc_pending_interrupt_legacy.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 130 +++++++++++++++++++++++++++++++++++++++
1 file changed, 130 insertions(+)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 0dbd385bf0..a67ab28661 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1678,6 +1678,134 @@ void ppc_cpu_do_interrupt(CPUState *cs)
powerpc_excp(cpu, cs->exception_index);
}
+static int ppc_pending_interrupt_p7(CPUPPCState *env)
+{
+ CPUState *cs = env_cpu(env);
+ bool async_deliver;
+
+ /* External reset */
+ if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
+ return PPC_INTERRUPT_RESET;
+ }
+
+ if (cs->halted) {
+ if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
+ (env->spr[SPR_LPCR] & LPCR_P7_PECE0)) {
+ return PPC_INTERRUPT_EXT;
+ }
+ if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
+ (env->spr[SPR_LPCR] & LPCR_P7_PECE1)) {
+ return PPC_INTERRUPT_DECR;
+ }
+ if ((env->pending_interrupts & PPC_INTERRUPT_MCK) &&
+ (env->spr[SPR_LPCR] & LPCR_P7_PECE2)) {
+ return PPC_INTERRUPT_MCK;
+ }
+ if ((env->pending_interrupts & PPC_INTERRUPT_HMI) &&
+ (env->spr[SPR_LPCR] & LPCR_P7_PECE2)) {
+ return PPC_INTERRUPT_HMI;
+ }
+ return 0;
+ }
+
+ /* Machine check exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
+ return PPC_INTERRUPT_MCK;
+ }
+
+ /*
+ * For interrupts that gate on MSR:EE, we need to do something a
+ * bit more subtle, as we need to let them through even when EE is
+ * clear when coming out of some power management states (in order
+ * for them to become a 0x100).
+ */
+ async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
+
+ /* Hypervisor decrementer exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
+ /* LPCR will be clear when not supported so this will work */
+ bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
+ if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
+ /* HDEC clears on delivery */
+ return PPC_INTERRUPT_HDECR;
+ }
+ }
+
+ /* Hypervisor virtualization interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
+ /* LPCR will be clear when not supported so this will work */
+ bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
+ if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
+ return PPC_INTERRUPT_HVIRT;
+ }
+ }
+
+ /* External interrupt can ignore MSR:EE under some circumstances */
+ if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
+ bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
+ bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
+ /* HEIC blocks delivery to the hypervisor */
+ if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
+ !FIELD_EX64(env->msr, MSR, PR))) ||
+ (env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
+ return PPC_INTERRUPT_EXT;
+ }
+ }
+ if (FIELD_EX64(env->msr, MSR, CE)) {
+ /* External critical interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
+ return PPC_INTERRUPT_CEXT;
+ }
+ }
+ if (async_deliver != 0) {
+ /* Watchdog timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
+ return PPC_INTERRUPT_WDT;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
+ return PPC_INTERRUPT_CDOORBELL;
+ }
+ /* Fixed interval timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
+ return PPC_INTERRUPT_FIT;
+ }
+ /* Programmable interval timer on embedded PowerPC */
+ if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
+ return PPC_INTERRUPT_PIT;
+ }
+ /* Decrementer exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
+ return PPC_INTERRUPT_DECR;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
+ return PPC_INTERRUPT_DOORBELL;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
+ return PPC_INTERRUPT_HDOORBELL;
+ }
+ if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
+ return PPC_INTERRUPT_PERFM;
+ }
+ /* Thermal interrupt */
+ if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
+ return PPC_INTERRUPT_THERM;
+ }
+ /* EBB exception */
+ if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
+ /*
+ * EBB exception must be taken in problem state and
+ * with BESCR_GE set.
+ */
+ if (FIELD_EX64(env->msr, MSR, PR) &&
+ (env->spr[SPR_BESCR] & BESCR_GE)) {
+ return PPC_INTERRUPT_EBB;
+ }
+ }
+ }
+
+ return 0;
+}
+
static int ppc_pending_interrupt_p8(CPUPPCState *env)
{
CPUState *cs = env_cpu(env);
@@ -2041,6 +2169,8 @@ static int ppc_pending_interrupt_legacy(CPUPPCState *env)
static int ppc_pending_interrupt(CPUPPCState *env)
{
switch (env->excp_model) {
+ case POWERPC_EXCP_POWER7:
+ return ppc_pending_interrupt_p7(env);
case POWERPC_EXCP_POWER8:
return ppc_pending_interrupt_p8(env);
case POWERPC_EXCP_POWER9:
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 10/13] target/ppc: remove unused interrupts from ppc_pending_interrupt_p7
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (8 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 09/13] target/ppc: create an interrupt masking method for POWER7 Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 11/13] target/ppc: remove ppc_store_lpcr from CONFIG_USER_ONLY builds Matheus Ferst
` (3 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
The Hypervisor Virtualization Interrupt was introduced in PowerISA v3.0.
Critical Input, Watchdog Timer, and Fixed Interval Timer are only
defined for embedded CPUs. The Programmable Interval Timer is 40x-only.
The Event-Based Branch Facility was added in PowerISA v2.07.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/excp_helper.c | 38 --------------------------------------
1 file changed, 38 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a67ab28661..b4c1198ea2 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1731,15 +1731,6 @@ static int ppc_pending_interrupt_p7(CPUPPCState *env)
}
}
- /* Hypervisor virtualization interrupt */
- if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
- /* LPCR will be clear when not supported so this will work */
- bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
- if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
- return PPC_INTERRUPT_HVIRT;
- }
- }
-
/* External interrupt can ignore MSR:EE under some circumstances */
if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
@@ -1751,28 +1742,10 @@ static int ppc_pending_interrupt_p7(CPUPPCState *env)
return PPC_INTERRUPT_EXT;
}
}
- if (FIELD_EX64(env->msr, MSR, CE)) {
- /* External critical interrupt */
- if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
- return PPC_INTERRUPT_CEXT;
- }
- }
if (async_deliver != 0) {
- /* Watchdog timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
- return PPC_INTERRUPT_WDT;
- }
if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
return PPC_INTERRUPT_CDOORBELL;
}
- /* Fixed interval timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
- return PPC_INTERRUPT_FIT;
- }
- /* Programmable interval timer on embedded PowerPC */
- if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
- return PPC_INTERRUPT_PIT;
- }
/* Decrementer exception */
if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
return PPC_INTERRUPT_DECR;
@@ -1790,17 +1763,6 @@ static int ppc_pending_interrupt_p7(CPUPPCState *env)
if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
return PPC_INTERRUPT_THERM;
}
- /* EBB exception */
- if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
- /*
- * EBB exception must be taken in problem state and
- * with BESCR_GE set.
- */
- if (FIELD_EX64(env->msr, MSR, PR) &&
- (env->spr[SPR_BESCR] & BESCR_GE)) {
- return PPC_INTERRUPT_EBB;
- }
- }
}
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 11/13] target/ppc: remove ppc_store_lpcr from CONFIG_USER_ONLY builds
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (9 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 10/13] target/ppc: remove unused interrupts from ppc_pending_interrupt_p7 Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 12/13] target/ppc: introduce ppc_maybe_interrupt Matheus Ferst
` (2 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Writes to LPCR are hypervisor privileged.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/cpu.c | 2 ++
target/ppc/cpu.h | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 401b6f9e63..9f261bde8e 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -73,6 +73,7 @@ void ppc_store_msr(CPUPPCState *env, target_ulong value)
hreg_store_msr(env, value, 0);
}
+#if !defined(CONFIG_USER_ONLY)
void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
{
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
@@ -82,6 +83,7 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
/* The gtse bit affects hflags */
hreg_compute_hflags(env);
}
+#endif
static inline void fpscr_set_rounding_mode(CPUPPCState *env)
{
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c7864bb3b1..5018296f02 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1370,9 +1370,9 @@ void ppc_translate_init(void);
#if !defined(CONFIG_USER_ONLY)
void ppc_store_sdr1(CPUPPCState *env, target_ulong value);
+void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
#endif /* !defined(CONFIG_USER_ONLY) */
void ppc_store_msr(CPUPPCState *env, target_ulong value);
-void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
void ppc_cpu_list(void);
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 12/13] target/ppc: introduce ppc_maybe_interrupt
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (10 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 11/13] target/ppc: remove ppc_store_lpcr from CONFIG_USER_ONLY builds Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 13/13] target/ppc: unify cpu->has_work based on cs->interrupt_request Matheus Ferst
2022-08-15 20:02 ` [RFC PATCH 00/13] PowerPC interrupt rework Cédric Le Goater
13 siblings, 0 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
This new method will check if any pending interrupt was unmasked and
then call cpu_interrupt/cpu_reset_interrupt accordingly. Code that
raises/lowers or masks/unmasks interrupts should call this method to
keep CPU_INTERRUPT_HARD coherent with env->pending_interrupts.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
hw/ppc/ppc.c | 7 +------
target/ppc/cpu.h | 1 +
target/ppc/excp_helper.c | 19 +++++++++++++++++++
target/ppc/helper_regs.c | 2 ++
target/ppc/translate.c | 8 ++++++--
5 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 77e611e81c..dc86c1c7db 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -42,7 +42,6 @@ static void cpu_ppc_tb_start (CPUPPCState *env);
void ppc_set_irq(PowerPCCPU *cpu, int irq, int level)
{
- CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
unsigned int old_pending;
bool locked = false;
@@ -57,19 +56,15 @@ void ppc_set_irq(PowerPCCPU *cpu, int irq, int level)
if (level) {
env->pending_interrupts |= irq;
- cpu_interrupt(cs, CPU_INTERRUPT_HARD);
} else {
env->pending_interrupts &= ~irq;
- if (env->pending_interrupts == 0) {
- cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
- }
}
if (old_pending != env->pending_interrupts) {
+ ppc_maybe_interrupt(env);
kvmppc_set_interrupt(cpu, irq, level);
}
-
trace_ppc_irq_set_exit(env, irq, level, env->pending_interrupts,
CPU(cpu)->interrupt_request);
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5018296f02..f65e0d7de8 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1358,6 +1358,7 @@ int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
int cpuid, void *opaque);
#ifndef CONFIG_USER_ONLY
+void ppc_maybe_interrupt(CPUPPCState *env);
void ppc_cpu_do_interrupt(CPUState *cpu);
bool ppc_cpu_exec_interrupt(CPUState *cpu, int int_req);
void ppc_cpu_do_system_reset(CPUState *cs);
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index b4c1198ea2..788dcd732a 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2143,6 +2143,23 @@ static int ppc_pending_interrupt(CPUPPCState *env)
}
}
+void ppc_maybe_interrupt(CPUPPCState *env)
+{
+ CPUState *cs = env_cpu(env);
+
+ if (ppc_pending_interrupt(env)) {
+ if (!qemu_mutex_iothread_locked()) {
+ qemu_mutex_lock_iothread();
+ cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+ qemu_mutex_unlock_iothread();
+ } else {
+ cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+ }
+ } else {
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+ }
+}
+
static void ppc_hw_interrupt(CPUPPCState *env, int pending_interrupt)
{
PowerPCCPU *cpu = env_archcpu(env);
@@ -2380,6 +2397,8 @@ void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
/* Condition for waking up at 0x100 */
env->resume_as_sreset = (insn != PPC_PM_STOP) ||
(env->spr[SPR_PSSCR] & PSSCR_EC);
+
+ ppc_maybe_interrupt(env);
}
#endif /* defined(TARGET_PPC64) */
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 12235ea2e9..2e85e124ab 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -260,6 +260,8 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
env->msr = value;
hreg_compute_hflags(env);
#if !defined(CONFIG_USER_ONLY)
+ ppc_maybe_interrupt(env);
+
if (unlikely(FIELD_EX64(env->msr, MSR, POW))) {
if (!env->pending_interrupts && (*env->check_pow)(env)) {
cs->halted = 1;
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 388337f81b..60dd66f736 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6174,7 +6174,8 @@ static void gen_wrtee(DisasContext *ctx)
t0 = tcg_temp_new();
tcg_gen_andi_tl(t0, cpu_gpr[rD(ctx->opcode)], (1 << MSR_EE));
tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(1 << MSR_EE));
- tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
+ tcg_gen_or_tl(t0, cpu_msr, t0);
+ gen_helper_store_msr(cpu_env, t0);
tcg_temp_free(t0);
/*
* Stop translation to have a chance to raise an exception if we
@@ -6192,7 +6193,10 @@ static void gen_wrteei(DisasContext *ctx)
#else
CHK_SV(ctx);
if (ctx->opcode & 0x00008000) {
- tcg_gen_ori_tl(cpu_msr, cpu_msr, (1 << MSR_EE));
+ TCGv t0 = tcg_temp_new();
+ tcg_gen_ori_tl(t0, cpu_msr, (1 << MSR_EE));
+ gen_helper_store_msr(cpu_env, t0);
+ tcg_temp_free(t0);
/* Stop translation to have a chance to raise an exception */
ctx->base.is_jmp = DISAS_EXIT_UPDATE;
} else {
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 13/13] target/ppc: unify cpu->has_work based on cs->interrupt_request
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (11 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 12/13] target/ppc: introduce ppc_maybe_interrupt Matheus Ferst
@ 2022-08-15 16:20 ` Matheus Ferst
2022-08-15 20:02 ` [RFC PATCH 00/13] PowerPC interrupt rework Cédric Le Goater
13 siblings, 0 replies; 27+ messages in thread
From: Matheus Ferst @ 2022-08-15 16:20 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Now that cs->interrupt_request indicates if there is any unmasked
interrupt, checking if the CPU has work to do can be simplified to a
single check that works for all CPU models.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/cpu_init.c | 212 +-----------------------------------------
1 file changed, 1 insertion(+), 211 deletions(-)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 850334545a..303e81596d 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5923,46 +5923,10 @@ static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr)
return false;
}
-static bool cpu_has_work_POWER7(CPUState *cs)
-{
- PowerPCCPU *cpu = POWERPC_CPU(cs);
- CPUPPCState *env = &cpu->env;
-
- if (cs->halted) {
- if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
- return false;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
- (env->spr[SPR_LPCR] & LPCR_P7_PECE0)) {
- return true;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
- (env->spr[SPR_LPCR] & LPCR_P7_PECE1)) {
- return true;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_MCK) &&
- (env->spr[SPR_LPCR] & LPCR_P7_PECE2)) {
- return true;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_HMI) &&
- (env->spr[SPR_LPCR] & LPCR_P7_PECE2)) {
- return true;
- }
- if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
- return true;
- }
- return false;
- } else {
- return FIELD_EX64(env->msr, MSR, EE) &&
- (cs->interrupt_request & CPU_INTERRUPT_HARD);
- }
-}
-
POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
- CPUClass *cc = CPU_CLASS(oc);
dc->fw_name = "PowerPC,POWER7";
dc->desc = "POWER7";
@@ -5971,7 +5935,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
pcc->init_proc = init_proc_POWER7;
pcc->check_pow = check_pow_nocheck;
- cc->has_work = cpu_has_work_POWER7;
pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -6087,54 +6050,10 @@ static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
return false;
}
-static bool cpu_has_work_POWER8(CPUState *cs)
-{
- PowerPCCPU *cpu = POWERPC_CPU(cs);
- CPUPPCState *env = &cpu->env;
-
- if (cs->halted) {
- if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
- return false;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
- (env->spr[SPR_LPCR] & LPCR_P8_PECE2)) {
- return true;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
- (env->spr[SPR_LPCR] & LPCR_P8_PECE3)) {
- return true;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_MCK) &&
- (env->spr[SPR_LPCR] & LPCR_P8_PECE4)) {
- return true;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_HMI) &&
- (env->spr[SPR_LPCR] & LPCR_P8_PECE4)) {
- return true;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
- (env->spr[SPR_LPCR] & LPCR_P8_PECE0)) {
- return true;
- }
- if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
- (env->spr[SPR_LPCR] & LPCR_P8_PECE1)) {
- return true;
- }
- if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
- return true;
- }
- return false;
- } else {
- return FIELD_EX64(env->msr, MSR, EE) &&
- (cs->interrupt_request & CPU_INTERRUPT_HARD);
- }
-}
-
POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
- CPUClass *cc = CPU_CLASS(oc);
dc->fw_name = "PowerPC,POWER8";
dc->desc = "POWER8";
@@ -6143,7 +6062,6 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
pcc->init_proc = init_proc_POWER8;
pcc->check_pow = check_pow_nocheck;
- cc->has_work = cpu_has_work_POWER8;
pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -6290,71 +6208,10 @@ static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr)
return false;
}
-static bool cpu_has_work_POWER9(CPUState *cs)
-{
- PowerPCCPU *cpu = POWERPC_CPU(cs);
- CPUPPCState *env = &cpu->env;
-
- if (cs->halted) {
- uint64_t psscr = env->spr[SPR_PSSCR];
-
- if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
- return false;
- }
-
- /* If EC is clear, just return true on any pending interrupt */
- if (!(psscr & PSSCR_EC)) {
- return true;
- }
- /* External Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
- (env->spr[SPR_LPCR] & LPCR_EEE)) {
- bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
- if (!heic || !FIELD_EX64_HV(env->msr) ||
- FIELD_EX64(env->msr, MSR, PR)) {
- return true;
- }
- }
- /* Decrementer Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
- (env->spr[SPR_LPCR] & LPCR_DEE)) {
- return true;
- }
- /* Machine Check or Hypervisor Maintenance Exception */
- if ((env->pending_interrupts & (PPC_INTERRUPT_MCK | PPC_INTERRUPT_HMI))
- && (env->spr[SPR_LPCR] & LPCR_OEE)) {
- return true;
- }
- /* Privileged Doorbell Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
- (env->spr[SPR_LPCR] & LPCR_PDEE)) {
- return true;
- }
- /* Hypervisor Doorbell Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
- (env->spr[SPR_LPCR] & LPCR_HDEE)) {
- return true;
- }
- /* Hypervisor virtualization exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
- (env->spr[SPR_LPCR] & LPCR_HVEE)) {
- return true;
- }
- if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
- return true;
- }
- return false;
- } else {
- return FIELD_EX64(env->msr, MSR, EE) &&
- (cs->interrupt_request & CPU_INTERRUPT_HARD);
- }
-}
-
POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
- CPUClass *cc = CPU_CLASS(oc);
dc->fw_name = "PowerPC,POWER9";
dc->desc = "POWER9";
@@ -6364,7 +6221,6 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
PCR_COMPAT_2_05;
pcc->init_proc = init_proc_POWER9;
pcc->check_pow = check_pow_nocheck;
- cc->has_work = cpu_has_work_POWER9;
pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -6507,71 +6363,10 @@ static bool ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr)
return false;
}
-static bool cpu_has_work_POWER10(CPUState *cs)
-{
- PowerPCCPU *cpu = POWERPC_CPU(cs);
- CPUPPCState *env = &cpu->env;
-
- if (cs->halted) {
- uint64_t psscr = env->spr[SPR_PSSCR];
-
- if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
- return false;
- }
-
- /* If EC is clear, just return true on any pending interrupt */
- if (!(psscr & PSSCR_EC)) {
- return true;
- }
- /* External Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
- (env->spr[SPR_LPCR] & LPCR_EEE)) {
- bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
- if (!heic || !FIELD_EX64_HV(env->msr) ||
- FIELD_EX64(env->msr, MSR, PR)) {
- return true;
- }
- }
- /* Decrementer Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
- (env->spr[SPR_LPCR] & LPCR_DEE)) {
- return true;
- }
- /* Machine Check or Hypervisor Maintenance Exception */
- if ((env->pending_interrupts & (PPC_INTERRUPT_MCK | PPC_INTERRUPT_HMI))
- && (env->spr[SPR_LPCR] & LPCR_OEE)) {
- return true;
- }
- /* Privileged Doorbell Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
- (env->spr[SPR_LPCR] & LPCR_PDEE)) {
- return true;
- }
- /* Hypervisor Doorbell Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
- (env->spr[SPR_LPCR] & LPCR_HDEE)) {
- return true;
- }
- /* Hypervisor virtualization exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
- (env->spr[SPR_LPCR] & LPCR_HVEE)) {
- return true;
- }
- if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
- return true;
- }
- return false;
- } else {
- return FIELD_EX64(env->msr, MSR, EE) &&
- (cs->interrupt_request & CPU_INTERRUPT_HARD);
- }
-}
-
POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
- CPUClass *cc = CPU_CLASS(oc);
dc->fw_name = "PowerPC,POWER10";
dc->desc = "POWER10";
@@ -6582,7 +6377,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
pcc->init_proc = init_proc_POWER10;
pcc->check_pow = check_pow_nocheck;
- cc->has_work = cpu_has_work_POWER10;
pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -7139,11 +6933,7 @@ static void ppc_cpu_set_pc(CPUState *cs, vaddr value)
static bool ppc_cpu_has_work(CPUState *cs)
{
- PowerPCCPU *cpu = POWERPC_CPU(cs);
- CPUPPCState *env = &cpu->env;
-
- return FIELD_EX64(env->msr, MSR, EE) &&
- (cs->interrupt_request & CPU_INTERRUPT_HARD);
+ return cs->interrupt_request & CPU_INTERRUPT_HARD;
}
static void ppc_cpu_reset(DeviceState *dev)
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 00/13] PowerPC interrupt rework
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
` (12 preceding siblings ...)
2022-08-15 16:20 ` [RFC PATCH 13/13] target/ppc: unify cpu->has_work based on cs->interrupt_request Matheus Ferst
@ 2022-08-15 20:02 ` Cédric Le Goater
2022-08-17 13:42 ` Matheus K. Ferst
13 siblings, 1 reply; 27+ messages in thread
From: Cédric Le Goater @ 2022-08-15 20:02 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc
Cc: danielhb413, david, groug, fbarrat, alex.bennee, Fabiano Rosas,
Nicholas Piggin
[ Adding Fabiano who reworked all exception models for 7.0 and Nick
who rewrote the Linux side sometime ago ]
On 8/15/22 18:20, Matheus Ferst wrote:
> Currently, PowerPC interrupts are handled as follows:
>
> 1) The CPU_INTERRUPT_HARD bit of cs->interrupt_request gates all
> interrupts;
> 2) The bits of env->pending_interrupts identify which particular
> interrupt is raised;
> 3) ppc_set_irq can be used to set/clear env->pending_interrupt bit and
> CPU_INTERRUPT_HARD, but some places access env->pending_interrupt
> directly;
> 4) ppc_cpu_exec_interrupt is called by cpu_handle_interrupt when
> cs->interrupt_request indicates that there is some interrupt pending.
> This method checks CPU_INTERRUPT_HARD and calls ppc_hw_interrupt. If
> env->pending_interrupt is zero after this call, CPU_INTERRUPT_HARD
> will be cleared.
> 5) ppc_hw_interrupt checks if there is any unmasked interrupt and calls
> powerpc_excp with the appropriate POWERPC_EXCP_* value. The method
> will also reset the corresponding bit in env->pending_interrupt for
> interrupts that clear on delivery.
>
> If all pending interrupts are masked, CPU_INTERRUPT_HARD will be set,
> but ppc_hw_interrupt will not deliver or clear the interrupt, so
> CPU_INTERRUPT_HARD will not be reset by ppc_cpu_exec_interrupt. With
> that, cs->has_work keeps returning true, creating a loop that acquires
> and release qemu_mutex_lock_iothread, causing the poor performance
> reported in [1].
>
> This patch series attempts to rework the PowerPC interrupt code to set
> CPU_INTERRUPT_HARD only when there are unmasked interrupts. Then
> cs->has_work can be simplified to a check of CPU_INTERRUPT_HARD, so it
> also only returns true when at least one interrupt can be delivered.
>
> To achieve that, we are basically following Alex Bannée's suggestion[2]
> in the original thread: the interrupt masking logic will be factored
> out of ppc_hw_interrupt in a new method, ppc_pending_interrupts. This
> method is then used to decide if CPU_INTERRUPT_HARD should be set or
> cleared after changes to MSR, LPCR, env->pending_interrupts, and
> power-management instructions.
>
> We used [3] to check for regressions at each patch in this series. After
> patch 12, booting a powernv machine with a newer skiboot with "-smp 4"
> goes from 1m09s to 20.79s.
whaou ! PowerNV is really an heavy weight platform, so that's a great
improvement.
Did you try KVM guests under PowerNV (L1 under an emulated L0) and KVM
under pseries (L2 under an emulated L1) ? Try some intensive I/O on a
SMP machine, like a large scp transfer.
We should try the MacOS images also.
Thanks,
C.
>
> [1] https://lists.gnu.org/archive/html/qemu-ppc/2022-06/msg00336.html
> [2] https://lists.gnu.org/archive/html/qemu-ppc/2022-06/msg00372.html
> [3] https://github.com/legoater/qemu-ppc-boot
>
> Matheus Ferst (13):
> target/ppc: define PPC_INTERRUPT_* values directly
> target/ppc: always use ppc_set_irq to set env->pending_interrupts
> target/ppc: move interrupt masking out of ppc_hw_interrupt
> target/ppc: prepare to split ppc_interrupt_pending by excp_model
> target/ppc: create a interrupt masking method for POWER9/POWER10
> target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9
> target/ppc: create a interrupt masking method for POWER8
> target/ppc: remove unused interrupts from ppc_pending_interrupt_p8
> target/ppc: create a interrupt masking method for POWER7
> target/ppc: remove unused interrupts from ppc_pending_interrupt_p7
> target/ppc: remove ppc_store_lpcr from CONFIG_USER_ONLY builds
> target/ppc: introduce ppc_maybe_interrupt
> target/ppc: unify cpu->has_work based on cs->interrupt_request
>
> hw/ppc/ppc.c | 17 +-
> hw/ppc/trace-events | 2 +-
> target/ppc/cpu.c | 2 +
> target/ppc/cpu.h | 43 +--
> target/ppc/cpu_init.c | 212 +------------
> target/ppc/excp_helper.c | 651 ++++++++++++++++++++++++++++++++-------
> target/ppc/helper_regs.c | 2 +
> target/ppc/misc_helper.c | 11 +-
> target/ppc/translate.c | 8 +-
> 9 files changed, 580 insertions(+), 368 deletions(-)
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 03/13] target/ppc: move interrupt masking out of ppc_hw_interrupt
2022-08-15 16:20 ` [RFC PATCH 03/13] target/ppc: move interrupt masking out of ppc_hw_interrupt Matheus Ferst
@ 2022-08-15 20:09 ` Fabiano Rosas
2022-08-17 13:42 ` Matheus K. Ferst
0 siblings, 1 reply; 27+ messages in thread
From: Fabiano Rosas @ 2022-08-15 20:09 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
> Move the interrupt masking logic to a new method, ppc_pending_interrupt,
> and only handle the interrupt processing in ppc_hw_interrupt.
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> target/ppc/excp_helper.c | 228 ++++++++++++++++++++++++---------------
> 1 file changed, 141 insertions(+), 87 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
<snip>
> @@ -1884,15 +1915,38 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
> + int pending_interrupt;
I would give this a simpler name to avoid confusion with the other two
pending_interrupt terms.
>
> - if (interrupt_request & CPU_INTERRUPT_HARD) {
> - ppc_hw_interrupt(env);
> - if (env->pending_interrupts == 0) {
> - cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
> - }
> - return true;
> + if ((interrupt_request & CPU_INTERRUPT_HARD) == 0) {
> + return false;
> }
> - return false;
> +
It seems we're assuming that after this point we certainly have some
pending interrupt...
> + pending_interrupt = ppc_pending_interrupt(env);
> + if (pending_interrupt == 0) {
...but how then is this able to return 0? Could the function's name be
made a bit clearer? Maybe interrupt = ppc_next_pending_interrupt or
something to that effect.
> + if (env->resume_as_sreset) {
> + /*
> + * This is a bug ! It means that has_work took us out of halt
> + * without anything to deliver while in a PM state that requires
> + * getting out via a 0x100
> + *
> + * This means we will incorrectly execute past the power management
> + * instruction instead of triggering a reset.
> + *
> + * It generally means a discrepancy between the wakeup conditions in
> + * the processor has_work implementation and the logic in this
> + * function.
> + */
> + cpu_abort(env_cpu(env),
> + "Wakeup from PM state but interrupt Undelivered");
This condition is BookS only. Perhaps it would be better to move it
inside each of the processor-specific functions. And since you're
merging has_work with pending_interrupts, can't you solve that issue
earlier? Specifically the "has_work tooks us out of halt..." part.
> + }
> + return false;
> + }
> +
> + ppc_hw_interrupt(env, pending_interrupt);
Some verbs would be nice. ppc_deliver_interrupt?
> + if (env->pending_interrupts == 0) {
> + cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> + }
> + return true;
> }
>
> #endif /* !CONFIG_USER_ONLY */
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 04/13] target/ppc: prepare to split ppc_interrupt_pending by excp_model
2022-08-15 16:20 ` [RFC PATCH 04/13] target/ppc: prepare to split ppc_interrupt_pending by excp_model Matheus Ferst
@ 2022-08-15 20:25 ` Fabiano Rosas
2022-08-17 13:42 ` Matheus K. Ferst
0 siblings, 1 reply; 27+ messages in thread
From: Fabiano Rosas @ 2022-08-15 20:25 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
> Rename the method to ppc_interrupt_pending_legacy and create a new
> ppc_interrupt_pending that will call the appropriate interrupt masking
> method based on env->excp_model.
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> target/ppc/excp_helper.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 8690017c70..59981efd16 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1678,7 +1678,7 @@ void ppc_cpu_do_interrupt(CPUState *cs)
> powerpc_excp(cpu, cs->exception_index);
> }
>
> -static int ppc_pending_interrupt(CPUPPCState *env)
> +static int ppc_pending_interrupt_legacy(CPUPPCState *env)
Won't this code continue to be used for the older CPUs? If so, I don't
think the term legacy is appropriate. It ends up being dependent on
context and what people's definitions of "legacy" are.
(if this function is removed in a later patch, then that's ok).
> {
> bool async_deliver;
>
> @@ -1790,6 +1790,14 @@ static int ppc_pending_interrupt(CPUPPCState *env)
> return 0;
> }
>
> +static int ppc_pending_interrupt(CPUPPCState *env)
> +{
> + switch (env->excp_model) {
> + default:
> + return ppc_pending_interrupt_legacy(env);
> + }
> +}
> +
> static void ppc_hw_interrupt(CPUPPCState *env, int pending_interrupt)
> {
> PowerPCCPU *cpu = env_archcpu(env);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 06/13] target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9
2022-08-15 16:20 ` [RFC PATCH 06/13] target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9 Matheus Ferst
@ 2022-08-15 21:23 ` Fabiano Rosas
2022-08-17 13:44 ` Matheus K. Ferst
0 siblings, 1 reply; 27+ messages in thread
From: Fabiano Rosas @ 2022-08-15 21:23 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
> Critical Input, Watchdog Timer, and Fixed Interval Timer are only
> defined for embedded CPUs. The Programmable Interval Timer is 40x-only.
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> target/ppc/excp_helper.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 2ca6a917b2..42b57019ba 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1780,28 +1780,10 @@ static int ppc_pending_interrupt_p9(CPUPPCState *env)
> return PPC_INTERRUPT_EXT;
> }
> }
> - if (FIELD_EX64(env->msr, MSR, CE)) {
> - /* External critical interrupt */
> - if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
> - return PPC_INTERRUPT_CEXT;
> - }
> - }
> if (async_deliver != 0) {
> - /* Watchdog timer on embedded PowerPC */
> - if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
> - return PPC_INTERRUPT_WDT;
> - }
> if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
> return PPC_INTERRUPT_CDOORBELL;
> }
This one too.
And the Thermal.
> - /* Fixed interval timer on embedded PowerPC */
> - if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
> - return PPC_INTERRUPT_FIT;
> - }
> - /* Programmable interval timer on embedded PowerPC */
> - if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
> - return PPC_INTERRUPT_PIT;
> - }
> /* Decrementer exception */
> if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
> return PPC_INTERRUPT_DECR;
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 05/13] target/ppc: create an interrupt masking method for POWER9/POWER10
2022-08-15 16:20 ` [RFC PATCH 05/13] target/ppc: create an interrupt masking method for POWER9/POWER10 Matheus Ferst
@ 2022-08-15 22:39 ` Fabiano Rosas
2022-08-17 13:43 ` Matheus K. Ferst
0 siblings, 1 reply; 27+ messages in thread
From: Fabiano Rosas @ 2022-08-15 22:39 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee, Matheus Ferst
Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
> Create an interrupt masking method for the POWER9 and POWER10
> processors. The new method is based on cpu_has_work_POWER{9,10} and
> ppc_pending_interrupt_legacy.
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> target/ppc/excp_helper.c | 160 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 160 insertions(+)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 59981efd16..2ca6a917b2 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1678,6 +1678,163 @@ void ppc_cpu_do_interrupt(CPUState *cs)
> powerpc_excp(cpu, cs->exception_index);
> }
>
> +static int ppc_pending_interrupt_p9(CPUPPCState *env)
> +{
> + CPUState *cs = env_cpu(env);
> + bool async_deliver = false;
I would suggest this to disentangle the PM stuff from MSR_EE:
if (cs->halted) {
if (env->spr[SPR_PSSCR] & PSSCR_EC) {
return p9_interrupt_powersave();
} else {
/*
* If EC is clear, any system-caused exception exits
* power-saving mode.
*/
ignore_msr_ee = true;
}
}
RESET (keep it duplicated)
MCHECK
if (!MSR_EE && !ignore_msr_ee) {
return 0;
}
MSR_EE interrupts
---
> +
> + /* External reset */
> + if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> + return PPC_INTERRUPT_RESET;
> + }
> +
> + if (cs->halted) {
> + uint64_t psscr = env->spr[SPR_PSSCR];
> +
> + if (!(psscr & PSSCR_EC)) {
> + /* If EC is clear, return any system-caused interrupt */
> + async_deliver = true;
This doesn't look correct, look at what the ISA says:
EC=0
Hardware will exit power-saving mode when the exception corresponding
to any system-caused interrupt occurs. Power-saving mode is exited
either at the instruction following the stop (if MSR_EE=0) or in the
corresponding interrupt handler (if MSR_EE=1).
So with MSR_EE=0 we should *not* deliver any interrupts, but return to NIP+4.
> + } else {
> + /* External Exception */
> + if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> + (env->spr[SPR_LPCR] & LPCR_EEE)) {
> + bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> + if (!heic || !FIELD_EX64_HV(env->msr) ||
> + FIELD_EX64(env->msr, MSR, PR)) {
> + return PPC_INTERRUPT_EXT;
> + }
> + }
> + /* Decrementer Exception */
> + if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
> + (env->spr[SPR_LPCR] & LPCR_DEE)) {
> + return PPC_INTERRUPT_DECR;
> + }
> + /* Machine Check or Hypervisor Maintenance Exception */
> + if (env->spr[SPR_LPCR] & LPCR_OEE) {
> + if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> + return PPC_INTERRUPT_MCK;
> + }
> + if (env->pending_interrupts & PPC_INTERRUPT_HMI) {
> + return PPC_INTERRUPT_HMI;
> + }
> + }
> + /* Privileged Doorbell Exception */
> + if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> + (env->spr[SPR_LPCR] & LPCR_PDEE)) {
> + return PPC_INTERRUPT_DOORBELL;
> + }
> + /* Hypervisor Doorbell Exception */
> + if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> + (env->spr[SPR_LPCR] & LPCR_HDEE)) {
> + return PPC_INTERRUPT_HDOORBELL;
> + }
> + /* Hypervisor virtualization exception */
> + if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
> + (env->spr[SPR_LPCR] & LPCR_HVEE)) {
> + return PPC_INTERRUPT_HVIRT;
> + }
> + return 0;
> + }
> + }
> +
> + /* Machine check exception */
> + if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> + return PPC_INTERRUPT_MCK;
> + }
> +
> + /*
> + * For interrupts that gate on MSR:EE, we need to do something a
> + * bit more subtle, as we need to let them through even when EE is
> + * clear when coming out of some power management states (in order
> + * for them to become a 0x100).
> + */
> + async_deliver |= FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
> +
> + /* Hypervisor decrementer exception */
> + if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
> + /* LPCR will be clear when not supported so this will work */
> + bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
> + if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
> + /* HDEC clears on delivery */
> + return PPC_INTERRUPT_HDECR;
> + }
> + }
> +
> + /* Hypervisor virtualization interrupt */
> + if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
> + /* LPCR will be clear when not supported so this will work */
> + bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
> + if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
> + return PPC_INTERRUPT_HVIRT;
> + }
> + }
> +
> + /* External interrupt can ignore MSR:EE under some circumstances */
> + if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
> + bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> + bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> + /* HEIC blocks delivery to the hypervisor */
> + if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
> + !FIELD_EX64(env->msr, MSR, PR))) ||
> + (env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
> + return PPC_INTERRUPT_EXT;
> + }
> + }
> + if (FIELD_EX64(env->msr, MSR, CE)) {
> + /* External critical interrupt */
> + if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
> + return PPC_INTERRUPT_CEXT;
> + }
> + }
> + if (async_deliver != 0) {
> + /* Watchdog timer on embedded PowerPC */
> + if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
> + return PPC_INTERRUPT_WDT;
> + }
> + if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
> + return PPC_INTERRUPT_CDOORBELL;
> + }
> + /* Fixed interval timer on embedded PowerPC */
> + if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
> + return PPC_INTERRUPT_FIT;
> + }
> + /* Programmable interval timer on embedded PowerPC */
> + if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
> + return PPC_INTERRUPT_PIT;
> + }
> + /* Decrementer exception */
> + if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
> + return PPC_INTERRUPT_DECR;
> + }
> + if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> + return PPC_INTERRUPT_DOORBELL;
> + }
> + if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
> + return PPC_INTERRUPT_HDOORBELL;
> + }
> + if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
> + return PPC_INTERRUPT_PERFM;
> + }
> + /* Thermal interrupt */
> + if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
> + return PPC_INTERRUPT_THERM;
> + }
> + /* EBB exception */
> + if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
> + /*
> + * EBB exception must be taken in problem state and
> + * with BESCR_GE set.
> + */
> + if (FIELD_EX64(env->msr, MSR, PR) &&
> + (env->spr[SPR_BESCR] & BESCR_GE)) {
> + return PPC_INTERRUPT_EBB;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> static int ppc_pending_interrupt_legacy(CPUPPCState *env)
> {
> bool async_deliver;
> @@ -1793,6 +1950,9 @@ static int ppc_pending_interrupt_legacy(CPUPPCState *env)
> static int ppc_pending_interrupt(CPUPPCState *env)
> {
> switch (env->excp_model) {
> + case POWERPC_EXCP_POWER9:
> + case POWERPC_EXCP_POWER10:
> + return ppc_pending_interrupt_p9(env);
> default:
> return ppc_pending_interrupt_legacy(env);
> }
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 00/13] PowerPC interrupt rework
2022-08-15 20:02 ` [RFC PATCH 00/13] PowerPC interrupt rework Cédric Le Goater
@ 2022-08-17 13:42 ` Matheus K. Ferst
0 siblings, 0 replies; 27+ messages in thread
From: Matheus K. Ferst @ 2022-08-17 13:42 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel, qemu-ppc
Cc: danielhb413, david, groug, fbarrat, alex.bennee, Fabiano Rosas,
Nicholas Piggin
On 15/08/2022 17:02, Cédric Le Goater wrote:
> [ Adding Fabiano who reworked all exception models for 7.0 and Nick
> who rewrote the Linux side sometime ago ]
>
> On 8/15/22 18:20, Matheus Ferst wrote:
>> Currently, PowerPC interrupts are handled as follows:
>>
>> 1) The CPU_INTERRUPT_HARD bit of cs->interrupt_request gates all
>> interrupts;
>> 2) The bits of env->pending_interrupts identify which particular
>> interrupt is raised;
>> 3) ppc_set_irq can be used to set/clear env->pending_interrupt bit and
>> CPU_INTERRUPT_HARD, but some places access env->pending_interrupt
>> directly;
>> 4) ppc_cpu_exec_interrupt is called by cpu_handle_interrupt when
>> cs->interrupt_request indicates that there is some interrupt pending.
>> This method checks CPU_INTERRUPT_HARD and calls ppc_hw_interrupt. If
>> env->pending_interrupt is zero after this call, CPU_INTERRUPT_HARD
>> will be cleared.
>> 5) ppc_hw_interrupt checks if there is any unmasked interrupt and calls
>> powerpc_excp with the appropriate POWERPC_EXCP_* value. The method
>> will also reset the corresponding bit in env->pending_interrupt for
>> interrupts that clear on delivery.
>>
>> If all pending interrupts are masked, CPU_INTERRUPT_HARD will be set,
>> but ppc_hw_interrupt will not deliver or clear the interrupt, so
>> CPU_INTERRUPT_HARD will not be reset by ppc_cpu_exec_interrupt. With
>> that, cs->has_work keeps returning true, creating a loop that acquires
>> and release qemu_mutex_lock_iothread, causing the poor performance
>> reported in [1].
>>
>> This patch series attempts to rework the PowerPC interrupt code to set
>> CPU_INTERRUPT_HARD only when there are unmasked interrupts. Then
>> cs->has_work can be simplified to a check of CPU_INTERRUPT_HARD, so it
>> also only returns true when at least one interrupt can be delivered.
>>
>> To achieve that, we are basically following Alex Bannée's suggestion[2]
>> in the original thread: the interrupt masking logic will be factored
>> out of ppc_hw_interrupt in a new method, ppc_pending_interrupts. This
>> method is then used to decide if CPU_INTERRUPT_HARD should be set or
>> cleared after changes to MSR, LPCR, env->pending_interrupts, and
>> power-management instructions.
>>
>> We used [3] to check for regressions at each patch in this series. After
>> patch 12, booting a powernv machine with a newer skiboot with "-smp 4"
>> goes from 1m09s to 20.79s.
>
> whaou ! PowerNV is really an heavy weight platform, so that's a great
> improvement.
>
Note that this result uses Frederic's test case, where one CPU
decompresses the kernel and the others keep spinning to deliver a masked
decrement interrupt. The improvement may not be that great with other
workloads.
> Did you try KVM guests under PowerNV (L1 under an emulated L0) and KVM
> under pseries (L2 under an emulated L1) ? Try some intensive I/O on a
> SMP machine, like a large scp transfer.
>
So far, I have mainly tested with buildroot boot+poweroff. I'll try
these other tests too.
> We should try the MacOS images also.
>
> Thanks,
>
> C.
Unfortunately, I can't test with MacOS :/
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 03/13] target/ppc: move interrupt masking out of ppc_hw_interrupt
2022-08-15 20:09 ` Fabiano Rosas
@ 2022-08-17 13:42 ` Matheus K. Ferst
2022-08-19 14:18 ` Fabiano Rosas
0 siblings, 1 reply; 27+ messages in thread
From: Matheus K. Ferst @ 2022-08-17 13:42 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee
On 15/08/2022 17:09, Fabiano Rosas wrote:
> Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
>
>> Move the interrupt masking logic to a new method, ppc_pending_interrupt,
>> and only handle the interrupt processing in ppc_hw_interrupt.
>>
>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> ---
>> target/ppc/excp_helper.c | 228 ++++++++++++++++++++++++---------------
>> 1 file changed, 141 insertions(+), 87 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>
> <snip>
>
>> @@ -1884,15 +1915,38 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>> {
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>> CPUPPCState *env = &cpu->env;
>> + int pending_interrupt;
>
> I would give this a simpler name to avoid confusion with the other two
> pending_interrupt terms.
>
>>
>> - if (interrupt_request & CPU_INTERRUPT_HARD) {
>> - ppc_hw_interrupt(env);
>> - if (env->pending_interrupts == 0) {
>> - cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
>> - }
>> - return true;
>> + if ((interrupt_request & CPU_INTERRUPT_HARD) == 0) {
>> + return false;
>> }
>> - return false;
>> +
>
> It seems we're assuming that after this point we certainly have some
> pending interrupt...
>
>> + pending_interrupt = ppc_pending_interrupt(env);
>> + if (pending_interrupt == 0) {
>
> ...but how then is this able to return 0?
At this point in the patch series, raising interrupts with ppc_set_irq
always sets CPU_INTERRUPT_HARD, so it is possible that all interrupts
are masked, and then ppc_pending_interrupt would return zero.
> Could the function's name be
> made a bit clearer? Maybe interrupt = ppc_next_pending_interrupt or
> something to that effect.
>
Maybe ppc_next_unmasked_interrupt?
>> + if (env->resume_as_sreset) {
>> + /*
>> + * This is a bug ! It means that has_work took us out of halt
>> + * without anything to deliver while in a PM state that requires
>> + * getting out via a 0x100
>> + *
>> + * This means we will incorrectly execute past the power management
>> + * instruction instead of triggering a reset.
>> + *
>> + * It generally means a discrepancy between the wakeup conditions in
>> + * the processor has_work implementation and the logic in this
>> + * function.
>> + */
>> + cpu_abort(env_cpu(env),
>> + "Wakeup from PM state but interrupt Undelivered");
>
> This condition is BookS only. Perhaps it would be better to move it
> inside each of the processor-specific functions. And since you're
> merging has_work with pending_interrupts, can't you solve that issue
> earlier? Specifically the "has_work tooks us out of halt..." part.
>
This condition would not be an error in ppc_pending_interrupt because
we'll call this method from other places in the following patches, like
ppc_set_irq. Maybe we should move it to a "case 0:" in ppc_hw_interrupt?
>> + }
>> + return false;
>> + }
>> +
>> + ppc_hw_interrupt(env, pending_interrupt);
>
> Some verbs would be nice. ppc_deliver_interrupt?
Agreed. Should we also make processor-specific versions of this method?
That way, we could get rid of the calls to ppc_decr_clear_on_delivery
and is_book3s_arch2x.
>
>> + if (env->pending_interrupts == 0) {
>> + cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
>> + }
>> + return true;
>> }
>>
>> #endif /* !CONFIG_USER_ONLY */
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 04/13] target/ppc: prepare to split ppc_interrupt_pending by excp_model
2022-08-15 20:25 ` Fabiano Rosas
@ 2022-08-17 13:42 ` Matheus K. Ferst
0 siblings, 0 replies; 27+ messages in thread
From: Matheus K. Ferst @ 2022-08-17 13:42 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee
On 15/08/2022 17:25, Fabiano Rosas wrote:
> Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
>
>> Rename the method to ppc_interrupt_pending_legacy and create a new
>> ppc_interrupt_pending that will call the appropriate interrupt masking
>> method based on env->excp_model.
>>
>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> ---
>> target/ppc/excp_helper.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 8690017c70..59981efd16 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1678,7 +1678,7 @@ void ppc_cpu_do_interrupt(CPUState *cs)
>> powerpc_excp(cpu, cs->exception_index);
>> }
>>
>> -static int ppc_pending_interrupt(CPUPPCState *env)
>> +static int ppc_pending_interrupt_legacy(CPUPPCState *env)
>
> Won't this code continue to be used for the older CPUs? If so, I don't
> think the term legacy is appropriate. It ends up being dependent on
> context and what people's definitions of "legacy" are.
>
> (if this function is removed in a later patch, then that's ok).
>
For this RFC, I created methods for CPUs that change the interrupt
masking logic when cs->halted is set. If the way we split the
interruption code in the following patches is acceptable, I'm planning
to add methods for all CPUs and remove ppc_pending_interrupt_legacy in
future versions of this patch series.
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 05/13] target/ppc: create an interrupt masking method for POWER9/POWER10
2022-08-15 22:39 ` Fabiano Rosas
@ 2022-08-17 13:43 ` Matheus K. Ferst
0 siblings, 0 replies; 27+ messages in thread
From: Matheus K. Ferst @ 2022-08-17 13:43 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee
On 15/08/2022 19:39, Fabiano Rosas wrote:
> Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
>
>> Create an interrupt masking method for the POWER9 and POWER10
>> processors. The new method is based on cpu_has_work_POWER{9,10} and
>> ppc_pending_interrupt_legacy.
>>
>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> ---
>> target/ppc/excp_helper.c | 160 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 160 insertions(+)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 59981efd16..2ca6a917b2 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1678,6 +1678,163 @@ void ppc_cpu_do_interrupt(CPUState *cs)
>> powerpc_excp(cpu, cs->exception_index);
>> }
>>
>> +static int ppc_pending_interrupt_p9(CPUPPCState *env)
>> +{
>> + CPUState *cs = env_cpu(env);
>> + bool async_deliver = false;
>
> I would suggest this to disentangle the PM stuff from MSR_EE:
>
> if (cs->halted) {
> if (env->spr[SPR_PSSCR] & PSSCR_EC) {
> return p9_interrupt_powersave();
> } else {
> /*
> * If EC is clear, any system-caused exception exits
> * power-saving mode.
> */
> ignore_msr_ee = true;
> }
> }
>
> RESET (keep it duplicated)
> MCHECK
>
> if (!MSR_EE && !ignore_msr_ee) {
> return 0;
> }
>
> MSR_EE interrupts
> ---
>
>> +
>> + /* External reset */
>> + if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
>> + return PPC_INTERRUPT_RESET;
>> + }
>> +
>> + if (cs->halted) {
>> + uint64_t psscr = env->spr[SPR_PSSCR];
>> +
>> + if (!(psscr & PSSCR_EC)) {
>> + /* If EC is clear, return any system-caused interrupt */
>> + async_deliver = true;
>
> This doesn't look correct, look at what the ISA says:
>
> EC=0
>
> Hardware will exit power-saving mode when the exception corresponding
> to any system-caused interrupt occurs. Power-saving mode is exited
> either at the instruction following the stop (if MSR_EE=0) or in the
> corresponding interrupt handler (if MSR_EE=1).
>
> So with MSR_EE=0 we should *not* deliver any interrupts, but return to NIP+4.
>
At first, I thought the powerpc_excp would handle that but looking again
at the code, that's not how we return to NIP+4. We want to return true
in cs->has_work even if no interrupt can be delivered, so
cpu_thread_is_idle returns false, and we return from qemu_wait_io_event
to another iteration of the loop in mttcg_cpu_thread_fn.
I think we have two options here:
i) Keep ignoring MSR[EE] here when !PSSCR[EC] and prevent the delivery
in ppc_hw_interrupt if !MSR[EE]. That way, we can still use
ppc_pending_interrupt to set CPU_INTERRUPT_HARD in ppc_set_irq in patch 12.
ii) Don't return interrupts that require MSR[EE] set and ignore
CPU_INTERRUPT_HARD if cs->halted==1 in cpu_has_work_POWER*. In this
case, we wouldn't be able to merge the cpu_has_work_* implementations in
patch 13.
>> + } else {
>> + /* External Exception */
>> + if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
>> + (env->spr[SPR_LPCR] & LPCR_EEE)) {
>> + bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>> + if (!heic || !FIELD_EX64_HV(env->msr) ||
>> + FIELD_EX64(env->msr, MSR, PR)) {
>> + return PPC_INTERRUPT_EXT;
>> + }
>> + }
>> + /* Decrementer Exception */
>> + if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
>> + (env->spr[SPR_LPCR] & LPCR_DEE)) {
>> + return PPC_INTERRUPT_DECR;
>> + }
>> + /* Machine Check or Hypervisor Maintenance Exception */
>> + if (env->spr[SPR_LPCR] & LPCR_OEE) {
>> + if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
>> + return PPC_INTERRUPT_MCK;
>> + }
>> + if (env->pending_interrupts & PPC_INTERRUPT_HMI) {
>> + return PPC_INTERRUPT_HMI;
>> + }
>> + }
>> + /* Privileged Doorbell Exception */
>> + if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
>> + (env->spr[SPR_LPCR] & LPCR_PDEE)) {
>> + return PPC_INTERRUPT_DOORBELL;
>> + }
>> + /* Hypervisor Doorbell Exception */
>> + if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
>> + (env->spr[SPR_LPCR] & LPCR_HDEE)) {
>> + return PPC_INTERRUPT_HDOORBELL;
>> + }
>> + /* Hypervisor virtualization exception */
>> + if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
>> + (env->spr[SPR_LPCR] & LPCR_HVEE)) {
>> + return PPC_INTERRUPT_HVIRT;
>> + }
>> + return 0;
>> + }
>> + }
>> +
>> + /* Machine check exception */
>> + if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
>> + return PPC_INTERRUPT_MCK;
>> + }
>> +
>> + /*
>> + * For interrupts that gate on MSR:EE, we need to do something a
>> + * bit more subtle, as we need to let them through even when EE is
>> + * clear when coming out of some power management states (in order
>> + * for them to become a 0x100).
>> + */
>> + async_deliver |= FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
>> +
>> + /* Hypervisor decrementer exception */
>> + if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
>> + /* LPCR will be clear when not supported so this will work */
>> + bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
>> + if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
>> + /* HDEC clears on delivery */
>> + return PPC_INTERRUPT_HDECR;
>> + }
>> + }
>> +
>> + /* Hypervisor virtualization interrupt */
>> + if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
>> + /* LPCR will be clear when not supported so this will work */
>> + bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
>> + if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
>> + return PPC_INTERRUPT_HVIRT;
>> + }
>> + }
>> +
>> + /* External interrupt can ignore MSR:EE under some circumstances */
>> + if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
>> + bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>> + bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>> + /* HEIC blocks delivery to the hypervisor */
>> + if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
>> + !FIELD_EX64(env->msr, MSR, PR))) ||
>> + (env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
>> + return PPC_INTERRUPT_EXT;
>> + }
>> + }
>> + if (FIELD_EX64(env->msr, MSR, CE)) {
>> + /* External critical interrupt */
>> + if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
>> + return PPC_INTERRUPT_CEXT;
>> + }
>> + }
>> + if (async_deliver != 0) {
>> + /* Watchdog timer on embedded PowerPC */
>> + if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
>> + return PPC_INTERRUPT_WDT;
>> + }
>> + if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>> + return PPC_INTERRUPT_CDOORBELL;
>> + }
>> + /* Fixed interval timer on embedded PowerPC */
>> + if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
>> + return PPC_INTERRUPT_FIT;
>> + }
>> + /* Programmable interval timer on embedded PowerPC */
>> + if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
>> + return PPC_INTERRUPT_PIT;
>> + }
>> + /* Decrementer exception */
>> + if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
>> + return PPC_INTERRUPT_DECR;
>> + }
>> + if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
>> + return PPC_INTERRUPT_DOORBELL;
>> + }
>> + if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
>> + return PPC_INTERRUPT_HDOORBELL;
>> + }
>> + if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
>> + return PPC_INTERRUPT_PERFM;
>> + }
>> + /* Thermal interrupt */
>> + if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
>> + return PPC_INTERRUPT_THERM;
>> + }
>> + /* EBB exception */
>> + if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
>> + /*
>> + * EBB exception must be taken in problem state and
>> + * with BESCR_GE set.
>> + */
>> + if (FIELD_EX64(env->msr, MSR, PR) &&
>> + (env->spr[SPR_BESCR] & BESCR_GE)) {
>> + return PPC_INTERRUPT_EBB;
>> + }
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int ppc_pending_interrupt_legacy(CPUPPCState *env)
>> {
>> bool async_deliver;
>> @@ -1793,6 +1950,9 @@ static int ppc_pending_interrupt_legacy(CPUPPCState *env)
>> static int ppc_pending_interrupt(CPUPPCState *env)
>> {
>> switch (env->excp_model) {
>> + case POWERPC_EXCP_POWER9:
>> + case POWERPC_EXCP_POWER10:
>> + return ppc_pending_interrupt_p9(env);
>> default:
>> return ppc_pending_interrupt_legacy(env);
>> }
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 06/13] target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9
2022-08-15 21:23 ` Fabiano Rosas
@ 2022-08-17 13:44 ` Matheus K. Ferst
2022-08-19 16:04 ` Fabiano Rosas
0 siblings, 1 reply; 27+ messages in thread
From: Matheus K. Ferst @ 2022-08-17 13:44 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee
On 15/08/2022 18:23, Fabiano Rosas wrote:
> Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
>
>> Critical Input, Watchdog Timer, and Fixed Interval Timer are only
>> defined for embedded CPUs. The Programmable Interval Timer is 40x-only.
>>
>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> ---
>> target/ppc/excp_helper.c | 18 ------------------
>> 1 file changed, 18 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 2ca6a917b2..42b57019ba 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1780,28 +1780,10 @@ static int ppc_pending_interrupt_p9(CPUPPCState *env)
>> return PPC_INTERRUPT_EXT;
>> }
>> }
>> - if (FIELD_EX64(env->msr, MSR, CE)) {
>> - /* External critical interrupt */
>> - if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
>> - return PPC_INTERRUPT_CEXT;
>> - }
>> - }
>> if (async_deliver != 0) {
>> - /* Watchdog timer on embedded PowerPC */
>> - if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
>> - return PPC_INTERRUPT_WDT;
>> - }
>> if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>> return PPC_INTERRUPT_CDOORBELL;
>> }
>
> This one too.
>
> And the Thermal.
There are some other interrupts that I was not sure if we should remove:
- PPC_INTERRUPT_PERFM doesn't seem to be used anywhere else. I guess it
will be used when we implement more PMU stuff, so I left it in all
ppc_pending_interrupt_* methods.
- PPC_INTERRUPT_RESET was treated in cpu_has_work_POWER*, but AFAICS,
it's only used in ppc6xx_set_irq and ppc970_set_irq, which means it can
only be raised on 6xx, 7xx, 970, and POWER5+. Should we remove it too?
>
>> - /* Fixed interval timer on embedded PowerPC */
>> - if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
>> - return PPC_INTERRUPT_FIT;
>> - }
>> - /* Programmable interval timer on embedded PowerPC */
>> - if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
>> - return PPC_INTERRUPT_PIT;
>> - }
>> /* Decrementer exception */
>> if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
>> return PPC_INTERRUPT_DECR;
Tḧanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 01/13] target/ppc: define PPC_INTERRUPT_* values directly
2022-08-15 16:20 ` [RFC PATCH 01/13] target/ppc: define PPC_INTERRUPT_* values directly Matheus Ferst
@ 2022-08-18 2:18 ` David Gibson
0 siblings, 0 replies; 27+ messages in thread
From: David Gibson @ 2022-08-18 2:18 UTC (permalink / raw)
To: Matheus Ferst
Cc: qemu-devel, qemu-ppc, clg, danielhb413, groug, fbarrat, alex.bennee
[-- Attachment #1: Type: text/plain, Size: 26194 bytes --]
On Mon, Aug 15, 2022 at 01:20:07PM -0300, Matheus Ferst wrote:
> This enum defines the bit positions in env->pending_interrupts for each
> interrupt. However, except for the comparison in kvmppc_set_interrupt,
> the values are always used as (1 << PPC_INTERRUPT_*). Define them
> directly like that to save some clutter. No functional change intended.
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Good idea.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/ppc.c | 10 +++---
> hw/ppc/trace-events | 2 +-
> target/ppc/cpu.h | 40 +++++++++++-----------
> target/ppc/cpu_init.c | 56 +++++++++++++++---------------
> target/ppc/excp_helper.c | 74 ++++++++++++++++++++--------------------
> target/ppc/misc_helper.c | 6 ++--
> 6 files changed, 94 insertions(+), 94 deletions(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 690f448cb9..77e611e81c 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -40,7 +40,7 @@
> static void cpu_ppc_tb_stop (CPUPPCState *env);
> static void cpu_ppc_tb_start (CPUPPCState *env);
>
> -void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
> +void ppc_set_irq(PowerPCCPU *cpu, int irq, int level)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> @@ -56,21 +56,21 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
> old_pending = env->pending_interrupts;
>
> if (level) {
> - env->pending_interrupts |= 1 << n_IRQ;
> + env->pending_interrupts |= irq;
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> } else {
> - env->pending_interrupts &= ~(1 << n_IRQ);
> + env->pending_interrupts &= ~irq;
> if (env->pending_interrupts == 0) {
> cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> }
>
> if (old_pending != env->pending_interrupts) {
> - kvmppc_set_interrupt(cpu, n_IRQ, level);
> + kvmppc_set_interrupt(cpu, irq, level);
> }
>
>
> - trace_ppc_irq_set_exit(env, n_IRQ, level, env->pending_interrupts,
> + trace_ppc_irq_set_exit(env, irq, level, env->pending_interrupts,
> CPU(cpu)->interrupt_request);
>
> if (locked) {
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index 5c0a215cad..c9ee1285b8 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -116,7 +116,7 @@ ppc40x_set_tb_clk(uint32_t value) "new frequency %" PRIu32
> ppc40x_timers_init(uint32_t value) "frequency %" PRIu32
>
> ppc_irq_set(void *env, uint32_t pin, uint32_t level) "env [%p] pin %d level %d"
> -ppc_irq_set_exit(void *env, uint32_t n_IRQ, uint32_t level, uint32_t pending, uint32_t request) "env [%p] n_IRQ %d level %d => pending 0x%08" PRIx32 " req 0x%08" PRIx32
> +ppc_irq_set_exit(void *env, uint32_t irq, uint32_t level, uint32_t pending, uint32_t request) "env [%p] irq 0x%05" PRIx32 " level %d => pending 0x%08" PRIx32 " req 0x%08" PRIx32
> ppc_irq_set_state(const char *name, uint32_t level) "\"%s\" level %d"
> ppc_irq_reset(const char *name) "%s"
> ppc_irq_cpu(const char *action) "%s"
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index a4c893cfad..c7864bb3b1 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2418,27 +2418,27 @@ enum {
> /* Hardware exceptions definitions */
> enum {
> /* External hardware exception sources */
> - PPC_INTERRUPT_RESET = 0, /* Reset exception */
> - PPC_INTERRUPT_WAKEUP, /* Wakeup exception */
> - PPC_INTERRUPT_MCK, /* Machine check exception */
> - PPC_INTERRUPT_EXT, /* External interrupt */
> - PPC_INTERRUPT_SMI, /* System management interrupt */
> - PPC_INTERRUPT_CEXT, /* Critical external interrupt */
> - PPC_INTERRUPT_DEBUG, /* External debug exception */
> - PPC_INTERRUPT_THERM, /* Thermal exception */
> + PPC_INTERRUPT_RESET = 0x00001, /* Reset exception */
> + PPC_INTERRUPT_WAKEUP = 0x00002, /* Wakeup exception */
> + PPC_INTERRUPT_MCK = 0x00004, /* Machine check exception */
> + PPC_INTERRUPT_EXT = 0x00008, /* External interrupt */
> + PPC_INTERRUPT_SMI = 0x00010, /* System management interrupt */
> + PPC_INTERRUPT_CEXT = 0x00020, /* Critical external interrupt */
> + PPC_INTERRUPT_DEBUG = 0x00040, /* External debug exception */
> + PPC_INTERRUPT_THERM = 0x00080, /* Thermal exception */
> /* Internal hardware exception sources */
> - PPC_INTERRUPT_DECR, /* Decrementer exception */
> - PPC_INTERRUPT_HDECR, /* Hypervisor decrementer exception */
> - PPC_INTERRUPT_PIT, /* Programmable interval timer interrupt */
> - PPC_INTERRUPT_FIT, /* Fixed interval timer interrupt */
> - PPC_INTERRUPT_WDT, /* Watchdog timer interrupt */
> - PPC_INTERRUPT_CDOORBELL, /* Critical doorbell interrupt */
> - PPC_INTERRUPT_DOORBELL, /* Doorbell interrupt */
> - PPC_INTERRUPT_PERFM, /* Performance monitor interrupt */
> - PPC_INTERRUPT_HMI, /* Hypervisor Maintenance interrupt */
> - PPC_INTERRUPT_HDOORBELL, /* Hypervisor Doorbell interrupt */
> - PPC_INTERRUPT_HVIRT, /* Hypervisor virtualization interrupt */
> - PPC_INTERRUPT_EBB, /* Event-based Branch exception */
> + PPC_INTERRUPT_DECR = 0x00100, /* Decrementer exception */
> + PPC_INTERRUPT_HDECR = 0x00200, /* Hypervisor decrementer exception */
> + PPC_INTERRUPT_PIT = 0x00400, /* Programmable interval timer int. */
> + PPC_INTERRUPT_FIT = 0x00800, /* Fixed interval timer interrupt */
> + PPC_INTERRUPT_WDT = 0x01000, /* Watchdog timer interrupt */
> + PPC_INTERRUPT_CDOORBELL = 0x02000, /* Critical doorbell interrupt */
> + PPC_INTERRUPT_DOORBELL = 0x04000, /* Doorbell interrupt */
> + PPC_INTERRUPT_PERFM = 0x08000, /* Performance monitor interrupt */
> + PPC_INTERRUPT_HMI = 0x10000, /* Hypervisor Maintenance interrupt */
> + PPC_INTERRUPT_HDOORBELL = 0x20000, /* Hypervisor Doorbell interrupt */
> + PPC_INTERRUPT_HVIRT = 0x40000, /* Hypervisor virtualization interrupt */
> + PPC_INTERRUPT_EBB = 0x80000, /* Event-based Branch exception */
> };
>
> /* Processor Compatibility mask (PCR) */
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index d1493a660c..850334545a 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5932,23 +5932,23 @@ static bool cpu_has_work_POWER7(CPUState *cs)
> if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
> return false;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> (env->spr[SPR_LPCR] & LPCR_P7_PECE0)) {
> return true;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DECR)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
> (env->spr[SPR_LPCR] & LPCR_P7_PECE1)) {
> return true;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_MCK)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_MCK) &&
> (env->spr[SPR_LPCR] & LPCR_P7_PECE2)) {
> return true;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HMI)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_HMI) &&
> (env->spr[SPR_LPCR] & LPCR_P7_PECE2)) {
> return true;
> }
> - if (env->pending_interrupts & (1u << PPC_INTERRUPT_RESET)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> return true;
> }
> return false;
> @@ -6096,31 +6096,31 @@ static bool cpu_has_work_POWER8(CPUState *cs)
> if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
> return false;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> (env->spr[SPR_LPCR] & LPCR_P8_PECE2)) {
> return true;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DECR)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
> (env->spr[SPR_LPCR] & LPCR_P8_PECE3)) {
> return true;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_MCK)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_MCK) &&
> (env->spr[SPR_LPCR] & LPCR_P8_PECE4)) {
> return true;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HMI)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_HMI) &&
> (env->spr[SPR_LPCR] & LPCR_P8_PECE4)) {
> return true;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DOORBELL)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> (env->spr[SPR_LPCR] & LPCR_P8_PECE0)) {
> return true;
> }
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HDOORBELL)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> (env->spr[SPR_LPCR] & LPCR_P8_PECE1)) {
> return true;
> }
> - if (env->pending_interrupts & (1u << PPC_INTERRUPT_RESET)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> return true;
> }
> return false;
> @@ -6307,7 +6307,7 @@ static bool cpu_has_work_POWER9(CPUState *cs)
> return true;
> }
> /* External Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> (env->spr[SPR_LPCR] & LPCR_EEE)) {
> bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> if (!heic || !FIELD_EX64_HV(env->msr) ||
> @@ -6316,31 +6316,31 @@ static bool cpu_has_work_POWER9(CPUState *cs)
> }
> }
> /* Decrementer Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DECR)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
> (env->spr[SPR_LPCR] & LPCR_DEE)) {
> return true;
> }
> /* Machine Check or Hypervisor Maintenance Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_MCK |
> - 1u << PPC_INTERRUPT_HMI)) && (env->spr[SPR_LPCR] & LPCR_OEE)) {
> + if ((env->pending_interrupts & (PPC_INTERRUPT_MCK | PPC_INTERRUPT_HMI))
> + && (env->spr[SPR_LPCR] & LPCR_OEE)) {
> return true;
> }
> /* Privileged Doorbell Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DOORBELL)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> (env->spr[SPR_LPCR] & LPCR_PDEE)) {
> return true;
> }
> /* Hypervisor Doorbell Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HDOORBELL)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> (env->spr[SPR_LPCR] & LPCR_HDEE)) {
> return true;
> }
> /* Hypervisor virtualization exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HVIRT)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
> (env->spr[SPR_LPCR] & LPCR_HVEE)) {
> return true;
> }
> - if (env->pending_interrupts & (1u << PPC_INTERRUPT_RESET)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> return true;
> }
> return false;
> @@ -6524,7 +6524,7 @@ static bool cpu_has_work_POWER10(CPUState *cs)
> return true;
> }
> /* External Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> (env->spr[SPR_LPCR] & LPCR_EEE)) {
> bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> if (!heic || !FIELD_EX64_HV(env->msr) ||
> @@ -6533,31 +6533,31 @@ static bool cpu_has_work_POWER10(CPUState *cs)
> }
> }
> /* Decrementer Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DECR)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
> (env->spr[SPR_LPCR] & LPCR_DEE)) {
> return true;
> }
> /* Machine Check or Hypervisor Maintenance Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_MCK |
> - 1u << PPC_INTERRUPT_HMI)) && (env->spr[SPR_LPCR] & LPCR_OEE)) {
> + if ((env->pending_interrupts & (PPC_INTERRUPT_MCK | PPC_INTERRUPT_HMI))
> + && (env->spr[SPR_LPCR] & LPCR_OEE)) {
> return true;
> }
> /* Privileged Doorbell Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_DOORBELL)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> (env->spr[SPR_LPCR] & LPCR_PDEE)) {
> return true;
> }
> /* Hypervisor Doorbell Exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HDOORBELL)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> (env->spr[SPR_LPCR] & LPCR_HDEE)) {
> return true;
> }
> /* Hypervisor virtualization exception */
> - if ((env->pending_interrupts & (1u << PPC_INTERRUPT_HVIRT)) &&
> + if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
> (env->spr[SPR_LPCR] & LPCR_HVEE)) {
> return true;
> }
> - if (env->pending_interrupts & (1u << PPC_INTERRUPT_RESET)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> return true;
> }
> return false;
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 7550aafed6..b9476b5d03 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1683,21 +1683,21 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> bool async_deliver;
>
> /* External reset */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_RESET)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_RESET);
> + if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_RESET;
> powerpc_excp(cpu, POWERPC_EXCP_RESET);
> return;
> }
> /* Machine check exception */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_MCK)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_MCK);
> + if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_MCK;
> powerpc_excp(cpu, POWERPC_EXCP_MCHECK);
> return;
> }
> #if 0 /* TODO */
> /* External debug exception */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_DEBUG)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DEBUG);
> + if (env->pending_interrupts & PPC_INTERRUPT_DEBUG) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_DEBUG;
> powerpc_excp(cpu, POWERPC_EXCP_DEBUG);
> return;
> }
> @@ -1712,19 +1712,19 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
>
> /* Hypervisor decrementer exception */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
> /* LPCR will be clear when not supported so this will work */
> bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
> if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
> /* HDEC clears on delivery */
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDECR);
> + env->pending_interrupts &= ~PPC_INTERRUPT_HDECR;
> powerpc_excp(cpu, POWERPC_EXCP_HDECR);
> return;
> }
> }
>
> /* Hypervisor virtualization interrupt */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_HVIRT)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
> /* LPCR will be clear when not supported so this will work */
> bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
> if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
> @@ -1734,7 +1734,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> }
>
> /* External interrupt can ignore MSR:EE under some circumstances */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_EXT)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
> bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> /* HEIC blocks delivery to the hypervisor */
> @@ -1751,45 +1751,45 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> }
> if (FIELD_EX64(env->msr, MSR, CE)) {
> /* External critical interrupt */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_CEXT)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
> powerpc_excp(cpu, POWERPC_EXCP_CRITICAL);
> return;
> }
> }
> if (async_deliver != 0) {
> /* Watchdog timer on embedded PowerPC */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_WDT)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_WDT);
> + if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_WDT;
> powerpc_excp(cpu, POWERPC_EXCP_WDT);
> return;
> }
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_CDOORBELL)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_CDOORBELL);
> + if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_CDOORBELL;
> powerpc_excp(cpu, POWERPC_EXCP_DOORCI);
> return;
> }
> /* Fixed interval timer on embedded PowerPC */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_FIT)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_FIT);
> + if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_FIT;
> powerpc_excp(cpu, POWERPC_EXCP_FIT);
> return;
> }
> /* Programmable interval timer on embedded PowerPC */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_PIT)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PIT);
> + if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_PIT;
> powerpc_excp(cpu, POWERPC_EXCP_PIT);
> return;
> }
> /* Decrementer exception */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_DECR)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
> if (ppc_decr_clear_on_delivery(env)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DECR);
> + env->pending_interrupts &= ~PPC_INTERRUPT_DECR;
> }
> powerpc_excp(cpu, POWERPC_EXCP_DECR);
> return;
> }
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL);
> + if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
> if (is_book3s_arch2x(env)) {
> powerpc_excp(cpu, POWERPC_EXCP_SDOOR);
> } else {
> @@ -1797,31 +1797,31 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> }
> return;
> }
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDOORBELL);
> + if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_HDOORBELL;
> powerpc_excp(cpu, POWERPC_EXCP_SDOOR_HV);
> return;
> }
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM);
> + if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_PERFM;
> powerpc_excp(cpu, POWERPC_EXCP_PERFM);
> return;
> }
> /* Thermal interrupt */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_THERM)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_THERM);
> + if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
> + env->pending_interrupts &= ~PPC_INTERRUPT_THERM;
> powerpc_excp(cpu, POWERPC_EXCP_THERM);
> return;
> }
> /* EBB exception */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_EBB)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
> /*
> * EBB exception must be taken in problem state and
> * with BESCR_GE set.
> */
> if (FIELD_EX64(env->msr, MSR, PR) &&
> (env->spr[SPR_BESCR] & BESCR_GE)) {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
> + env->pending_interrupts &= ~PPC_INTERRUPT_EBB;
>
> if (env->spr[SPR_BESCR] & BESCR_PMEO) {
> powerpc_excp(cpu, POWERPC_EXCP_PERFM_EBB);
> @@ -2098,7 +2098,7 @@ static void do_ebb(CPUPPCState *env, int ebb_excp)
> if (FIELD_EX64(env->msr, MSR, PR)) {
> powerpc_excp(cpu, ebb_excp);
> } else {
> - env->pending_interrupts |= 1 << PPC_INTERRUPT_EBB;
> + env->pending_interrupts |= PPC_INTERRUPT_EBB;
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> }
> @@ -2209,7 +2209,7 @@ void helper_msgclr(CPUPPCState *env, target_ulong rb)
> return;
> }
>
> - env->pending_interrupts &= ~(1 << irq);
> + env->pending_interrupts &= ~irq;
> }
>
> void helper_msgsnd(target_ulong rb)
> @@ -2228,7 +2228,7 @@ void helper_msgsnd(target_ulong rb)
> CPUPPCState *cenv = &cpu->env;
>
> if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
> - cenv->pending_interrupts |= 1 << irq;
> + cenv->pending_interrupts |= irq;
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> }
> @@ -2253,7 +2253,7 @@ void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb)
> return;
> }
>
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDOORBELL);
> + env->pending_interrupts &= ~PPC_INTERRUPT_HDOORBELL;
> }
>
> static void book3s_msgsnd_common(int pir, int irq)
> @@ -2267,7 +2267,7 @@ static void book3s_msgsnd_common(int pir, int irq)
>
> /* TODO: broadcast message to all threads of the same processor */
> if (cenv->spr_cb[SPR_PIR].default_value == pir) {
> - cenv->pending_interrupts |= 1 << irq;
> + cenv->pending_interrupts |= irq;
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> }
> @@ -2294,7 +2294,7 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb)
> return;
> }
>
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL);
> + env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
> }
>
> /*
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index b0a5e7ce76..05e35572bc 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -163,7 +163,7 @@ target_ulong helper_load_dpdes(CPUPPCState *env)
> helper_hfscr_facility_check(env, HFSCR_MSGP, "load DPDES", HFSCR_IC_MSGP);
>
> /* TODO: TCG supports only one thread */
> - if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) {
> + if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> dpdes = 1;
> }
>
> @@ -185,10 +185,10 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val)
> }
>
> if (val & 0x1) {
> - env->pending_interrupts |= 1 << PPC_INTERRUPT_DOORBELL;
> + env->pending_interrupts |= PPC_INTERRUPT_DOORBELL;
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> } else {
> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL);
> + env->pending_interrupts &= ~PPC_INTERRUPT_DOORBELL;
> }
> }
> #endif /* defined(TARGET_PPC64) */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 03/13] target/ppc: move interrupt masking out of ppc_hw_interrupt
2022-08-17 13:42 ` Matheus K. Ferst
@ 2022-08-19 14:18 ` Fabiano Rosas
0 siblings, 0 replies; 27+ messages in thread
From: Fabiano Rosas @ 2022-08-19 14:18 UTC (permalink / raw)
To: Matheus K. Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee
"Matheus K. Ferst" <matheus.ferst@eldorado.org.br> writes:
> On 15/08/2022 17:09, Fabiano Rosas wrote:
>> Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
>>
>>> Move the interrupt masking logic to a new method, ppc_pending_interrupt,
>>> and only handle the interrupt processing in ppc_hw_interrupt.
>>>
>>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>>> ---
>>> target/ppc/excp_helper.c | 228 ++++++++++++++++++++++++---------------
>>> 1 file changed, 141 insertions(+), 87 deletions(-)
>>>
>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>>
>> <snip>
>>
>>> @@ -1884,15 +1915,38 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>>> {
>>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>>> CPUPPCState *env = &cpu->env;
>>> + int pending_interrupt;
>>
>> I would give this a simpler name to avoid confusion with the other two
>> pending_interrupt terms.
>>
>>>
>>> - if (interrupt_request & CPU_INTERRUPT_HARD) {
>>> - ppc_hw_interrupt(env);
>>> - if (env->pending_interrupts == 0) {
>>> - cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
>>> - }
>>> - return true;
>>> + if ((interrupt_request & CPU_INTERRUPT_HARD) == 0) {
>>> + return false;
>>> }
>>> - return false;
>>> +
>>
>> It seems we're assuming that after this point we certainly have some
>> pending interrupt...
>>
>>> + pending_interrupt = ppc_pending_interrupt(env);
>>> + if (pending_interrupt == 0) {
>>
>> ...but how then is this able to return 0?
>
> At this point in the patch series, raising interrupts with ppc_set_irq
> always sets CPU_INTERRUPT_HARD, so it is possible that all interrupts
> are masked, and then ppc_pending_interrupt would return zero.
>
>> Could the function's name be
>> made a bit clearer? Maybe interrupt = ppc_next_pending_interrupt or
>> something to that effect.
>>
>
> Maybe ppc_next_unmasked_interrupt?
Could be. Although your suggestion below of moving the pending=0 case
into ppc_hw_interrupt already clears up the confusion quite a lot.
>>> + if (env->resume_as_sreset) {
>>> + /*
>>> + * This is a bug ! It means that has_work took us out of halt
>>> + * without anything to deliver while in a PM state that requires
>>> + * getting out via a 0x100
>>> + *
>>> + * This means we will incorrectly execute past the power management
>>> + * instruction instead of triggering a reset.
>>> + *
>>> + * It generally means a discrepancy between the wakeup conditions in
>>> + * the processor has_work implementation and the logic in this
>>> + * function.
>>> + */
>>> + cpu_abort(env_cpu(env),
>>> + "Wakeup from PM state but interrupt Undelivered");
>>
>> This condition is BookS only. Perhaps it would be better to move it
>> inside each of the processor-specific functions. And since you're
>> merging has_work with pending_interrupts, can't you solve that issue
>> earlier? Specifically the "has_work tooks us out of halt..." part.
>>
>
> This condition would not be an error in ppc_pending_interrupt because
> we'll call this method from other places in the following patches, like
> ppc_set_irq. Maybe we should move it to a "case 0:" in ppc_hw_interrupt?
>
Good idea, this condition is very unlikely (impossible?) to happen so
there's no need to have it prominent in this function like this. Should
it be turned into an assert as well perhaps?
>>> + }
>>> + return false;
>>> + }
>>> +
>>> + ppc_hw_interrupt(env, pending_interrupt);
>>
>> Some verbs would be nice. ppc_deliver_interrupt?
>
> Agreed. Should we also make processor-specific versions of this method?
> That way, we could get rid of the calls to ppc_decr_clear_on_delivery
> and is_book3s_arch2x.
Yes, let's see what it looks like.
What interrupts exist for a given CPU along with their implementation
specific behavior are data that's linked to the CPU/group of CPUs. In
theory we could have these decisions made at build time/CPU selection
time and just operate on the smaller subset of data at runtime.
The is_book3s_arch2x check can already be removed. After the
powerpc_excp functions were split there's nothing handling DOORI; and
SDOOR is now only handled by powerpc_books. So we would just be changing
the "exception not implemented" message from DOORI to SDOOR.
... which will probably lead to some exploration of why we are
generating exceptions in TCG code for interrupts that are never
delivered/handled.
>>
>>> + if (env->pending_interrupts == 0) {
>>> + cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
>>> + }
>>> + return true;
>>> }
>>>
>>> #endif /* !CONFIG_USER_ONLY */
>
> Thanks,
> Matheus K. Ferst
> Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
> Analista de Software
> Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 06/13] target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9
2022-08-17 13:44 ` Matheus K. Ferst
@ 2022-08-19 16:04 ` Fabiano Rosas
0 siblings, 0 replies; 27+ messages in thread
From: Fabiano Rosas @ 2022-08-19 16:04 UTC (permalink / raw)
To: Matheus K. Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, fbarrat, alex.bennee
"Matheus K. Ferst" <matheus.ferst@eldorado.org.br> writes:
> On 15/08/2022 18:23, Fabiano Rosas wrote:
>> Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
>>
>>> Critical Input, Watchdog Timer, and Fixed Interval Timer are only
>>> defined for embedded CPUs. The Programmable Interval Timer is 40x-only.
>>>
>>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>>> ---
>>> target/ppc/excp_helper.c | 18 ------------------
>>> 1 file changed, 18 deletions(-)
>>>
>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>>> index 2ca6a917b2..42b57019ba 100644
>>> --- a/target/ppc/excp_helper.c
>>> +++ b/target/ppc/excp_helper.c
>>> @@ -1780,28 +1780,10 @@ static int ppc_pending_interrupt_p9(CPUPPCState *env)
>>> return PPC_INTERRUPT_EXT;
>>> }
>>> }
>>> - if (FIELD_EX64(env->msr, MSR, CE)) {
>>> - /* External critical interrupt */
>>> - if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
>>> - return PPC_INTERRUPT_CEXT;
>>> - }
>>> - }
>>> if (async_deliver != 0) {
>>> - /* Watchdog timer on embedded PowerPC */
>>> - if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
>>> - return PPC_INTERRUPT_WDT;
>>> - }
>>> if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>>> return PPC_INTERRUPT_CDOORBELL;
>>> }
>>
>> This one too.
>>
>> And the Thermal.
>
> There are some other interrupts that I was not sure if we should remove:
I would keep them simply because that is an unrelated cleanup. Here you
are removing the ones that are not present in those CPUs at all. I think
the discussion about how/what QEMU emulates is a different one. If we
determine that they are indeed not used and that is not a mistake, we
could replace them with a placeholder comment or even some explanation
of why we don't need them.
> - PPC_INTERRUPT_PERFM doesn't seem to be used anywhere else. I guess it
> will be used when we implement more PMU stuff, so I left it in all
> ppc_pending_interrupt_* methods.
> - PPC_INTERRUPT_RESET was treated in cpu_has_work_POWER*, but AFAICS,
> it's only used in ppc6xx_set_irq and ppc970_set_irq, which means it can
> only be raised on 6xx, 7xx, 970, and POWER5+. Should we remove it too?
I'm not sure if we have an external interrupt source that affects the
CPU like that. I see that we simply call powerpc_excp to reset the CPUs
when we need it.
Cédric, any thoughts?
>>
>>> - /* Fixed interval timer on embedded PowerPC */
>>> - if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
>>> - return PPC_INTERRUPT_FIT;
>>> - }
>>> - /* Programmable interval timer on embedded PowerPC */
>>> - if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
>>> - return PPC_INTERRUPT_PIT;
>>> - }
>>> /* Decrementer exception */
>>> if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
>>> return PPC_INTERRUPT_DECR;
>
> Tḧanks,
> Matheus K. Ferst
> Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
> Analista de Software
> Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2022-08-19 16:10 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-15 16:20 [RFC PATCH 00/13] PowerPC interrupt rework Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 01/13] target/ppc: define PPC_INTERRUPT_* values directly Matheus Ferst
2022-08-18 2:18 ` David Gibson
2022-08-15 16:20 ` [RFC PATCH 02/13] target/ppc: always use ppc_set_irq to set env->pending_interrupts Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 03/13] target/ppc: move interrupt masking out of ppc_hw_interrupt Matheus Ferst
2022-08-15 20:09 ` Fabiano Rosas
2022-08-17 13:42 ` Matheus K. Ferst
2022-08-19 14:18 ` Fabiano Rosas
2022-08-15 16:20 ` [RFC PATCH 04/13] target/ppc: prepare to split ppc_interrupt_pending by excp_model Matheus Ferst
2022-08-15 20:25 ` Fabiano Rosas
2022-08-17 13:42 ` Matheus K. Ferst
2022-08-15 16:20 ` [RFC PATCH 05/13] target/ppc: create an interrupt masking method for POWER9/POWER10 Matheus Ferst
2022-08-15 22:39 ` Fabiano Rosas
2022-08-17 13:43 ` Matheus K. Ferst
2022-08-15 16:20 ` [RFC PATCH 06/13] target/ppc: remove embedded interrupts from ppc_pending_interrupt_p9 Matheus Ferst
2022-08-15 21:23 ` Fabiano Rosas
2022-08-17 13:44 ` Matheus K. Ferst
2022-08-19 16:04 ` Fabiano Rosas
2022-08-15 16:20 ` [RFC PATCH 07/13] target/ppc: create an interrupt masking method for POWER8 Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 08/13] target/ppc: remove unused interrupts from ppc_pending_interrupt_p8 Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 09/13] target/ppc: create an interrupt masking method for POWER7 Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 10/13] target/ppc: remove unused interrupts from ppc_pending_interrupt_p7 Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 11/13] target/ppc: remove ppc_store_lpcr from CONFIG_USER_ONLY builds Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 12/13] target/ppc: introduce ppc_maybe_interrupt Matheus Ferst
2022-08-15 16:20 ` [RFC PATCH 13/13] target/ppc: unify cpu->has_work based on cs->interrupt_request Matheus Ferst
2022-08-15 20:02 ` [RFC PATCH 00/13] PowerPC interrupt rework Cédric Le Goater
2022-08-17 13:42 ` Matheus K. Ferst
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.