Re: [PATCH v1 08/17] xen/riscv: add IMSIC state save/restore

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 8/13/26 11:30 AM, Baptiste Le Duc wrote:
> > IMSIC state is currently needed only to track which physical CPU owns a
> > vCPU's IMSIC interrupt file. This is required because the physical CPU
> > ID is part of the physical address used to map the IMSIC file.
> >
> > Add imsic_state_save() to record the current pCPU for a vCPU. When the
> > vCPU is migrated to a different pCPU, the mapping will need to be updated.
> >
> > When imsic_state_restore() is called, VGEIN is already assigned to the
> > vCPU and the guest interrupt file is already mapped, and, as only h/w
> > interrupt files are used for now, nothing specific needs to be done.
> > Action is only required when the vCPU is moved to a different pCPU, which
> > requires recalculating VGEIN and the mapping for the new guest interrupt
> > file. That will be handled separately by vcpu_move_irqs(), which is
> > introduced in a follow-up patch; until then this case is guarded by a
> > BUG_ON(), which is fine.
> >
> > Co-developed-by: Romain Caritey <Romain.Caritey@xxxxxxxxxxxxx>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> >
> > diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
> > index 2a792e756c..406bc68cbc 100644
> > --- a/xen/arch/riscv/imsic.c
> > +++ b/xen/arch/riscv/imsic.c
> > @@ -20,6 +20,7 @@
> >   #include <xen/init.h>
> >   #include <xen/libfdt/libfdt.h>
> >   #include <xen/macros.h>
> > +#include <xen/rwlock.h>
> >   #include <xen/sched.h>
> >   #include <xen/smp.h>
> >   #include <xen/spinlock.h>
> > @@ -418,6 +419,28 @@ int imsic_map_guest_file(struct vcpu *v, unsigned int 
> > vsfile_id)
> >       return res;
> >   }
> >   
> > +void imsic_state_save(struct vcpu *v)
> > +{
> > +    struct vimsic_state *imsic_state = v->arch.vimsic_state;
> > +    unsigned long flags;
> > +
> > +    /*
> > +     * SW interrupt file always has ->vsfile_pcpu = NR_CPUS so nothing specific
> > +     * should be done in this case.
> > +     */
> > +    if ( !vcpu_guest_file_id(v) )
> > +        return;
>
> > +
> > +    write_lock_irqsave(&imsic_state->vsfile_lock, flags);
> > +    imsic_state->vsfile_pcpu = cpuid_to_hartid(v->processor);
> How will you detect a migration is needed? Don't you need to first know
> if ->vsfile_pcpu is different to cpuid_to_hartid(v->processor)? (I
> didn't take a look to other patchs for the moment, so the
> explanations might be later.)
Migration (if you are speaking about migration of vCPU from one pCPU to 
another) is completely different path. Look at sched_move_irqs().
~ Oleksii