Re: [PATCH v1 10/17] xen/riscv: introduce vintc_state_{save,restore}()

On 20.07.2026 18:02, Oleksii Kurochko wrote:
> Virtual interrupt controller state must be preserved across vCPU context
> switches: for AIA, a vCPU's guest interrupt file lives in the IMSIC of
> the pCPU it runs on, so the related state has to be saved when the vCPU
> is descheduled and re-established when it is scheduled again.
> 
> Introduce vintc_state_save()/vintc_state_restore() wrappers around new
> store_state()/restore_state() hooks in struct vintc_ops, so that the
> context switch path can save/restore this state without knowing which
> vINTC variant a domain uses.

Same issue with naming as mentioned for patch 08.

> No callers are wired up yet: the vAPLIC implementation of the hooks is
> added by the follow-up patch,

Neither "follow-up patch" nor "patch" alone nor "follow-up commit" should
appear in a description. You simply don't know how many other commits are
going to come between the two.

> --- a/xen/arch/riscv/include/asm/intc.h
> +++ b/xen/arch/riscv/include/asm/intc.h
> @@ -64,6 +64,12 @@ struct vintc_ops {
>  
>      /* Deinitialize some vINTC-related stuff for a vCPU */
>      void (*vcpu_deinit)(struct vcpu *v);
> +
> +    /* Store virtual interrupt controller state */
> +    void (*store_state)(struct vcpu *v);
> +
> +    /* Restore virtual interrupt controller state */
> +    void (*restore_state)(struct vcpu *v);
>  };

The parameters are properly named "v" here. Why ...

> @@ -91,4 +97,7 @@ void domain_vintc_deinit(struct domain *d);
>  
>  bool vintc_reserve_virq(const struct domain *d, unsigned int virq);
>  
> +void vintc_state_save(struct vcpu *vcpu);
> +void vintc_state_restore(struct vcpu *vcpu);

... is it "vcpu" here and ...

> --- a/xen/arch/riscv/intc.c
> +++ b/xen/arch/riscv/intc.c
> @@ -163,3 +163,17 @@ bool vintc_reserve_virq(const struct domain *d, unsigned int virq)
>  
>      return !test_and_set_bit(virq, d->arch.vintc->used_irqs);
>  }
> +
> +void vintc_state_save(struct vcpu *vcpu)
> +{
> +    const struct vintc_ops *ops = vcpu->domain->arch.vintc->ops;
> +
> +    ops->store_state(vcpu);
> +}
> +
> +void vintc_state_restore(struct vcpu *vcpu)
> +{
> +    const struct vintc_ops *ops = vcpu->domain->arch.vintc->ops;
> +
> +    ops->restore_state(vcpu);
> +}

... here? Consistent and predictable naming of parameters / variables _is_
important.

Jan