Re: [PATCH v6 14/23] xen/riscv: introduce minimal virtual APLIC (vAPLIC) infrastructure

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

On 7/22/26 9:40 AM, Jan Beulich wrote:
> On 20.07.2026 17:59, Oleksii Kurochko wrote:
>> At the current development stage, only domain vINTC init and deinit
>> operations are required, so implement those first.
>>
>> Initialize vAPLIC's domaincfg to with the interrupt-enable bit set and
>> MSI delivery mode selected as the current solution is exepcted to have
>> always IMSIC, and initialize vintc->ops.
>>
>> Other operations such as emulate_load(), emulate_store(), and is_access()
>> will be needed once guests are running and MMIO accesses to APLIC MMIO
>> range must be handled. These will be introduced separately later.
>>
>> Introduce a structure to describe a virtual interrupt controller (vINTC)
>> and a vintc_ops structure, which provides operations to emulate load and
>> store accesses to interrupt controller MMIOs and to check whether a given
>> address falls within the MMIO range of a specific virtual interrupt
>> controller.
>> Note that already existed init_ops field in struct vintc will be init-ed
>> for APLIC in the follow up patch.
>>
>> The vAPLIC implementation of these operations will be provided later
>> once guests can be run and these operations are actually needed.
>>
>> Introduce these structures here as they are required for the implementation
>> of domain_vaplic_init() and domain_vaplic_alloc(). Also, introduce
>> vaplic_init() and init vintc_ops->vcpu_init() with it.
>>
>> Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> albeit I (continue to) think that ...
> 
>> --- a/xen/arch/riscv/include/asm/intc.h
>> +++ b/xen/arch/riscv/include/asm/intc.h
>> @@ -17,6 +17,7 @@ enum intc_variant {
>>   struct cpu_user_regs;
>>   struct irq_desc;
>>   struct kernel_info;
>> +struct vcpu;
>>   
>>   struct intc_info {
>>       enum intc_variant hw_variant;
>> @@ -53,8 +54,19 @@ struct vintc_init_ops {
>>       int (*make_domu_dt_node)(struct kernel_info *kinfo);
>>   };
>>   
>> +struct vintc_ops {
>> +    /* Initialize some vINTC-related stuff for a vCPU */
>> +    int (*vcpu_init)(struct vcpu *v);
>> +
>> +    /* Deinitialize some vINTC-related stuff for a vCPU */
>> +    void (*vcpu_deinit)(struct vcpu *v);
>> +};
>> +
>>   struct vintc {
>> +    /* Used during domain build only; dropped afterwards. */
>>       const struct vintc_init_ops *init_ops;
> 
> ... "dropped" in the comment is misleading - there's nothing being dropped
> here, as that's technically impossible. Iirc you don't even poison the
> field once domain build is finished.

I will rephrase that in the following way:

/* Callbacks invoked during domain construction only. */

~ Oleksii