Re: [PATCH v4 20/25] xen/riscv: create APLIC DT node for guest domains

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

On 6/29/26 5:26 PM, Jan Beulich wrote:
> On 26.06.2026 17:46, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/include/asm/imsic.h
>> +++ b/xen/arch/riscv/include/asm/imsic.h
>> @@ -78,6 +78,7 @@ struct vimsic_state {
>>   };
>>   
>>   struct dt_device_node;
>> +struct kernel_info;
>>   struct vcpu;
>>   
>>   int imsic_init(const struct dt_device_node *node);
>> @@ -93,4 +94,6 @@ int vcpu_imsic_init(struct vcpu *v);
>>   void vcpu_imsic_deinit(struct vcpu *v);
>>   unsigned int vcpu_guest_file_id(const struct vcpu *v);
>>   
>> +int vimsic_make_domu_dt_node(struct kernel_info *kinfo, unsigned int *phandle);
> 
> Doesn't this belong in the earlier patch?

Yes, it should be in the earlier patch. I will move this decl. to prev 
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,6 +21,11 @@
>>   
>>   #include "aplic-priv.h"
>>   
>> +#define VAPLIC_COMPATIBLE "riscv,aplic"
>> +#define VAPLIC_NUM_SOURCES 96
> 
> So for the IMSIC the host value is relevant, but for the APLIC the count is
> an arbitrary(?) compile time constant? Why?
If you are referring to the number of IMSIC interrupt identities, the 
IMSIC node has separate properties for the host and guest:

```yaml
riscv,num-ids:
   $ref: /schemas/types.yaml#/definitions/uint32
   minimum: 63
   maximum: 2047
   description:
     Number of interrupt identities supported by the IMSIC interrupt file.

riscv,num-guest-ids:
   $ref: /schemas/types.yaml#/definitions/uint32
   minimum: 63
   maximum: 2047
   description:
     Number of interrupt identities supported by the IMSIC guest 
interrupt file. If not specified, it is assumed to be the same as the 
value of the `riscv,num-ids` property.
```

Therefore, if `riscv,num-guest-ids` is not explicitly specified, the 
predefined value `GUEST_IMSIC_NUM_IDS` is used.

No equivalent property exists for APLIC, which is why the value is 
explicitly hardcoded here. To some extent, the chosen number is 
arbitrary, but I used the same value that QEMU uses when creating the 
APLIC node.

It probably makes sense to limit this value in a similar way to IMSIC, 
at least for now, since the case where the number of VAPLIC sources 
exceeds the number of host APLIC sources is not supported (assuming that 
configuration is even valid). In other words, we could use:
   min(VAPLIC_NUM_SOURCES, host_num_sources)

Would that make sense?

> 
>> @@ -29,6 +36,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 = 0;
> 
> I'm pretty sure I did point out this (or a very similar) pointless initializer
> before.
> 

I will drop initializer.

Thanks.

~ Oleksii