Re: [PATCH v6 2/7] xen/riscv: aplic_init() implementation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.07.2025 11:01, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/aplic-priv.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +/*
> + * xen/arch/riscv/aplic-priv.h
> + *
> + * Private part of aplic.h header.
> + *
> + * RISC-V Advanced Platform-Level Interrupt Controller support
> + *
> + * Copyright (c) Microchip.
> + * Copyright (c) Vates.
> + */
> +
> +#ifndef ASM_RISCV_PRIV_APLIC_H
> +#define ASM_RISCV_PRIV_APLIC_H
> +
> +#include <xen/types.h>
> +
> +#include <asm/aplic.h>
> +#include <asm/imsic.h>

Why is the latter one needed here? Certainly not for ...

> +struct aplic_priv {
> +    /* Base physical address and size */
> +    paddr_t paddr_start;
> +    size_t  size;
> +
> +    /* Registers */
> +    volatile struct aplic_regs __iomem *regs;
> +
> +    /* IMSIC configuration */
> +    const struct imsic_config *imsic_cfg;

... this field decl (which itself serves as a forward decl of the struct
tag).

> --- a/xen/arch/riscv/aplic.c
> +++ b/xen/arch/riscv/aplic.c
> @@ -9,19 +9,118 @@
>   * Copyright (c) 2024-2025 Vates
>   */
>  
> +#include <xen/device_tree.h>
>  #include <xen/errno.h>
>  #include <xen/init.h>
>  #include <xen/irq.h>
> +#include <xen/mm.h>
>  #include <xen/sections.h>
>  #include <xen/types.h>
> +#include <xen/vmap.h>
> +
> +#include "aplic-priv.h"
>  
>  #include <asm/device.h>
> +#include <asm/imsic.h>
>  #include <asm/intc.h>
> +#include <asm/riscv_encoding.h>
> +
> +#define APLIC_DEFAULT_PRIORITY  1
> +
> +static struct aplic_priv aplic;

Is this altered post-init? IOW can it be __ro_after_init? Or otherwise at
least __read_mostly?

With these two taken care of (one way or another):
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
If you clarify what is wanted / needed, I'm also happy to make adjustments
while committing.

Jan