Re: [PATCH v1] xen/riscv: route unhandled interrupts to do_unexpected_trap()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 1/29/26 4:43 PM, Jan Beulich wrote:
> On 29.01.2026 15:40, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/traps.c
> > +++ b/xen/arch/riscv/traps.c
> > @@ -196,6 +196,7 @@ void do_trap(struct cpu_user_regs *cpu_regs)
> >           {
> >               /* Handle interrupt */
> >               unsigned long icause = cause & ~CAUSE_IRQ_FLAG;
> > +            bool intr_handled = true;
> Of course I don't know what your further plans are here, so maybe doing
> it this way really is desirable. As the code is right now, I wonder if
> you couldn't make this a 2-line change, ...
>
> > @@ -204,10 +205,12 @@ void do_trap(struct cpu_user_regs *cpu_regs)
> >                   break;
> ... using return here and ...
>
> >               default:
> > +                intr_handled = false;
> >                   break;
> >               }
> >   
> > -            break;
> > +            if ( intr_handled )
> > +                break;
> ... simply dropping this break altogether.
Well, your change is better but it won't apply to my current code of do_trap():
    ....
    default:
        if ( cause & CAUSE_IRQ_FLAG )
        {
            /* Handle interrupt */
            unsigned long icause = cause & ~CAUSE_IRQ_FLAG;
            bool intr_handled = true;

            switch ( icause )
            {
            case IRQ_S_EXT:
                intc_handle_external_irqs(cpu_regs);
                break;
            ...
            default:
                intr_handled = false;
                break;
            }

            if ( intr_handled )
                break;
        }

        do_unexpected_trap(cpu_regs);
        break;
    }

    if ( cpu_regs->hstatus & HSTATUS_SPV )
        check_for_pcpu_work();
}

So if to use return instead of break here, I will miss the call of 
check_for_pcpu_work()
which is syncing interrupt and check if some softirq should be done:

static void check_for_pcpu_work(void)
{
    ASSERT(!local_irq_is_enabled());

    while ( softirq_pending(smp_processor_id()) )
    {
        local_irq_enable();
        do_softirq();
        local_irq_disable();
    }

    vcpu_flush_interrupts(current);

    vcpu_sync_interrupts(current);
}

~ Oleksii