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

[Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>]

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


> 
> No callers are wired up yet: the vAPLIC implementation of the hooks is


> added by the follow-up patch, and the calls from the context switch path
> will be introduced together with vCPU context switch support.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
>
> diff --git a/xen/arch/riscv/include/asm/intc.h 
> b/xen/arch/riscv/include/asm/intc.h
> index 2ee5d1533c..f4f0ce365c 100644
> --- 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);
>  };


>  
>  struct vintc {
> @@ -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);


> +
>  #endif /* ASM__RISCV__INTERRUPT_CONTOLLER_H */
> diff --git a/xen/arch/riscv/intc.c b/xen/arch/riscv/intc.c
> index 372c8d3a20..879d513374 100644
> --- 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;
Is there a situation where ops could be NULL? If yes, add a check.
> +
> +    ops->store_state(vcpu);
> +}
> +
> +void vintc_state_restore(struct vcpu *vcpu)
> +{
> +    const struct vintc_ops *ops = vcpu->domain->arch.vintc->ops;
Same as above

-- 
Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>