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

[Oleksii Kurochko]

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