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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 8/12/26 3:57 PM, Jan Beulich wrote:
> On 20.07.2026 18:02, Oleksii Kurochko wrote:
> > @@ -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;
> How does the ->vsfile_pcpu sentinel value matter here, when you're checking
> ->guest_file_id?
Comment is incorrect. I will fix it.

> And anyway, there being dependencies like this one on the other big series
> makes it rather hard to review things.
>
> > +    write_lock_irqsave(&imsic_state->vsfile_lock, flags);
> > +    imsic_state->vsfile_pcpu = cpuid_to_hartid(v->processor);
> As discussed for another patch in this series, this will need to change then
> as well.
I will update that properly.

> > +    write_unlock_irqrestore(&imsic_state->vsfile_lock, flags);
> > +}
> > +
> > +void imsic_state_restore(struct vcpu *v)
> > +{
> > +    /* Nothing to do */
> > +}
> "save" and "restore" have meaning other than what you intend here, aiui. Once
> again without call sites it remains unclear when exactly these functions would
> be called. Which makes it close to impossible to suggest better names.
I will add some extra context and/or re-shuffle patches to make it more 
clearer. Anyway as you explained me in another thread a name is really 
incorrect. I will use imsic_ctxt_switch_{to,from}() instead.
Thanks.

~ Oleksii