[v3,6/8] RISC-V: KVM: Add ONE_REG interface for AIA CSRs

  We implement ONE_REG interface for AIA CSRs as a separate subtype
under the CSR ONE_REG interface.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 arch/riscv/include/uapi/asm/kvm.h | 8 ++++++++
 arch/riscv/kvm/vcpu.c             | 8 ++++++++
 2 files changed, 16 insertions(+)
  

On Mon, Apr 03, 2023 at 03:03:08PM +0530, Anup Patel wrote:
> We implement ONE_REG interface for AIA CSRs as a separate subtype
> under the CSR ONE_REG interface.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  arch/riscv/include/uapi/asm/kvm.h | 8 ++++++++
>  arch/riscv/kvm/vcpu.c             | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 182023dc9a51..cbc3e74fa670 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -79,6 +79,10 @@ struct kvm_riscv_csr {
>  	unsigned long scounteren;
>  };
>  
> +/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> +struct kvm_riscv_aia_csr {
> +};
> +
>  /* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
>  struct kvm_riscv_timer {
>  	__u64 frequency;
> @@ -107,6 +111,7 @@ enum KVM_RISCV_ISA_EXT_ID {
>  	KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
>  	KVM_RISCV_ISA_EXT_ZICBOM,
>  	KVM_RISCV_ISA_EXT_ZBB,

Looks like this patch is also based on "[PATCH] RISC-V: KVM: Allow Zbb
extension for Guest/VM"

Thanks,
drew
  

On Mon, Apr 3, 2023 at 5:01 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Apr 03, 2023 at 03:03:08PM +0530, Anup Patel wrote:
> > We implement ONE_REG interface for AIA CSRs as a separate subtype
> > under the CSR ONE_REG interface.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  arch/riscv/include/uapi/asm/kvm.h | 8 ++++++++
> >  arch/riscv/kvm/vcpu.c             | 8 ++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> > index 182023dc9a51..cbc3e74fa670 100644
> > --- a/arch/riscv/include/uapi/asm/kvm.h
> > +++ b/arch/riscv/include/uapi/asm/kvm.h
> > @@ -79,6 +79,10 @@ struct kvm_riscv_csr {
> >       unsigned long scounteren;
> >  };
> >
> > +/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > +struct kvm_riscv_aia_csr {
> > +};
> > +
> >  /* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> >  struct kvm_riscv_timer {
> >       __u64 frequency;
> > @@ -107,6 +111,7 @@ enum KVM_RISCV_ISA_EXT_ID {
> >       KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
> >       KVM_RISCV_ISA_EXT_ZICBOM,
> >       KVM_RISCV_ISA_EXT_ZBB,
>
> Looks like this patch is also based on "[PATCH] RISC-V: KVM: Allow Zbb
> extension for Guest/VM"

Yes, do you want me to change the order of dependency?

Regards,
Anup
  

On Mon, Apr 03, 2023 at 05:34:57PM +0530, Anup Patel wrote:
> On Mon, Apr 3, 2023 at 5:01 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > On Mon, Apr 03, 2023 at 03:03:08PM +0530, Anup Patel wrote:
> > > We implement ONE_REG interface for AIA CSRs as a separate subtype
> > > under the CSR ONE_REG interface.
> > >
> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > ---
> > >  arch/riscv/include/uapi/asm/kvm.h | 8 ++++++++
> > >  arch/riscv/kvm/vcpu.c             | 8 ++++++++
> > >  2 files changed, 16 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> > > index 182023dc9a51..cbc3e74fa670 100644
> > > --- a/arch/riscv/include/uapi/asm/kvm.h
> > > +++ b/arch/riscv/include/uapi/asm/kvm.h
> > > @@ -79,6 +79,10 @@ struct kvm_riscv_csr {
> > >       unsigned long scounteren;
> > >  };
> > >
> > > +/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > > +struct kvm_riscv_aia_csr {
> > > +};
> > > +
> > >  /* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > >  struct kvm_riscv_timer {
> > >       __u64 frequency;
> > > @@ -107,6 +111,7 @@ enum KVM_RISCV_ISA_EXT_ID {
> > >       KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
> > >       KVM_RISCV_ISA_EXT_ZICBOM,
> > >       KVM_RISCV_ISA_EXT_ZBB,
> >
> > Looks like this patch is also based on "[PATCH] RISC-V: KVM: Allow Zbb
> > extension for Guest/VM"
> 
> Yes, do you want me to change the order of dependency?

It's probably best if neither depend on each other, since they're
independent, but otherwise the order doesn't matter. It'd be nice to call
the order out in the cover letter to give patchwork a chance at automatic
build testing, though. To call it out, I believe adding

Based-on: 20230401112730.2105240-1-apatel@ventanamicro.com

to the cover letter should work.

Thanks,
drew
  

On Mon, Apr 03, 2023 at 03:03:08PM +0530, Anup Patel wrote:
> We implement ONE_REG interface for AIA CSRs as a separate subtype
> under the CSR ONE_REG interface.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  arch/riscv/include/uapi/asm/kvm.h | 8 ++++++++
>  arch/riscv/kvm/vcpu.c             | 8 ++++++++
>  2 files changed, 16 insertions(+)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
  

On Mon, Apr 3, 2023 at 3:03 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> We implement ONE_REG interface for AIA CSRs as a separate subtype
> under the CSR ONE_REG interface.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  arch/riscv/include/uapi/asm/kvm.h | 8 ++++++++
>  arch/riscv/kvm/vcpu.c             | 8 ++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 182023dc9a51..cbc3e74fa670 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -79,6 +79,10 @@ struct kvm_riscv_csr {
>         unsigned long scounteren;
>  };
>
> +/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> +struct kvm_riscv_aia_csr {
> +};
> +
>  /* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
>  struct kvm_riscv_timer {
>         __u64 frequency;
> @@ -107,6 +111,7 @@ enum KVM_RISCV_ISA_EXT_ID {
>         KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
>         KVM_RISCV_ISA_EXT_ZICBOM,
>         KVM_RISCV_ISA_EXT_ZBB,
> +       KVM_RISCV_ISA_EXT_SSAIA,
>         KVM_RISCV_ISA_EXT_MAX,
>  };
>
> @@ -153,8 +158,11 @@ enum KVM_RISCV_SBI_EXT_ID {
>  /* Control and status registers are mapped as type 3 */
>  #define KVM_REG_RISCV_CSR              (0x03 << KVM_REG_RISCV_TYPE_SHIFT)
>  #define KVM_REG_RISCV_CSR_GENERAL      (0x0 << KVM_REG_RISCV_SUBTYPE_SHIFT)
> +#define KVM_REG_RISCV_CSR_AIA          (0x1 << KVM_REG_RISCV_SUBTYPE_SHIFT)
>  #define KVM_REG_RISCV_CSR_REG(name)    \
>                 (offsetof(struct kvm_riscv_csr, name) / sizeof(unsigned long))
> +#define KVM_REG_RISCV_CSR_AIA_REG(name)        \
> +       (offsetof(struct kvm_riscv_aia_csr, name) / sizeof(unsigned long))
>
>  /* Timer registers are mapped as type 4 */
>  #define KVM_REG_RISCV_TIMER            (0x04 << KVM_REG_RISCV_TYPE_SHIFT)
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index aca6b4fb7519..15507cd3a595 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -58,6 +58,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
>         [KVM_RISCV_ISA_EXT_I] = RISCV_ISA_EXT_i,
>         [KVM_RISCV_ISA_EXT_M] = RISCV_ISA_EXT_m,
>
> +       KVM_ISA_EXT_ARR(SSAIA),
>         KVM_ISA_EXT_ARR(SSTC),
>         KVM_ISA_EXT_ARR(SVINVAL),
>         KVM_ISA_EXT_ARR(SVPBMT),
> @@ -97,6 +98,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
>         case KVM_RISCV_ISA_EXT_C:
>         case KVM_RISCV_ISA_EXT_I:
>         case KVM_RISCV_ISA_EXT_M:
> +       case KVM_RISCV_ISA_EXT_SSAIA:
>         case KVM_RISCV_ISA_EXT_SSTC:
>         case KVM_RISCV_ISA_EXT_SVINVAL:
>         case KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
> @@ -520,6 +522,9 @@ static int kvm_riscv_vcpu_get_reg_csr(struct kvm_vcpu *vcpu,
>         case KVM_REG_RISCV_CSR_GENERAL:
>                 rc = kvm_riscv_vcpu_general_get_csr(vcpu, reg_num, &reg_val);
>                 break;
> +       case KVM_REG_RISCV_CSR_AIA:
> +               rc = kvm_riscv_vcpu_aia_get_csr(vcpu, reg_num, &reg_val);
> +               break;
>         default:
>                 rc = -EINVAL;
>                 break;
> @@ -556,6 +561,9 @@ static int kvm_riscv_vcpu_set_reg_csr(struct kvm_vcpu *vcpu,
>         case KVM_REG_RISCV_CSR_GENERAL:
>                 rc = kvm_riscv_vcpu_general_set_csr(vcpu, reg_num, reg_val);
>                 break;
> +       case KVM_REG_RISCV_CSR_AIA:
> +               rc = kvm_riscv_vcpu_aia_set_csr(vcpu, reg_num, reg_val);
> +               break;
>         default:
>                 rc = -EINVAL;
>                 break;
> --
> 2.34.1
>


Reviewed-by: Atish Patra <atishp@rivosinc.com>
  

On Mon, Apr 03, 2023 at 02:23:01PM +0200, Andrew Jones wrote:
> On Mon, Apr 03, 2023 at 05:34:57PM +0530, Anup Patel wrote:
> > On Mon, Apr 3, 2023 at 5:01 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> > >
> > > On Mon, Apr 03, 2023 at 03:03:08PM +0530, Anup Patel wrote:
> > > > We implement ONE_REG interface for AIA CSRs as a separate subtype
> > > > under the CSR ONE_REG interface.
> > > >
> > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > > ---
> > > >  arch/riscv/include/uapi/asm/kvm.h | 8 ++++++++
> > > >  arch/riscv/kvm/vcpu.c             | 8 ++++++++
> > > >  2 files changed, 16 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> > > > index 182023dc9a51..cbc3e74fa670 100644
> > > > --- a/arch/riscv/include/uapi/asm/kvm.h
> > > > +++ b/arch/riscv/include/uapi/asm/kvm.h
> > > > @@ -79,6 +79,10 @@ struct kvm_riscv_csr {
> > > >       unsigned long scounteren;
> > > >  };
> > > >
> > > > +/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > > > +struct kvm_riscv_aia_csr {
> > > > +};
> > > > +
> > > >  /* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > > >  struct kvm_riscv_timer {
> > > >       __u64 frequency;
> > > > @@ -107,6 +111,7 @@ enum KVM_RISCV_ISA_EXT_ID {
> > > >       KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
> > > >       KVM_RISCV_ISA_EXT_ZICBOM,
> > > >       KVM_RISCV_ISA_EXT_ZBB,
> > >
> > > Looks like this patch is also based on "[PATCH] RISC-V: KVM: Allow Zbb
> > > extension for Guest/VM"
> > 
> > Yes, do you want me to change the order of dependency?
> 
> It's probably best if neither depend on each other, since they're
> independent, but otherwise the order doesn't matter. It'd be nice to call
> the order out in the cover letter to give patchwork a chance at automatic
> build testing, though. To call it out, I believe adding
> 
> Based-on: 20230401112730.2105240-1-apatel@ventanamicro.com
> 
> to the cover letter should work.

I also just noticed that this based on "RISC-V: KVM: Add ONE_REG
interface to enable/disable SBI extensions"[1] and it needs to be
in order to pick up the KVM_REG_RISCV_SUBTYPE_MASK and
KVM_REG_RISCV_SUBTYPE_SHIFT defines. It'd be good to call that
patch out with Based-on.

[1]: 20230331174542.2067560-2-apatel@ventanamicro.com

Thanks,
drew
  

On Tue, Apr 04, 2023 at 01:52:43PM +0200, Andrew Jones wrote:
> On Mon, Apr 03, 2023 at 02:23:01PM +0200, Andrew Jones wrote:

> > It's probably best if neither depend on each other, since they're
> > independent, but otherwise the order doesn't matter. It'd be nice to call
> > the order out in the cover letter to give patchwork a chance at automatic
> > build testing, though. To call it out, I believe adding
> > 
> > Based-on: 20230401112730.2105240-1-apatel@ventanamicro.com
> > 
> > to the cover letter should work.
> 
> I also just noticed that this based on "RISC-V: KVM: Add ONE_REG
> interface to enable/disable SBI extensions"[1] and it needs to be
> in order to pick up the KVM_REG_RISCV_SUBTYPE_MASK and
> KVM_REG_RISCV_SUBTYPE_SHIFT defines. It'd be good to call that
> patch out with Based-on.
> 
> [1]: 20230331174542.2067560-2-apatel@ventanamicro.com

I've been waiting for a review on that for a while.. It's been 3
weeks, so just gonna merge it and see what breaks!
  

On Tue, Apr 04, 2023 at 01:52:43PM +0200, Andrew Jones wrote:
> On Mon, Apr 03, 2023 at 02:23:01PM +0200, Andrew Jones wrote:
> > On Mon, Apr 03, 2023 at 05:34:57PM +0530, Anup Patel wrote:
> > > On Mon, Apr 3, 2023 at 5:01 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> > > >
> > > > On Mon, Apr 03, 2023 at 03:03:08PM +0530, Anup Patel wrote:
> > > > > We implement ONE_REG interface for AIA CSRs as a separate subtype
> > > > > under the CSR ONE_REG interface.
> > > > >
> > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > > > ---
> > > > >  arch/riscv/include/uapi/asm/kvm.h | 8 ++++++++
> > > > >  arch/riscv/kvm/vcpu.c             | 8 ++++++++
> > > > >  2 files changed, 16 insertions(+)
> > > > >
> > > > > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> > > > > index 182023dc9a51..cbc3e74fa670 100644
> > > > > --- a/arch/riscv/include/uapi/asm/kvm.h
> > > > > +++ b/arch/riscv/include/uapi/asm/kvm.h
> > > > > @@ -79,6 +79,10 @@ struct kvm_riscv_csr {
> > > > >       unsigned long scounteren;
> > > > >  };
> > > > >
> > > > > +/* AIA CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > > > > +struct kvm_riscv_aia_csr {
> > > > > +};
> > > > > +
> > > > >  /* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> > > > >  struct kvm_riscv_timer {
> > > > >       __u64 frequency;
> > > > > @@ -107,6 +111,7 @@ enum KVM_RISCV_ISA_EXT_ID {
> > > > >       KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
> > > > >       KVM_RISCV_ISA_EXT_ZICBOM,
> > > > >       KVM_RISCV_ISA_EXT_ZBB,
> > > >
> > > > Looks like this patch is also based on "[PATCH] RISC-V: KVM: Allow Zbb
> > > > extension for Guest/VM"
> > > 
> > > Yes, do you want me to change the order of dependency?
> > 
> > It's probably best if neither depend on each other, since they're
> > independent, but otherwise the order doesn't matter. It'd be nice to call
> > the order out in the cover letter to give patchwork a chance at automatic
> > build testing, though. To call it out, I believe adding
> > 
> > Based-on: 20230401112730.2105240-1-apatel@ventanamicro.com
> > 
> > to the cover letter should work.
> 
> I also just noticed that this based on "RISC-V: KVM: Add ONE_REG
> interface to enable/disable SBI extensions"[1] and it needs to be
> in order to pick up the KVM_REG_RISCV_SUBTYPE_MASK and
> KVM_REG_RISCV_SUBTYPE_SHIFT defines. It'd be good to call that
> patch out with Based-on.
> 
> [1]: 20230331174542.2067560-2-apatel@ventanamicro.com

And "RISC-V IPI Improvements",
20230328035223.1480939-1-apatel@ventanamicro.com, which is required
for riscv_get_intc_hwnode()

Thanks,
drew
  

On Tue, Apr 04, 2023 at 12:58:41PM +0100, Conor Dooley wrote:
> On Tue, Apr 04, 2023 at 01:52:43PM +0200, Andrew Jones wrote:
> > On Mon, Apr 03, 2023 at 02:23:01PM +0200, Andrew Jones wrote:
> 
> > > It's probably best if neither depend on each other, since they're
> > > independent, but otherwise the order doesn't matter. It'd be nice to call
> > > the order out in the cover letter to give patchwork a chance at automatic
> > > build testing, though. To call it out, I believe adding
> > > 
> > > Based-on: 20230401112730.2105240-1-apatel@ventanamicro.com
> > > 
> > > to the cover letter should work.
> > 
> > I also just noticed that this based on "RISC-V: KVM: Add ONE_REG
> > interface to enable/disable SBI extensions"[1] and it needs to be
> > in order to pick up the KVM_REG_RISCV_SUBTYPE_MASK and
> > KVM_REG_RISCV_SUBTYPE_SHIFT defines. It'd be good to call that
> > patch out with Based-on.
> > 
> > [1]: 20230331174542.2067560-2-apatel@ventanamicro.com
> 
> I've been waiting for a review on that for a while.. It's been 3
> weeks, so just gonna merge it and see what breaks!

I did in fact break some stuff, but the output was no worse than if the
dependencies had not been specified...
I've fixed it (I think!) and told it to ignore the old state, so it'll
re-run against the stuff it missed.

Cheers,
Conor.