Re: [PATCH v2 16/26] xen/riscv: create APLIC DT node for guest domains

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.05.2026 16:43, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/imsic.c
> +++ b/xen/arch/riscv/imsic.c
> @@ -14,6 +14,7 @@
>  #include <xen/cpumask.h>
>  #include <xen/device_tree.h>
>  #include <xen/errno.h>
> +#include <xen/fdt-kernel.h>
>  #include <xen/init.h>
>  #include <xen/macros.h>
>  #include <xen/sched.h>
> @@ -522,3 +523,9 @@ int __init imsic_init(const struct dt_device_node *node)
>  
>      return rc;
>  }
> +
> +int __init vimsic_make_domu_dt_node(struct kernel_info *kinfo,
> +                                    unsigned int *phandle)
> +{
> +    return -EOPNOTSUPP;
> +}

This, I assume, is going to be filled properly by the next patch.

> --- a/xen/arch/riscv/vaplic.c
> +++ b/xen/arch/riscv/vaplic.c
> @@ -9,6 +9,8 @@
>   */
>  
>  #include <xen/errno.h>
> +#include <xen/fdt-kernel.h>
> +#include <xen/libfdt/libfdt.h>
>  #include <xen/sched.h>
>  #include <xen/xvmalloc.h>
>  
> @@ -19,8 +21,11 @@
>  
>  #include "aplic-priv.h"
>  
> +#define VAPLIC_COMPATIBLE "riscv,aplic"
>  #define VAPLIC_NUM_SOURCES 96
>  
> +#define FDT_VAPLIC_INT_CELLS 2
> +
>  static int cf_check vcpu_vaplic_init(struct vcpu *v)
>  {
>      int rc = 0;
> @@ -47,6 +52,73 @@ static int cf_check vcpu_vaplic_init(struct vcpu *v)
>      return rc;
>  }
>  
> +static int __init cf_check vaplic_make_domu_dt_node(struct kernel_info 
> *kinfo)

Again - why __init here and ...

> +{
> +    int res = 0;
> +    void *fdt = kinfo->fdt;
> +    unsigned int msi_parent_phandle;
> +    char vaplic_name[128];
> +    paddr_t aplic_addr = GUEST_APLIC_S_BASE;
> +    paddr_t aplic_size = APLIC_SIZE(kinfo->bd.d->max_vcpus);
> +    const __be32 reg[] = {
> +        cpu_to_be32(aplic_addr >> 32),
> +        cpu_to_be32(aplic_addr),
> +        cpu_to_be32(aplic_size >> 32),
> +        cpu_to_be32(aplic_size),
> +    };
> +    struct vintc *vintc = kinfo->bd.d->arch.vintc;
> +
> +    res = snprintf(vaplic_name, sizeof(vaplic_name), "/soc/aplic@%x",
> +                   GUEST_APLIC_S_BASE);
> +    if ( res >= sizeof(vaplic_name) )
> +    {
> +        dprintk(XENLOG_DEBUG, "vaplic name is truncated\n");
> +        return -ENOBUFS;
> +    }
> +
> +    res = vimsic_make_domu_dt_node(kinfo, &msi_parent_phandle);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_begin_node(fdt, vaplic_name);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#interrupt-cells", FDT_VAPLIC_INT_CELLS);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "reg", reg, sizeof(reg));
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "riscv,num-sources", vintc->irq_nums);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "interrupt-controller", NULL, 0);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_string(fdt, "compatible", VAPLIC_COMPATIBLE);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "msi-parent", msi_parent_phandle);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "phandle", kinfo->phandle_intc);
> +    if ( res )
> +        return res;
> +
> +    return fdt_end_node(fdt);
> +}
> +
> +static const struct vintc_init_ops __initdata init_ops = {

... __initdata here? If you really want to have the option of moving
domain creation stuff to .init.* when only dom0less is configured,
then a proper abstraction is needed, along the lines of
init_or_livepatch.

> @@ -60,13 +132,14 @@ int __init domain_vaplic_init(struct domain *d)
>  
>      d->arch.vintc = &vaplic->vintc;
>      d->arch.vintc->ops = &vintc_ops;
> +    d->arch.vintc->init_ops = &init_ops;
>  
> -    vaplic->regs.domaincfg = APLIC_DOMAINCFG_IE | APLIC_DOMAINCFG_DM;
> +    vaplic->regs.domaincfg = APLIC_DOMAINCFG_IE | APLIC_DOMAINCFG_DM |
> +                             APLIC_DOMAINCFG_RO80;

This looks unrelated. I don't mind it being done here, but then it
wants mentioning in the description. Or maybe I simply don't understand
what this is about.

Jan