Re: [Qemu-devel] [PATCH 03/15] openpic: fix sense and priority bits

[Alexander Graf]

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 <address@hidden>
> ---
> 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
> 
>