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

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?

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.

> +    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.

Jan