Re: [PATCH v2 09/26] xen/riscv: introduce init interrupt controller operations

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.05.2026 16:43, Oleksii Kurochko wrote:
> Introduce intc_hw_init_ops structure to avoid risky mix of init
> function and non-init function.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> Changes in v2:
>  - New patch.
> ---
>  xen/arch/riscv/aplic.c            |  7 +++++--
>  xen/arch/riscv/include/asm/intc.h | 10 +++++++---
>  xen/arch/riscv/intc.c             | 10 ++++++++--
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
> index 739e8dab3498..97dc0ef731f0 100644
> --- a/xen/arch/riscv/aplic.c
> +++ b/xen/arch/riscv/aplic.c
> @@ -306,12 +306,15 @@ static const hw_irq_controller aplic_xen_irq_type = {
>  
>  static const struct intc_hw_operations aplic_ops = {
>      .info                = &aplic_info,
> -    .init                = aplic_init,
>      .host_irq_type       = &aplic_xen_irq_type,
>      .handle_interrupt    = aplic_handle_interrupt,
>      .set_irq_type        = aplic_set_irq_type,
>  };
>  
> +static const struct intc_hw_init_ops __initdata aplic_init_ops = {
> +    .init                = aplic_init,
> +};

const wants to pair with __initconst. Then:
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

However, I have another comment for consideration:

> --- a/xen/arch/riscv/intc.c
> +++ b/xen/arch/riscv/intc.c
> @@ -12,9 +12,13 @@
>  
>  static const struct intc_hw_operations *__ro_after_init intc_hw_ops;
>  
> -void __init register_intc_ops(const struct intc_hw_operations *ops)
> +static const struct intc_hw_init_ops *__initdata intc_hw_init_ops;
> +
> +void __init register_intc_ops(const struct intc_hw_operations *ops,
> +                              const struct intc_hw_init_ops *init_ops)
>  {
>      intc_hw_ops = ops;
> +    intc_hw_init_ops = init_ops;
>  }

Again following what we do e.g. in x86'es IOMMU code, instead of passing
two pointers to the function, have struct intc_hw_init_ops have a
const struct intc_hw_operations * member which then can be used to
set intc_hw_ops here? Both will always come in pairs anyway.

Jan