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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

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@xxxxxxxxxxxxx>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
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