Re: [PATCH v6 17/23] xen/riscv: create APLIC DT node for guest domains

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

On 7/23/26 3:08 PM, Jan Beulich wrote:
> On 20.07.2026 17:59, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/aplic-priv.h
>> +++ b/xen/arch/riscv/aplic-priv.h
>> @@ -34,4 +34,18 @@ struct aplic_priv {
>>       const struct imsic_config *imsic_cfg;
>>   };
>>   
>> +/*
>> + * Value is inspired by what QEMU is using for riscv,num-sources property for
>> + * APLIC node.
>> + */
>> +#define GUEST_APLIC_MAX_SOURCES 96U
>> +
>> +/*
>> + * Specifies the number of wired interrupt sources supported by guest APLIC
>> + * domain.
>> + * Could be limited by host interrupt controller and is identical for every
>> + * domain.
>> + */
>> +extern unsigned int guest_aplic_num_sources;
> 
> It being only IMSIC which is supported right now together with APLIC, what
> exactly does the word "wired" mean to express in the comment? MSI, as their
> name says, aren't "wired interrupts" but messages on the system bus.

Technically, APLIC always works with interrupts delivered via dedicated 
physical connections (wired interrupts). The difference is that the 
APLIC can operate in two modes: direct mode and MSI mode. In our case, 
all interrupts are converted into MSIs, which are then sent over the 
system bus to a hart's IMSIC.

To avoid confusion, I think the term "wired" can simply be omitted. So 
I'll remove it.


> 
> You may also want to add "for now" to the latter sentence.

Agree I will add this.

> 
>> @@ -29,6 +37,74 @@ static void cf_check vaplic_deinit(struct vcpu *v)
>>       return vcpu_imsic_deinit(v);
>>   }
>>   
>> +static int __init cf_check vaplic_make_domu_dt_node(struct kernel_info *kinfo)
>> +{
>> +    struct domain *d = kinfo->bd.d;
>> +    int res;
>> +    void *fdt = kinfo->fdt;
>> +    unsigned int msi_parent_phandle;
>> +    char vaplic_name[32];
>> +    unsigned int aplic_size = APLIC_SIZE(d->max_vcpus);
>> +    const __be32 reg[] = {
>> +        cpu_to_be32(GUEST_APLIC_S_BASE >> 32),
>> +        cpu_to_be32(GUEST_APLIC_S_BASE),
>> +        cpu_to_be32(0),
>> +        cpu_to_be32(aplic_size),
>> +    };
>> +
>> +    BUILD_BUG_ON(APLIC_SIZE(MAX_VIRT_CPUS) > UINT_MAX);
>> +
>> +    res = snprintf(vaplic_name, sizeof(vaplic_name), "/soc/aplic@%lx",
> 
> Well, what should I say? I really expected you to have gone through all of your
> pending patches. replacing sizeof() by ARRAY_SIZE() in cases like this one.

No, you shouldn't but thanks for reminding me. I will replace sizeof() 
with ARRAY_SIZE().

> 
> With this adjusted and with the question above sorted one way or the other:
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks.

~ Oleksii