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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 1/29/26 6:03 PM, Jan Beulich wrote:
> On 29.01.2026 17:56, Oleksii Kurochko wrote:
> > 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()
> Ah, I see. But how should I have known without the description saying anything
> along these lines?
Of course, without proper description it was impossible to understand that.

> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
Thanks.

~ Oleksii