Re: [PATCH v1 09/27] xen/riscv: implement make_intc_domU_node()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 4/1/26 4:38 PM, Jan Beulich wrote:
> On 10.03.2026 18:08, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/dom0less-build.c
> > +++ b/xen/arch/riscv/dom0less-build.c
> > @@ -3,6 +3,15 @@
> >   #include <xen/fdt-kernel.h>
> >   #include <xen/init.h>
> >   
> > +#include <asm/intc.h>
> > +
> > +int __init make_intc_domU_node(struct kernel_info *kinfo)
> > +{
> > +    intc_make_domu_dt_node(kinfo);
> > +
> > +    return 0;
> > +}
> Is this wrapper really needed? Can't what's intc_make_domu_dt_node() right
> now become make_intc_domU_node()?
With current implementation no as intc_hw_ops used inside 
intc_make_domu_dt_node() is declared as static.
But I can introduce:

enum intc_version intc_hw_version(void)
{
   return intc_hw_ops->info->hw_version;
}

and the in make_intc_domU_node() just use switch/case to call interrupt 
controller specific functions.
Would it be better? It will also help to ...

> > @@ -41,6 +41,10 @@ struct intc_hw_operations {
> >   
> >       /* handle external interrupt */
> >       void (*handle_interrupt)(struct cpu_user_regs *regs);
> > +
> > +    /* Create interrupt controller node for domain */
> > +    int (*make_dom_dt_node)(const struct kernel_info *kinfo,
> > +                            const struct dt_device_node *intc);
> An __init-only hook is somewhat risky, just to mention it. In IOMMU code
> besides struct iommu_ops we have struct iommu_init_ops, just to give an
> example of where the same could have been the case.
.. not introduce hooks in this structure which won't exist after init.

> > --- a/xen/arch/riscv/intc.c
> > +++ b/xen/arch/riscv/intc.c
> > @@ -67,3 +67,11 @@ void intc_route_irq_to_xen(struct irq_desc *desc, unsigned 
> > int priority)
> >       intc_set_irq_type(desc, desc->arch.type);
> >       intc_set_irq_priority(desc, priority);
> >   }
> > +
> > +int __init intc_make_domu_dt_node(const struct kernel_info *kinfo)
> > +{
> > +    if ( intc_hw_ops && intc_hw_ops->make_dom_dt_node )
> > +        return intc_hw_ops->make_dom_dt_node(kinfo, intc_hw_ops->info->node);
> > +
> > +    return -ENOSYS;
> How do you justify this choice of return value? This isn't even a hypercall
> handler.
That make_dom_dt_node() isn't provided by interrupt controller, so isn't 
supported or as it mentioned in the comment "not implemented in 
public/errno.h:
  XEN_ERRNO(ENOSYS,     38)     /* Function not implemented */

~ Oleksii