Re: [PATCH v2 33/39] xen/riscv: dump old interrupt file to memory

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/23/26 6:16 PM, Baptiste Le Duc wrote:
> On 2026-09-23 18:02 +0200, Oleksii Kurochko wrote:
> > On 9/23/26 5:15 PM, Baptiste Le Duc wrote:
> > > > At the old interrupt file, dump to memory all the eip and eie arrays).
> > > Typo `)`
> > Will drop `)`.
> >
> > > > After this step is done, the old interrupt file is no longer in use so
> > > > old intrrupt file VGEIN could be released.
> > > >
> > > > Restoring of old interrupt file state will be done in follow-up
> > > > patch.
> > >
> > > > There are cases where it is needed to specify on which cpu it is
> > > > necessary to VGEIN should be released so update vgein_release() to
> > > The sentence miss a verb, here is a proposal:
> > > ```
> > > There are cases where the cpu on which the VGEIN is released needs to
> > > be specified, so update vgein_release() to deal with that.
> > > ```
> > I think it could be dropped at all as vgein_release() stub is just
> > introduced here and not updated.
> >
> > > Moreover, could you explain me the cases you are talking about? It's not
> > > clear by reading the commit message in the first place.
> > For example, during migration of vCPU, vCPU->processor points to new CPU
> > where it will be run but we still have to free VGEIN on the prev.
> > ->processor.
> >
> > > > deal with that.
> > >
> > >
> > >
> > > >
> > >
> > > > Keep BUG_ON("unimplemented") placeholder in imsic_migrate_vcpu() to guard
> > >
> > > > against silent incorrect behaviour or unexpected panics in guest VMs until
> > > > the function is fully implemented.
> > >
> > > > vgein_release() is stub for now and will be introduced later.
> > >
> > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > > >
> > > > diff --git a/xen/arch/riscv/aia.c b/xen/arch/riscv/aia.c
> > > > index 75c82bcfa1..be3901ec0c 100644
> > > > --- a/xen/arch/riscv/aia.c
> > > > +++ b/xen/arch/riscv/aia.c
> > > > @@ -30,3 +30,8 @@ unsigned int vgein_assign(struct vcpu *v)
> > > >    
> > > >        return 0;
> > > >    }
> > > > +
> > > > +void vgein_release(struct vcpu *v, unsigned int vgein_id, unsigned int cpu)
> > > > +{
> > > > +    BUG_ON("unimplemented\n");
> > > > +}
> > > > diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
> > > > index 5e9f6995e4..3cba58e0c1 100644
> > > > --- a/xen/arch/riscv/imsic.c
> > > > +++ b/xen/arch/riscv/imsic.c
> > > > @@ -56,6 +56,24 @@ static unsigned int __ro_after_init guest_num_msis;
> > > >     */
> > > >    #define GUEST_IMSIC_MAX_MSIS 255U
> > > >    
> > > > +/*
> > > > + * The interrupt identities an IMSIC interrupt file provides are 0 (which is
> > > > + * never valid, but still occupies a bit) up to IMSIC_MAX_ID inclusive, so
> > > > + * IMSIC_MAX_ID + 1 bits have to be covered.
> > > > + */
> > > > +#define IMSIC_MAX_EIX DIV_ROUND_UP(IMSIC_MAX_ID + 1, BITS_PER_TYPE(uint64_t))
> > >
> > > > +
> > > > +struct imsic_mrif_eix {
> > > > +    unsigned long eip[BITS_PER_TYPE(uint64_t) / BITS_PER_LONG];
> > > > +    unsigned long eie[BITS_PER_TYPE(uint64_t) / BITS_PER_LONG];
> > >
> > > > +};
> > > > +
> > > > +struct imsic_mrif {
> > > > +    struct imsic_mrif_eix eix[IMSIC_MAX_EIX];
> > > > +    unsigned long eithreshold;
> > > > +    unsigned long eidelivery;
> > > > +};
> > > > +
> > > Maybe I didn't get something but I couldn't find anything in the commit
> > > message explaining why do we use mrif here.
> > It is just convenient way to temporary store h/w interrupt file. Also it
> > could be used not only for ...
> >
> > > Moreover, mrif, as described in aia spec (8.3 Memory-resident interrupt
> > > files), seems to be only usable with IOMMU that Xen doesn't support.
> > ... IOMMU but also to support more guest interrupts file implemented by
> > IMSIC (basically what I am calling as software interrupt file). Without
> > memory-resident interrupt files, the number of virtual RISC-V harts that
> > can directly receive MSIs from devices is limited by the total number of
> > guest interrupt files implemented by all IMSICs in the system, because
> > all MSIs to RISC-V harts must go through IMSICs. For a single RISC-V
> > hart, the number of guest interrupt files is the GEILEN parameter
> > defined by the Privileged Architecture, which can be at most 31 for RV32
> > and 63 for RV64.
> >
> > > If you want to have something in memory that could store some interrupt
> > > file info, we should take another name to not be confusing.
> > It seems like it is okay to use memory residential interrupt file (mrif)
> > here based on KVM's code who are using mrif for the same purpose I
> > described above.
> >
> > In short, MRIF is a joint virtualization technology shared between the
> > IOMMU and the hypervisor. The IOMMU uses the MRIF as a memory target to
> > land incoming hardware MSIs, while the hypervisor manages these MRIFs in
> > RAM as software data structures to support an effectively unlimited
> > number of vCPUs that don't currently hold a physical IMSIC guest file slot.
> Yes, I read the spec to understand but here, mrif doesn't catch the MSIs
> right?
Yes, because right now only h/w if(s) are supported. For now it is just 
a storage where we are saving temporary h/w interrupt file (which has 
the same structure as MRIF).
When s/w if(s) will be supported we will need to store somewhere an 
interrupt file in RAM and considering that the structure of h/w if == 
s/w if and basically == mrif thereby mrif is a good candidate. Even 
more, ...
> so it's not exactly the behaviour mentioned or you have in mind
> to add full support when IOMMU will be supported?
... IOMMU points to the the same structure so again h/w  if == s/w if == 
mrif from organization point of you.
And answering your question we have in mind to support IOMMU for IMSIC 
purposes and then we still will need mrif structure.
~ Oleksii