From: Andrew Jones <ajones@ventanamicro.com>
To: Atish Patra <atishp@atishpatra.org>
Cc: kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org,
virtualization@lists.linux-foundation.org, anup@brainfault.org,
pbonzini@redhat.com, paul.walmsley@sifive.com,
palmer@dabbelt.com, aou@eecs.berkeley.edu, jgross@suse.com,
srivatsa@csail.mit.edu, guoren@kernel.org,
conor.dooley@microchip.com
Subject: Re: [PATCH v3 07/13] RISC-V: KVM: Add support for SBI extension registers
Date: Wed, 20 Dec 2023 07:16:26 +0100 [thread overview]
Message-ID: <20231220-c59e78c640f00495c3976343@orel> (raw)
In-Reply-To: <CAOnJCU+h_ed-x7L5oFwWTWoKAq8M3Vk8XUQo1-jhPPaty5Y7Jg@mail.gmail.com>
On Tue, Dec 19, 2023 at 11:58:27AM -0800, Atish Patra wrote:
> On Sun, Dec 17, 2023 at 12:40 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > Some SBI extensions have state that needs to be saved / restored
> > when migrating the VM. Provide a get/set-one-reg register type
> > for SBI extension registers. Each SBI extension that uses this type
> > will have its own subtype. There are currently no subtypes defined.
> > The next patch introduces the first one.
> >
> > Reviewed-by: Anup Patel <anup@brainfault.org>
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> > arch/riscv/include/asm/kvm_vcpu_sbi.h | 4 ++
> > arch/riscv/include/uapi/asm/kvm.h | 3 ++
> > arch/riscv/kvm/vcpu_onereg.c | 42 +++++++++++++++++--
> > arch/riscv/kvm/vcpu_sbi.c | 58 +++++++++++++++++++++++++++
> > 4 files changed, 103 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> > index 99c23bb37a37..dd60f73b5c36 100644
> > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> > @@ -60,6 +60,10 @@ int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg);
> > int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg);
> > +int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu,
> > + const struct kvm_one_reg *reg);
> > +int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu,
> > + const struct kvm_one_reg *reg);
> > const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
> > struct kvm_vcpu *vcpu, unsigned long extid);
> > bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx);
> > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> > index e961d79622fb..30f89a0e855f 100644
> > --- a/arch/riscv/include/uapi/asm/kvm.h
> > +++ b/arch/riscv/include/uapi/asm/kvm.h
> > @@ -242,6 +242,9 @@ enum KVM_RISCV_SBI_EXT_ID {
> > #define KVM_REG_RISCV_VECTOR_REG(n) \
> > ((n) + sizeof(struct __riscv_v_ext_state) / sizeof(unsigned long))
> >
> > +/* Registers for specific SBI extensions are mapped as type 10 */
> > +#define KVM_REG_RISCV_SBI (0x0a << KVM_REG_RISCV_TYPE_SHIFT)
> > +
>
>
> nit comment: KVM_REG_RISCV_SBI looks bit odd when we already have
> KVM_REG_RISCV_SBI_EXT for
> extension enabling/disabling.
>
> How about renaming this to KVM_REG_RISCV_SBI_EXT_STATE or something
> similar indicate that this
> for a specific extension state ?
OK, will do for v4.
>
>
> > /* Device Control API: RISC-V AIA */
> > #define KVM_DEV_RISCV_APLIC_ALIGN 0x1000
> > #define KVM_DEV_RISCV_APLIC_SIZE 0x4000
> > diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> > index 11cdbf844291..901480e73817 100644
> > --- a/arch/riscv/kvm/vcpu_onereg.c
> > +++ b/arch/riscv/kvm/vcpu_onereg.c
> > @@ -961,6 +961,29 @@ static unsigned long num_sbi_ext_regs(struct kvm_vcpu *vcpu)
> > return copy_sbi_ext_reg_indices(vcpu, NULL);
> > }
> >
> > +static inline unsigned long num_sbi_regs(struct kvm_vcpu *vcpu)
> > +{
> > + return 0;
> > +}
> > +
> > +static int copy_sbi_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
> > +{
> > + int n = num_sbi_regs(vcpu);
> > +
> > + for (int i = 0; i < n; i++) {
> > + u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 |
> > + KVM_REG_RISCV_SBI | i;
> > +
> > + if (uindices) {
> > + if (put_user(reg, uindices))
> > + return -EFAULT;
> > + uindices++;
> > + }
> > + }
> > +
> > + return n;
> > +}
> > +
> > static inline unsigned long num_vector_regs(const struct kvm_vcpu *vcpu)
> > {
> > if (!riscv_isa_extension_available(vcpu->arch.isa, v))
> > @@ -1028,6 +1051,7 @@ unsigned long kvm_riscv_vcpu_num_regs(struct kvm_vcpu *vcpu)
> > res += num_vector_regs(vcpu);
> > res += num_isa_ext_regs(vcpu);
> > res += num_sbi_ext_regs(vcpu);
> > + res += num_sbi_regs(vcpu);
> >
> > return res;
> > }
> > @@ -1083,6 +1107,12 @@ int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu,
> > ret = copy_sbi_ext_reg_indices(vcpu, uindices);
> > if (ret < 0)
> > return ret;
> > + uindices += ret;
> > +
> > + ret = copy_sbi_reg_indices(vcpu, uindices);
> > + if (ret < 0)
> > + return ret;
> > + uindices += ret;
> >
> > return 0;
> > }
> > @@ -1105,12 +1135,14 @@ int kvm_riscv_vcpu_set_reg(struct kvm_vcpu *vcpu,
> > case KVM_REG_RISCV_FP_D:
> > return kvm_riscv_vcpu_set_reg_fp(vcpu, reg,
> > KVM_REG_RISCV_FP_D);
> > + case KVM_REG_RISCV_VECTOR:
> > + return kvm_riscv_vcpu_set_reg_vector(vcpu, reg);
> > case KVM_REG_RISCV_ISA_EXT:
> > return kvm_riscv_vcpu_set_reg_isa_ext(vcpu, reg);
> > case KVM_REG_RISCV_SBI_EXT:
> > return kvm_riscv_vcpu_set_reg_sbi_ext(vcpu, reg);
> > - case KVM_REG_RISCV_VECTOR:
> > - return kvm_riscv_vcpu_set_reg_vector(vcpu, reg);
> > + case KVM_REG_RISCV_SBI:
> > + return kvm_riscv_vcpu_set_reg_sbi(vcpu, reg);
> > default:
> > break;
> > }
> > @@ -1136,12 +1168,14 @@ int kvm_riscv_vcpu_get_reg(struct kvm_vcpu *vcpu,
> > case KVM_REG_RISCV_FP_D:
> > return kvm_riscv_vcpu_get_reg_fp(vcpu, reg,
> > KVM_REG_RISCV_FP_D);
> > + case KVM_REG_RISCV_VECTOR:
> > + return kvm_riscv_vcpu_get_reg_vector(vcpu, reg);
> > case KVM_REG_RISCV_ISA_EXT:
> > return kvm_riscv_vcpu_get_reg_isa_ext(vcpu, reg);
> > case KVM_REG_RISCV_SBI_EXT:
> > return kvm_riscv_vcpu_get_reg_sbi_ext(vcpu, reg);
> > - case KVM_REG_RISCV_VECTOR:
> > - return kvm_riscv_vcpu_get_reg_vector(vcpu, reg);
> > + case KVM_REG_RISCV_SBI:
> > + return kvm_riscv_vcpu_get_reg_sbi(vcpu, reg);
> > default:
> > break;
> > }
> > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> > index 088daaa23dd8..834176242ddf 100644
> > --- a/arch/riscv/kvm/vcpu_sbi.c
> > +++ b/arch/riscv/kvm/vcpu_sbi.c
> > @@ -325,6 +325,64 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu,
> > return 0;
> > }
> >
> > +int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu,
> > + const struct kvm_one_reg *reg)
> > +{
> > + unsigned long __user *uaddr =
> > + (unsigned long __user *)(unsigned long)reg->addr;
> > + unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
> > + KVM_REG_SIZE_MASK |
> > + KVM_REG_RISCV_SBI);
> > + unsigned long reg_subtype, reg_val;
> > +
> > + if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
> > + return -EINVAL;
> > +
> > + if (copy_from_user(®_val, uaddr, KVM_REG_SIZE(reg->id)))
> > + return -EFAULT;
> > +
> > + reg_subtype = reg_num & KVM_REG_RISCV_SUBTYPE_MASK;
> > + reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
> > +
> > + switch (reg_subtype) {
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu,
> > + const struct kvm_one_reg *reg)
> > +{
> > + unsigned long __user *uaddr =
> > + (unsigned long __user *)(unsigned long)reg->addr;
> > + unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
> > + KVM_REG_SIZE_MASK |
> > + KVM_REG_RISCV_SBI);
> > + unsigned long reg_subtype, reg_val;
> > + int ret;
> > +
> > + if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
> > + return -EINVAL;
> > +
> > + reg_subtype = reg_num & KVM_REG_RISCV_SUBTYPE_MASK;
> > + reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
> > +
> > + switch (reg_subtype) {
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + if (ret)
> > + return ret;
> > +
> > + if (copy_to_user(uaddr, ®_val, KVM_REG_SIZE(reg->id)))
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
> > +
> > const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
> > struct kvm_vcpu *vcpu, unsigned long extid)
> > {
> > --
> > 2.43.0
> >
>
> Other than that, lgtm.
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-12-20 6:16 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-17 20:40 [PATCH v3 00/13] RISC-V: Add steal-time support Andrew Jones
2023-12-17 20:40 ` [PATCH v3 01/13] RISC-V: paravirt: Add skeleton for pv-time support Andrew Jones
2023-12-19 0:48 ` Atish Patra
2023-12-19 14:29 ` Andrew Jones
2023-12-17 20:40 ` [PATCH v3 02/13] RISC-V: Add SBI STA extension definitions Andrew Jones
2023-12-19 0:49 ` Atish Patra
2023-12-17 20:40 ` [PATCH v3 03/13] RISC-V: paravirt: Implement steal-time support Andrew Jones
2023-12-19 1:41 ` Atish Patra
2023-12-20 6:41 ` Anup Patel
2023-12-17 20:40 ` [PATCH v3 04/13] RISC-V: KVM: Add SBI STA extension skeleton Andrew Jones
2023-12-19 1:42 ` Atish Patra
2023-12-17 20:40 ` [PATCH v3 05/13] RISC-V: KVM: Add steal-update vcpu request Andrew Jones
2023-12-19 1:46 ` Atish Patra
2023-12-17 20:40 ` [PATCH v3 06/13] RISC-V: KVM: Add SBI STA info to vcpu_arch Andrew Jones
2023-12-19 1:49 ` Atish Patra
2023-12-17 20:40 ` [PATCH v3 07/13] RISC-V: KVM: Add support for SBI extension registers Andrew Jones
2023-12-19 19:58 ` Atish Patra
2023-12-20 6:16 ` Andrew Jones [this message]
2023-12-20 6:19 ` Anup Patel
2023-12-20 6:32 ` Andrew Jones
2023-12-17 20:40 ` [PATCH v3 08/13] RISC-V: KVM: Add support for SBI STA registers Andrew Jones
2023-12-17 20:40 ` [PATCH v3 09/13] RISC-V: KVM: Implement SBI STA extension Andrew Jones
2023-12-19 21:52 ` Atish Patra
2023-12-17 20:40 ` [PATCH v3 10/13] RISC-V: KVM: selftests: Move sbi_ecall to processor.c Andrew Jones
2023-12-17 20:40 ` [PATCH v3 11/13] RISC-V: KVM: selftests: Add guest_sbi_probe_extension Andrew Jones
2023-12-19 21:57 ` Atish Patra
2023-12-20 6:28 ` Andrew Jones
2023-12-17 20:40 ` [PATCH v3 12/13] RISC-V: KVM: selftests: Add steal_time test support Andrew Jones
2023-12-19 23:39 ` Atish Patra
2023-12-17 20:40 ` [PATCH v3 13/13] RISC-V: KVM: selftests: Add get-reg-list test for STA registers Andrew Jones
2023-12-19 23:40 ` Atish Patra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231220-c59e78c640f00495c3976343@orel \
--to=ajones@ventanamicro.com \
--cc=anup@brainfault.org \
--cc=aou@eecs.berkeley.edu \
--cc=atishp@atishpatra.org \
--cc=conor.dooley@microchip.com \
--cc=guoren@kernel.org \
--cc=jgross@suse.com \
--cc=kvm-riscv@lists.infradead.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=pbonzini@redhat.com \
--cc=srivatsa@csail.mit.edu \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox