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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 8/13/26 11:51 AM, Jan Beulich wrote:
> On 13.08.2026 11:34, Oleksii Kurochko wrote:
> > On 8/13/26 11:30 AM, Baptiste Le Duc wrote:
> > > > --- 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().
> See how terminology is important. As said elsewhere, "save state" and
> "restore state" don't make clear at all in which situation they're to be
> used.
I totally agree that it is important.

Just to clarify it now (before I started to re-shuffle and/or adding 
extra patches to have better context how this functions will be called) 
I will add some information here. So imsic_state_save() and 
imsic_state_restore() is going to be called from context_switch() 
function when one vCPU is de-scheduled and new vCPU is scheduled (so no 
migration here at all, yes it could happen but it is still a separate 
path and so separate question). Considering that my understanding that 
during context_switch() I have to save state of IMSIC which corresponds 
to vCPU which is going to be de-scheduled and restore a state of IMSIC 
of vCPU which is going to be scheduled.
With the current context is imsic_state_save() and imsic_state_restore() 
are correct names?
> Also, can both of you please adjust Roger's email address when replying?
Could you please clarify what is wrong with it? For example, in this 
patch series:
  [PATCH v2 0/2] vpci: allow unaligned accesses by the hardware domain

This one is used: Roger Pau Monne <roger@xxxxxxxxxxxxxx>

~ Oleksii