Re: [PATCH v9 16/20] xen/riscv: implement IRQ routing for device passthrough

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.09.2026 17:07, Oleksii Kurochko wrote:
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -205,7 +205,7 @@ void __init init_IRQ(void)
>  static inline struct irq_guest *irq_get_guest_info(struct irq_desc *desc)
>  {
>      ASSERT(spin_is_locked(&desc->lock));
> -    ASSERT(test_bit(_IRQ_GUEST, &desc->status));
> +    ASSERT(desc->status & IRQ_GUEST);
>      ASSERT(desc->action != NULL);
>  
>      return desc->action->dev_id;

Why did an Arm change slip into a RISC-V patch?

> --- a/xen/arch/riscv/aplic.c
> +++ b/xen/arch/riscv/aplic.c
> @@ -325,9 +325,40 @@ static const hw_irq_controller aplic_xen_irq_type = {
>      .set_affinity = aplic_set_irq_affinity,
>  };
>  
> +static unsigned int cf_check aplic_guest_irq_startup(struct irq_desc *desc)
> +{
> +    BUG_ON("unimplemented");
> +}
> +
> +/*
> + * Shared by ->shutdown(), ->enable(), ->disable() and ->end(), which have
> + * no state.
> + */
> +static void cf_check aplic_guest_irq_stub(struct irq_desc *desc)
> +{
> +    BUG_ON("unimplemented");
> +}
> +
> +static void cf_check aplic_guest_set_irq_affinity(struct irq_desc *desc,
> +                                                  const cpumask_t *mask)
> +{
> +    BUG_ON("unimplemented");
> +}
> +
> +static const hw_irq_controller aplic_guest = {
> +    .typename     = "aplic",

This is indistinguishable from aplic_xen_irq_type. Naming of both variables
also isn't consistent

> --- a/xen/arch/riscv/include/asm/intc.h
> +++ b/xen/arch/riscv/include/asm/intc.h
> @@ -15,6 +15,7 @@ enum intc_variant {
>  };
>  
>  struct cpu_user_regs;
> +struct domain;
>  struct irq_desc;
>  struct kernel_info;
>  struct vcpu;

Why is this suddenly necessary? There are ...

> @@ -76,10 +82,13 @@ void register_intc_ops(const struct intc_hw_init_ops 
> *init_ops);
>  void intc_init(void);
>  
>  void intc_route_irq_to_xen(struct irq_desc *desc, unsigned int priority);
> +int intc_route_irq_to_guest(struct irq_desc *desc, unsigned int priority);
>  
>  void intc_handle_external_irqs(struct cpu_user_regs *regs);
>  
>  int domain_vintc_init(struct domain *d);
>  void domain_vintc_deinit(struct domain *d);
>  
> +int vintc_reserve_virq(const struct domain *d, unsigned int virq);

... existing uses of struct domain already, immediately ahead of this added
line.

> @@ -78,6 +82,22 @@ void intc_route_irq_to_xen(struct irq_desc *desc, unsigned 
> int priority)
>      intc_set_irq_priority(desc, priority);
>  }
>  
> +int intc_route_irq_to_guest(struct irq_desc *desc,
> +                            unsigned int priority)
> +{
> +    ASSERT(spin_is_locked(&desc->lock));
> +
> +    ASSERT(intc_hw_ops->guest_irq_type);
> +
> +    desc->handler = intc_hw_ops->guest_irq_type;
> +    __set_bit(_IRQ_GUEST, &desc->status);

The revlog says you replaced all of these bitops.

> @@ -227,3 +251,240 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int 
> irq)
>      spin_unlock(&desc->lock);
>      irq_exit();
>  }
> +
> +static struct irq_guest *irq_get_guest_info(struct irq_desc *desc)
> +{
> +    ASSERT(spin_is_locked(&desc->lock));
> +    ASSERT(test_bit(_IRQ_GUEST, &desc->status));

Same here. And more elsewhere.

> +/* Route an IRQ to a specific guest */
> +int route_irq_to_guest(struct domain *d, unsigned int virq,
> +                       unsigned int irq, const char *devname)
> +{
> +    struct irq_guest *info;
> +    struct irq_desc *desc = irq_to_desc(irq);
> +    unsigned long flags;
> +    int retval = 0;
> +
> +    if ( d->is_dying )
> +        return -EINVAL;
> +
> +    info = xvzalloc(struct irq_guest);
> +    if ( !info )
> +        return -ENOMEM;
> +
> +    info->d = d;
> +    info->virq = virq;
> +
> +    info->action.dev_id = info;
> +    info->action.name = devname;
> +    /* The action is part of 'info', thus it is freed together with it. */
> +    info->action.free_on_release = false;

The revlog says this line doesn't exist anymore.

Jan