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

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

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