Re: [PATCH v1 33/43] hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)

[Richard Henderson]

On 4/15/22 02:40, Xiaojuan Yang wrote:
> +static void pch_pic_update_irq(LoongArchPCHPIC *s, uint32_t mask,
> +                               int level, int hi)
> +{
> +    uint32_t val, irq;
> +
> +    if (level == 1) {
> +        if (hi) {
> +            val = mask & s->intirr_hi & (~s->int_mask_hi);
> +            irq = find_first_bit((unsigned long *)&val, 32);

This does not work -- you're accessing beyond the end of the uint32_t for all LP64 hosts. 
 I think you just want ctz32()...


> +            if (irq != 32) {
> +                s->intisr_hi |= 1ULL << irq;
> +                qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 1);
> +            }

... which should in fact only be tested if val != 0, which is to what this IF 
equates.

Is there a good reason that this function is treating hi and lo separately, as opposed to 
simply doing all of the computation on uint64_t?


r~