Re: [PATCH v2 13/26] xen/riscv: introduce per-vCPU IMSIC state

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.05.2026 16:43, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/imsic.c
> +++ b/xen/arch/riscv/imsic.c
> @@ -16,6 +16,7 @@
>  #include <xen/errno.h>
>  #include <xen/init.h>
>  #include <xen/macros.h>
> +#include <xen/sched.h>
>  #include <xen/smp.h>
>  #include <xen/spinlock.h>
>  #include <xen/xvmalloc.h>
> @@ -56,6 +57,16 @@ do {                            \
>      csr_clear(CSR_SIREG, v);    \
>  } while (0)
>  
> +unsigned int vcpu_guest_file_id(const struct vcpu *v)
> +{
> +    return ACCESS_ONCE(v->arch.vimsic_state->guest_file_id);
> +}
> +
> +void imsic_set_guest_file_id(const struct vcpu *v, unsigned int 
> guest_file_id)

Some people will demand that "const" be omitted in cases like this one, as
it only works ...

> +{
> +    ACCESS_ONCE(v->arch.vimsic_state->guest_file_id) = guest_file_id;

... as long as vimsic_state is a pointer (and not a sub-structure).

> @@ -312,6 +323,30 @@ static int imsic_parse_node(const struct dt_device_node 
> *node,
>      return 0;
>  }
>  
> +int vcpu_imsic_init(struct vcpu *v)
> +{
> +    struct vimsic_state *imsic_state;
> +
> +    /* Allocate IMSIC context */
> +    imsic_state = xvzalloc(struct vimsic_state);
> +    if ( !imsic_state )
> +        return -ENOMEM;
> +
> +    v->arch.vimsic_state = imsic_state;
> +
> +    /* Setup IMSIC context  */
> +    rwlock_init(&imsic_state->vsfile_lock);
> +
> +    imsic_state->vsfile_pcpu = NR_CPUS;
> +
> +    return 0;
> +}
> +
> +void vcpu_imsic_deinit(const struct vcpu *v)
> +{
> +    xvfree(v->arch.vimsic_state);

Better XVFREE(), for the function to be idempotent.

> --- a/xen/arch/riscv/include/asm/imsic.h
> +++ b/xen/arch/riscv/include/asm/imsic.h
> @@ -11,6 +11,7 @@
>  #ifndef ASM_RISCV_IMSIC_H
>  #define ASM_RISCV_IMSIC_H
>  
> +#include <xen/rwlock.h>
>  #include <xen/spinlock.h>
>  #include <xen/stdbool.h>
>  #include <xen/types.h>
> @@ -61,7 +62,24 @@ struct imsic_config {
>      spinlock_t lock;
>  };
>  
> +struct vimsic_state {
> +    /* IMSIC VS-file */
> +    rwlock_t vsfile_lock;
> +    /*
> +     * (guest_file_id == 0) -> s/w IMSIC SW-file
> +     * (guest_file_id > 0) -> h/w IMSIC VS-file
> +     */
> +    unsigned int guest_file_id;
> +    /*
> +     * (vsfile_pcpu >= 0) => h/w IMSIC VS-file
> +     * (vsfile_pcpu == NR_CPUS) => s/w IMSIC SW-file
> +     */
> +    unsigned int vsfile_pcpu;
> +};

In the comments, what does SW stand for? Not "software" I assume, as
that's already expressed by s/w.

Jan