Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[yangxiaojuan]

On 2022/5/7 下午11:31, Richard Henderson wrote:

> > +    if (level) {
> >         
> >         +        /* if not enable return false */
> >         
> >         +        if (((s->enable[enable_index]) & (1 <<
> >         enable_mask)) == 0) {
> >         
> >         +            return;
> >         
> >         +        }
> >         
> >         +        s->coreisr[cpu][coreisr_index] |= (1 <<
> >         coreisr_mask);
> >         
> >         +        qemu_set_irq(s->parent_irq[cpu][ipnum], level);
> >         
> >         +    } else {
> >         
> >         +        s->coreisr[cpu][coreisr_index] &= ~(1 <<
> >         coreisr_mask);
> >         
> >         +        qemu_set_irq(s->parent_irq[cpu][ipnum], level);
> >         
> >         +    }
> >         
> >       
>       
>       This final bit, updating the cpu irq is also wrong, in that it
>       should be unconditional. This is the only way that it will work
>       for the usage in updating the enable mask.
>       
>       
>       I think you are not considering when the MAP registers overlap
>       outputs.  For instance, if all 256 bits of EXT_IOIMap contain 0,
>       then all of EXT_IOI[n*32+31 : n*32] overlap.  When that happens,
>       you cannot lower the level of the cpu pin until all of the
>       matching ioi interrupts are low.
>       
>       
>     
    Thanks, i should consider the MAP
      registers overlap outputs.
    And i want to add 'uint32_t
      sw_isr_group[256 / 32]', when each bit of sw_isr_group[n*32+31 :
      n*32] is 0, then lower the level of the cpu pin.
    
    Thanks.
    Xiaojuan