Re: [PATCH v5 17/26] xen/riscv: introduce minimal virtual APLIC (vAPLIC) infrastructure

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 7/9/26 5:39 PM, Jan Beulich wrote:
> On 06.07.2026 17:57, 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.
> How would domaincfg be initialized on real hardware? 
Xen will initialize that in aplic_init_hw_interrupts():
writel(APLIC_DOMAINCFG_IE | APLIC_DOMAINCFG_DM, &aplic.regs->domaincfg);

I can see that maybe
> firmware would have to set DM suitably (and you may mean to take firmware's
> role here). 
I don't think that firmware will do that (and OpenSBI for example 
doesn't do that). If firmware can do that we for sure want to control in 
Xen what is written to ->domaincfg.
> But isn't setting at least IE entirely the OSes responsibility?
At least, Linux setups ->domaincfg once at the boot time:

        /* Setup APLIC domaincfg register */
        val = readl(priv->regs + APLIC_DOMAINCFG);
        val |= APLIC_DOMAINCFG_IE;
        if (msi_mode)
                val |= APLIC_DOMAINCFG_DM;
        writel(val, priv->regs + APLIC_DOMAINCFG);
        if (readl(priv->regs + APLIC_DOMAINCFG) != val)
                dev_warn(priv->dev, "unable to write 0x%x in domaincfg\n", val);

And don't touch this register anymore, even for interrupt disablement it 
isn't used.
So Xen can just does once:
writel(APLIC_DOMAINCFG_IE | APLIC_DOMAINCFG_DM, &aplic.regs->domaincfg);

and then just properly handle access of a guest to domaincfg.

> > --- a/xen/arch/riscv/include/asm/aplic.h
> > +++ b/xen/arch/riscv/include/asm/aplic.h
> > @@ -15,6 +15,8 @@
> >   
> >   #include <asm/imsic.h>
> >   
> > +/* domaincfg bits 31:24 are read-only 0x80 */
> > +#define APLIC_DOMAINCFG_RO      (0x80U << 24)
> Bit 7 is also documented as read-only 0. Wouldn't the comment better reflect
> that as well?
Not sure, bits 31:24 are read-only *0x80* but bit 7 is read-only *0*.

> >   #define APLIC_DOMAINCFG_IE      BIT(8, U)
> >   #define APLIC_DOMAINCFG_DM      BIT(2, U)
> Wouldn't you better spell out BE as well?
I can add:
  #define APLIC_DOMAINCFG_BE       BIT(0, U)

But it isn't used at the moment (Linux also defines it but never 
actually using it). Do you want still to add that now?
Thanks.

~ Oleksii