Re: [PATCH v1 11/17] xen/riscv: add vAPLIC state save/restore hooks

On 20.07.2026 18:02, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/vaplic.h
> +++ b/xen/arch/riscv/include/asm/vaplic.h
> @@ -34,4 +34,7 @@ struct vaplic {
>  int domain_vaplic_init(struct domain *d);
>  void domain_vaplic_deinit(struct domain *d);
>  
> +void vaplic_state_save(struct vcpu *v);
> +void vaplic_state_restore(struct vcpu *v);

Why would these be needed? Can't ...

> --- a/xen/arch/riscv/vaplic.c
> +++ b/xen/arch/riscv/vaplic.c
> @@ -400,9 +400,27 @@ static const struct mmio_handler_ops vaplic_mmio_ops = {
>      .write = vaplic_mmio_write,
>  };
>  
> +void vaplic_state_save(struct vcpu *v)

... both be static? And don't they want to be cf_check?

> +{
> +    if ( has_msi_support() )
> +        imsic_state_save(v);
> +    else
> +        BUG_ON("unimplemented");
> +}
> +
> +void vaplic_state_restore(struct vcpu *v)
> +{
> +    if ( has_msi_support() )
> +        imsic_state_restore(v);
> +    else
> +        BUG_ON("unimplemented");
> +}

If you're merely forwarding the calls, why can't ...

>  static const struct vintc_ops vintc_ops = {
>      .vcpu_init = vaplic_init,
>      .vcpu_deinit = vaplic_deinit,
> +    .store_state = vaplic_state_save,
> +    .restore_state = vaplic_state_restore,

... imsic_state_{save,restore}() be used directly here? And whatever other
pair of handlers for the case when it's not IMSIC?

Jan