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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

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