* [Qemu-devel] [PATCH 01/15] openpic: fix debug prints
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 17:31 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 02/15] openpic: lower interrupt when reading the MSI register Scott Wood
` (13 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Fix various format errors when debug prints are enabled. Also
cause error checking to happen even when debug prints are not
enabled, and consistently use 0x for hex output.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index 93e8208..72a5bc9 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -45,7 +45,11 @@
#ifdef DEBUG_OPENPIC
#define DPRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
#else
-#define DPRINTF(fmt, ...) do { } while (0)
+#define DPRINTF(fmt, ...) do { \
+ if (0) { \
+ printf(fmt , ## __VA_ARGS__); \
+ } \
+ } while (0)
#endif
#define MAX_CPU 15
@@ -421,7 +425,7 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
IRQ_src_t *src;
src = &opp->src[n_IRQ];
- DPRINTF("openpic: set irq %d = %d ipvp=%08x\n",
+ DPRINTF("openpic: set irq %d = %d ipvp=0x%08x\n",
n_IRQ, level, src->ipvp);
if (src->ipvp & IPVP_SENSE_MASK) {
/* level-sensitive irq */
@@ -511,7 +515,8 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
IRQ_dst_t *dst;
int idx;
- DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
+ DPRINTF("%s: addr %#" HWADDR_PRIx " <= 0x%08" PRIx64 "\n",
+ __func__, addr, val);
if (addr & 0xF)
return;
switch (addr) {
@@ -573,7 +578,7 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
OpenPICState *opp = opaque;
uint32_t retval;
- DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+ DPRINTF("%s: addr %#" HWADDR_PRIx "\n", __func__, addr);
retval = 0xFFFFFFFF;
if (addr & 0xF)
return retval;
@@ -619,7 +624,7 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
default:
break;
}
- DPRINTF("%s: => %08x\n", __func__, retval);
+ DPRINTF("%s: => 0x%08x\n", __func__, retval);
return retval;
}
@@ -630,7 +635,8 @@ static void openpic_tmr_write(void *opaque, hwaddr addr, uint64_t val,
OpenPICState *opp = opaque;
int idx;
- DPRINTF("%s: addr %08x <= %08x\n", __func__, addr, val);
+ DPRINTF("%s: addr %#" HWADDR_PRIx " <= 0x%08" PRIx64 "\n",
+ __func__, addr, val);
if (addr & 0xF)
return;
idx = (addr >> 6) & 0x3;
@@ -667,7 +673,7 @@ static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len)
uint32_t retval = -1;
int idx;
- DPRINTF("%s: addr %08x\n", __func__, addr);
+ DPRINTF("%s: addr %#" HWADDR_PRIx "\n", __func__, addr);
if (addr & 0xF) {
goto out;
}
@@ -693,7 +699,7 @@ static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len)
}
out:
- DPRINTF("%s: => %08x\n", __func__, retval);
+ DPRINTF("%s: => 0x%08x\n", __func__, retval);
return retval;
}
@@ -704,7 +710,8 @@ static void openpic_src_write(void *opaque, hwaddr addr, uint64_t val,
OpenPICState *opp = opaque;
int idx;
- DPRINTF("%s: addr %08x <= %08x\n", __func__, addr, val);
+ DPRINTF("%s: addr %#" HWADDR_PRIx " <= 0x%08" PRIx64 "\n",
+ __func__, addr, val);
if (addr & 0xF)
return;
addr = addr & 0xFFF0;
@@ -724,7 +731,7 @@ static uint64_t openpic_src_read(void *opaque, uint64_t addr, unsigned len)
uint32_t retval;
int idx;
- DPRINTF("%s: addr %08x\n", __func__, addr);
+ DPRINTF("%s: addr %#" HWADDR_PRIx "\n", __func__, addr);
retval = 0xFFFFFFFF;
if (addr & 0xF)
return retval;
@@ -737,7 +744,7 @@ static uint64_t openpic_src_read(void *opaque, uint64_t addr, unsigned len)
/* EXVP / IFEVP / IEEVP */
retval = read_IRQreg_ipvp(opp, idx);
}
- DPRINTF("%s: => %08x\n", __func__, retval);
+ DPRINTF("%s: => 0x%08x\n", __func__, retval);
return retval;
}
@@ -749,7 +756,8 @@ static void openpic_msi_write(void *opaque, hwaddr addr, uint64_t val,
int idx = opp->irq_msi;
int srs, ibs;
- DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
+ DPRINTF("%s: addr %#" HWADDR_PRIx " <= 0x%08" PRIx64 "\n",
+ __func__, addr, val);
if (addr & 0xF) {
return;
}
@@ -774,7 +782,7 @@ static uint64_t openpic_msi_read(void *opaque, hwaddr addr, unsigned size)
uint64_t r = 0;
int i, srs;
- DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+ DPRINTF("%s: addr %#" HWADDR_PRIx "\n", __func__, addr);
if (addr & 0xF) {
return -1;
}
@@ -812,7 +820,7 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
IRQ_dst_t *dst;
int s_IRQ, n_IRQ;
- DPRINTF("%s: cpu %d addr " TARGET_FMT_plx " <= %08x\n", __func__, idx,
+ DPRINTF("%s: cpu %d addr %#" HWADDR_PRIx " <= 0x%08x\n", __func__, idx,
addr, val);
if (idx < 0) {
@@ -882,7 +890,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
uint32_t retval;
int n_IRQ;
- DPRINTF("%s: cpu %d addr " TARGET_FMT_plx "\n", __func__, idx, addr);
+ DPRINTF("%s: cpu %d addr %#" HWADDR_PRIx "\n", __func__, idx, addr);
retval = 0xFFFFFFFF;
if (idx < 0) {
@@ -949,7 +957,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
default:
break;
}
- DPRINTF("%s: => %08x\n", __func__, retval);
+ DPRINTF("%s: => 0x%08x\n", __func__, retval);
return retval;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 01/15] openpic: fix debug prints
2012-12-22 2:15 ` [Qemu-devel] [PATCH 01/15] openpic: fix debug prints Scott Wood
@ 2013-01-03 17:31 ` Alexander Graf
2013-01-03 19:41 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 17:31 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> Fix various format errors when debug prints are enabled. Also
> cause error checking to happen even when debug prints are not
> enabled, and consistently use 0x for hex output.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> hw/openpic.c | 40 ++++++++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 93e8208..72a5bc9 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -45,7 +45,11 @@
> #ifdef DEBUG_OPENPIC
static const int debug_openpic = 1;
#else
static const int debug_openpic = 0;
> #define DPRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
> #else
> -#define DPRINTF(fmt, ...) do { } while (0)
> +#define DPRINTF(fmt, ...) do { \
> + if (0) { \
if (debug_openpic)
> + printf(fmt , ## __VA_ARGS__); \
> + } \
> + } while (0)
> #endif
That way we don't need to duplicate the print define. Let me fix that up for you while applying the patch.
Alex
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 01/15] openpic: fix debug prints
2013-01-03 17:31 ` Alexander Graf
@ 2013-01-03 19:41 ` Scott Wood
0 siblings, 0 replies; 47+ messages in thread
From: Scott Wood @ 2013-01-03 19:41 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, qemu-devel
On 01/03/2013 11:31:49 AM, Alexander Graf wrote:
>
> On 22.12.2012, at 03:15, Scott Wood wrote:
>
> > Fix various format errors when debug prints are enabled. Also
> > cause error checking to happen even when debug prints are not
> > enabled, and consistently use 0x for hex output.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > ---
> > hw/openpic.c | 40 ++++++++++++++++++++++++----------------
> > 1 file changed, 24 insertions(+), 16 deletions(-)
> >
> > diff --git a/hw/openpic.c b/hw/openpic.c
> > index 93e8208..72a5bc9 100644
> > --- a/hw/openpic.c
> > +++ b/hw/openpic.c
> > @@ -45,7 +45,11 @@
> > #ifdef DEBUG_OPENPIC
>
> static const int debug_openpic = 1;
> #else
> static const int debug_openpic = 0;
>
> > #define DPRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); }
> while (0)
> > #else
> > -#define DPRINTF(fmt, ...) do { } while (0)
> > +#define DPRINTF(fmt, ...) do { \
> > + if (0) { \
>
> if (debug_openpic)
>
> > + printf(fmt , ## __VA_ARGS__); \
> > + } \
> > + } while (0)
> > #endif
>
> That way we don't need to duplicate the print define. Let me fix that
> up for you while applying the patch.
OK, thanks.
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 02/15] openpic: lower interrupt when reading the MSI register
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
2012-12-22 2:15 ` [Qemu-devel] [PATCH 01/15] openpic: fix debug prints Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 17:52 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 03/15] openpic: fix sense and priority bits Scott Wood
` (12 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
This will stop things from breaking once it's properly treated as a
level-triggered interrupt. Note that it's the MPIC's MSI cascade
interrupts that are level-triggered; the individual MSIs are
edge-triggered.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/openpic.c b/hw/openpic.c
index 72a5bc9..02f793b 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -801,6 +801,7 @@ static uint64_t openpic_msi_read(void *opaque, hwaddr addr, unsigned size)
r = opp->msi[srs].msir;
/* Clear on read */
opp->msi[srs].msir = 0;
+ openpic_set_irq(opp, opp->irq_msi + srs, 0);
break;
case 0x120: /* MSISR */
for (i = 0; i < MAX_MSI; i++) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 02/15] openpic: lower interrupt when reading the MSI register
2012-12-22 2:15 ` [Qemu-devel] [PATCH 02/15] openpic: lower interrupt when reading the MSI register Scott Wood
@ 2013-01-03 17:52 ` Alexander Graf
0 siblings, 0 replies; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 17:52 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> This will stop things from breaking once it's properly treated as a
> level-triggered interrupt. Note that it's the MPIC's MSI cascade
> interrupts that are level-triggered; the individual MSIs are
> edge-triggered.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Thanks, applied to ppc-next.
Alex
> ---
> hw/openpic.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 72a5bc9..02f793b 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -801,6 +801,7 @@ static uint64_t openpic_msi_read(void *opaque, hwaddr addr, unsigned size)
> r = opp->msi[srs].msir;
> /* Clear on read */
> opp->msi[srs].msir = 0;
> + openpic_set_irq(opp, opp->irq_msi + srs, 0);
> break;
> case 0x120: /* MSISR */
> for (i = 0; i < MAX_MSI; i++) {
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 03/15] openpic: fix sense and priority bits
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
2012-12-22 2:15 ` [Qemu-devel] [PATCH 01/15] openpic: fix debug prints Scott Wood
2012-12-22 2:15 ` [Qemu-devel] [PATCH 02/15] openpic: lower interrupt when reading the MSI register Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 17:51 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 04/15] ppc/booke: fix crit/mcheck/debug exceptions Scott Wood
` (11 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Previously, the sense and priority bits were masked off when writing
to IVPR, and all interrupts were treated as edge-triggered (despite
the existence of code for handling level-triggered interrupts).
Polarity is implemented only as storage. We don't simulate the
bad effects that you'd get on real hardware if you set this incorrectly,
but at least the guest sees the right thing when it reads back the register.
Sense now controls level/edge on FSL external interrupts (and all
interrupts on non-FSL MPIC). FSL internal interrupts do not have a sense
bit (reads as zero), but are level. FSL timers and IPIs do not have
sense or polarity bits (read as zero), and are edge-triggered. To
accommodate FSL internal interrupts, QEMU's internal notion of whether an
interrupt is level-triggered is separated from the IVPR bit.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 55 insertions(+), 6 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index 02f793b..34449a7 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -189,6 +189,9 @@ typedef struct IRQ_src_t {
uint32_t ide; /* IRQ destination register */
int last_cpu;
int pending; /* TRUE if IRQ is pending */
+ bool level; /* level-triggered */
+ bool fslint; /* FSL internal interrupt -- level only */
+ bool fslspecial; /* FSL timer/IPI interrupt, edge, no polarity */
} IRQ_src_t;
#define IPVP_MASK_SHIFT 31
@@ -427,7 +430,7 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
src = &opp->src[n_IRQ];
DPRINTF("openpic: set irq %d = %d ipvp=0x%08x\n",
n_IRQ, level, src->ipvp);
- if (src->ipvp & IPVP_SENSE_MASK) {
+ if (src->level) {
/* level-sensitive irq */
src->pending = level;
if (!level) {
@@ -459,6 +462,14 @@ static void openpic_reset(DeviceState *d)
for (i = 0; i < opp->max_irq; i++) {
opp->src[i].ipvp = opp->ipvp_reset;
opp->src[i].ide = opp->ide_reset;
+
+ if (opp->src[i].fslint) {
+ opp->src[i].ipvp |= IPVP_POLARITY_MASK;
+ }
+
+ if (!opp->src[i].fslint && !opp->src[i].fslspecial) {
+ opp->src[i].level = !!(opp->ipvp_reset & IPVP_SENSE_MASK);
+ }
}
/* Initialise IRQ destinations */
for (i = 0; i < MAX_CPU; i++) {
@@ -499,10 +510,30 @@ static inline void write_IRQreg_ide(OpenPICState *opp, int n_IRQ, uint32_t val)
static inline void write_IRQreg_ipvp(OpenPICState *opp, int n_IRQ, uint32_t val)
{
- /* NOTE: not fully accurate for special IRQs, but simple and sufficient */
+ uint32_t mask;
+
+ /* NOTE when implementing newer FSL MPIC models: starting with v4.0,
+ * the polarity bit is read-only on internal interrupts.
+ */
+ mask = IPVP_MASK_MASK | IPVP_PRIORITY_MASK | IPVP_SENSE_MASK |
+ IPVP_POLARITY_MASK | opp->vector_mask;
+
/* ACTIVITY bit is read-only */
- opp->src[n_IRQ].ipvp = (opp->src[n_IRQ].ipvp & IPVP_ACTIVITY_MASK) |
- (val & (IPVP_MASK_MASK | IPVP_PRIORITY_MASK | opp->vector_mask));
+ opp->src[n_IRQ].ipvp =
+ (opp->src[n_IRQ].ipvp & IPVP_ACTIVITY_MASK) | (val & mask);
+
+ /* For FSL internal interrupts, The sense bit is reserved and zero,
+ * and the interrupt is always level-triggered. Timers and IPIs
+ * have no sense or polarity bits, and are edge-triggered.
+ */
+ if (opp->src[n_IRQ].fslint) {
+ opp->src[n_IRQ].ipvp &= ~IPVP_SENSE_MASK;
+ } else if (opp->src[n_IRQ].fslspecial) {
+ opp->src[n_IRQ].ipvp &= ~(IPVP_POLARITY_MASK | IPVP_SENSE_MASK);
+ } else {
+ opp->src[n_IRQ].level = !!(opp->src[n_IRQ].ipvp & IPVP_SENSE_MASK);
+ }
+
openpic_update_irq(opp, n_IRQ);
DPRINTF("Set IPVP %d to 0x%08x -> 0x%08x\n", n_IRQ, val,
opp->src[n_IRQ].ipvp);
@@ -934,7 +965,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
}
IRQ_resetbit(&dst->raised, n_IRQ);
dst->raised.next = -1;
- if (!(src->ipvp & IPVP_SENSE_MASK)) {
+ if (!src->level) {
/* edge-sensitive IRQ */
src->ipvp &= ~IPVP_ACTIVITY_MASK;
src->pending = 0;
@@ -942,7 +973,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
if ((n_IRQ >= opp->irq_ipi0) && (n_IRQ < (opp->irq_ipi0 + MAX_IPI))) {
src->ide &= ~(1 << idx);
- if (src->ide && !(src->ipvp & IPVP_SENSE_MASK)) {
+ if (src->ide && !src->level) {
/* trigger on CPUs that didn't know about it yet */
openpic_set_irq(opp, n_IRQ, 1);
openpic_set_irq(opp, n_IRQ, 0);
@@ -1226,7 +1257,25 @@ static int openpic_init(SysBusDevice *dev)
opp->brr1 = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
msi_supported = true;
list = list_be;
+
+ for (i = 0; i < FSL_MPIC_20_MAX_EXT; i++) {
+ opp->src[i].level = false;
+ }
+
+ /* Internal interrupts, including message and MSI */
+ for (i = 16; i < MAX_SRC; i++) {
+ opp->src[i].fslint = true;
+ opp->src[i].level = true;
+ }
+
+ /* timers and IPIs */
+ for (i = MAX_SRC; i < MAX_IRQ; i++) {
+ opp->src[i].fslspecial = true;
+ opp->src[i].level = false;
+ }
+
break;
+
case OPENPIC_MODEL_RAVEN:
opp->nb_irqs = RAVEN_MAX_EXT;
opp->vid = VID_REVISION_1_3;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 03/15] openpic: fix sense and priority bits
2012-12-22 2:15 ` [Qemu-devel] [PATCH 03/15] openpic: fix sense and priority bits Scott Wood
@ 2013-01-03 17:51 ` Alexander Graf
2013-01-03 20:12 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 17:51 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> Previously, the sense and priority bits were masked off when writing
> to IVPR, and all interrupts were treated as edge-triggered (despite
> the existence of code for handling level-triggered interrupts).
>
> Polarity is implemented only as storage. We don't simulate the
> bad effects that you'd get on real hardware if you set this incorrectly,
> but at least the guest sees the right thing when it reads back the register.
>
> Sense now controls level/edge on FSL external interrupts (and all
> interrupts on non-FSL MPIC). FSL internal interrupts do not have a sense
> bit (reads as zero), but are level. FSL timers and IPIs do not have
> sense or polarity bits (read as zero), and are edge-triggered. To
> accommodate FSL internal interrupts, QEMU's internal notion of whether an
> interrupt is level-triggered is separated from the IVPR bit.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> hw/openpic.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 02f793b..34449a7 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -189,6 +189,9 @@ typedef struct IRQ_src_t {
> uint32_t ide; /* IRQ destination register */
> int last_cpu;
> int pending; /* TRUE if IRQ is pending */
> + bool level; /* level-triggered */
> + bool fslint; /* FSL internal interrupt -- level only */
> + bool fslspecial; /* FSL timer/IPI interrupt, edge, no polarity */
This really looks more like an "irqtype" enum, no?
enum irqtype {
IRQ_TYPE_NORMAL = 0,
IRQ_TYPE_FSLINT,
IRQ_TYPE_FSLSPECIAL,
}
Alex
> } IRQ_src_t;
>
> #define IPVP_MASK_SHIFT 31
> @@ -427,7 +430,7 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
> src = &opp->src[n_IRQ];
> DPRINTF("openpic: set irq %d = %d ipvp=0x%08x\n",
> n_IRQ, level, src->ipvp);
> - if (src->ipvp & IPVP_SENSE_MASK) {
> + if (src->level) {
> /* level-sensitive irq */
> src->pending = level;
> if (!level) {
> @@ -459,6 +462,14 @@ static void openpic_reset(DeviceState *d)
> for (i = 0; i < opp->max_irq; i++) {
> opp->src[i].ipvp = opp->ipvp_reset;
> opp->src[i].ide = opp->ide_reset;
> +
> + if (opp->src[i].fslint) {
> + opp->src[i].ipvp |= IPVP_POLARITY_MASK;
> + }
> +
> + if (!opp->src[i].fslint && !opp->src[i].fslspecial) {
> + opp->src[i].level = !!(opp->ipvp_reset & IPVP_SENSE_MASK);
> + }
> }
> /* Initialise IRQ destinations */
> for (i = 0; i < MAX_CPU; i++) {
> @@ -499,10 +510,30 @@ static inline void write_IRQreg_ide(OpenPICState *opp, int n_IRQ, uint32_t val)
>
> static inline void write_IRQreg_ipvp(OpenPICState *opp, int n_IRQ, uint32_t val)
> {
> - /* NOTE: not fully accurate for special IRQs, but simple and sufficient */
> + uint32_t mask;
> +
> + /* NOTE when implementing newer FSL MPIC models: starting with v4.0,
> + * the polarity bit is read-only on internal interrupts.
> + */
> + mask = IPVP_MASK_MASK | IPVP_PRIORITY_MASK | IPVP_SENSE_MASK |
> + IPVP_POLARITY_MASK | opp->vector_mask;
> +
> /* ACTIVITY bit is read-only */
> - opp->src[n_IRQ].ipvp = (opp->src[n_IRQ].ipvp & IPVP_ACTIVITY_MASK) |
> - (val & (IPVP_MASK_MASK | IPVP_PRIORITY_MASK | opp->vector_mask));
> + opp->src[n_IRQ].ipvp =
> + (opp->src[n_IRQ].ipvp & IPVP_ACTIVITY_MASK) | (val & mask);
> +
> + /* For FSL internal interrupts, The sense bit is reserved and zero,
> + * and the interrupt is always level-triggered. Timers and IPIs
> + * have no sense or polarity bits, and are edge-triggered.
> + */
> + if (opp->src[n_IRQ].fslint) {
> + opp->src[n_IRQ].ipvp &= ~IPVP_SENSE_MASK;
> + } else if (opp->src[n_IRQ].fslspecial) {
> + opp->src[n_IRQ].ipvp &= ~(IPVP_POLARITY_MASK | IPVP_SENSE_MASK);
> + } else {
> + opp->src[n_IRQ].level = !!(opp->src[n_IRQ].ipvp & IPVP_SENSE_MASK);
> + }
> +
> openpic_update_irq(opp, n_IRQ);
> DPRINTF("Set IPVP %d to 0x%08x -> 0x%08x\n", n_IRQ, val,
> opp->src[n_IRQ].ipvp);
> @@ -934,7 +965,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
> }
> IRQ_resetbit(&dst->raised, n_IRQ);
> dst->raised.next = -1;
> - if (!(src->ipvp & IPVP_SENSE_MASK)) {
> + if (!src->level) {
> /* edge-sensitive IRQ */
> src->ipvp &= ~IPVP_ACTIVITY_MASK;
> src->pending = 0;
> @@ -942,7 +973,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
>
> if ((n_IRQ >= opp->irq_ipi0) && (n_IRQ < (opp->irq_ipi0 + MAX_IPI))) {
> src->ide &= ~(1 << idx);
> - if (src->ide && !(src->ipvp & IPVP_SENSE_MASK)) {
> + if (src->ide && !src->level) {
> /* trigger on CPUs that didn't know about it yet */
> openpic_set_irq(opp, n_IRQ, 1);
> openpic_set_irq(opp, n_IRQ, 0);
> @@ -1226,7 +1257,25 @@ static int openpic_init(SysBusDevice *dev)
> opp->brr1 = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
> msi_supported = true;
> list = list_be;
> +
> + for (i = 0; i < FSL_MPIC_20_MAX_EXT; i++) {
> + opp->src[i].level = false;
> + }
> +
> + /* Internal interrupts, including message and MSI */
> + for (i = 16; i < MAX_SRC; i++) {
> + opp->src[i].fslint = true;
> + opp->src[i].level = true;
> + }
> +
> + /* timers and IPIs */
> + for (i = MAX_SRC; i < MAX_IRQ; i++) {
> + opp->src[i].fslspecial = true;
> + opp->src[i].level = false;
> + }
> +
> break;
> +
> case OPENPIC_MODEL_RAVEN:
> opp->nb_irqs = RAVEN_MAX_EXT;
> opp->vid = VID_REVISION_1_3;
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 03/15] openpic: fix sense and priority bits
2013-01-03 17:51 ` Alexander Graf
@ 2013-01-03 20:12 ` Scott Wood
0 siblings, 0 replies; 47+ messages in thread
From: Scott Wood @ 2013-01-03 20:12 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, qemu-devel
On 01/03/2013 11:51:56 AM, Alexander Graf wrote:
>
> On 22.12.2012, at 03:15, Scott Wood wrote:
>
> > Previously, the sense and priority bits were masked off when writing
> > to IVPR, and all interrupts were treated as edge-triggered (despite
> > the existence of code for handling level-triggered interrupts).
> >
> > Polarity is implemented only as storage. We don't simulate the
> > bad effects that you'd get on real hardware if you set this
> incorrectly,
> > but at least the guest sees the right thing when it reads back the
> register.
> >
> > Sense now controls level/edge on FSL external interrupts (and all
> > interrupts on non-FSL MPIC). FSL internal interrupts do not have a
> sense
> > bit (reads as zero), but are level. FSL timers and IPIs do not have
> > sense or polarity bits (read as zero), and are edge-triggered. To
> > accommodate FSL internal interrupts, QEMU's internal notion of
> whether an
> > interrupt is level-triggered is separated from the IVPR bit.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > ---
> > hw/openpic.c | 61
> ++++++++++++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 55 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/openpic.c b/hw/openpic.c
> > index 02f793b..34449a7 100644
> > --- a/hw/openpic.c
> > +++ b/hw/openpic.c
> > @@ -189,6 +189,9 @@ typedef struct IRQ_src_t {
> > uint32_t ide; /* IRQ destination register */
> > int last_cpu;
> > int pending; /* TRUE if IRQ is pending */
> > + bool level; /* level-triggered */
> > + bool fslint; /* FSL internal interrupt -- level only */
> > + bool fslspecial; /* FSL timer/IPI interrupt, edge, no polarity
> */
>
> This really looks more like an "irqtype" enum, no?
>
> enum irqtype {
> IRQ_TYPE_NORMAL = 0,
> IRQ_TYPE_FSLINT,
> IRQ_TYPE_FSLSPECIAL,
> }
OK. At one point they could both be set, before I looked more closely
at how the special interrupts are defined in hardware.
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 04/15] ppc/booke: fix crit/mcheck/debug exceptions
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (2 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 03/15] openpic: fix sense and priority bits Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 17:57 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 05/15] openpic: make register names correspond better with hw docs Scott Wood
` (10 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Book E does not play games with certain bits of xSRR1 being MSR save
bits and others being error status. xSRR1 is the old MSR, period.
This was causing things like MSR[CE] to be lost, even in the saved
version, as soon as you take an exception.
rfci/rfdi/rfmci are fixed to pass the actual xSRR1 register contents,
rather than the register number.
Put FIXME comments on the hack that is "asrr0/1". The whole point of
separate exception levels is so that you can, for example, take a machine
check or debug interrupt without corrupting critical-level operations.
The right xSRR0/1 set needs to be chosen based on CPU type flags.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
target-ppc/excp_helper.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 5e34ad0..41037a7 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -84,7 +84,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
" => %08x (%02x)\n", env->nip, excp, env->error_code);
/* new srr1 value excluding must-be-zero bits */
- msr = env->msr & ~0x783f0000ULL;
+ if (excp_model == POWERPC_EXCP_BOOKE) {
+ msr = env->msr;
+ } else {
+ msr = env->msr & ~0x783f0000ULL;
+ }
/* new interrupt handler msr */
new_msr = env->msr & ((target_ulong)1 << MSR_ME);
@@ -145,6 +149,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
srr1 = SPR_40x_SRR3;
break;
case POWERPC_EXCP_BOOKE:
+ /* FIXME: choose one or the other based on CPU type */
srr0 = SPR_BOOKE_MCSRR0;
srr1 = SPR_BOOKE_MCSRR1;
asrr0 = SPR_BOOKE_CSRR0;
@@ -275,6 +280,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
case POWERPC_EXCP_DEBUG: /* Debug interrupt */
switch (excp_model) {
case POWERPC_EXCP_BOOKE:
+ /* FIXME: choose one or the other based on CPU type */
srr0 = SPR_BOOKE_DSRR0;
srr1 = SPR_BOOKE_DSRR1;
asrr0 = SPR_BOOKE_CSRR0;
@@ -836,8 +842,13 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
void helper_rfi(CPUPPCState *env)
{
- do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
- ~((target_ulong)0x783F0000), 1);
+ if (env->excp_model == POWERPC_EXCP_BOOKE) {
+ do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
+ ~((target_ulong)0), 0);
+ } else {
+ do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
+ ~((target_ulong)0x783F0000), 1);
+ }
}
#if defined(TARGET_PPC64)
@@ -864,20 +875,22 @@ void helper_40x_rfci(CPUPPCState *env)
void helper_rfci(CPUPPCState *env)
{
- do_rfi(env, env->spr[SPR_BOOKE_CSRR0], SPR_BOOKE_CSRR1,
- ~((target_ulong)0x3FFF0000), 0);
+ do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1],
+ ~((target_ulong)0), 0);
}
void helper_rfdi(CPUPPCState *env)
{
- do_rfi(env, env->spr[SPR_BOOKE_DSRR0], SPR_BOOKE_DSRR1,
- ~((target_ulong)0x3FFF0000), 0);
+ /* FIXME: choose CSRR1 or DSRR1 based on cpu type */
+ do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1],
+ ~((target_ulong)0), 0);
}
void helper_rfmci(CPUPPCState *env)
{
- do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], SPR_BOOKE_MCSRR1,
- ~((target_ulong)0x3FFF0000), 0);
+ /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */
+ do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1],
+ ~((target_ulong)0), 0);
}
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 04/15] ppc/booke: fix crit/mcheck/debug exceptions
2012-12-22 2:15 ` [Qemu-devel] [PATCH 04/15] ppc/booke: fix crit/mcheck/debug exceptions Scott Wood
@ 2013-01-03 17:57 ` Alexander Graf
0 siblings, 0 replies; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 17:57 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> Book E does not play games with certain bits of xSRR1 being MSR save
> bits and others being error status. xSRR1 is the old MSR, period.
> This was causing things like MSR[CE] to be lost, even in the saved
> version, as soon as you take an exception.
>
> rfci/rfdi/rfmci are fixed to pass the actual xSRR1 register contents,
> rather than the register number.
>
> Put FIXME comments on the hack that is "asrr0/1". The whole point of
> separate exception levels is so that you can, for example, take a machine
> check or debug interrupt without corrupting critical-level operations.
> The right xSRR0/1 set needs to be chosen based on CPU type flags.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Thanks, applied to ppc-next.
Alex
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 05/15] openpic: make register names correspond better with hw docs
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (3 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 04/15] ppc/booke: fix crit/mcheck/debug exceptions Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:18 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 06/15] openpic: rework critical interrupt support Scott Wood
` (9 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
The base openpic specification doesn't provide abbreviated register
names, so it's somewhat understandable that the QEMU code made up
its own, except that most of the names that QEMU used didn't correspond
to the terminology used by any implementation I could find.
In some cases, like PCTP, the phrase "processor current task priority"
could be found in the openpic spec when describing the concept, but
the register itself was labelled "current task priority register"
and every implementation seems to use either CTPR or the full phrase.
In other cases, individual implementations disagree on what to call
the register. The implementations I have documentation for are
Freescale, Raven (MCP750), and IBM. The Raven docs tend to not use
abbreviations at all. The IBM MPIC isn't implemented in QEMU. Thus,
where there's disagreement I chose to use the Freescale abbreviations.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
BTW, I'm still not sure where the first "P" in QEMU's "IPVP" came from.
---
hw/openpic.c | 362 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 181 insertions(+), 181 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index 34449a7..7647368 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -61,7 +61,7 @@
#define VID 0x03 /* MPIC version ID */
/* OpenPIC capability flags */
-#define OPENPIC_FLAG_IDE_CRIT (1 << 0)
+#define OPENPIC_FLAG_IDR_CRIT (1 << 0)
/* OpenPIC address map */
#define OPENPIC_GLB_REG_START 0x0
@@ -118,19 +118,19 @@
#define FSL_BRR1_IPMJ (0x00 << 8) /* 8 bit IP major number */
#define FSL_BRR1_IPMN 0x00 /* 8 bit IP minor number */
-#define FREP_NIRQ_SHIFT 16
-#define FREP_NCPU_SHIFT 8
-#define FREP_VID_SHIFT 0
+#define FRR_NIRQ_SHIFT 16
+#define FRR_NCPU_SHIFT 8
+#define FRR_VID_SHIFT 0
#define VID_REVISION_1_2 2
#define VID_REVISION_1_3 3
-#define VENI_GENERIC 0x00000000 /* Generic Vendor ID */
+#define VIR_GENERIC 0x00000000 /* Generic Vendor ID */
-#define GLBC_RESET 0x80000000
+#define GCR_RESET 0x80000000
-#define TIBC_CI 0x80000000 /* count inhibit */
-#define TICC_TOG 0x80000000 /* toggles when decrement to zero */
+#define TBCR_CI 0x80000000 /* count inhibit */
+#define TCCR_TOG 0x80000000 /* toggles when decrement to zero */
#define IDR_EP_SHIFT 31
#define IDR_EP_MASK (1 << IDR_EP_SHIFT)
@@ -185,8 +185,8 @@ typedef struct IRQ_queue_t {
} IRQ_queue_t;
typedef struct IRQ_src_t {
- uint32_t ipvp; /* IRQ vector/priority register */
- uint32_t ide; /* IRQ destination register */
+ uint32_t ivpr; /* IRQ vector/priority register */
+ uint32_t idr; /* IRQ destination register */
int last_cpu;
int pending; /* TRUE if IRQ is pending */
bool level; /* level-triggered */
@@ -194,27 +194,27 @@ typedef struct IRQ_src_t {
bool fslspecial; /* FSL timer/IPI interrupt, edge, no polarity */
} IRQ_src_t;
-#define IPVP_MASK_SHIFT 31
-#define IPVP_MASK_MASK (1 << IPVP_MASK_SHIFT)
-#define IPVP_ACTIVITY_SHIFT 30
-#define IPVP_ACTIVITY_MASK (1 << IPVP_ACTIVITY_SHIFT)
-#define IPVP_MODE_SHIFT 29
-#define IPVP_MODE_MASK (1 << IPVP_MODE_SHIFT)
-#define IPVP_POLARITY_SHIFT 23
-#define IPVP_POLARITY_MASK (1 << IPVP_POLARITY_SHIFT)
-#define IPVP_SENSE_SHIFT 22
-#define IPVP_SENSE_MASK (1 << IPVP_SENSE_SHIFT)
-
-#define IPVP_PRIORITY_MASK (0xF << 16)
-#define IPVP_PRIORITY(_ipvpr_) ((int)(((_ipvpr_) & IPVP_PRIORITY_MASK) >> 16))
-#define IPVP_VECTOR(opp, _ipvpr_) ((_ipvpr_) & (opp)->vector_mask)
+#define IVPR_MASK_SHIFT 31
+#define IVPR_MASK_MASK (1 << IVPR_MASK_SHIFT)
+#define IVPR_ACTIVITY_SHIFT 30
+#define IVPR_ACTIVITY_MASK (1 << IVPR_ACTIVITY_SHIFT)
+#define IVPR_MODE_SHIFT 29
+#define IVPR_MODE_MASK (1 << IVPR_MODE_SHIFT)
+#define IVPR_POLARITY_SHIFT 23
+#define IVPR_POLARITY_MASK (1 << IVPR_POLARITY_SHIFT)
+#define IVPR_SENSE_SHIFT 22
+#define IVPR_SENSE_MASK (1 << IVPR_SENSE_SHIFT)
+
+#define IVPR_PRIORITY_MASK (0xF << 16)
+#define IVPR_PRIORITY(_ivprr_) ((int)(((_ivprr_) & IVPR_PRIORITY_MASK) >> 16))
+#define IVPR_VECTOR(opp, _ivprr_) ((_ivprr_) & (opp)->vector_mask)
/* IDE[EP/CI] are only for FSL MPIC prior to v4.0 */
-#define IDE_EP 0x80000000 /* external pin */
-#define IDE_CI 0x40000000 /* critical interrupt */
+#define IDR_EP 0x80000000 /* external pin */
+#define IDR_CI 0x40000000 /* critical interrupt */
typedef struct IRQ_dst_t {
- uint32_t pctp; /* CPU current task priority */
+ uint32_t ctpr; /* CPU current task priority */
IRQ_queue_t raised;
IRQ_queue_t servicing;
qemu_irq *irqs;
@@ -229,22 +229,22 @@ typedef struct OpenPICState {
uint32_t flags;
uint32_t nb_irqs;
uint32_t vid;
- uint32_t veni; /* Vendor identification register */
+ uint32_t vir; /* Vendor identification register */
uint32_t vector_mask;
- uint32_t tifr_reset;
- uint32_t ipvp_reset;
- uint32_t ide_reset;
+ uint32_t tfrr_reset;
+ uint32_t ivpr_reset;
+ uint32_t idr_reset;
uint32_t brr1;
/* Sub-regions */
MemoryRegion sub_io_mem[5];
/* Global registers */
- uint32_t frep; /* Feature reporting register */
- uint32_t glbc; /* Global configuration register */
- uint32_t pint; /* Processor initialization register */
+ uint32_t frr; /* Feature reporting register */
+ uint32_t gcr; /* Global configuration register */
+ uint32_t pir; /* Processor initialization register */
uint32_t spve; /* Spurious vector register */
- uint32_t tifr; /* Timer frequency reporting register */
+ uint32_t tfrr; /* Timer frequency reporting register */
/* Source registers */
IRQ_src_t src[MAX_IRQ];
/* Local registers per output pin */
@@ -252,8 +252,8 @@ typedef struct OpenPICState {
uint32_t nb_cpus;
/* Timer registers */
struct {
- uint32_t ticc; /* Global timer current count register */
- uint32_t tibc; /* Global timer base count register */
+ uint32_t tccr; /* Global timer current count register */
+ uint32_t tbcr; /* Global timer base count register */
} timers[MAX_TMR];
/* Shared MSI registers */
struct {
@@ -299,11 +299,11 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
for (i = 0; i < opp->max_irq; i++) {
if (IRQ_testbit(q, i)) {
- DPRINTF("IRQ_check: irq %d set ipvp_pr=%d pr=%d\n",
- i, IPVP_PRIORITY(opp->src[i].ipvp), priority);
- if (IPVP_PRIORITY(opp->src[i].ipvp) > priority) {
+ DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
+ i, IVPR_PRIORITY(opp->src[i].ivpr), priority);
+ if (IVPR_PRIORITY(opp->src[i].ivpr) > priority) {
next = i;
- priority = IPVP_PRIORITY(opp->src[i].ipvp);
+ priority = IVPR_PRIORITY(opp->src[i].ivpr);
}
}
}
@@ -331,8 +331,8 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
dst = &opp->dst[n_CPU];
src = &opp->src[n_IRQ];
- priority = IPVP_PRIORITY(src->ipvp);
- if (priority <= dst->pctp) {
+ priority = IVPR_PRIORITY(src->ivpr);
+ if (priority <= dst->ctpr) {
/* Too low priority */
DPRINTF("%s: IRQ %d has too low priority on CPU %d\n",
__func__, n_IRQ, n_CPU);
@@ -344,7 +344,7 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
__func__, n_IRQ, n_CPU);
return;
}
- src->ipvp |= IPVP_ACTIVITY_MASK;
+ src->ivpr |= IVPR_ACTIVITY_MASK;
IRQ_setbit(&dst->raised, n_IRQ);
if (priority < dst->raised.priority) {
/* An higher priority IRQ is already raised */
@@ -377,34 +377,34 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
DPRINTF("%s: IRQ %d is not pending\n", __func__, n_IRQ);
return;
}
- if (src->ipvp & IPVP_MASK_MASK) {
+ if (src->ivpr & IVPR_MASK_MASK) {
/* Interrupt source is disabled */
DPRINTF("%s: IRQ %d is disabled\n", __func__, n_IRQ);
return;
}
- if (IPVP_PRIORITY(src->ipvp) == 0) {
+ if (IVPR_PRIORITY(src->ivpr) == 0) {
/* Priority set to zero */
DPRINTF("%s: IRQ %d has 0 priority\n", __func__, n_IRQ);
return;
}
- if (src->ipvp & IPVP_ACTIVITY_MASK) {
+ if (src->ivpr & IVPR_ACTIVITY_MASK) {
/* IRQ already active */
DPRINTF("%s: IRQ %d is already active\n", __func__, n_IRQ);
return;
}
- if (src->ide == 0) {
+ if (src->idr == 0) {
/* No target */
DPRINTF("%s: IRQ %d has no target\n", __func__, n_IRQ);
return;
}
- if (src->ide == (1 << src->last_cpu)) {
+ if (src->idr == (1 << src->last_cpu)) {
/* Only one CPU is allowed to receive this IRQ */
IRQ_local_pipe(opp, src->last_cpu, n_IRQ);
- } else if (!(src->ipvp & IPVP_MODE_MASK)) {
+ } else if (!(src->ivpr & IVPR_MODE_MASK)) {
/* Directed delivery mode */
for (i = 0; i < opp->nb_cpus; i++) {
- if (src->ide & (1 << i)) {
+ if (src->idr & (1 << i)) {
IRQ_local_pipe(opp, i, n_IRQ);
}
}
@@ -413,7 +413,7 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
for (i = src->last_cpu + 1; i != src->last_cpu; i++) {
if (i == opp->nb_cpus)
i = 0;
- if (src->ide & (1 << i)) {
+ if (src->idr & (1 << i)) {
IRQ_local_pipe(opp, i, n_IRQ);
src->last_cpu = i;
break;
@@ -428,13 +428,13 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
IRQ_src_t *src;
src = &opp->src[n_IRQ];
- DPRINTF("openpic: set irq %d = %d ipvp=0x%08x\n",
- n_IRQ, level, src->ipvp);
+ DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n",
+ n_IRQ, level, src->ivpr);
if (src->level) {
/* level-sensitive irq */
src->pending = level;
if (!level) {
- src->ipvp &= ~IPVP_ACTIVITY_MASK;
+ src->ivpr &= ~IVPR_ACTIVITY_MASK;
}
} else {
/* edge-sensitive irq */
@@ -449,31 +449,31 @@ static void openpic_reset(DeviceState *d)
OpenPICState *opp = FROM_SYSBUS(typeof (*opp), sysbus_from_qdev(d));
int i;
- opp->glbc = GLBC_RESET;
+ opp->gcr = GCR_RESET;
/* Initialise controller registers */
- opp->frep = ((opp->nb_irqs - 1) << FREP_NIRQ_SHIFT) |
- ((opp->nb_cpus - 1) << FREP_NCPU_SHIFT) |
- (opp->vid << FREP_VID_SHIFT);
+ opp->frr = ((opp->nb_irqs - 1) << FRR_NIRQ_SHIFT) |
+ ((opp->nb_cpus - 1) << FRR_NCPU_SHIFT) |
+ (opp->vid << FRR_VID_SHIFT);
- opp->pint = 0;
+ opp->pir = 0;
opp->spve = -1 & opp->vector_mask;
- opp->tifr = opp->tifr_reset;
+ opp->tfrr = opp->tfrr_reset;
/* Initialise IRQ sources */
for (i = 0; i < opp->max_irq; i++) {
- opp->src[i].ipvp = opp->ipvp_reset;
- opp->src[i].ide = opp->ide_reset;
+ opp->src[i].ivpr = opp->ivpr_reset;
+ opp->src[i].idr = opp->idr_reset;
if (opp->src[i].fslint) {
- opp->src[i].ipvp |= IPVP_POLARITY_MASK;
+ opp->src[i].ivpr |= IVPR_POLARITY_MASK;
}
if (!opp->src[i].fslint && !opp->src[i].fslspecial) {
- opp->src[i].level = !!(opp->ipvp_reset & IPVP_SENSE_MASK);
+ opp->src[i].level = !!(opp->ivpr_reset & IVPR_SENSE_MASK);
}
}
/* Initialise IRQ destinations */
for (i = 0; i < MAX_CPU; i++) {
- opp->dst[i].pctp = 15;
+ opp->dst[i].ctpr = 15;
memset(&opp->dst[i].raised, 0, sizeof(IRQ_queue_t));
opp->dst[i].raised.next = -1;
memset(&opp->dst[i].servicing, 0, sizeof(IRQ_queue_t));
@@ -481,62 +481,62 @@ static void openpic_reset(DeviceState *d)
}
/* Initialise timers */
for (i = 0; i < MAX_TMR; i++) {
- opp->timers[i].ticc = 0;
- opp->timers[i].tibc = TIBC_CI;
+ opp->timers[i].tccr = 0;
+ opp->timers[i].tbcr = TBCR_CI;
}
/* Go out of RESET state */
- opp->glbc = 0;
+ opp->gcr = 0;
}
-static inline uint32_t read_IRQreg_ide(OpenPICState *opp, int n_IRQ)
+static inline uint32_t read_IRQreg_idr(OpenPICState *opp, int n_IRQ)
{
- return opp->src[n_IRQ].ide;
+ return opp->src[n_IRQ].idr;
}
-static inline uint32_t read_IRQreg_ipvp(OpenPICState *opp, int n_IRQ)
+static inline uint32_t read_IRQreg_ivpr(OpenPICState *opp, int n_IRQ)
{
- return opp->src[n_IRQ].ipvp;
+ return opp->src[n_IRQ].ivpr;
}
-static inline void write_IRQreg_ide(OpenPICState *opp, int n_IRQ, uint32_t val)
+static inline void write_IRQreg_idr(OpenPICState *opp, int n_IRQ, uint32_t val)
{
uint32_t tmp;
- tmp = val & (IDE_EP | IDE_CI);
+ tmp = val & (IDR_EP | IDR_CI);
tmp |= val & ((1ULL << MAX_CPU) - 1);
- opp->src[n_IRQ].ide = tmp;
- DPRINTF("Set IDE %d to 0x%08x\n", n_IRQ, opp->src[n_IRQ].ide);
+ opp->src[n_IRQ].idr = tmp;
+ DPRINTF("Set IDE %d to 0x%08x\n", n_IRQ, opp->src[n_IRQ].idr);
}
-static inline void write_IRQreg_ipvp(OpenPICState *opp, int n_IRQ, uint32_t val)
+static inline void write_IRQreg_ivpr(OpenPICState *opp, int n_IRQ, uint32_t val)
{
uint32_t mask;
/* NOTE when implementing newer FSL MPIC models: starting with v4.0,
* the polarity bit is read-only on internal interrupts.
*/
- mask = IPVP_MASK_MASK | IPVP_PRIORITY_MASK | IPVP_SENSE_MASK |
- IPVP_POLARITY_MASK | opp->vector_mask;
+ mask = IVPR_MASK_MASK | IVPR_PRIORITY_MASK | IVPR_SENSE_MASK |
+ IVPR_POLARITY_MASK | opp->vector_mask;
/* ACTIVITY bit is read-only */
- opp->src[n_IRQ].ipvp =
- (opp->src[n_IRQ].ipvp & IPVP_ACTIVITY_MASK) | (val & mask);
+ opp->src[n_IRQ].ivpr =
+ (opp->src[n_IRQ].ivpr & IVPR_ACTIVITY_MASK) | (val & mask);
/* For FSL internal interrupts, The sense bit is reserved and zero,
* and the interrupt is always level-triggered. Timers and IPIs
* have no sense or polarity bits, and are edge-triggered.
*/
if (opp->src[n_IRQ].fslint) {
- opp->src[n_IRQ].ipvp &= ~IPVP_SENSE_MASK;
+ opp->src[n_IRQ].ivpr &= ~IVPR_SENSE_MASK;
} else if (opp->src[n_IRQ].fslspecial) {
- opp->src[n_IRQ].ipvp &= ~(IPVP_POLARITY_MASK | IPVP_SENSE_MASK);
+ opp->src[n_IRQ].ivpr &= ~(IVPR_POLARITY_MASK | IVPR_SENSE_MASK);
} else {
- opp->src[n_IRQ].level = !!(opp->src[n_IRQ].ipvp & IPVP_SENSE_MASK);
+ opp->src[n_IRQ].level = !!(opp->src[n_IRQ].ivpr & IVPR_SENSE_MASK);
}
openpic_update_irq(opp, n_IRQ);
- DPRINTF("Set IPVP %d to 0x%08x -> 0x%08x\n", n_IRQ, val,
- opp->src[n_IRQ].ipvp);
+ DPRINTF("Set IVPR %d to 0x%08x -> 0x%08x\n", n_IRQ, val,
+ opp->src[n_IRQ].ivpr);
}
static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
@@ -563,37 +563,37 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
case 0xB0:
openpic_cpu_write_internal(opp, addr, val, get_current_cpu());
break;
- case 0x1000: /* FREP */
+ case 0x1000: /* FRR */
break;
- case 0x1020: /* GLBC */
- if (val & GLBC_RESET) {
+ case 0x1020: /* GCR */
+ if (val & GCR_RESET) {
openpic_reset(&opp->busdev.qdev);
}
break;
- case 0x1080: /* VENI */
+ case 0x1080: /* VIR */
break;
- case 0x1090: /* PINT */
+ case 0x1090: /* PIR */
for (idx = 0; idx < opp->nb_cpus; idx++) {
- if ((val & (1 << idx)) && !(opp->pint & (1 << idx))) {
+ if ((val & (1 << idx)) && !(opp->pir & (1 << idx))) {
DPRINTF("Raise OpenPIC RESET output for CPU %d\n", idx);
dst = &opp->dst[idx];
qemu_irq_raise(dst->irqs[OPENPIC_OUTPUT_RESET]);
- } else if (!(val & (1 << idx)) && (opp->pint & (1 << idx))) {
+ } else if (!(val & (1 << idx)) && (opp->pir & (1 << idx))) {
DPRINTF("Lower OpenPIC RESET output for CPU %d\n", idx);
dst = &opp->dst[idx];
qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_RESET]);
}
}
- opp->pint = val;
+ opp->pir = val;
break;
- case 0x10A0: /* IPI_IPVP */
+ case 0x10A0: /* IPI_IVPR */
case 0x10B0:
case 0x10C0:
case 0x10D0:
{
int idx;
idx = (addr - 0x10A0) >> 4;
- write_IRQreg_ipvp(opp, opp->irq_ipi0 + idx, val);
+ write_IRQreg_ivpr(opp, opp->irq_ipi0 + idx, val);
}
break;
case 0x10E0: /* SPVE */
@@ -614,16 +614,16 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
if (addr & 0xF)
return retval;
switch (addr) {
- case 0x1000: /* FREP */
- retval = opp->frep;
+ case 0x1000: /* FRR */
+ retval = opp->frr;
break;
- case 0x1020: /* GLBC */
- retval = opp->glbc;
+ case 0x1020: /* GCR */
+ retval = opp->gcr;
break;
- case 0x1080: /* VENI */
- retval = opp->veni;
+ case 0x1080: /* VIR */
+ retval = opp->vir;
break;
- case 0x1090: /* PINT */
+ case 0x1090: /* PIR */
retval = 0x00000000;
break;
case 0x00: /* Block Revision Register1 (BRR1) */
@@ -639,14 +639,14 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
case 0xB0:
retval = openpic_cpu_read_internal(opp, addr, get_current_cpu());
break;
- case 0x10A0: /* IPI_IPVP */
+ case 0x10A0: /* IPI_IVPR */
case 0x10B0:
case 0x10C0:
case 0x10D0:
{
int idx;
idx = (addr - 0x10A0) >> 4;
- retval = read_IRQreg_ipvp(opp, opp->irq_ipi0 + idx);
+ retval = read_IRQreg_ivpr(opp, opp->irq_ipi0 + idx);
}
break;
case 0x10E0: /* SPVE */
@@ -674,26 +674,26 @@ static void openpic_tmr_write(void *opaque, hwaddr addr, uint64_t val,
addr = addr & 0x30;
if (addr == 0x0) {
- /* TIFR (TFRR) */
- opp->tifr = val;
+ /* TFRR */
+ opp->tfrr = val;
return;
}
switch (addr & 0x30) {
- case 0x00: /* TICC (GTCCR) */
+ case 0x00: /* TCCR */
break;
- case 0x10: /* TIBC (GTBCR) */
- if ((opp->timers[idx].ticc & TICC_TOG) != 0 &&
- (val & TIBC_CI) == 0 &&
- (opp->timers[idx].tibc & TIBC_CI) != 0) {
- opp->timers[idx].ticc &= ~TICC_TOG;
+ case 0x10: /* TBCR */
+ if ((opp->timers[idx].tccr & TCCR_TOG) != 0 &&
+ (val & TBCR_CI) == 0 &&
+ (opp->timers[idx].tbcr & TBCR_CI) != 0) {
+ opp->timers[idx].tccr &= ~TCCR_TOG;
}
- opp->timers[idx].tibc = val;
+ opp->timers[idx].tbcr = val;
break;
- case 0x20: /* TIVP (GTIVPR) */
- write_IRQreg_ipvp(opp, opp->irq_tim0 + idx, val);
+ case 0x20: /* TVPR */
+ write_IRQreg_ivpr(opp, opp->irq_tim0 + idx, val);
break;
- case 0x30: /* TIDE (GTIDR) */
- write_IRQreg_ide(opp, opp->irq_tim0 + idx, val);
+ case 0x30: /* TDR */
+ write_IRQreg_idr(opp, opp->irq_tim0 + idx, val);
break;
}
}
@@ -710,22 +710,22 @@ static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len)
}
idx = (addr >> 6) & 0x3;
if (addr == 0x0) {
- /* TIFR (TFRR) */
- retval = opp->tifr;
+ /* TFRR */
+ retval = opp->tfrr;
goto out;
}
switch (addr & 0x30) {
- case 0x00: /* TICC (GTCCR) */
- retval = opp->timers[idx].ticc;
+ case 0x00: /* TCCR */
+ retval = opp->timers[idx].tccr;
break;
- case 0x10: /* TIBC (GTBCR) */
- retval = opp->timers[idx].tibc;
+ case 0x10: /* TBCR */
+ retval = opp->timers[idx].tbcr;
break;
- case 0x20: /* TIPV (TIPV) */
- retval = read_IRQreg_ipvp(opp, opp->irq_tim0 + idx);
+ case 0x20: /* TIPV */
+ retval = read_IRQreg_ivpr(opp, opp->irq_tim0 + idx);
break;
case 0x30: /* TIDE (TIDR) */
- retval = read_IRQreg_ide(opp, opp->irq_tim0 + idx);
+ retval = read_IRQreg_idr(opp, opp->irq_tim0 + idx);
break;
}
@@ -749,10 +749,10 @@ static void openpic_src_write(void *opaque, hwaddr addr, uint64_t val,
idx = addr >> 5;
if (addr & 0x10) {
/* EXDE / IFEDE / IEEDE */
- write_IRQreg_ide(opp, idx, val);
+ write_IRQreg_idr(opp, idx, val);
} else {
/* EXVP / IFEVP / IEEVP */
- write_IRQreg_ipvp(opp, idx, val);
+ write_IRQreg_ivpr(opp, idx, val);
}
}
@@ -770,10 +770,10 @@ static uint64_t openpic_src_read(void *opaque, uint64_t addr, unsigned len)
idx = addr >> 5;
if (addr & 0x10) {
/* EXDE / IFEDE / IEEDE */
- retval = read_IRQreg_ide(opp, idx);
+ retval = read_IRQreg_idr(opp, idx);
} else {
/* EXVP / IFEVP / IEEVP */
- retval = read_IRQreg_ipvp(opp, idx);
+ retval = read_IRQreg_ivpr(opp, idx);
}
DPRINTF("%s: => 0x%08x\n", __func__, retval);
@@ -870,22 +870,22 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
case 0x70:
idx = (addr - 0x40) >> 4;
/* we use IDE as mask which CPUs to deliver the IPI to still. */
- write_IRQreg_ide(opp, opp->irq_ipi0 + idx,
- opp->src[opp->irq_ipi0 + idx].ide | val);
+ write_IRQreg_idr(opp, opp->irq_ipi0 + idx,
+ opp->src[opp->irq_ipi0 + idx].idr | val);
openpic_set_irq(opp, opp->irq_ipi0 + idx, 1);
openpic_set_irq(opp, opp->irq_ipi0 + idx, 0);
break;
- case 0x80: /* PCTP */
- dst->pctp = val & 0x0000000F;
+ case 0x80: /* CTPR */
+ dst->ctpr = val & 0x0000000F;
break;
case 0x90: /* WHOAMI */
/* Read-only register */
break;
- case 0xA0: /* PIAC */
+ case 0xA0: /* IACK */
/* Read-only register */
break;
- case 0xB0: /* PEOI */
- DPRINTF("PEOI\n");
+ case 0xB0: /* EOI */
+ DPRINTF("EOI\n");
s_IRQ = IRQ_get_next(opp, &dst->servicing);
IRQ_resetbit(&dst->servicing, s_IRQ);
dst->servicing.next = -1;
@@ -896,7 +896,7 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
src = &opp->src[n_IRQ];
if (n_IRQ != -1 &&
(s_IRQ == -1 ||
- IPVP_PRIORITY(src->ipvp) > dst->servicing.priority)) {
+ IVPR_PRIORITY(src->ivpr) > dst->servicing.priority)) {
DPRINTF("Raise OpenPIC INT output cpu %d irq %d\n",
idx, n_IRQ);
openpic_irq_raise(opp, idx, src);
@@ -934,56 +934,56 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
dst = &opp->dst[idx];
addr &= 0xFF0;
switch (addr) {
- case 0x80: /* PCTP */
- retval = dst->pctp;
+ case 0x80: /* CTPR */
+ retval = dst->ctpr;
break;
case 0x90: /* WHOAMI */
retval = idx;
break;
- case 0xA0: /* PIAC */
+ case 0xA0: /* IACK */
DPRINTF("Lower OpenPIC INT output\n");
qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_INT]);
n_IRQ = IRQ_get_next(opp, &dst->raised);
- DPRINTF("PIAC: irq=%d\n", n_IRQ);
+ DPRINTF("IACK: irq=%d\n", n_IRQ);
if (n_IRQ == -1) {
/* No more interrupt pending */
retval = opp->spve;
} else {
src = &opp->src[n_IRQ];
- if (!(src->ipvp & IPVP_ACTIVITY_MASK) ||
- !(IPVP_PRIORITY(src->ipvp) > dst->pctp)) {
+ if (!(src->ivpr & IVPR_ACTIVITY_MASK) ||
+ !(IVPR_PRIORITY(src->ivpr) > dst->ctpr)) {
/* - Spurious level-sensitive IRQ
* - Priorities has been changed
* and the pending IRQ isn't allowed anymore
*/
- src->ipvp &= ~IPVP_ACTIVITY_MASK;
+ src->ivpr &= ~IVPR_ACTIVITY_MASK;
retval = opp->spve;
} else {
/* IRQ enter servicing state */
IRQ_setbit(&dst->servicing, n_IRQ);
- retval = IPVP_VECTOR(opp, src->ipvp);
+ retval = IVPR_VECTOR(opp, src->ivpr);
}
IRQ_resetbit(&dst->raised, n_IRQ);
dst->raised.next = -1;
if (!src->level) {
/* edge-sensitive IRQ */
- src->ipvp &= ~IPVP_ACTIVITY_MASK;
+ src->ivpr &= ~IVPR_ACTIVITY_MASK;
src->pending = 0;
}
if ((n_IRQ >= opp->irq_ipi0) && (n_IRQ < (opp->irq_ipi0 + MAX_IPI))) {
- src->ide &= ~(1 << idx);
- if (src->ide && !src->level) {
+ src->idr &= ~(1 << idx);
+ if (src->idr && !src->level) {
/* trigger on CPUs that didn't know about it yet */
openpic_set_irq(opp, n_IRQ, 1);
openpic_set_irq(opp, n_IRQ, 0);
/* if all CPUs knew about it, set active bit again */
- src->ipvp |= IPVP_ACTIVITY_MASK;
+ src->ivpr |= IVPR_ACTIVITY_MASK;
}
}
}
break;
- case 0xB0: /* PEOI */
+ case 0xB0: /* EOI */
retval = 0;
break;
default:
@@ -1115,15 +1115,15 @@ static void openpic_save(QEMUFile* f, void *opaque)
OpenPICState *opp = (OpenPICState *)opaque;
unsigned int i;
- qemu_put_be32s(f, &opp->glbc);
- qemu_put_be32s(f, &opp->veni);
- qemu_put_be32s(f, &opp->pint);
+ qemu_put_be32s(f, &opp->gcr);
+ qemu_put_be32s(f, &opp->vir);
+ qemu_put_be32s(f, &opp->pir);
qemu_put_be32s(f, &opp->spve);
- qemu_put_be32s(f, &opp->tifr);
+ qemu_put_be32s(f, &opp->tfrr);
for (i = 0; i < opp->max_irq; i++) {
- qemu_put_be32s(f, &opp->src[i].ipvp);
- qemu_put_be32s(f, &opp->src[i].ide);
+ qemu_put_be32s(f, &opp->src[i].ivpr);
+ qemu_put_be32s(f, &opp->src[i].idr);
qemu_put_sbe32s(f, &opp->src[i].last_cpu);
qemu_put_sbe32s(f, &opp->src[i].pending);
}
@@ -1131,14 +1131,14 @@ static void openpic_save(QEMUFile* f, void *opaque)
qemu_put_be32s(f, &opp->nb_cpus);
for (i = 0; i < opp->nb_cpus; i++) {
- qemu_put_be32s(f, &opp->dst[i].pctp);
+ qemu_put_be32s(f, &opp->dst[i].ctpr);
openpic_save_IRQ_queue(f, &opp->dst[i].raised);
openpic_save_IRQ_queue(f, &opp->dst[i].servicing);
}
for (i = 0; i < MAX_TMR; i++) {
- qemu_put_be32s(f, &opp->timers[i].ticc);
- qemu_put_be32s(f, &opp->timers[i].tibc);
+ qemu_put_be32s(f, &opp->timers[i].tccr);
+ qemu_put_be32s(f, &opp->timers[i].tbcr);
}
}
@@ -1161,15 +1161,15 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
if (version_id != 1)
return -EINVAL;
- qemu_get_be32s(f, &opp->glbc);
- qemu_get_be32s(f, &opp->veni);
- qemu_get_be32s(f, &opp->pint);
+ qemu_get_be32s(f, &opp->gcr);
+ qemu_get_be32s(f, &opp->vir);
+ qemu_get_be32s(f, &opp->pir);
qemu_get_be32s(f, &opp->spve);
- qemu_get_be32s(f, &opp->tifr);
+ qemu_get_be32s(f, &opp->tfrr);
for (i = 0; i < opp->max_irq; i++) {
- qemu_get_be32s(f, &opp->src[i].ipvp);
- qemu_get_be32s(f, &opp->src[i].ide);
+ qemu_get_be32s(f, &opp->src[i].ivpr);
+ qemu_get_be32s(f, &opp->src[i].idr);
qemu_get_sbe32s(f, &opp->src[i].last_cpu);
qemu_get_sbe32s(f, &opp->src[i].pending);
}
@@ -1177,14 +1177,14 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
qemu_get_be32s(f, &opp->nb_cpus);
for (i = 0; i < opp->nb_cpus; i++) {
- qemu_get_be32s(f, &opp->dst[i].pctp);
+ qemu_get_be32s(f, &opp->dst[i].ctpr);
openpic_load_IRQ_queue(f, &opp->dst[i].raised);
openpic_load_IRQ_queue(f, &opp->dst[i].servicing);
}
for (i = 0; i < MAX_TMR; i++) {
- qemu_get_be32s(f, &opp->timers[i].ticc);
- qemu_get_be32s(f, &opp->timers[i].tibc);
+ qemu_get_be32s(f, &opp->timers[i].tccr);
+ qemu_get_be32s(f, &opp->timers[i].tbcr);
}
return 0;
@@ -1194,7 +1194,7 @@ static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src)
{
int n_ci = IDR_CI0_SHIFT - n_CPU;
- if ((opp->flags & OPENPIC_FLAG_IDE_CRIT) && (src->ide & (1 << n_ci))) {
+ if ((opp->flags & OPENPIC_FLAG_IDR_CRIT) && (src->idr & (1 << n_ci))) {
qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_CINT]);
} else {
qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
@@ -1242,14 +1242,14 @@ static int openpic_init(SysBusDevice *dev)
switch (opp->model) {
case OPENPIC_MODEL_FSL_MPIC_20:
default:
- opp->flags |= OPENPIC_FLAG_IDE_CRIT;
+ opp->flags |= OPENPIC_FLAG_IDR_CRIT;
opp->nb_irqs = 80;
opp->vid = VID_REVISION_1_2;
- opp->veni = VENI_GENERIC;
+ opp->vir = VIR_GENERIC;
opp->vector_mask = 0xFFFF;
- opp->tifr_reset = 0;
- opp->ipvp_reset = IPVP_MASK_MASK;
- opp->ide_reset = 1 << 0;
+ opp->tfrr_reset = 0;
+ opp->ivpr_reset = IVPR_MASK_MASK;
+ opp->idr_reset = 1 << 0;
opp->max_irq = FSL_MPIC_20_MAX_IRQ;
opp->irq_ipi0 = FSL_MPIC_20_IPI_IRQ;
opp->irq_tim0 = FSL_MPIC_20_TMR_IRQ;
@@ -1279,11 +1279,11 @@ static int openpic_init(SysBusDevice *dev)
case OPENPIC_MODEL_RAVEN:
opp->nb_irqs = RAVEN_MAX_EXT;
opp->vid = VID_REVISION_1_3;
- opp->veni = VENI_GENERIC;
+ opp->vir = VIR_GENERIC;
opp->vector_mask = 0xFF;
- opp->tifr_reset = 4160000;
- opp->ipvp_reset = IPVP_MASK_MASK | IPVP_MODE_MASK;
- opp->ide_reset = 0;
+ opp->tfrr_reset = 4160000;
+ opp->ivpr_reset = IVPR_MASK_MASK | IVPR_MODE_MASK;
+ opp->idr_reset = 0;
opp->max_irq = RAVEN_MAX_IRQ;
opp->irq_ipi0 = RAVEN_IPI_IRQ;
opp->irq_tim0 = RAVEN_TMR_IRQ;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 05/15] openpic: make register names correspond better with hw docs
2012-12-22 2:15 ` [Qemu-devel] [PATCH 05/15] openpic: make register names correspond better with hw docs Scott Wood
@ 2013-01-03 18:18 ` Alexander Graf
0 siblings, 0 replies; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 18:18 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> The base openpic specification doesn't provide abbreviated register
> names, so it's somewhat understandable that the QEMU code made up
> its own, except that most of the names that QEMU used didn't correspond
> to the terminology used by any implementation I could find.
>
> In some cases, like PCTP, the phrase "processor current task priority"
> could be found in the openpic spec when describing the concept, but
> the register itself was labelled "current task priority register"
> and every implementation seems to use either CTPR or the full phrase.
>
> In other cases, individual implementations disagree on what to call
> the register. The implementations I have documentation for are
> Freescale, Raven (MCP750), and IBM. The Raven docs tend to not use
> abbreviations at all. The IBM MPIC isn't implemented in QEMU. Thus,
> where there's disagreement I chose to use the Freescale abbreviations.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Thanks, applied and fixed up to apply to the current state of ppc-next. Please rebase 3/15 against ppc-next when you redo it.
Alex
> ---
> BTW, I'm still not sure where the first "P" in QEMU's "IPVP" came from.
> ---
> hw/openpic.c | 362 +++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 181 insertions(+), 181 deletions(-)
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 34449a7..7647368 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -61,7 +61,7 @@
> #define VID 0x03 /* MPIC version ID */
>
> /* OpenPIC capability flags */
> -#define OPENPIC_FLAG_IDE_CRIT (1 << 0)
> +#define OPENPIC_FLAG_IDR_CRIT (1 << 0)
>
> /* OpenPIC address map */
> #define OPENPIC_GLB_REG_START 0x0
> @@ -118,19 +118,19 @@
> #define FSL_BRR1_IPMJ (0x00 << 8) /* 8 bit IP major number */
> #define FSL_BRR1_IPMN 0x00 /* 8 bit IP minor number */
>
> -#define FREP_NIRQ_SHIFT 16
> -#define FREP_NCPU_SHIFT 8
> -#define FREP_VID_SHIFT 0
> +#define FRR_NIRQ_SHIFT 16
> +#define FRR_NCPU_SHIFT 8
> +#define FRR_VID_SHIFT 0
>
> #define VID_REVISION_1_2 2
> #define VID_REVISION_1_3 3
>
> -#define VENI_GENERIC 0x00000000 /* Generic Vendor ID */
> +#define VIR_GENERIC 0x00000000 /* Generic Vendor ID */
>
> -#define GLBC_RESET 0x80000000
> +#define GCR_RESET 0x80000000
>
> -#define TIBC_CI 0x80000000 /* count inhibit */
> -#define TICC_TOG 0x80000000 /* toggles when decrement to zero */
> +#define TBCR_CI 0x80000000 /* count inhibit */
> +#define TCCR_TOG 0x80000000 /* toggles when decrement to zero */
>
> #define IDR_EP_SHIFT 31
> #define IDR_EP_MASK (1 << IDR_EP_SHIFT)
> @@ -185,8 +185,8 @@ typedef struct IRQ_queue_t {
> } IRQ_queue_t;
>
> typedef struct IRQ_src_t {
> - uint32_t ipvp; /* IRQ vector/priority register */
> - uint32_t ide; /* IRQ destination register */
> + uint32_t ivpr; /* IRQ vector/priority register */
> + uint32_t idr; /* IRQ destination register */
> int last_cpu;
> int pending; /* TRUE if IRQ is pending */
> bool level; /* level-triggered */
> @@ -194,27 +194,27 @@ typedef struct IRQ_src_t {
> bool fslspecial; /* FSL timer/IPI interrupt, edge, no polarity */
> } IRQ_src_t;
>
> -#define IPVP_MASK_SHIFT 31
> -#define IPVP_MASK_MASK (1 << IPVP_MASK_SHIFT)
> -#define IPVP_ACTIVITY_SHIFT 30
> -#define IPVP_ACTIVITY_MASK (1 << IPVP_ACTIVITY_SHIFT)
> -#define IPVP_MODE_SHIFT 29
> -#define IPVP_MODE_MASK (1 << IPVP_MODE_SHIFT)
> -#define IPVP_POLARITY_SHIFT 23
> -#define IPVP_POLARITY_MASK (1 << IPVP_POLARITY_SHIFT)
> -#define IPVP_SENSE_SHIFT 22
> -#define IPVP_SENSE_MASK (1 << IPVP_SENSE_SHIFT)
> -
> -#define IPVP_PRIORITY_MASK (0xF << 16)
> -#define IPVP_PRIORITY(_ipvpr_) ((int)(((_ipvpr_) & IPVP_PRIORITY_MASK) >> 16))
> -#define IPVP_VECTOR(opp, _ipvpr_) ((_ipvpr_) & (opp)->vector_mask)
> +#define IVPR_MASK_SHIFT 31
> +#define IVPR_MASK_MASK (1 << IVPR_MASK_SHIFT)
> +#define IVPR_ACTIVITY_SHIFT 30
> +#define IVPR_ACTIVITY_MASK (1 << IVPR_ACTIVITY_SHIFT)
> +#define IVPR_MODE_SHIFT 29
> +#define IVPR_MODE_MASK (1 << IVPR_MODE_SHIFT)
> +#define IVPR_POLARITY_SHIFT 23
> +#define IVPR_POLARITY_MASK (1 << IVPR_POLARITY_SHIFT)
> +#define IVPR_SENSE_SHIFT 22
> +#define IVPR_SENSE_MASK (1 << IVPR_SENSE_SHIFT)
> +
> +#define IVPR_PRIORITY_MASK (0xF << 16)
> +#define IVPR_PRIORITY(_ivprr_) ((int)(((_ivprr_) & IVPR_PRIORITY_MASK) >> 16))
> +#define IVPR_VECTOR(opp, _ivprr_) ((_ivprr_) & (opp)->vector_mask)
>
> /* IDE[EP/CI] are only for FSL MPIC prior to v4.0 */
> -#define IDE_EP 0x80000000 /* external pin */
> -#define IDE_CI 0x40000000 /* critical interrupt */
> +#define IDR_EP 0x80000000 /* external pin */
> +#define IDR_CI 0x40000000 /* critical interrupt */
>
> typedef struct IRQ_dst_t {
> - uint32_t pctp; /* CPU current task priority */
> + uint32_t ctpr; /* CPU current task priority */
> IRQ_queue_t raised;
> IRQ_queue_t servicing;
> qemu_irq *irqs;
> @@ -229,22 +229,22 @@ typedef struct OpenPICState {
> uint32_t flags;
> uint32_t nb_irqs;
> uint32_t vid;
> - uint32_t veni; /* Vendor identification register */
> + uint32_t vir; /* Vendor identification register */
> uint32_t vector_mask;
> - uint32_t tifr_reset;
> - uint32_t ipvp_reset;
> - uint32_t ide_reset;
> + uint32_t tfrr_reset;
> + uint32_t ivpr_reset;
> + uint32_t idr_reset;
> uint32_t brr1;
>
> /* Sub-regions */
> MemoryRegion sub_io_mem[5];
>
> /* Global registers */
> - uint32_t frep; /* Feature reporting register */
> - uint32_t glbc; /* Global configuration register */
> - uint32_t pint; /* Processor initialization register */
> + uint32_t frr; /* Feature reporting register */
> + uint32_t gcr; /* Global configuration register */
> + uint32_t pir; /* Processor initialization register */
> uint32_t spve; /* Spurious vector register */
> - uint32_t tifr; /* Timer frequency reporting register */
> + uint32_t tfrr; /* Timer frequency reporting register */
> /* Source registers */
> IRQ_src_t src[MAX_IRQ];
> /* Local registers per output pin */
> @@ -252,8 +252,8 @@ typedef struct OpenPICState {
> uint32_t nb_cpus;
> /* Timer registers */
> struct {
> - uint32_t ticc; /* Global timer current count register */
> - uint32_t tibc; /* Global timer base count register */
> + uint32_t tccr; /* Global timer current count register */
> + uint32_t tbcr; /* Global timer base count register */
> } timers[MAX_TMR];
> /* Shared MSI registers */
> struct {
> @@ -299,11 +299,11 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
>
> for (i = 0; i < opp->max_irq; i++) {
> if (IRQ_testbit(q, i)) {
> - DPRINTF("IRQ_check: irq %d set ipvp_pr=%d pr=%d\n",
> - i, IPVP_PRIORITY(opp->src[i].ipvp), priority);
> - if (IPVP_PRIORITY(opp->src[i].ipvp) > priority) {
> + DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
> + i, IVPR_PRIORITY(opp->src[i].ivpr), priority);
> + if (IVPR_PRIORITY(opp->src[i].ivpr) > priority) {
> next = i;
> - priority = IPVP_PRIORITY(opp->src[i].ipvp);
> + priority = IVPR_PRIORITY(opp->src[i].ivpr);
> }
> }
> }
> @@ -331,8 +331,8 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
>
> dst = &opp->dst[n_CPU];
> src = &opp->src[n_IRQ];
> - priority = IPVP_PRIORITY(src->ipvp);
> - if (priority <= dst->pctp) {
> + priority = IVPR_PRIORITY(src->ivpr);
> + if (priority <= dst->ctpr) {
> /* Too low priority */
> DPRINTF("%s: IRQ %d has too low priority on CPU %d\n",
> __func__, n_IRQ, n_CPU);
> @@ -344,7 +344,7 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
> __func__, n_IRQ, n_CPU);
> return;
> }
> - src->ipvp |= IPVP_ACTIVITY_MASK;
> + src->ivpr |= IVPR_ACTIVITY_MASK;
> IRQ_setbit(&dst->raised, n_IRQ);
> if (priority < dst->raised.priority) {
> /* An higher priority IRQ is already raised */
> @@ -377,34 +377,34 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
> DPRINTF("%s: IRQ %d is not pending\n", __func__, n_IRQ);
> return;
> }
> - if (src->ipvp & IPVP_MASK_MASK) {
> + if (src->ivpr & IVPR_MASK_MASK) {
> /* Interrupt source is disabled */
> DPRINTF("%s: IRQ %d is disabled\n", __func__, n_IRQ);
> return;
> }
> - if (IPVP_PRIORITY(src->ipvp) == 0) {
> + if (IVPR_PRIORITY(src->ivpr) == 0) {
> /* Priority set to zero */
> DPRINTF("%s: IRQ %d has 0 priority\n", __func__, n_IRQ);
> return;
> }
> - if (src->ipvp & IPVP_ACTIVITY_MASK) {
> + if (src->ivpr & IVPR_ACTIVITY_MASK) {
> /* IRQ already active */
> DPRINTF("%s: IRQ %d is already active\n", __func__, n_IRQ);
> return;
> }
> - if (src->ide == 0) {
> + if (src->idr == 0) {
> /* No target */
> DPRINTF("%s: IRQ %d has no target\n", __func__, n_IRQ);
> return;
> }
>
> - if (src->ide == (1 << src->last_cpu)) {
> + if (src->idr == (1 << src->last_cpu)) {
> /* Only one CPU is allowed to receive this IRQ */
> IRQ_local_pipe(opp, src->last_cpu, n_IRQ);
> - } else if (!(src->ipvp & IPVP_MODE_MASK)) {
> + } else if (!(src->ivpr & IVPR_MODE_MASK)) {
> /* Directed delivery mode */
> for (i = 0; i < opp->nb_cpus; i++) {
> - if (src->ide & (1 << i)) {
> + if (src->idr & (1 << i)) {
> IRQ_local_pipe(opp, i, n_IRQ);
> }
> }
> @@ -413,7 +413,7 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
> for (i = src->last_cpu + 1; i != src->last_cpu; i++) {
> if (i == opp->nb_cpus)
> i = 0;
> - if (src->ide & (1 << i)) {
> + if (src->idr & (1 << i)) {
> IRQ_local_pipe(opp, i, n_IRQ);
> src->last_cpu = i;
> break;
> @@ -428,13 +428,13 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
> IRQ_src_t *src;
>
> src = &opp->src[n_IRQ];
> - DPRINTF("openpic: set irq %d = %d ipvp=0x%08x\n",
> - n_IRQ, level, src->ipvp);
> + DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n",
> + n_IRQ, level, src->ivpr);
> if (src->level) {
> /* level-sensitive irq */
> src->pending = level;
> if (!level) {
> - src->ipvp &= ~IPVP_ACTIVITY_MASK;
> + src->ivpr &= ~IVPR_ACTIVITY_MASK;
> }
> } else {
> /* edge-sensitive irq */
> @@ -449,31 +449,31 @@ static void openpic_reset(DeviceState *d)
> OpenPICState *opp = FROM_SYSBUS(typeof (*opp), sysbus_from_qdev(d));
> int i;
>
> - opp->glbc = GLBC_RESET;
> + opp->gcr = GCR_RESET;
> /* Initialise controller registers */
> - opp->frep = ((opp->nb_irqs - 1) << FREP_NIRQ_SHIFT) |
> - ((opp->nb_cpus - 1) << FREP_NCPU_SHIFT) |
> - (opp->vid << FREP_VID_SHIFT);
> + opp->frr = ((opp->nb_irqs - 1) << FRR_NIRQ_SHIFT) |
> + ((opp->nb_cpus - 1) << FRR_NCPU_SHIFT) |
> + (opp->vid << FRR_VID_SHIFT);
>
> - opp->pint = 0;
> + opp->pir = 0;
> opp->spve = -1 & opp->vector_mask;
> - opp->tifr = opp->tifr_reset;
> + opp->tfrr = opp->tfrr_reset;
> /* Initialise IRQ sources */
> for (i = 0; i < opp->max_irq; i++) {
> - opp->src[i].ipvp = opp->ipvp_reset;
> - opp->src[i].ide = opp->ide_reset;
> + opp->src[i].ivpr = opp->ivpr_reset;
> + opp->src[i].idr = opp->idr_reset;
>
> if (opp->src[i].fslint) {
> - opp->src[i].ipvp |= IPVP_POLARITY_MASK;
> + opp->src[i].ivpr |= IVPR_POLARITY_MASK;
> }
>
> if (!opp->src[i].fslint && !opp->src[i].fslspecial) {
> - opp->src[i].level = !!(opp->ipvp_reset & IPVP_SENSE_MASK);
> + opp->src[i].level = !!(opp->ivpr_reset & IVPR_SENSE_MASK);
> }
> }
> /* Initialise IRQ destinations */
> for (i = 0; i < MAX_CPU; i++) {
> - opp->dst[i].pctp = 15;
> + opp->dst[i].ctpr = 15;
> memset(&opp->dst[i].raised, 0, sizeof(IRQ_queue_t));
> opp->dst[i].raised.next = -1;
> memset(&opp->dst[i].servicing, 0, sizeof(IRQ_queue_t));
> @@ -481,62 +481,62 @@ static void openpic_reset(DeviceState *d)
> }
> /* Initialise timers */
> for (i = 0; i < MAX_TMR; i++) {
> - opp->timers[i].ticc = 0;
> - opp->timers[i].tibc = TIBC_CI;
> + opp->timers[i].tccr = 0;
> + opp->timers[i].tbcr = TBCR_CI;
> }
> /* Go out of RESET state */
> - opp->glbc = 0;
> + opp->gcr = 0;
> }
>
> -static inline uint32_t read_IRQreg_ide(OpenPICState *opp, int n_IRQ)
> +static inline uint32_t read_IRQreg_idr(OpenPICState *opp, int n_IRQ)
> {
> - return opp->src[n_IRQ].ide;
> + return opp->src[n_IRQ].idr;
> }
>
> -static inline uint32_t read_IRQreg_ipvp(OpenPICState *opp, int n_IRQ)
> +static inline uint32_t read_IRQreg_ivpr(OpenPICState *opp, int n_IRQ)
> {
> - return opp->src[n_IRQ].ipvp;
> + return opp->src[n_IRQ].ivpr;
> }
>
> -static inline void write_IRQreg_ide(OpenPICState *opp, int n_IRQ, uint32_t val)
> +static inline void write_IRQreg_idr(OpenPICState *opp, int n_IRQ, uint32_t val)
> {
> uint32_t tmp;
>
> - tmp = val & (IDE_EP | IDE_CI);
> + tmp = val & (IDR_EP | IDR_CI);
> tmp |= val & ((1ULL << MAX_CPU) - 1);
> - opp->src[n_IRQ].ide = tmp;
> - DPRINTF("Set IDE %d to 0x%08x\n", n_IRQ, opp->src[n_IRQ].ide);
> + opp->src[n_IRQ].idr = tmp;
> + DPRINTF("Set IDE %d to 0x%08x\n", n_IRQ, opp->src[n_IRQ].idr);
> }
>
> -static inline void write_IRQreg_ipvp(OpenPICState *opp, int n_IRQ, uint32_t val)
> +static inline void write_IRQreg_ivpr(OpenPICState *opp, int n_IRQ, uint32_t val)
> {
> uint32_t mask;
>
> /* NOTE when implementing newer FSL MPIC models: starting with v4.0,
> * the polarity bit is read-only on internal interrupts.
> */
> - mask = IPVP_MASK_MASK | IPVP_PRIORITY_MASK | IPVP_SENSE_MASK |
> - IPVP_POLARITY_MASK | opp->vector_mask;
> + mask = IVPR_MASK_MASK | IVPR_PRIORITY_MASK | IVPR_SENSE_MASK |
> + IVPR_POLARITY_MASK | opp->vector_mask;
>
> /* ACTIVITY bit is read-only */
> - opp->src[n_IRQ].ipvp =
> - (opp->src[n_IRQ].ipvp & IPVP_ACTIVITY_MASK) | (val & mask);
> + opp->src[n_IRQ].ivpr =
> + (opp->src[n_IRQ].ivpr & IVPR_ACTIVITY_MASK) | (val & mask);
>
> /* For FSL internal interrupts, The sense bit is reserved and zero,
> * and the interrupt is always level-triggered. Timers and IPIs
> * have no sense or polarity bits, and are edge-triggered.
> */
> if (opp->src[n_IRQ].fslint) {
> - opp->src[n_IRQ].ipvp &= ~IPVP_SENSE_MASK;
> + opp->src[n_IRQ].ivpr &= ~IVPR_SENSE_MASK;
> } else if (opp->src[n_IRQ].fslspecial) {
> - opp->src[n_IRQ].ipvp &= ~(IPVP_POLARITY_MASK | IPVP_SENSE_MASK);
> + opp->src[n_IRQ].ivpr &= ~(IVPR_POLARITY_MASK | IVPR_SENSE_MASK);
> } else {
> - opp->src[n_IRQ].level = !!(opp->src[n_IRQ].ipvp & IPVP_SENSE_MASK);
> + opp->src[n_IRQ].level = !!(opp->src[n_IRQ].ivpr & IVPR_SENSE_MASK);
> }
>
> openpic_update_irq(opp, n_IRQ);
> - DPRINTF("Set IPVP %d to 0x%08x -> 0x%08x\n", n_IRQ, val,
> - opp->src[n_IRQ].ipvp);
> + DPRINTF("Set IVPR %d to 0x%08x -> 0x%08x\n", n_IRQ, val,
> + opp->src[n_IRQ].ivpr);
> }
>
> static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
> @@ -563,37 +563,37 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
> case 0xB0:
> openpic_cpu_write_internal(opp, addr, val, get_current_cpu());
> break;
> - case 0x1000: /* FREP */
> + case 0x1000: /* FRR */
> break;
> - case 0x1020: /* GLBC */
> - if (val & GLBC_RESET) {
> + case 0x1020: /* GCR */
> + if (val & GCR_RESET) {
> openpic_reset(&opp->busdev.qdev);
> }
> break;
> - case 0x1080: /* VENI */
> + case 0x1080: /* VIR */
> break;
> - case 0x1090: /* PINT */
> + case 0x1090: /* PIR */
> for (idx = 0; idx < opp->nb_cpus; idx++) {
> - if ((val & (1 << idx)) && !(opp->pint & (1 << idx))) {
> + if ((val & (1 << idx)) && !(opp->pir & (1 << idx))) {
> DPRINTF("Raise OpenPIC RESET output for CPU %d\n", idx);
> dst = &opp->dst[idx];
> qemu_irq_raise(dst->irqs[OPENPIC_OUTPUT_RESET]);
> - } else if (!(val & (1 << idx)) && (opp->pint & (1 << idx))) {
> + } else if (!(val & (1 << idx)) && (opp->pir & (1 << idx))) {
> DPRINTF("Lower OpenPIC RESET output for CPU %d\n", idx);
> dst = &opp->dst[idx];
> qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_RESET]);
> }
> }
> - opp->pint = val;
> + opp->pir = val;
> break;
> - case 0x10A0: /* IPI_IPVP */
> + case 0x10A0: /* IPI_IVPR */
> case 0x10B0:
> case 0x10C0:
> case 0x10D0:
> {
> int idx;
> idx = (addr - 0x10A0) >> 4;
> - write_IRQreg_ipvp(opp, opp->irq_ipi0 + idx, val);
> + write_IRQreg_ivpr(opp, opp->irq_ipi0 + idx, val);
> }
> break;
> case 0x10E0: /* SPVE */
> @@ -614,16 +614,16 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
> if (addr & 0xF)
> return retval;
> switch (addr) {
> - case 0x1000: /* FREP */
> - retval = opp->frep;
> + case 0x1000: /* FRR */
> + retval = opp->frr;
> break;
> - case 0x1020: /* GLBC */
> - retval = opp->glbc;
> + case 0x1020: /* GCR */
> + retval = opp->gcr;
> break;
> - case 0x1080: /* VENI */
> - retval = opp->veni;
> + case 0x1080: /* VIR */
> + retval = opp->vir;
> break;
> - case 0x1090: /* PINT */
> + case 0x1090: /* PIR */
> retval = 0x00000000;
> break;
> case 0x00: /* Block Revision Register1 (BRR1) */
> @@ -639,14 +639,14 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
> case 0xB0:
> retval = openpic_cpu_read_internal(opp, addr, get_current_cpu());
> break;
> - case 0x10A0: /* IPI_IPVP */
> + case 0x10A0: /* IPI_IVPR */
> case 0x10B0:
> case 0x10C0:
> case 0x10D0:
> {
> int idx;
> idx = (addr - 0x10A0) >> 4;
> - retval = read_IRQreg_ipvp(opp, opp->irq_ipi0 + idx);
> + retval = read_IRQreg_ivpr(opp, opp->irq_ipi0 + idx);
> }
> break;
> case 0x10E0: /* SPVE */
> @@ -674,26 +674,26 @@ static void openpic_tmr_write(void *opaque, hwaddr addr, uint64_t val,
> addr = addr & 0x30;
>
> if (addr == 0x0) {
> - /* TIFR (TFRR) */
> - opp->tifr = val;
> + /* TFRR */
> + opp->tfrr = val;
> return;
> }
> switch (addr & 0x30) {
> - case 0x00: /* TICC (GTCCR) */
> + case 0x00: /* TCCR */
> break;
> - case 0x10: /* TIBC (GTBCR) */
> - if ((opp->timers[idx].ticc & TICC_TOG) != 0 &&
> - (val & TIBC_CI) == 0 &&
> - (opp->timers[idx].tibc & TIBC_CI) != 0) {
> - opp->timers[idx].ticc &= ~TICC_TOG;
> + case 0x10: /* TBCR */
> + if ((opp->timers[idx].tccr & TCCR_TOG) != 0 &&
> + (val & TBCR_CI) == 0 &&
> + (opp->timers[idx].tbcr & TBCR_CI) != 0) {
> + opp->timers[idx].tccr &= ~TCCR_TOG;
> }
> - opp->timers[idx].tibc = val;
> + opp->timers[idx].tbcr = val;
> break;
> - case 0x20: /* TIVP (GTIVPR) */
> - write_IRQreg_ipvp(opp, opp->irq_tim0 + idx, val);
> + case 0x20: /* TVPR */
> + write_IRQreg_ivpr(opp, opp->irq_tim0 + idx, val);
> break;
> - case 0x30: /* TIDE (GTIDR) */
> - write_IRQreg_ide(opp, opp->irq_tim0 + idx, val);
> + case 0x30: /* TDR */
> + write_IRQreg_idr(opp, opp->irq_tim0 + idx, val);
> break;
> }
> }
> @@ -710,22 +710,22 @@ static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len)
> }
> idx = (addr >> 6) & 0x3;
> if (addr == 0x0) {
> - /* TIFR (TFRR) */
> - retval = opp->tifr;
> + /* TFRR */
> + retval = opp->tfrr;
> goto out;
> }
> switch (addr & 0x30) {
> - case 0x00: /* TICC (GTCCR) */
> - retval = opp->timers[idx].ticc;
> + case 0x00: /* TCCR */
> + retval = opp->timers[idx].tccr;
> break;
> - case 0x10: /* TIBC (GTBCR) */
> - retval = opp->timers[idx].tibc;
> + case 0x10: /* TBCR */
> + retval = opp->timers[idx].tbcr;
> break;
> - case 0x20: /* TIPV (TIPV) */
> - retval = read_IRQreg_ipvp(opp, opp->irq_tim0 + idx);
> + case 0x20: /* TIPV */
> + retval = read_IRQreg_ivpr(opp, opp->irq_tim0 + idx);
> break;
> case 0x30: /* TIDE (TIDR) */
> - retval = read_IRQreg_ide(opp, opp->irq_tim0 + idx);
> + retval = read_IRQreg_idr(opp, opp->irq_tim0 + idx);
> break;
> }
>
> @@ -749,10 +749,10 @@ static void openpic_src_write(void *opaque, hwaddr addr, uint64_t val,
> idx = addr >> 5;
> if (addr & 0x10) {
> /* EXDE / IFEDE / IEEDE */
> - write_IRQreg_ide(opp, idx, val);
> + write_IRQreg_idr(opp, idx, val);
> } else {
> /* EXVP / IFEVP / IEEVP */
> - write_IRQreg_ipvp(opp, idx, val);
> + write_IRQreg_ivpr(opp, idx, val);
> }
> }
>
> @@ -770,10 +770,10 @@ static uint64_t openpic_src_read(void *opaque, uint64_t addr, unsigned len)
> idx = addr >> 5;
> if (addr & 0x10) {
> /* EXDE / IFEDE / IEEDE */
> - retval = read_IRQreg_ide(opp, idx);
> + retval = read_IRQreg_idr(opp, idx);
> } else {
> /* EXVP / IFEVP / IEEVP */
> - retval = read_IRQreg_ipvp(opp, idx);
> + retval = read_IRQreg_ivpr(opp, idx);
> }
> DPRINTF("%s: => 0x%08x\n", __func__, retval);
>
> @@ -870,22 +870,22 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
> case 0x70:
> idx = (addr - 0x40) >> 4;
> /* we use IDE as mask which CPUs to deliver the IPI to still. */
> - write_IRQreg_ide(opp, opp->irq_ipi0 + idx,
> - opp->src[opp->irq_ipi0 + idx].ide | val);
> + write_IRQreg_idr(opp, opp->irq_ipi0 + idx,
> + opp->src[opp->irq_ipi0 + idx].idr | val);
> openpic_set_irq(opp, opp->irq_ipi0 + idx, 1);
> openpic_set_irq(opp, opp->irq_ipi0 + idx, 0);
> break;
> - case 0x80: /* PCTP */
> - dst->pctp = val & 0x0000000F;
> + case 0x80: /* CTPR */
> + dst->ctpr = val & 0x0000000F;
> break;
> case 0x90: /* WHOAMI */
> /* Read-only register */
> break;
> - case 0xA0: /* PIAC */
> + case 0xA0: /* IACK */
> /* Read-only register */
> break;
> - case 0xB0: /* PEOI */
> - DPRINTF("PEOI\n");
> + case 0xB0: /* EOI */
> + DPRINTF("EOI\n");
> s_IRQ = IRQ_get_next(opp, &dst->servicing);
> IRQ_resetbit(&dst->servicing, s_IRQ);
> dst->servicing.next = -1;
> @@ -896,7 +896,7 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
> src = &opp->src[n_IRQ];
> if (n_IRQ != -1 &&
> (s_IRQ == -1 ||
> - IPVP_PRIORITY(src->ipvp) > dst->servicing.priority)) {
> + IVPR_PRIORITY(src->ivpr) > dst->servicing.priority)) {
> DPRINTF("Raise OpenPIC INT output cpu %d irq %d\n",
> idx, n_IRQ);
> openpic_irq_raise(opp, idx, src);
> @@ -934,56 +934,56 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
> dst = &opp->dst[idx];
> addr &= 0xFF0;
> switch (addr) {
> - case 0x80: /* PCTP */
> - retval = dst->pctp;
> + case 0x80: /* CTPR */
> + retval = dst->ctpr;
> break;
> case 0x90: /* WHOAMI */
> retval = idx;
> break;
> - case 0xA0: /* PIAC */
> + case 0xA0: /* IACK */
> DPRINTF("Lower OpenPIC INT output\n");
> qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_INT]);
> n_IRQ = IRQ_get_next(opp, &dst->raised);
> - DPRINTF("PIAC: irq=%d\n", n_IRQ);
> + DPRINTF("IACK: irq=%d\n", n_IRQ);
> if (n_IRQ == -1) {
> /* No more interrupt pending */
> retval = opp->spve;
> } else {
> src = &opp->src[n_IRQ];
> - if (!(src->ipvp & IPVP_ACTIVITY_MASK) ||
> - !(IPVP_PRIORITY(src->ipvp) > dst->pctp)) {
> + if (!(src->ivpr & IVPR_ACTIVITY_MASK) ||
> + !(IVPR_PRIORITY(src->ivpr) > dst->ctpr)) {
> /* - Spurious level-sensitive IRQ
> * - Priorities has been changed
> * and the pending IRQ isn't allowed anymore
> */
> - src->ipvp &= ~IPVP_ACTIVITY_MASK;
> + src->ivpr &= ~IVPR_ACTIVITY_MASK;
> retval = opp->spve;
> } else {
> /* IRQ enter servicing state */
> IRQ_setbit(&dst->servicing, n_IRQ);
> - retval = IPVP_VECTOR(opp, src->ipvp);
> + retval = IVPR_VECTOR(opp, src->ivpr);
> }
> IRQ_resetbit(&dst->raised, n_IRQ);
> dst->raised.next = -1;
> if (!src->level) {
> /* edge-sensitive IRQ */
> - src->ipvp &= ~IPVP_ACTIVITY_MASK;
> + src->ivpr &= ~IVPR_ACTIVITY_MASK;
> src->pending = 0;
> }
>
> if ((n_IRQ >= opp->irq_ipi0) && (n_IRQ < (opp->irq_ipi0 + MAX_IPI))) {
> - src->ide &= ~(1 << idx);
> - if (src->ide && !src->level) {
> + src->idr &= ~(1 << idx);
> + if (src->idr && !src->level) {
> /* trigger on CPUs that didn't know about it yet */
> openpic_set_irq(opp, n_IRQ, 1);
> openpic_set_irq(opp, n_IRQ, 0);
> /* if all CPUs knew about it, set active bit again */
> - src->ipvp |= IPVP_ACTIVITY_MASK;
> + src->ivpr |= IVPR_ACTIVITY_MASK;
> }
> }
> }
> break;
> - case 0xB0: /* PEOI */
> + case 0xB0: /* EOI */
> retval = 0;
> break;
> default:
> @@ -1115,15 +1115,15 @@ static void openpic_save(QEMUFile* f, void *opaque)
> OpenPICState *opp = (OpenPICState *)opaque;
> unsigned int i;
>
> - qemu_put_be32s(f, &opp->glbc);
> - qemu_put_be32s(f, &opp->veni);
> - qemu_put_be32s(f, &opp->pint);
> + qemu_put_be32s(f, &opp->gcr);
> + qemu_put_be32s(f, &opp->vir);
> + qemu_put_be32s(f, &opp->pir);
> qemu_put_be32s(f, &opp->spve);
> - qemu_put_be32s(f, &opp->tifr);
> + qemu_put_be32s(f, &opp->tfrr);
>
> for (i = 0; i < opp->max_irq; i++) {
> - qemu_put_be32s(f, &opp->src[i].ipvp);
> - qemu_put_be32s(f, &opp->src[i].ide);
> + qemu_put_be32s(f, &opp->src[i].ivpr);
> + qemu_put_be32s(f, &opp->src[i].idr);
> qemu_put_sbe32s(f, &opp->src[i].last_cpu);
> qemu_put_sbe32s(f, &opp->src[i].pending);
> }
> @@ -1131,14 +1131,14 @@ static void openpic_save(QEMUFile* f, void *opaque)
> qemu_put_be32s(f, &opp->nb_cpus);
>
> for (i = 0; i < opp->nb_cpus; i++) {
> - qemu_put_be32s(f, &opp->dst[i].pctp);
> + qemu_put_be32s(f, &opp->dst[i].ctpr);
> openpic_save_IRQ_queue(f, &opp->dst[i].raised);
> openpic_save_IRQ_queue(f, &opp->dst[i].servicing);
> }
>
> for (i = 0; i < MAX_TMR; i++) {
> - qemu_put_be32s(f, &opp->timers[i].ticc);
> - qemu_put_be32s(f, &opp->timers[i].tibc);
> + qemu_put_be32s(f, &opp->timers[i].tccr);
> + qemu_put_be32s(f, &opp->timers[i].tbcr);
> }
> }
>
> @@ -1161,15 +1161,15 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
> if (version_id != 1)
> return -EINVAL;
>
> - qemu_get_be32s(f, &opp->glbc);
> - qemu_get_be32s(f, &opp->veni);
> - qemu_get_be32s(f, &opp->pint);
> + qemu_get_be32s(f, &opp->gcr);
> + qemu_get_be32s(f, &opp->vir);
> + qemu_get_be32s(f, &opp->pir);
> qemu_get_be32s(f, &opp->spve);
> - qemu_get_be32s(f, &opp->tifr);
> + qemu_get_be32s(f, &opp->tfrr);
>
> for (i = 0; i < opp->max_irq; i++) {
> - qemu_get_be32s(f, &opp->src[i].ipvp);
> - qemu_get_be32s(f, &opp->src[i].ide);
> + qemu_get_be32s(f, &opp->src[i].ivpr);
> + qemu_get_be32s(f, &opp->src[i].idr);
> qemu_get_sbe32s(f, &opp->src[i].last_cpu);
> qemu_get_sbe32s(f, &opp->src[i].pending);
> }
> @@ -1177,14 +1177,14 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
> qemu_get_be32s(f, &opp->nb_cpus);
>
> for (i = 0; i < opp->nb_cpus; i++) {
> - qemu_get_be32s(f, &opp->dst[i].pctp);
> + qemu_get_be32s(f, &opp->dst[i].ctpr);
> openpic_load_IRQ_queue(f, &opp->dst[i].raised);
> openpic_load_IRQ_queue(f, &opp->dst[i].servicing);
> }
>
> for (i = 0; i < MAX_TMR; i++) {
> - qemu_get_be32s(f, &opp->timers[i].ticc);
> - qemu_get_be32s(f, &opp->timers[i].tibc);
> + qemu_get_be32s(f, &opp->timers[i].tccr);
> + qemu_get_be32s(f, &opp->timers[i].tbcr);
> }
>
> return 0;
> @@ -1194,7 +1194,7 @@ static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src)
> {
> int n_ci = IDR_CI0_SHIFT - n_CPU;
>
> - if ((opp->flags & OPENPIC_FLAG_IDE_CRIT) && (src->ide & (1 << n_ci))) {
> + if ((opp->flags & OPENPIC_FLAG_IDR_CRIT) && (src->idr & (1 << n_ci))) {
> qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_CINT]);
> } else {
> qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
> @@ -1242,14 +1242,14 @@ static int openpic_init(SysBusDevice *dev)
> switch (opp->model) {
> case OPENPIC_MODEL_FSL_MPIC_20:
> default:
> - opp->flags |= OPENPIC_FLAG_IDE_CRIT;
> + opp->flags |= OPENPIC_FLAG_IDR_CRIT;
> opp->nb_irqs = 80;
> opp->vid = VID_REVISION_1_2;
> - opp->veni = VENI_GENERIC;
> + opp->vir = VIR_GENERIC;
> opp->vector_mask = 0xFFFF;
> - opp->tifr_reset = 0;
> - opp->ipvp_reset = IPVP_MASK_MASK;
> - opp->ide_reset = 1 << 0;
> + opp->tfrr_reset = 0;
> + opp->ivpr_reset = IVPR_MASK_MASK;
> + opp->idr_reset = 1 << 0;
> opp->max_irq = FSL_MPIC_20_MAX_IRQ;
> opp->irq_ipi0 = FSL_MPIC_20_IPI_IRQ;
> opp->irq_tim0 = FSL_MPIC_20_TMR_IRQ;
> @@ -1279,11 +1279,11 @@ static int openpic_init(SysBusDevice *dev)
> case OPENPIC_MODEL_RAVEN:
> opp->nb_irqs = RAVEN_MAX_EXT;
> opp->vid = VID_REVISION_1_3;
> - opp->veni = VENI_GENERIC;
> + opp->vir = VIR_GENERIC;
> opp->vector_mask = 0xFF;
> - opp->tifr_reset = 4160000;
> - opp->ipvp_reset = IPVP_MASK_MASK | IPVP_MODE_MASK;
> - opp->ide_reset = 0;
> + opp->tfrr_reset = 4160000;
> + opp->ivpr_reset = IVPR_MASK_MASK | IVPR_MODE_MASK;
> + opp->idr_reset = 0;
> opp->max_irq = RAVEN_MAX_IRQ;
> opp->irq_ipi0 = RAVEN_IPI_IRQ;
> opp->irq_tim0 = RAVEN_TMR_IRQ;
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 06/15] openpic: rework critical interrupt support
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (4 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 05/15] openpic: make register names correspond better with hw docs Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:31 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 07/15] openpic: make ctpr signed Scott Wood
` (8 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Critical interrupts on FSL MPIC are not supposed to pay
attention to priority, IACK, EOI, etc. On the currently modeled
version it's not supposed to pay attention to the mask bit either.
Also reorganize to make it easier to implement newer FSL MPIC models,
which encode interrupt level information differently and support
mcheck as well as crit, and to reduce problems for later patches
in this set.
Still missing is the ability to lower the CINT signal to the core,
as IACK/EOI is not used. This will come with general IRQ-source-driven
lowering in the next patch.
New state is added which is not serialized, but instead is recomputed
in openpic_load() by calling the appropriate write_IRQreg function.
This should have the side effect of causing the IRQ outputs to be
raised appropriately on load, which was missing.
The serialization format is altered by swapping ivpr and idr (we'd like
IDR to be restored before we run the IVPR logic), and moving interrupts
to the end (so that other state has been restored by the time we run the
IDR/IVPR logic. Serialization for this driver is not yet in a state
where backwards compatibility is reasonable (assuming it works at all),
and the current serialization format was not built for extensibility.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 110 ++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 76 insertions(+), 34 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index 7647368..94a7807 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -187,7 +187,9 @@ typedef struct IRQ_queue_t {
typedef struct IRQ_src_t {
uint32_t ivpr; /* IRQ vector/priority register */
uint32_t idr; /* IRQ destination register */
+ uint32_t destmask; /* bitmap of CPU destinations */
int last_cpu;
+ int output; /* IRQ level, e.g. OPENPIC_OUTPUT_INT */
int pending; /* TRUE if IRQ is pending */
bool level; /* level-triggered */
bool fslint; /* FSL internal interrupt -- level only */
@@ -265,8 +267,6 @@ typedef struct OpenPICState {
uint32_t irq_msi;
} OpenPICState;
-static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src);
-
static inline void IRQ_setbit(IRQ_queue_t *q, int n_IRQ)
{
q->pending++;
@@ -331,6 +331,19 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
dst = &opp->dst[n_CPU];
src = &opp->src[n_IRQ];
+
+ if (src->output != OPENPIC_OUTPUT_INT) {
+ /* On Freescale MPIC, critical interrupts ignore priority,
+ * IACK, EOI, etc. Before MPIC v4.1 they also ignore
+ * masking.
+ */
+ src->ivpr |= IVPR_ACTIVITY_MASK;
+ DPRINTF("%s: Raise OpenPIC output %d cpu %d irq %d\n",
+ __func__, src->output, n_CPU, n_IRQ);
+ qemu_irq_raise(opp->dst[n_CPU].irqs[src->output]);
+ return;
+ }
+
priority = IVPR_PRIORITY(src->ivpr);
if (priority <= dst->ctpr) {
/* Too low priority */
@@ -361,7 +374,7 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
return;
}
DPRINTF("Raise OpenPIC INT output cpu %d irq %d\n", n_CPU, n_IRQ);
- openpic_irq_raise(opp, n_CPU, src);
+ qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
}
/* update pic state because registers for n_IRQ have changed value */
@@ -404,7 +417,7 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
} else if (!(src->ivpr & IVPR_MODE_MASK)) {
/* Directed delivery mode */
for (i = 0; i < opp->nb_cpus; i++) {
- if (src->idr & (1 << i)) {
+ if (src->destmask & (1 << i)) {
IRQ_local_pipe(opp, i, n_IRQ);
}
}
@@ -413,7 +426,7 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
for (i = src->last_cpu + 1; i != src->last_cpu; i++) {
if (i == opp->nb_cpus)
i = 0;
- if (src->idr & (1 << i)) {
+ if (src->destmask & (1 << i)) {
IRQ_local_pipe(opp, i, n_IRQ);
src->last_cpu = i;
break;
@@ -500,12 +513,45 @@ static inline uint32_t read_IRQreg_ivpr(OpenPICState *opp, int n_IRQ)
static inline void write_IRQreg_idr(OpenPICState *opp, int n_IRQ, uint32_t val)
{
- uint32_t tmp;
+ IRQ_src_t *src = &opp->src[n_IRQ];
+ uint32_t normal_mask = (1UL << opp->nb_cpus) - 1;
+ uint32_t crit_mask = 0;
+ uint32_t mask = normal_mask;
+ int crit_shift = IDR_EP_SHIFT - opp->nb_cpus;
+ int i;
+
+ if (opp->flags & OPENPIC_FLAG_IDR_CRIT) {
+ crit_mask = mask << crit_shift;
+ mask |= crit_mask | IDR_EP;
+ }
+
+ src->idr = val & mask;
+ DPRINTF("Set IDE %d to 0x%08x\n", n_IRQ, src->idr);
+
+ if (opp->flags & OPENPIC_FLAG_IDR_CRIT) {
+ if (src->idr & crit_mask) {
+ if (src->idr & normal_mask) {
+ DPRINTF("%s: IRQ configured for multiple output types, using critical\n",
+ __func__);
+ }
+
+ src->output = OPENPIC_OUTPUT_CINT;
+ src->destmask = 0;
+
+ for (i = 0; i < opp->nb_cpus; i++) {
+ int n_ci = IDR_CI0_SHIFT - i;
- tmp = val & (IDR_EP | IDR_CI);
- tmp |= val & ((1ULL << MAX_CPU) - 1);
- opp->src[n_IRQ].idr = tmp;
- DPRINTF("Set IDE %d to 0x%08x\n", n_IRQ, opp->src[n_IRQ].idr);
+ if (src->idr & (1UL << n_ci)) {
+ src->destmask |= 1UL << i;
+ }
+ }
+ } else {
+ src->output = OPENPIC_OUTPUT_INT;
+ src->destmask = src->idr & normal_mask;
+ }
+ } else {
+ src->destmask = src->idr;
+ }
}
static inline void write_IRQreg_ivpr(OpenPICState *opp, int n_IRQ, uint32_t val)
@@ -899,7 +945,7 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
IVPR_PRIORITY(src->ivpr) > dst->servicing.priority)) {
DPRINTF("Raise OpenPIC INT output cpu %d irq %d\n",
idx, n_IRQ);
- openpic_irq_raise(opp, idx, src);
+ qemu_irq_raise(opp->dst[idx].irqs[OPENPIC_OUTPUT_INT]);
}
break;
default:
@@ -1121,13 +1167,6 @@ static void openpic_save(QEMUFile* f, void *opaque)
qemu_put_be32s(f, &opp->spve);
qemu_put_be32s(f, &opp->tfrr);
- for (i = 0; i < opp->max_irq; i++) {
- qemu_put_be32s(f, &opp->src[i].ivpr);
- qemu_put_be32s(f, &opp->src[i].idr);
- qemu_put_sbe32s(f, &opp->src[i].last_cpu);
- qemu_put_sbe32s(f, &opp->src[i].pending);
- }
-
qemu_put_be32s(f, &opp->nb_cpus);
for (i = 0; i < opp->nb_cpus; i++) {
@@ -1140,6 +1179,13 @@ static void openpic_save(QEMUFile* f, void *opaque)
qemu_put_be32s(f, &opp->timers[i].tccr);
qemu_put_be32s(f, &opp->timers[i].tbcr);
}
+
+ for (i = 0; i < opp->max_irq; i++) {
+ qemu_put_be32s(f, &opp->src[i].ivpr);
+ qemu_put_be32s(f, &opp->src[i].idr);
+ qemu_put_sbe32s(f, &opp->src[i].last_cpu);
+ qemu_put_sbe32s(f, &opp->src[i].pending);
+ }
}
static void openpic_load_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
@@ -1167,13 +1213,6 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
qemu_get_be32s(f, &opp->spve);
qemu_get_be32s(f, &opp->tfrr);
- for (i = 0; i < opp->max_irq; i++) {
- qemu_get_be32s(f, &opp->src[i].ivpr);
- qemu_get_be32s(f, &opp->src[i].idr);
- qemu_get_sbe32s(f, &opp->src[i].last_cpu);
- qemu_get_sbe32s(f, &opp->src[i].pending);
- }
-
qemu_get_be32s(f, &opp->nb_cpus);
for (i = 0; i < opp->nb_cpus; i++) {
@@ -1187,18 +1226,21 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
qemu_get_be32s(f, &opp->timers[i].tbcr);
}
- return 0;
-}
+ for (i = 0; i < opp->max_irq; i++) {
+ uint32_t val;
-static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src)
-{
- int n_ci = IDR_CI0_SHIFT - n_CPU;
+ val = qemu_get_be32(f);
+ write_IRQreg_idr(opp, i, val);
+ val = qemu_get_be32(f);
+ write_IRQreg_ivpr(opp, i, val);
- if ((opp->flags & OPENPIC_FLAG_IDR_CRIT) && (src->idr & (1 << n_ci))) {
- qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_CINT]);
- } else {
- qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
+ qemu_get_be32s(f, &opp->src[i].ivpr);
+ qemu_get_be32s(f, &opp->src[i].idr);
+ qemu_get_sbe32s(f, &opp->src[i].last_cpu);
+ qemu_get_sbe32s(f, &opp->src[i].pending);
}
+
+ return 0;
}
struct memreg {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 06/15] openpic: rework critical interrupt support
2012-12-22 2:15 ` [Qemu-devel] [PATCH 06/15] openpic: rework critical interrupt support Scott Wood
@ 2013-01-03 18:31 ` Alexander Graf
2013-01-03 23:07 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 18:31 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> Critical interrupts on FSL MPIC are not supposed to pay
> attention to priority, IACK, EOI, etc. On the currently modeled
> version it's not supposed to pay attention to the mask bit either.
>
> Also reorganize to make it easier to implement newer FSL MPIC models,
> which encode interrupt level information differently and support
> mcheck as well as crit, and to reduce problems for later patches
> in this set.
>
> Still missing is the ability to lower the CINT signal to the core,
> as IACK/EOI is not used. This will come with general IRQ-source-driven
> lowering in the next patch.
>
> New state is added which is not serialized, but instead is recomputed
> in openpic_load() by calling the appropriate write_IRQreg function.
> This should have the side effect of causing the IRQ outputs to be
> raised appropriately on load, which was missing.
>
> The serialization format is altered by swapping ivpr and idr (we'd like
> IDR to be restored before we run the IVPR logic), and moving interrupts
> to the end (so that other state has been restored by the time we run the
> IDR/IVPR logic. Serialization for this driver is not yet in a state
> where backwards compatibility is reasonable (assuming it works at all),
> and the current serialization format was not built for extensibility.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Thanks, applied to ppc-next (with adjustments).
Alex
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 06/15] openpic: rework critical interrupt support
2013-01-03 18:31 ` Alexander Graf
@ 2013-01-03 23:07 ` Scott Wood
2013-01-04 8:04 ` Alexander Graf
2013-01-04 20:46 ` [Qemu-devel] [Qemu-ppc] " Blue Swirl
0 siblings, 2 replies; 47+ messages in thread
From: Scott Wood @ 2013-01-03 23:07 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, qemu-devel
On 01/03/2013 12:31:47 PM, Alexander Graf wrote:
>
> On 22.12.2012, at 03:15, Scott Wood wrote:
>
> > Critical interrupts on FSL MPIC are not supposed to pay
> > attention to priority, IACK, EOI, etc. On the currently modeled
> > version it's not supposed to pay attention to the mask bit either.
> >
> > Also reorganize to make it easier to implement newer FSL MPIC
> models,
> > which encode interrupt level information differently and support
> > mcheck as well as crit, and to reduce problems for later patches
> > in this set.
> >
> > Still missing is the ability to lower the CINT signal to the core,
> > as IACK/EOI is not used. This will come with general
> IRQ-source-driven
> > lowering in the next patch.
> >
> > New state is added which is not serialized, but instead is
> recomputed
> > in openpic_load() by calling the appropriate write_IRQreg function.
> > This should have the side effect of causing the IRQ outputs to be
> > raised appropriately on load, which was missing.
> >
> > The serialization format is altered by swapping ivpr and idr (we'd
> like
> > IDR to be restored before we run the IVPR logic), and moving
> interrupts
> > to the end (so that other state has been restored by the time we
> run the
> > IDR/IVPR logic. Serialization for this driver is not yet in a state
> > where backwards compatibility is reasonable (assuming it works at
> all),
> > and the current serialization format was not built for
> extensibility.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>
> Thanks, applied to ppc-next (with adjustments).
It looks like one of the adjustments was wrapping an error string --
does QEMU not follow the Linux rule of not wrapping error strings? If
not, maybe that should change.
QEMU's checkpatch appears to retain the line-length exception, but the
set of log functions hasn't been updated (not even to include printf)...
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 06/15] openpic: rework critical interrupt support
2013-01-03 23:07 ` Scott Wood
@ 2013-01-04 8:04 ` Alexander Graf
2013-01-04 20:46 ` [Qemu-devel] [Qemu-ppc] " Blue Swirl
1 sibling, 0 replies; 47+ messages in thread
From: Alexander Graf @ 2013-01-04 8:04 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 04.01.2013, at 00:07, Scott Wood wrote:
> On 01/03/2013 12:31:47 PM, Alexander Graf wrote:
>> On 22.12.2012, at 03:15, Scott Wood wrote:
>> > Critical interrupts on FSL MPIC are not supposed to pay
>> > attention to priority, IACK, EOI, etc. On the currently modeled
>> > version it's not supposed to pay attention to the mask bit either.
>> >
>> > Also reorganize to make it easier to implement newer FSL MPIC models,
>> > which encode interrupt level information differently and support
>> > mcheck as well as crit, and to reduce problems for later patches
>> > in this set.
>> >
>> > Still missing is the ability to lower the CINT signal to the core,
>> > as IACK/EOI is not used. This will come with general IRQ-source-driven
>> > lowering in the next patch.
>> >
>> > New state is added which is not serialized, but instead is recomputed
>> > in openpic_load() by calling the appropriate write_IRQreg function.
>> > This should have the side effect of causing the IRQ outputs to be
>> > raised appropriately on load, which was missing.
>> >
>> > The serialization format is altered by swapping ivpr and idr (we'd like
>> > IDR to be restored before we run the IVPR logic), and moving interrupts
>> > to the end (so that other state has been restored by the time we run the
>> > IDR/IVPR logic. Serialization for this driver is not yet in a state
>> > where backwards compatibility is reasonable (assuming it works at all),
>> > and the current serialization format was not built for extensibility.
>> >
>> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>> Thanks, applied to ppc-next (with adjustments).
>
> It looks like one of the adjustments was wrapping an error string -- does QEMU not follow the Linux rule of not wrapping error strings? If not, maybe that should change.
At least not that I'm aware of :).
> QEMU's checkpatch appears to retain the line-length exception, but the set of log functions hasn't been updated (not even to include printf)...
Yeah, checkpatch was simply copied from Linux. I suppose nobody adjusted that bit.
Alex
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 06/15] openpic: rework critical interrupt support
2013-01-03 23:07 ` Scott Wood
2013-01-04 8:04 ` Alexander Graf
@ 2013-01-04 20:46 ` Blue Swirl
2013-01-04 20:49 ` Scott Wood
1 sibling, 1 reply; 47+ messages in thread
From: Blue Swirl @ 2013-01-04 20:46 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, Alexander Graf, qemu-devel
On Thu, Jan 3, 2013 at 11:07 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 01/03/2013 12:31:47 PM, Alexander Graf wrote:
>>
>>
>> On 22.12.2012, at 03:15, Scott Wood wrote:
>>
>> > Critical interrupts on FSL MPIC are not supposed to pay
>> > attention to priority, IACK, EOI, etc. On the currently modeled
>> > version it's not supposed to pay attention to the mask bit either.
>> >
>> > Also reorganize to make it easier to implement newer FSL MPIC models,
>> > which encode interrupt level information differently and support
>> > mcheck as well as crit, and to reduce problems for later patches
>> > in this set.
>> >
>> > Still missing is the ability to lower the CINT signal to the core,
>> > as IACK/EOI is not used. This will come with general IRQ-source-driven
>> > lowering in the next patch.
>> >
>> > New state is added which is not serialized, but instead is recomputed
>> > in openpic_load() by calling the appropriate write_IRQreg function.
>> > This should have the side effect of causing the IRQ outputs to be
>> > raised appropriately on load, which was missing.
>> >
>> > The serialization format is altered by swapping ivpr and idr (we'd like
>> > IDR to be restored before we run the IVPR logic), and moving interrupts
>> > to the end (so that other state has been restored by the time we run the
>> > IDR/IVPR logic. Serialization for this driver is not yet in a state
>> > where backwards compatibility is reasonable (assuming it works at all),
>> > and the current serialization format was not built for extensibility.
>> >
>> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>>
>> Thanks, applied to ppc-next (with adjustments).
>
>
> It looks like one of the adjustments was wrapping an error string -- does
> QEMU not follow the Linux rule of not wrapping error strings? If not, maybe
> that should change.
There was some discussion earlier and IIRC the outcome was that error
strings should contain unique IDs to make them greppable.
>
> QEMU's checkpatch appears to retain the line-length exception, but the set
> of log functions hasn't been updated (not even to include printf)...
The exception should be removed.
>
> -Scott
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 06/15] openpic: rework critical interrupt support
2013-01-04 20:46 ` [Qemu-devel] [Qemu-ppc] " Blue Swirl
@ 2013-01-04 20:49 ` Scott Wood
2013-01-04 21:17 ` Blue Swirl
0 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2013-01-04 20:49 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-ppc, Alexander Graf, qemu-devel
On 01/04/2013 02:46:00 PM, Blue Swirl wrote:
> On Thu, Jan 3, 2013 at 11:07 PM, Scott Wood <scottwood@freescale.com>
> wrote:
> > It looks like one of the adjustments was wrapping an error string
> -- does
> > QEMU not follow the Linux rule of not wrapping error strings? If
> not, maybe
> > that should change.
>
> There was some discussion earlier and IIRC the outcome was that error
> strings should contain unique IDs to make them greppable.
Sigh. Are there any examples of this being done? How is uniqueness
ensured?
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 06/15] openpic: rework critical interrupt support
2013-01-04 20:49 ` Scott Wood
@ 2013-01-04 21:17 ` Blue Swirl
2013-01-04 21:25 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Blue Swirl @ 2013-01-04 21:17 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, Alexander Graf, qemu-devel
On Fri, Jan 4, 2013 at 8:49 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 01/04/2013 02:46:00 PM, Blue Swirl wrote:
>>
>> On Thu, Jan 3, 2013 at 11:07 PM, Scott Wood <scottwood@freescale.com>
>> wrote:
>> > It looks like one of the adjustments was wrapping an error string --
>> > does
>> > QEMU not follow the Linux rule of not wrapping error strings? If not,
>> > maybe
>> > that should change.
>>
>> There was some discussion earlier and IIRC the outcome was that error
>> strings should contain unique IDs to make them greppable.
>
>
> Sigh. Are there any examples of this being done? How is uniqueness
> ensured?
Here's one example:
http://h71000.www7.hp.com/doc/73final/6023/6023pro_001.html
:-)
>
> -Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 06/15] openpic: rework critical interrupt support
2013-01-04 21:17 ` Blue Swirl
@ 2013-01-04 21:25 ` Scott Wood
0 siblings, 0 replies; 47+ messages in thread
From: Scott Wood @ 2013-01-04 21:25 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-ppc, Alexander Graf, qemu-devel
On 01/04/2013 03:17:21 PM, Blue Swirl wrote:
> On Fri, Jan 4, 2013 at 8:49 PM, Scott Wood <scottwood@freescale.com>
> wrote:
> > On 01/04/2013 02:46:00 PM, Blue Swirl wrote:
> >>
> >> On Thu, Jan 3, 2013 at 11:07 PM, Scott Wood
> <scottwood@freescale.com>
> >> wrote:
> >> > It looks like one of the adjustments was wrapping an error
> string --
> >> > does
> >> > QEMU not follow the Linux rule of not wrapping error strings?
> If not,
> >> > maybe
> >> > that should change.
> >>
> >> There was some discussion earlier and IIRC the outcome was that
> error
> >> strings should contain unique IDs to make them greppable.
> >
> >
> > Sigh. Are there any examples of this being done? How is uniqueness
> > ensured?
>
> Here's one example:
> http://h71000.www7.hp.com/doc/73final/6023/6023pro_001.html
>
> :-)
I meant in QEMU. :-P
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 07/15] openpic: make ctpr signed
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (5 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 06/15] openpic: rework critical interrupt support Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:33 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 08/15] openpic/fsl: critical interrupts ignore mask before v4.1 Scott Wood
` (7 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Other priorities are signed, so avoid comparisons between
signed and unsigned.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index 94a7807..9d22e9c 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -216,7 +216,7 @@ typedef struct IRQ_src_t {
#define IDR_CI 0x40000000 /* critical interrupt */
typedef struct IRQ_dst_t {
- uint32_t ctpr; /* CPU current task priority */
+ int32_t ctpr; /* CPU current task priority */
IRQ_queue_t raised;
IRQ_queue_t servicing;
qemu_irq *irqs;
@@ -1170,7 +1170,7 @@ static void openpic_save(QEMUFile* f, void *opaque)
qemu_put_be32s(f, &opp->nb_cpus);
for (i = 0; i < opp->nb_cpus; i++) {
- qemu_put_be32s(f, &opp->dst[i].ctpr);
+ qemu_put_sbe32s(f, &opp->dst[i].ctpr);
openpic_save_IRQ_queue(f, &opp->dst[i].raised);
openpic_save_IRQ_queue(f, &opp->dst[i].servicing);
}
@@ -1216,7 +1216,7 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
qemu_get_be32s(f, &opp->nb_cpus);
for (i = 0; i < opp->nb_cpus; i++) {
- qemu_get_be32s(f, &opp->dst[i].ctpr);
+ qemu_get_sbe32s(f, &opp->dst[i].ctpr);
openpic_load_IRQ_queue(f, &opp->dst[i].raised);
openpic_load_IRQ_queue(f, &opp->dst[i].servicing);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 08/15] openpic/fsl: critical interrupts ignore mask before v4.1
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (6 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 07/15] openpic: make ctpr signed Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:37 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 09/15] openpic: always call IRQ_check from IRQ_get_next Scott Wood
` (6 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index 9d22e9c..268f312 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -194,6 +194,7 @@ typedef struct IRQ_src_t {
bool level; /* level-triggered */
bool fslint; /* FSL internal interrupt -- level only */
bool fslspecial; /* FSL timer/IPI interrupt, edge, no polarity */
+ bool nomask; /* critical interrupts ignore mask on some FSL MPICs */
} IRQ_src_t;
#define IVPR_MASK_SHIFT 31
@@ -390,7 +391,7 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
DPRINTF("%s: IRQ %d is not pending\n", __func__, n_IRQ);
return;
}
- if (src->ivpr & IVPR_MASK_MASK) {
+ if ((src->ivpr & IVPR_MASK_MASK) && !src->nomask) {
/* Interrupt source is disabled */
DPRINTF("%s: IRQ %d is disabled\n", __func__, n_IRQ);
return;
@@ -536,6 +537,7 @@ static inline void write_IRQreg_idr(OpenPICState *opp, int n_IRQ, uint32_t val)
}
src->output = OPENPIC_OUTPUT_CINT;
+ src->nomask = true;
src->destmask = 0;
for (i = 0; i < opp->nb_cpus; i++) {
@@ -547,6 +549,7 @@ static inline void write_IRQreg_idr(OpenPICState *opp, int n_IRQ, uint32_t val)
}
} else {
src->output = OPENPIC_OUTPUT_INT;
+ src->nomask = false;
src->destmask = src->idr & normal_mask;
}
} else {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 08/15] openpic/fsl: critical interrupts ignore mask before v4.1
2012-12-22 2:15 ` [Qemu-devel] [PATCH 08/15] openpic/fsl: critical interrupts ignore mask before v4.1 Scott Wood
@ 2013-01-03 18:37 ` Alexander Graf
0 siblings, 0 replies; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 18:37 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> hw/openpic.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 9d22e9c..268f312 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -194,6 +194,7 @@ typedef struct IRQ_src_t {
> bool level; /* level-triggered */
> bool fslint; /* FSL internal interrupt -- level only */
> bool fslspecial; /* FSL timer/IPI interrupt, edge, no polarity */
> + bool nomask; /* critical interrupts ignore mask on some FSL MPICs */
This should be :1 - we have quite a big number of sources :).
Fixed up locally and applied to ppc-next.
Alex
> } IRQ_src_t;
>
> #define IVPR_MASK_SHIFT 31
> @@ -390,7 +391,7 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
> DPRINTF("%s: IRQ %d is not pending\n", __func__, n_IRQ);
> return;
> }
> - if (src->ivpr & IVPR_MASK_MASK) {
> + if ((src->ivpr & IVPR_MASK_MASK) && !src->nomask) {
> /* Interrupt source is disabled */
> DPRINTF("%s: IRQ %d is disabled\n", __func__, n_IRQ);
> return;
> @@ -536,6 +537,7 @@ static inline void write_IRQreg_idr(OpenPICState *opp, int n_IRQ, uint32_t val)
> }
>
> src->output = OPENPIC_OUTPUT_CINT;
> + src->nomask = true;
> src->destmask = 0;
>
> for (i = 0; i < opp->nb_cpus; i++) {
> @@ -547,6 +549,7 @@ static inline void write_IRQreg_idr(OpenPICState *opp, int n_IRQ, uint32_t val)
> }
> } else {
> src->output = OPENPIC_OUTPUT_INT;
> + src->nomask = false;
> src->destmask = src->idr & normal_mask;
> }
> } else {
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 09/15] openpic: always call IRQ_check from IRQ_get_next
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (7 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 08/15] openpic/fsl: critical interrupts ignore mask before v4.1 Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:42 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 10/15] Revert "openpic: Accelerate pending irq search" Scott Wood
` (5 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Previously the code relied on the queue's "next" field getting
set to -1 sometime between an update to the bitmap, and the next
call to IRQ_get_next. Sometimes this happened after the update.
Sometimes it happened before the check. Sometimes it didn't happen
at all.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index 268f312..e2e7079 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -316,10 +316,8 @@ out:
static int IRQ_get_next(OpenPICState *opp, IRQ_queue_t *q)
{
- if (q->next == -1) {
- /* XXX: optimize */
- IRQ_check(opp, q);
- }
+ /* XXX: optimize */
+ IRQ_check(opp, q);
return q->next;
}
@@ -366,7 +364,7 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
__func__, n_IRQ, dst->raised.next, n_CPU);
return;
}
- IRQ_get_next(opp, &dst->raised);
+ IRQ_check(opp, &dst->raised);
if (IRQ_get_next(opp, &dst->servicing) != -1 &&
priority <= dst->servicing.priority) {
DPRINTF("%s: IRQ %d is hidden by servicing IRQ %d on CPU %d\n",
@@ -937,7 +935,6 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
DPRINTF("EOI\n");
s_IRQ = IRQ_get_next(opp, &dst->servicing);
IRQ_resetbit(&dst->servicing, s_IRQ);
- dst->servicing.next = -1;
/* Set up next servicing IRQ */
s_IRQ = IRQ_get_next(opp, &dst->servicing);
/* Check queued interrupts. */
@@ -1013,7 +1010,6 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
retval = IVPR_VECTOR(opp, src->ivpr);
}
IRQ_resetbit(&dst->raised, n_IRQ);
- dst->raised.next = -1;
if (!src->level) {
/* edge-sensitive IRQ */
src->ivpr &= ~IVPR_ACTIVITY_MASK;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 09/15] openpic: always call IRQ_check from IRQ_get_next
2012-12-22 2:15 ` [Qemu-devel] [PATCH 09/15] openpic: always call IRQ_check from IRQ_get_next Scott Wood
@ 2013-01-03 18:42 ` Alexander Graf
2013-01-03 20:09 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 18:42 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> Previously the code relied on the queue's "next" field getting
> set to -1 sometime between an update to the bitmap, and the next
> call to IRQ_get_next. Sometimes this happened after the update.
> Sometimes it happened before the check. Sometimes it didn't happen
> at all.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Have you verified that we don't run the check too often then? It's quite costly, no?
Applied nevertheless to ppc-next.
Alex
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 09/15] openpic: always call IRQ_check from IRQ_get_next
2013-01-03 18:42 ` Alexander Graf
@ 2013-01-03 20:09 ` Scott Wood
0 siblings, 0 replies; 47+ messages in thread
From: Scott Wood @ 2013-01-03 20:09 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, qemu-devel
On 01/03/2013 12:42:09 PM, Alexander Graf wrote:
>
> On 22.12.2012, at 03:15, Scott Wood wrote:
>
> > Previously the code relied on the queue's "next" field getting
> > set to -1 sometime between an update to the bitmap, and the next
> > call to IRQ_get_next. Sometimes this happened after the update.
> > Sometimes it happened before the check. Sometimes it didn't happen
> > at all.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>
> Have you verified that we don't run the check too often then? It's
> quite costly, no?
Correctness takes precedence over speed, as does
readability/maintainability if the difference is minor. In any case,
the check gets faster later in the patchset.
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 10/15] Revert "openpic: Accelerate pending irq search"
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (8 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 09/15] openpic: always call IRQ_check from IRQ_get_next Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2012-12-22 2:15 ` [Qemu-devel] [PATCH 11/15] openpic: use standard bitmap operations Scott Wood
` (4 subsequent siblings)
14 siblings, 0 replies; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
This reverts commit a9bd83f4c65de0058659ede009fa1a241f379edd.
This counting approach is not robust against setting a bit that
was already set, or clearing a bit that was already clear. Perhaps
that is considered a bug, but besides the lack of any documentation
for that restriction, it's a pretty unpleasant way for the problem
to manifest itself.
It could be made more robust by testing the current value of the
bit before changing the count, but a later patch speeds up IRQ_check
in all cases, not just when there's nothing pending. Hopefully that
should be adequate to address performance concerns.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index e2e7079..cc3e514 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -181,7 +181,6 @@ typedef struct IRQ_queue_t {
uint32_t queue[BF_WIDTH(MAX_IRQ)];
int next;
int priority;
- int pending; /* nr of pending bits in queue */
} IRQ_queue_t;
typedef struct IRQ_src_t {
@@ -270,13 +269,11 @@ typedef struct OpenPICState {
static inline void IRQ_setbit(IRQ_queue_t *q, int n_IRQ)
{
- q->pending++;
set_bit(q->queue, n_IRQ);
}
static inline void IRQ_resetbit(IRQ_queue_t *q, int n_IRQ)
{
- q->pending--;
reset_bit(q->queue, n_IRQ);
}
@@ -292,12 +289,6 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
next = -1;
priority = -1;
-
- if (!q->pending) {
- /* IRQ bitmap is empty */
- goto out;
- }
-
for (i = 0; i < opp->max_irq; i++) {
if (IRQ_testbit(q, i)) {
DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
@@ -308,8 +299,6 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
}
}
}
-
-out:
q->next = next;
q->priority = priority;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 11/15] openpic: use standard bitmap operations
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (9 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 10/15] Revert "openpic: Accelerate pending irq search" Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:49 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time Scott Wood
` (3 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Besides the private implementation being redundant, namespace collisions
prevented the use of other things in bitops.h.
Serialization does get a bit more awkward, unfortunately, since the
standard bitmap operations are "unsigned long" rather than "uint32_t",
though in exchange we will get faster queue lookups on 64-bit hosts once
we search a word at a time.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 55 +++++++++++++++++++++++++++++--------------------------
1 file changed, 29 insertions(+), 26 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index cc3e514..f2ac286 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -39,6 +39,7 @@
#include "openpic.h"
#include "sysbus.h"
#include "pci/msi.h"
+#include "qemu/bitops.h"
//#define DEBUG_OPENPIC
@@ -145,24 +146,6 @@
#define MSIIR_IBS_SHIFT 24
#define MSIIR_IBS_MASK (0x1f << MSIIR_IBS_SHIFT)
-#define BF_WIDTH(_bits_) \
-(((_bits_) + (sizeof(uint32_t) * 8) - 1) / (sizeof(uint32_t) * 8))
-
-static inline void set_bit(uint32_t *field, int bit)
-{
- field[bit >> 5] |= 1 << (bit & 0x1F);
-}
-
-static inline void reset_bit(uint32_t *field, int bit)
-{
- field[bit >> 5] &= ~(1 << (bit & 0x1F));
-}
-
-static inline int test_bit(uint32_t *field, int bit)
-{
- return (field[bit >> 5] & 1 << (bit & 0x1F)) != 0;
-}
-
static int get_current_cpu(void)
{
if (!cpu_single_env) {
@@ -178,7 +161,10 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
uint32_t val, int idx);
typedef struct IRQ_queue_t {
- uint32_t queue[BF_WIDTH(MAX_IRQ)];
+ /* Round up to the nearest 64 IRQs so that the queue length
+ * won't change when moving between 32 and 64 bit hosts.
+ */
+ unsigned long queue[BITS_TO_LONGS((MAX_IRQ + 63) & ~63)];
int next;
int priority;
} IRQ_queue_t;
@@ -269,17 +255,17 @@ typedef struct OpenPICState {
static inline void IRQ_setbit(IRQ_queue_t *q, int n_IRQ)
{
- set_bit(q->queue, n_IRQ);
+ set_bit(n_IRQ, q->queue);
}
static inline void IRQ_resetbit(IRQ_queue_t *q, int n_IRQ)
{
- reset_bit(q->queue, n_IRQ);
+ clear_bit(n_IRQ, q->queue);
}
static inline int IRQ_testbit(IRQ_queue_t *q, int n_IRQ)
{
- return test_bit(q->queue, n_IRQ);
+ return test_bit(n_IRQ, q->queue);
}
static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
@@ -1137,8 +1123,16 @@ static void openpic_save_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
{
unsigned int i;
- for (i = 0; i < BF_WIDTH(MAX_IRQ); i++)
- qemu_put_be32s(f, &q->queue[i]);
+ for (i = 0; i < ARRAY_SIZE(q->queue); i++) {
+ /* Always put the lower half of a 64-bit long first, in case we
+ * restore on a 32-bit host. The least significant bits correspond
+ * to lower IRQ numbers in the bitmap.
+ */
+ qemu_put_be32(f, (uint32_t)q->queue[i]);
+#if LONG_MAX > 0x7FFFFFFF
+ qemu_put_be32(f, (uint32_t)(q->queue[i] >> 32));
+#endif
+ }
qemu_put_sbe32s(f, &q->next);
qemu_put_sbe32s(f, &q->priority);
@@ -1180,8 +1174,17 @@ static void openpic_load_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
{
unsigned int i;
- for (i = 0; i < BF_WIDTH(MAX_IRQ); i++)
- qemu_get_be32s(f, &q->queue[i]);
+ for (i = 0; i < ARRAY_SIZE(q->queue); i++) {
+ unsigned long val;
+
+ val = qemu_get_be32(f);
+#if LONG_MAX > 0x7FFFFFFF
+ val <<= 32;
+ val |= qemu_get_be32(f);
+#endif
+
+ q->queue[i] = val;
+ }
qemu_get_sbe32s(f, &q->next);
qemu_get_sbe32s(f, &q->priority);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 11/15] openpic: use standard bitmap operations
2012-12-22 2:15 ` [Qemu-devel] [PATCH 11/15] openpic: use standard bitmap operations Scott Wood
@ 2013-01-03 18:49 ` Alexander Graf
0 siblings, 0 replies; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 18:49 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> Besides the private implementation being redundant, namespace collisions
> prevented the use of other things in bitops.h.
>
> Serialization does get a bit more awkward, unfortunately, since the
> standard bitmap operations are "unsigned long" rather than "uint32_t",
> though in exchange we will get faster queue lookups on 64-bit hosts once
> we search a word at a time.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Thanks, applied to ppc-next. I wouldn't worry about save/restore for now though. We need to move that to savevm anyways and there should be a generic bitmap save/restore helper available.
Alex
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (10 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 11/15] openpic: use standard bitmap operations Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:53 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 13/15] openpic: add some bounds checking for IRQ numbers Scott Wood
` (2 subsequent siblings)
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Search the queue more efficiently by first looking for a non-zero word,
and then using the common bit-searching function to find the bit within
the word. It would be even nicer if bitops_ffsl() could be hooked up
to the compiler intrinsic so that bit-searching instructions could be
used, but that's another matter.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index f2ac286..5accff5 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -270,21 +270,35 @@ static inline int IRQ_testbit(IRQ_queue_t *q, int n_IRQ)
static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
{
- int next, i;
+ int next, word, irq, base;
int priority;
next = -1;
priority = -1;
- for (i = 0; i < opp->max_irq; i++) {
- if (IRQ_testbit(q, i)) {
+
+ for (word = 0, base = 0; word < ARRAY_SIZE(q->queue);
+ word++, base += BITS_PER_LONG) {
+ unsigned long map = q->queue[word];
+
+ if (!map) {
+ continue;
+ }
+
+ while (map) {
+ int offset = bitops_ffsl(map);
+ irq = base + offset;
+ map &= ~(1UL << offset);
+
DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
- i, IVPR_PRIORITY(opp->src[i].ivpr), priority);
- if (IVPR_PRIORITY(opp->src[i].ivpr) > priority) {
- next = i;
- priority = IVPR_PRIORITY(opp->src[i].ivpr);
+ irq, IVPR_PRIORITY(opp->src[irq].ivpr), priority);
+
+ if (IVPR_PRIORITY(opp->src[irq].ivpr) > priority) {
+ next = irq;
+ priority = IVPR_PRIORITY(opp->src[irq].ivpr);
}
}
}
+
q->next = next;
q->priority = priority;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time
2012-12-22 2:15 ` [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time Scott Wood
@ 2013-01-03 18:53 ` Alexander Graf
2013-01-03 20:07 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 18:53 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> Search the queue more efficiently by first looking for a non-zero word,
> and then using the common bit-searching function to find the bit within
> the word. It would be even nicer if bitops_ffsl() could be hooked up
> to the compiler intrinsic so that bit-searching instructions could be
> used, but that's another matter.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
What we really want is a bitmap wide ffs() bipops helper function that returns the first set bit in a bitmap and can optimize the hell out of that operation inside of itself. I don't think this belongs to the OpenPIC code.
Alex
> ---
> hw/openpic.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index f2ac286..5accff5 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -270,21 +270,35 @@ static inline int IRQ_testbit(IRQ_queue_t *q, int n_IRQ)
>
> static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
> {
> - int next, i;
> + int next, word, irq, base;
> int priority;
>
> next = -1;
> priority = -1;
> - for (i = 0; i < opp->max_irq; i++) {
> - if (IRQ_testbit(q, i)) {
> +
> + for (word = 0, base = 0; word < ARRAY_SIZE(q->queue);
> + word++, base += BITS_PER_LONG) {
> + unsigned long map = q->queue[word];
> +
> + if (!map) {
> + continue;
> + }
> +
> + while (map) {
> + int offset = bitops_ffsl(map);
> + irq = base + offset;
> + map &= ~(1UL << offset);
> +
> DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
> - i, IVPR_PRIORITY(opp->src[i].ivpr), priority);
> - if (IVPR_PRIORITY(opp->src[i].ivpr) > priority) {
> - next = i;
> - priority = IVPR_PRIORITY(opp->src[i].ivpr);
> + irq, IVPR_PRIORITY(opp->src[irq].ivpr), priority);
> +
> + if (IVPR_PRIORITY(opp->src[irq].ivpr) > priority) {
> + next = irq;
> + priority = IVPR_PRIORITY(opp->src[irq].ivpr);
> }
> }
> }
> +
> q->next = next;
> q->priority = priority;
> }
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time
2013-01-03 18:53 ` Alexander Graf
@ 2013-01-03 20:07 ` Scott Wood
2013-01-03 20:31 ` Alexander Graf
0 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2013-01-03 20:07 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, qemu-devel
On 01/03/2013 12:53:13 PM, Alexander Graf wrote:
>
> On 22.12.2012, at 03:15, Scott Wood wrote:
>
> > Search the queue more efficiently by first looking for a non-zero
> word,
> > and then using the common bit-searching function to find the bit
> within
> > the word. It would be even nicer if bitops_ffsl() could be hooked
> up
> > to the compiler intrinsic so that bit-searching instructions could
> be
> > used, but that's another matter.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>
> What we really want is a bitmap wide ffs() bipops helper function
> that returns the first set bit in a bitmap and can optimize the hell
> out of that operation inside of itself. I don't think this belongs to
> the OpenPIC code.
Well, we do have find_next_bit() in bitops.c, but it looks
comparitively complicated in order to be generic and simply return a
value rather than perform an action on each bit set. I suspect that
the code in this patch would be faster, and avoids the need for me to
follow all the twists and turns of find_next_bit() to figure out
whether the undocumented interface is actually exactly what I guess it
to be (e.g. what does it return when no bit is found?).
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time
2013-01-03 20:07 ` Scott Wood
@ 2013-01-03 20:31 ` Alexander Graf
2013-01-03 20:32 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 20:31 UTC (permalink / raw)
To: Scott Wood; +Cc: <qemu-ppc@nongnu.org>, <qemu-devel@nongnu.org>
Am 03.01.2013 um 21:07 schrieb Scott Wood <scottwood@freescale.com>:
> On 01/03/2013 12:53:13 PM, Alexander Graf wrote:
>> On 22.12.2012, at 03:15, Scott Wood wrote:
>> > Search the queue more efficiently by first looking for a non-zero word,
>> > and then using the common bit-searching function to find the bit within
>> > the word. It would be even nicer if bitops_ffsl() could be hooked up
>> > to the compiler intrinsic so that bit-searching instructions could be
>> > used, but that's another matter.
>> >
>> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>> What we really want is a bitmap wide ffs() bipops helper function that returns the first set bit in a bitmap and can optimize the hell out of that operation inside of itself. I don't think this belongs to the OpenPIC code.
>
> Well, we do have find_next_bit() in bitops.c, but it looks comparitively complicated in order to be generic and simply return a value rather than perform an action on each bit set. I suspect that the code in this patch would be faster, and avoids the need for me to follow all the twists and turns of find_next_bit() to figure out whether the undocumented interface is actually exactly what I guess it to be (e.g. what does it return when no bit is found?).
I would just call it bit_ffs and follow the same semantics.
Alex
>
> -Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time
2013-01-03 20:31 ` Alexander Graf
@ 2013-01-03 20:32 ` Scott Wood
2013-01-03 20:57 ` Alexander Graf
0 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2013-01-03 20:32 UTC (permalink / raw)
To: Alexander Graf; +Cc: <qemu-ppc@nongnu.org>, <qemu-devel@nongnu.org>
On 01/03/2013 02:31:30 PM, Alexander Graf wrote:
>
>
> Am 03.01.2013 um 21:07 schrieb Scott Wood <scottwood@freescale.com>:
>
> > On 01/03/2013 12:53:13 PM, Alexander Graf wrote:
> >> On 22.12.2012, at 03:15, Scott Wood wrote:
> >> > Search the queue more efficiently by first looking for a
> non-zero word,
> >> > and then using the common bit-searching function to find the bit
> within
> >> > the word. It would be even nicer if bitops_ffsl() could be
> hooked up
> >> > to the compiler intrinsic so that bit-searching instructions
> could be
> >> > used, but that's another matter.
> >> >
> >> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> >> What we really want is a bitmap wide ffs() bipops helper function
> that returns the first set bit in a bitmap and can optimize the hell
> out of that operation inside of itself. I don't think this belongs to
> the OpenPIC code.
> >
> > Well, we do have find_next_bit() in bitops.c, but it looks
> comparitively complicated in order to be generic and simply return a
> value rather than perform an action on each bit set. I suspect that
> the code in this patch would be faster, and avoids the need for me to
> follow all the twists and turns of find_next_bit() to figure out
> whether the undocumented interface is actually exactly what I guess
> it to be (e.g. what does it return when no bit is found?).
>
> I would just call it bit_ffs and follow the same semantics.
I'm not sure how that's an answer to what I wrote...
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time
2013-01-03 20:32 ` Scott Wood
@ 2013-01-03 20:57 ` Alexander Graf
2013-01-03 21:52 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 20:57 UTC (permalink / raw)
To: Scott Wood; +Cc: <qemu-ppc@nongnu.org>, <qemu-devel@nongnu.org>
On 03.01.2013, at 21:32, Scott Wood wrote:
> On 01/03/2013 02:31:30 PM, Alexander Graf wrote:
>> Am 03.01.2013 um 21:07 schrieb Scott Wood <scottwood@freescale.com>:
>> > On 01/03/2013 12:53:13 PM, Alexander Graf wrote:
>> >> On 22.12.2012, at 03:15, Scott Wood wrote:
>> >> > Search the queue more efficiently by first looking for a non-zero word,
>> >> > and then using the common bit-searching function to find the bit within
>> >> > the word. It would be even nicer if bitops_ffsl() could be hooked up
>> >> > to the compiler intrinsic so that bit-searching instructions could be
>> >> > used, but that's another matter.
>> >> >
>> >> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>> >> What we really want is a bitmap wide ffs() bipops helper function that returns the first set bit in a bitmap and can optimize the hell out of that operation inside of itself. I don't think this belongs to the OpenPIC code.
>> >
>> > Well, we do have find_next_bit() in bitops.c, but it looks comparitively complicated in order to be generic and simply return a value rather than perform an action on each bit set. I suspect that the code in this patch would be faster, and avoids the need for me to follow all the twists and turns of find_next_bit() to figure out whether the undocumented interface is actually exactly what I guess it to be (e.g. what does it return when no bit is found?).
>> I would just call it bit_ffs and follow the same semantics.
>
> I'm not sure how that's an answer to what I wrote...
The answer is "man ffs" and it will tell you what semantics the function would have. However given that find_next_bit() is defined in Linux, why not use its semantics? Can't we just copy Linux code here?
Whether it's faster or not is another question, but I'd like to keep bitops internals as internal to bitops as we can. We could even completely hide the bitops implementation details behind a private struct definition. But that would mean that we'd have to create an indirect memory access for the bitmap itself :(.
One advantage of keeping these bits internal would be that we could even add more accelerations. We could for example add another bitmap layer on top of the bitmap internally, similar to how the MSIs get another bitmap across all combined MSI registers in the MPIC. And find_next_bit could use that to speed up the lookup for really big bitmaps.
Alex
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time
2013-01-03 20:57 ` Alexander Graf
@ 2013-01-03 21:52 ` Scott Wood
0 siblings, 0 replies; 47+ messages in thread
From: Scott Wood @ 2013-01-03 21:52 UTC (permalink / raw)
To: Alexander Graf; +Cc: <qemu-ppc@nongnu.org>, <qemu-devel@nongnu.org>
On 01/03/2013 02:57:33 PM, Alexander Graf wrote:
>
> On 03.01.2013, at 21:32, Scott Wood wrote:
>
> > On 01/03/2013 02:31:30 PM, Alexander Graf wrote:
> >> Am 03.01.2013 um 21:07 schrieb Scott Wood
> <scottwood@freescale.com>:
> >> > On 01/03/2013 12:53:13 PM, Alexander Graf wrote:
> >> >> On 22.12.2012, at 03:15, Scott Wood wrote:
> >> >> > Search the queue more efficiently by first looking for a
> non-zero word,
> >> >> > and then using the common bit-searching function to find the
> bit within
> >> >> > the word. It would be even nicer if bitops_ffsl() could be
> hooked up
> >> >> > to the compiler intrinsic so that bit-searching instructions
> could be
> >> >> > used, but that's another matter.
> >> >> >
> >> >> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> >> >> What we really want is a bitmap wide ffs() bipops helper
> function that returns the first set bit in a bitmap and can optimize
> the hell out of that operation inside of itself. I don't think this
> belongs to the OpenPIC code.
> >> >
> >> > Well, we do have find_next_bit() in bitops.c, but it looks
> comparitively complicated in order to be generic and simply return a
> value rather than perform an action on each bit set. I suspect that
> the code in this patch would be faster, and avoids the need for me to
> follow all the twists and turns of find_next_bit() to figure out
> whether the undocumented interface is actually exactly what I guess
> it to be (e.g. what does it return when no bit is found?).
> >> I would just call it bit_ffs and follow the same semantics.
> >
> > I'm not sure how that's an answer to what I wrote...
>
> The answer is "man ffs" and it will tell you what semantics the
> function would have.
Not completely, because ffs() doesn't operate on multiple words.
> However given that find_next_bit() is defined in Linux, why not use
> its semantics? Can't we just copy Linux code here?
As I said, the function *has* been copied. Its semantics differ from
ffs() (which is good, since ffs() has annoying semantics such as adding
one to the result). It took a fair bit of staring at the function to
be convinced that the not-found case returns "size" under all
circumstances and that there isn't a bug when size is not a multiple of
BITS_PER_LONG.
> Whether it's faster or not is another question, but I'd like to keep
> bitops internals as internal to bitops as we can.
I think you get diminishing (and eventually negative) returns if you
push it too far, but fine, I'll send a v2 using find_next_bit().
> We could even completely hide the bitops implementation details
> behind a private struct definition. But that would mean that we'd
> have to create an indirect memory access for the bitmap itself :(.
>
> One advantage of keeping these bits internal would be that we could
> even add more accelerations. We could for example add another bitmap
> layer on top of the bitmap internally, similar to how the MSIs get
> another bitmap across all combined MSI registers in the MPIC. And
> find_next_bit could use that to speed up the lookup for really big
> bitmaps.
Perhaps, if it's shown to be a performance problem still.
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 13/15] openpic: add some bounds checking for IRQ numbers
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (11 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 12/15] openpic: IRQ_check: search the queue a word at a time Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:55 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 14/15] openpic: move IACK to its own function Scott Wood
2012-12-22 2:15 ` [Qemu-devel] [PATCH 15/15] openpic: fix CTPR and de-assertion of interrupts Scott Wood
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
The two checks with abort() guard against potential QEMU-internal
problems, but the EOI check stops the guest from causing updates to queue
position -1 and other havoc if it writes EOI with no interrupt in
service.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/hw/openpic.c b/hw/openpic.c
index 5accff5..a3fcefd 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -289,6 +289,10 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
irq = base + offset;
map &= ~(1UL << offset);
+ if (irq >= MAX_IRQ) {
+ abort();
+ }
+
DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
irq, IVPR_PRIORITY(opp->src[irq].ivpr), priority);
@@ -428,6 +432,11 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
OpenPICState *opp = opaque;
IRQ_src_t *src;
+ if (n_IRQ >= MAX_IRQ) {
+ fprintf(stderr, "%s: IRQ %d out of range\n", __func__, n_IRQ);
+ abort();
+ }
+
src = &opp->src[n_IRQ];
DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n",
n_IRQ, level, src->ivpr);
@@ -923,6 +932,12 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
case 0xB0: /* EOI */
DPRINTF("EOI\n");
s_IRQ = IRQ_get_next(opp, &dst->servicing);
+
+ if (s_IRQ < 0) {
+ DPRINTF("%s: EOI with no interrupt in service\n", __func__);
+ break;
+ }
+
IRQ_resetbit(&dst->servicing, s_IRQ);
/* Set up next servicing IRQ */
s_IRQ = IRQ_get_next(opp, &dst->servicing);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 13/15] openpic: add some bounds checking for IRQ numbers
2012-12-22 2:15 ` [Qemu-devel] [PATCH 13/15] openpic: add some bounds checking for IRQ numbers Scott Wood
@ 2013-01-03 18:55 ` Alexander Graf
2013-01-03 19:54 ` [Qemu-devel] [Qemu-ppc] " Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 18:55 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 22.12.2012, at 03:15, Scott Wood wrote:
> The two checks with abort() guard against potential QEMU-internal
> problems, but the EOI check stops the guest from causing updates to queue
> position -1 and other havoc if it writes EOI with no interrupt in
> service.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Did you ever actually experience this? MAX_IRQ should match the memory region size, so we shouldn't be able to receive any interrupt above it.
I might be inclined to accept an assert() there for internal sanity checking though. The last hunk looks fine.
Alex
> ---
> hw/openpic.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 5accff5..a3fcefd 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -289,6 +289,10 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
> irq = base + offset;
> map &= ~(1UL << offset);
>
> + if (irq >= MAX_IRQ) {
> + abort();
> + }
> +
> DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
> irq, IVPR_PRIORITY(opp->src[irq].ivpr), priority);
>
> @@ -428,6 +432,11 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
> OpenPICState *opp = opaque;
> IRQ_src_t *src;
>
> + if (n_IRQ >= MAX_IRQ) {
> + fprintf(stderr, "%s: IRQ %d out of range\n", __func__, n_IRQ);
> + abort();
> + }
> +
> src = &opp->src[n_IRQ];
> DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n",
> n_IRQ, level, src->ivpr);
> @@ -923,6 +932,12 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
> case 0xB0: /* EOI */
> DPRINTF("EOI\n");
> s_IRQ = IRQ_get_next(opp, &dst->servicing);
> +
> + if (s_IRQ < 0) {
> + DPRINTF("%s: EOI with no interrupt in service\n", __func__);
> + break;
> + }
> +
> IRQ_resetbit(&dst->servicing, s_IRQ);
> /* Set up next servicing IRQ */
> s_IRQ = IRQ_get_next(opp, &dst->servicing);
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 13/15] openpic: add some bounds checking for IRQ numbers
2013-01-03 18:55 ` Alexander Graf
@ 2013-01-03 19:54 ` Scott Wood
2013-01-03 21:07 ` Alexander Graf
0 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2013-01-03 19:54 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, qemu-devel
On 01/03/2013 12:55:26 PM, Alexander Graf wrote:
>
> On 22.12.2012, at 03:15, Scott Wood wrote:
>
> > The two checks with abort() guard against potential QEMU-internal
> > problems, but the EOI check stops the guest from causing updates to
> queue
> > position -1 and other havoc if it writes EOI with no interrupt in
> > service.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>
> Did you ever actually experience this?
Which one? EOI with no interrupt in service can be triggered by bad
guest behavior, and I did see it happen when the guest was confused by
another bug in QEMU's openpic (which is fixed elsewhere), resulting in
an IRQ number of -1 being thrown around. The other checks were to try
to be more robust against bad IRQ numbers in general.
> MAX_IRQ should match the memory region size, so we shouldn't be able
> to receive any interrupt above it.
Right, that's why I didn't add checking to the MMIO code. In IRQ_check
it could happen due to bad bitmap contents (e.g. after a checkpoint
restore), and in openpic_set_irq() it could happen if some device
raises an IRQ that is out of bounds.
> I might be inclined to accept an assert() there for internal sanity
> checking though. The last hunk looks fine.
Assert instead of abort is fine (there seem to be plenty of uses of
both in QEMU), though for the openpic_set_irq() case it would be nice
to be able to print the bad IRQ number before dying.
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 13/15] openpic: add some bounds checking for IRQ numbers
2013-01-03 19:54 ` [Qemu-devel] [Qemu-ppc] " Scott Wood
@ 2013-01-03 21:07 ` Alexander Graf
2013-01-03 21:20 ` Scott Wood
0 siblings, 1 reply; 47+ messages in thread
From: Alexander Graf @ 2013-01-03 21:07 UTC (permalink / raw)
To: Scott Wood; +Cc: qemu-ppc, qemu-devel
On 03.01.2013, at 20:54, Scott Wood wrote:
> On 01/03/2013 12:55:26 PM, Alexander Graf wrote:
>> On 22.12.2012, at 03:15, Scott Wood wrote:
>> > The two checks with abort() guard against potential QEMU-internal
>> > problems, but the EOI check stops the guest from causing updates to queue
>> > position -1 and other havoc if it writes EOI with no interrupt in
>> > service.
>> >
>> > Signed-off-by: Scott Wood <scottwood@freescale.com>
>> Did you ever actually experience this?
>
> Which one? EOI with no interrupt in service can be triggered by bad guest behavior, and I did see it happen when the guest was confused by another bug in QEMU's openpic (which is fixed elsewhere), resulting in an IRQ number of -1 being thrown around.
That's the last hunk, which as I said is fine :).
> The other checks were to try to be more robust against bad IRQ numbers in general.
>
>> MAX_IRQ should match the memory region size, so we shouldn't be able to receive any interrupt above it.
>
> Right, that's why I didn't add checking to the MMIO code. In IRQ_check it could happen due to bad bitmap contents (e.g. after a checkpoint restore), and in openpic_set_irq() it could happen if some device raises an IRQ that is out of bounds.
How would a device raise an IRQ that is out of bounds? Devices can only raise IRQs that are passed down from the init function and that only creates MAX_INT irq lines.
>
>> I might be inclined to accept an assert() there for internal sanity checking though. The last hunk looks fine.
>
> Assert instead of abort is fine (there seem to be plenty of uses of both in QEMU), though for the openpic_set_irq() case it would be nice to be able to print the bad IRQ number before dying.
Well, that's why I was asking where you've seen this happen. It really shouldn't. Ever. :)
But the checks as is don't hurt either. If it makes you happy, I can apply the patch.
Alex
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 13/15] openpic: add some bounds checking for IRQ numbers
2013-01-03 21:07 ` Alexander Graf
@ 2013-01-03 21:20 ` Scott Wood
0 siblings, 0 replies; 47+ messages in thread
From: Scott Wood @ 2013-01-03 21:20 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, qemu-devel
On 01/03/2013 03:07:49 PM, Alexander Graf wrote:
>
> On 03.01.2013, at 20:54, Scott Wood wrote:
>
> > On 01/03/2013 12:55:26 PM, Alexander Graf wrote:
> >> On 22.12.2012, at 03:15, Scott Wood wrote:
> >> > The two checks with abort() guard against potential QEMU-internal
> >> > problems, but the EOI check stops the guest from causing updates
> to queue
> >> > position -1 and other havoc if it writes EOI with no interrupt in
> >> > service.
> >> >
> >> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> >> Did you ever actually experience this?
> >
> > Which one? EOI with no interrupt in service can be triggered by
> bad guest behavior, and I did see it happen when the guest was
> confused by another bug in QEMU's openpic (which is fixed elsewhere),
> resulting in an IRQ number of -1 being thrown around.
>
> That's the last hunk, which as I said is fine :).
I would have found the issue in that hunk faster if I had array bounds
checking elsewhere, which was what led me to add it in certain places.
I'm not sure why I didn't add it in the place that would have helped
find the EOI bug, though (IRQ_resetbit). :-P
> > The other checks were to try to be more robust against bad IRQ
> numbers in general.
> >
> >> MAX_IRQ should match the memory region size, so we shouldn't be
> able to receive any interrupt above it.
> >
> > Right, that's why I didn't add checking to the MMIO code. In
> IRQ_check it could happen due to bad bitmap contents (e.g. after a
> checkpoint restore), and in openpic_set_irq() it could happen if some
> device raises an IRQ that is out of bounds.
>
> How would a device raise an IRQ that is out of bounds? Devices can
> only raise IRQs that are passed down from the init function and that
> only creates MAX_INT irq lines.
OK, so it looks like there would need to be a bug in the qdev gpio
mechanism rather than the devices -- but the interface boundary of
openpic.c does take an int rather than a pointer.
> >> I might be inclined to accept an assert() there for internal
> sanity checking though. The last hunk looks fine.
> >
> > Assert instead of abort is fine (there seem to be plenty of uses of
> both in QEMU), though for the openpic_set_irq() case it would be nice
> to be able to print the bad IRQ number before dying.
>
> Well, that's why I was asking where you've seen this happen. It
> really shouldn't. Ever. :)
That's why it's assert/abort and not some less severe form of error
handling. :-)
-Scott
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 14/15] openpic: move IACK to its own function
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (12 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 13/15] openpic: add some bounds checking for IRQ numbers Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 18:59 ` Alexander Graf
2012-12-22 2:15 ` [Qemu-devel] [PATCH 15/15] openpic: fix CTPR and de-assertion of interrupts Scott Wood
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Besides making the code cleaner, we will need a separate way to access
IACK in order to implement EPR (external proxy) interrupt delivery.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 95 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index a3fcefd..ee944ea 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -963,14 +963,64 @@ static void openpic_cpu_write(void *opaque, hwaddr addr, uint64_t val,
openpic_cpu_write_internal(opaque, addr, val, (addr & 0x1f000) >> 12);
}
+
+static uint32_t openpic_iack(OpenPICState *opp, IRQ_dst_t *dst, int cpu)
+{
+ IRQ_src_t *src;
+ int retval, irq;
+
+ DPRINTF("Lower OpenPIC INT output\n");
+ qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_INT]);
+
+ irq = IRQ_get_next(opp, &dst->raised);
+ DPRINTF("IACK: irq=%d\n", irq);
+
+ if (irq == -1) {
+ /* No more interrupt pending */
+ return opp->spve;
+ }
+
+ src = &opp->src[irq];
+ if (!(src->ivpr & IVPR_ACTIVITY_MASK) ||
+ !(IVPR_PRIORITY(src->ivpr) > dst->ctpr)) {
+ /* - Spurious level-sensitive IRQ
+ * - Priorities has been changed
+ * and the pending IRQ isn't allowed anymore
+ */
+ src->ivpr &= ~IVPR_ACTIVITY_MASK;
+ retval = opp->spve;
+ } else {
+ /* IRQ enter servicing state */
+ IRQ_setbit(&dst->servicing, irq);
+ retval = IVPR_VECTOR(opp, src->ivpr);
+ }
+ IRQ_resetbit(&dst->raised, irq);
+ if (!src->level) {
+ /* edge-sensitive IRQ */
+ src->ivpr &= ~IVPR_ACTIVITY_MASK;
+ src->pending = 0;
+ }
+
+ if ((irq >= opp->irq_ipi0) && (irq < (opp->irq_ipi0 + MAX_IPI))) {
+ src->idr &= ~(1 << cpu);
+ if (src->idr && !src->level) {
+ /* trigger on CPUs that didn't know about it yet */
+ openpic_set_irq(opp, irq, 1);
+ openpic_set_irq(opp, irq, 0);
+ /* if all CPUs knew about it, set active bit again */
+ src->ivpr |= IVPR_ACTIVITY_MASK;
+ }
+ }
+
+ return retval;
+}
+
static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
int idx)
{
OpenPICState *opp = opaque;
- IRQ_src_t *src;
IRQ_dst_t *dst;
uint32_t retval;
- int n_IRQ;
DPRINTF("%s: cpu %d addr %#" HWADDR_PRIx "\n", __func__, idx, addr);
retval = 0xFFFFFFFF;
@@ -991,46 +1041,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
retval = idx;
break;
case 0xA0: /* IACK */
- DPRINTF("Lower OpenPIC INT output\n");
- qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_INT]);
- n_IRQ = IRQ_get_next(opp, &dst->raised);
- DPRINTF("IACK: irq=%d\n", n_IRQ);
- if (n_IRQ == -1) {
- /* No more interrupt pending */
- retval = opp->spve;
- } else {
- src = &opp->src[n_IRQ];
- if (!(src->ivpr & IVPR_ACTIVITY_MASK) ||
- !(IVPR_PRIORITY(src->ivpr) > dst->ctpr)) {
- /* - Spurious level-sensitive IRQ
- * - Priorities has been changed
- * and the pending IRQ isn't allowed anymore
- */
- src->ivpr &= ~IVPR_ACTIVITY_MASK;
- retval = opp->spve;
- } else {
- /* IRQ enter servicing state */
- IRQ_setbit(&dst->servicing, n_IRQ);
- retval = IVPR_VECTOR(opp, src->ivpr);
- }
- IRQ_resetbit(&dst->raised, n_IRQ);
- if (!src->level) {
- /* edge-sensitive IRQ */
- src->ivpr &= ~IVPR_ACTIVITY_MASK;
- src->pending = 0;
- }
-
- if ((n_IRQ >= opp->irq_ipi0) && (n_IRQ < (opp->irq_ipi0 + MAX_IPI))) {
- src->idr &= ~(1 << idx);
- if (src->idr && !src->level) {
- /* trigger on CPUs that didn't know about it yet */
- openpic_set_irq(opp, n_IRQ, 1);
- openpic_set_irq(opp, n_IRQ, 0);
- /* if all CPUs knew about it, set active bit again */
- src->ivpr |= IVPR_ACTIVITY_MASK;
- }
- }
- }
+ retval = openpic_iack(opp, dst, idx);
break;
case 0xB0: /* EOI */
retval = 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 15/15] openpic: fix CTPR and de-assertion of interrupts
2012-12-22 2:15 [Qemu-devel] [PATCH 00/15] openpic: cleanups and fixes Scott Wood
` (13 preceding siblings ...)
2012-12-22 2:15 ` [Qemu-devel] [PATCH 14/15] openpic: move IACK to its own function Scott Wood
@ 2012-12-22 2:15 ` Scott Wood
2013-01-03 19:00 ` Alexander Graf
14 siblings, 1 reply; 47+ messages in thread
From: Scott Wood @ 2012-12-22 2:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: Scott Wood, qemu-ppc, qemu-devel
Properly implement level-triggered interrupts by withdrawing an
interrupt from the raised queue if the interrupt source de-asserts.
Also withdraw from the raised queue if the interrupt becomes masked.
When CTPR is written, check whether we need to raise or lower the
interrupt output.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
hw/openpic.c | 184 +++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 125 insertions(+), 59 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index ee944ea..8cf9467 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -206,6 +206,9 @@ typedef struct IRQ_dst_t {
IRQ_queue_t raised;
IRQ_queue_t servicing;
qemu_irq *irqs;
+
+ /* Count of IRQ sources asserting on non-INT outputs */
+ uint32_t outputs_active[OPENPIC_OUTPUT_NB];
} IRQ_dst_t;
typedef struct OpenPICState {
@@ -315,7 +318,8 @@ static int IRQ_get_next(OpenPICState *opp, IRQ_queue_t *q)
return q->next;
}
-static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
+static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ,
+ bool active, bool was_active)
{
IRQ_dst_t *dst;
IRQ_src_t *src;
@@ -324,79 +328,113 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
dst = &opp->dst[n_CPU];
src = &opp->src[n_IRQ];
+ DPRINTF("%s: IRQ %d active %d was %d\n",
+ __func__, n_IRQ, active, was_active);
+
if (src->output != OPENPIC_OUTPUT_INT) {
+ DPRINTF("%s: output %d irq %d active %d was %d count %d\n",
+ __func__, src->output, n_IRQ, active, was_active,
+ dst->outputs_active[src->output]);
+
/* On Freescale MPIC, critical interrupts ignore priority,
* IACK, EOI, etc. Before MPIC v4.1 they also ignore
* masking.
*/
- src->ivpr |= IVPR_ACTIVITY_MASK;
- DPRINTF("%s: Raise OpenPIC output %d cpu %d irq %d\n",
- __func__, src->output, n_CPU, n_IRQ);
- qemu_irq_raise(opp->dst[n_CPU].irqs[src->output]);
+ if (active) {
+ if (!was_active && dst->outputs_active[src->output]++ == 0) {
+ DPRINTF("%s: Raise OpenPIC output %d cpu %d irq %d\n",
+ __func__, src->output, n_CPU, n_IRQ);
+ qemu_irq_raise(dst->irqs[src->output]);
+ }
+ } else {
+ if (was_active && --dst->outputs_active[src->output] == 0) {
+ DPRINTF("%s: Lower OpenPIC output %d cpu %d irq %d\n",
+ __func__, src->output, n_CPU, n_IRQ);
+ qemu_irq_lower(dst->irqs[src->output]);
+ }
+ }
+
return;
}
priority = IVPR_PRIORITY(src->ivpr);
- if (priority <= dst->ctpr) {
- /* Too low priority */
- DPRINTF("%s: IRQ %d has too low priority on CPU %d\n",
- __func__, n_IRQ, n_CPU);
- return;
- }
- if (IRQ_testbit(&dst->raised, n_IRQ)) {
- /* Interrupt miss */
- DPRINTF("%s: IRQ %d was missed on CPU %d\n",
- __func__, n_IRQ, n_CPU);
- return;
- }
- src->ivpr |= IVPR_ACTIVITY_MASK;
- IRQ_setbit(&dst->raised, n_IRQ);
- if (priority < dst->raised.priority) {
- /* An higher priority IRQ is already raised */
- DPRINTF("%s: IRQ %d is hidden by raised IRQ %d on CPU %d\n",
- __func__, n_IRQ, dst->raised.next, n_CPU);
- return;
+
+ /* Even if the interrupt doesn't have enough priority,
+ * it is still raised, in case ctpr is lowered later.
+ */
+ if (active) {
+ IRQ_setbit(&dst->raised, n_IRQ);
+ } else {
+ IRQ_resetbit(&dst->raised, n_IRQ);
}
+
IRQ_check(opp, &dst->raised);
- if (IRQ_get_next(opp, &dst->servicing) != -1 &&
- priority <= dst->servicing.priority) {
- DPRINTF("%s: IRQ %d is hidden by servicing IRQ %d on CPU %d\n",
- __func__, n_IRQ, dst->servicing.next, n_CPU);
- /* Already servicing a higher priority IRQ */
- return;
+
+ if (active && priority <= dst->ctpr) {
+ DPRINTF("%s: IRQ %d priority %d too low for ctpr %d on CPU %d\n",
+ __func__, n_IRQ, priority, dst->ctpr, n_CPU);
+ active = 0;
+ }
+
+ if (active) {
+ if (IRQ_get_next(opp, &dst->servicing) >= 0 &&
+ priority <= dst->servicing.priority) {
+ DPRINTF("%s: IRQ %d is hidden by servicing IRQ %d on CPU %d\n",
+ __func__, n_IRQ, dst->servicing.next, n_CPU);
+ } else {
+ DPRINTF("%s: Raise OpenPIC INT output cpu %d irq %d/%d\n",
+ __func__, n_CPU, n_IRQ, dst->raised.next);
+ qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
+ }
+ } else {
+ IRQ_get_next(opp, &dst->servicing);
+ if (dst->raised.priority > dst->ctpr &&
+ dst->raised.priority > dst->servicing.priority) {
+ DPRINTF("%s: IRQ %d inactive, IRQ %d prio %d remains above %d/%d, CPU %d\n",
+ __func__, n_IRQ, dst->raised.next, dst->raised.priority,
+ dst->ctpr, dst->servicing.priority, n_CPU);
+ /* IRQ line stays asserted */
+ } else {
+ DPRINTF("%s: IRQ %d inactive, current prio %d/%d, CPU %d\n",
+ __func__, n_IRQ, dst->ctpr, dst->servicing.priority, n_CPU);
+ qemu_irq_lower(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
+ }
}
- DPRINTF("Raise OpenPIC INT output cpu %d irq %d\n", n_CPU, n_IRQ);
- qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
}
/* update pic state because registers for n_IRQ have changed value */
static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
{
IRQ_src_t *src;
+ bool active, was_active;
int i;
src = &opp->src[n_IRQ];
+ active = src->pending;
- if (!src->pending) {
- /* no irq pending */
- DPRINTF("%s: IRQ %d is not pending\n", __func__, n_IRQ);
- return;
- }
if ((src->ivpr & IVPR_MASK_MASK) && !src->nomask) {
/* Interrupt source is disabled */
DPRINTF("%s: IRQ %d is disabled\n", __func__, n_IRQ);
- return;
+ active = false;
}
- if (IVPR_PRIORITY(src->ivpr) == 0) {
- /* Priority set to zero */
- DPRINTF("%s: IRQ %d has 0 priority\n", __func__, n_IRQ);
+
+ was_active = !!(src->ivpr & IVPR_ACTIVITY_MASK);
+
+ /*
+ * We don't have a similar check for already-active because
+ * ctpr may have changed and we need to withdraw the interrupt.
+ */
+ if (!active && !was_active) {
+ DPRINTF("%s: IRQ %d is already inactive\n", __func__, n_IRQ);
return;
}
- if (src->ivpr & IVPR_ACTIVITY_MASK) {
- /* IRQ already active */
- DPRINTF("%s: IRQ %d is already active\n", __func__, n_IRQ);
- return;
+
+ if (active) {
+ src->ivpr |= IVPR_ACTIVITY_MASK;
+ } else {
+ src->ivpr &= ~IVPR_ACTIVITY_MASK;
}
+
if (src->idr == 0) {
/* No target */
DPRINTF("%s: IRQ %d has no target\n", __func__, n_IRQ);
@@ -405,12 +443,12 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
if (src->idr == (1 << src->last_cpu)) {
/* Only one CPU is allowed to receive this IRQ */
- IRQ_local_pipe(opp, src->last_cpu, n_IRQ);
+ IRQ_local_pipe(opp, src->last_cpu, n_IRQ, active, was_active);
} else if (!(src->ivpr & IVPR_MODE_MASK)) {
/* Directed delivery mode */
for (i = 0; i < opp->nb_cpus; i++) {
if (src->destmask & (1 << i)) {
- IRQ_local_pipe(opp, i, n_IRQ);
+ IRQ_local_pipe(opp, i, n_IRQ, active, was_active);
}
}
} else {
@@ -419,7 +457,7 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
if (i == opp->nb_cpus)
i = 0;
if (src->destmask & (1 << i)) {
- IRQ_local_pipe(opp, i, n_IRQ);
+ IRQ_local_pipe(opp, i, n_IRQ, active, was_active);
src->last_cpu = i;
break;
}
@@ -443,15 +481,25 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
if (src->level) {
/* level-sensitive irq */
src->pending = level;
- if (!level) {
- src->ivpr &= ~IVPR_ACTIVITY_MASK;
- }
+ openpic_update_irq(opp, n_IRQ);
} else {
/* edge-sensitive irq */
- if (level)
+ if (level) {
src->pending = 1;
+ openpic_update_irq(opp, n_IRQ);
+ }
+
+ if (src->output != OPENPIC_OUTPUT_INT) {
+ /* Edge-triggered interrupts shouldn't be used
+ * with non-INT delivery, but just in case,
+ * try to make it do something sane rather than
+ * cause an interrupt storm. This is close to
+ * what you'd probably see happen in real hardware.
+ */
+ src->pending = 0;
+ openpic_update_irq(opp, n_IRQ);
+ }
}
- openpic_update_irq(opp, n_IRQ);
}
static void openpic_reset(DeviceState *d)
@@ -922,6 +970,21 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
break;
case 0x80: /* CTPR */
dst->ctpr = val & 0x0000000F;
+
+ DPRINTF("%s: set CPU %d ctpr to %d, raised %d servicing %d\n",
+ __func__, idx, dst->ctpr, dst->raised.priority,
+ dst->servicing.priority);
+
+ if (dst->raised.priority <= dst->ctpr) {
+ DPRINTF("%s: Lower OpenPIC INT output cpu %d due to ctpr\n",
+ __func__, idx);
+ qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_INT]);
+ } else if (dst->raised.priority > dst->servicing.priority) {
+ DPRINTF("%s: Raise OpenPIC INT output cpu %d irq %d\n",
+ __func__, idx, dst->raised.next);
+ qemu_irq_raise(dst->irqs[OPENPIC_OUTPUT_INT]);
+ }
+
break;
case 0x90: /* WHOAMI */
/* Read-only register */
@@ -983,22 +1046,21 @@ static uint32_t openpic_iack(OpenPICState *opp, IRQ_dst_t *dst, int cpu)
src = &opp->src[irq];
if (!(src->ivpr & IVPR_ACTIVITY_MASK) ||
!(IVPR_PRIORITY(src->ivpr) > dst->ctpr)) {
- /* - Spurious level-sensitive IRQ
- * - Priorities has been changed
- * and the pending IRQ isn't allowed anymore
- */
- src->ivpr &= ~IVPR_ACTIVITY_MASK;
+ fprintf(stderr, "%s: bad raised IRQ %d ctpr %d ivpr 0x%08x\n",
+ __func__, irq, dst->ctpr, src->ivpr);
+ openpic_update_irq(opp, irq);
retval = opp->spve;
} else {
/* IRQ enter servicing state */
IRQ_setbit(&dst->servicing, irq);
retval = IVPR_VECTOR(opp, src->ivpr);
}
- IRQ_resetbit(&dst->raised, irq);
+
if (!src->level) {
/* edge-sensitive IRQ */
src->ivpr &= ~IVPR_ACTIVITY_MASK;
src->pending = 0;
+ IRQ_resetbit(&dst->raised, irq);
}
if ((irq >= opp->irq_ipi0) && (irq < (opp->irq_ipi0 + MAX_IPI))) {
@@ -1195,6 +1257,8 @@ static void openpic_save(QEMUFile* f, void *opaque)
qemu_put_sbe32s(f, &opp->dst[i].ctpr);
openpic_save_IRQ_queue(f, &opp->dst[i].raised);
openpic_save_IRQ_queue(f, &opp->dst[i].servicing);
+ qemu_put_buffer(f, (uint8_t *)&opp->dst[i].outputs_active,
+ sizeof(opp->dst[i].outputs_active));
}
for (i = 0; i < MAX_TMR; i++) {
@@ -1250,6 +1314,8 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
qemu_get_sbe32s(f, &opp->dst[i].ctpr);
openpic_load_IRQ_queue(f, &opp->dst[i].raised);
openpic_load_IRQ_queue(f, &opp->dst[i].servicing);
+ qemu_get_buffer(f, (uint8_t *)&opp->dst[i].outputs_active,
+ sizeof(opp->dst[i].outputs_active));
}
for (i = 0; i < MAX_TMR; i++) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 47+ messages in thread