Re: [PATCH v1 07/17] xen/riscv: introduce vCPU AIA initialization

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 8/13/26 11:24 AM, Baptiste Le Duc wrote:
> > Introduce vcpu_aia_init() to initialize the AIA-related state needed
> > for a vCPU to have a working guest interrupt file.
> >
> > A guest (VS) interrupt file must be mapped to one of a pCPU's
> > hardware interrupt files (if they exist), so the pCPU a vCPU will actually
> > run on needs to be known first. arch_vcpu_create() is therefore not a
> > suitable place to call vcpu_aia_init(), since the pCPU assigned to a
> > vCPU can still change before it is first scheduled. To avoid
> > reassigning the VS interrupt file id and remapping it to a different
> > pCPU's hardware interrupt file, vcpu_aia_init() will instead be
> > called from a later point in the scheduling path (e.g.
> > continue_to_new_vcpu()), to be introduced in a follow-up patch. Since
> > it will end up being called from a non-__init context, it is not
> > itself marked __init.
>
> > Introduce imsic_update_state() to update a vCPU's guest IMSIC state
> > (the guest interrupt file id and the pCPU whose hardware interrupt
> > file it is mapped to) as a single consistent unit. This state can be
> > read concurrently, e.g. by a future helper that checks whether a
> > vCPU has a pending IMSIC interrupt, though no such consumer exists
> > yet at this stage - so it is protected by a lock.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> >
> > diff --git a/xen/arch/riscv/aia.c b/xen/arch/riscv/aia.c
> > index 4f7f46f58f..ed19600d46 100644
> > --- a/xen/arch/riscv/aia.c
> > +++ b/xen/arch/riscv/aia.c
> > @@ -14,6 +14,7 @@
> >   #include <asm/cpufeature.h>
> >   #include <asm/csr.h>
> >   #include <asm/current.h>
> > +#include <asm/imsic.h>
> >   
> >   struct vgein_ctrl {
> >       unsigned long bmp;
> > @@ -36,6 +37,35 @@ bool aia_usable(void)
> >       return _aia_usable;
> >   }
> >   
> > +void vcpu_aia_init(struct vcpu *v)
> > +{
> > +    unsigned int new_vsfile_id;
> > +    int rc;
> > +
> > +    if ( !aia_usable() )
> > +        return;
> > +
> > +    new_vsfile_id = vgein_assign(v);
> > +
> > +    /*
> > +     * vgein_assign() returns 0 when no free h/w guest interrupt file is
> > +     * available (including GEILEN == 0); imsic_map_guest_file() maps nothing
> > +     * in that case.
> > +     */
> > +    rc = imsic_map_guest_file(v, new_vsfile_id);
> > +    if ( rc )
> > +    {
> > +        /* Can't continue w/o correctly mapped IMSIC interrupt file */
> > +        domain_crash(v->domain);
> The vgein id assigned a few lines up is not released here. The domain is dying
> anyway, but the guest interrupt file stays marked in use on that pCPU forever,
> since nothing else ever calls vgein_release(). A vgein_release(v,
> new_vsfile_id) before the domain_crash() would fix it.
Yes, agree with that vgein_release() is missed. I've mentioned that 
before in reply to Jan B.
Thanks!

~ Oleksii