| Message ID | 1356142552-13453-7-git-send-email-scottwood@freescale.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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 >
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
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
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
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 {
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(-)