All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anup Patel <anup@brainfault.org>
To: Atish Patra <atish.patra@wdc.com>
Cc: "linux-kernel@vger.kernel.org List"
	<linux-kernel@vger.kernel.org>, Anup Patel <anup.patel@wdc.com>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	kvm-riscv@lists.infradead.org, KVM General <kvm@vger.kernel.org>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Vincent Chen <vincent.chen@sifive.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>
Subject: Re: [PATCH v4 2/5] RISC-V: KVM: Reorganize SBI code by moving SBI v0.1 to its own file
Date: Sat, 6 Nov 2021 10:13:35 +0530	[thread overview]
Message-ID: <CAAhSdy12kq6AHwRkgAoPaSxSDp6LM3oFj03Zyt8_jkQtBZRLZA@mail.gmail.com> (raw)
In-Reply-To: <20211105235852.3011900-3-atish.patra@wdc.com>

On Sat, Nov 6, 2021 at 5:29 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> With SBI v0.2, there may be more SBI extensions in future. It makes more
> sense to group related extensions in separate files. Guest kernel will
> choose appropriate SBI version dynamically.
>
> Move the existing implementation to a separate file so that it can be
> removed in future without much conflict.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/include/asm/kvm_vcpu_sbi.h |   2 +
>  arch/riscv/kvm/Makefile               |   1 +
>  arch/riscv/kvm/vcpu_sbi.c             | 151 +++-----------------------
>  arch/riscv/kvm/vcpu_sbi_v01.c         | 129 ++++++++++++++++++++++
>  4 files changed, 149 insertions(+), 134 deletions(-)
>  create mode 100644 arch/riscv/kvm/vcpu_sbi_v01.c
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index 1a4cb0db2d0b..704151969ceb 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -25,5 +25,7 @@ struct kvm_vcpu_sbi_extension {
>                        bool *exit);
>  };
>
> +void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run);
>  const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(unsigned long extid);
> +
>  #endif /* __RISCV_KVM_VCPU_SBI_H__ */
> diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile
> index 30cdd1df0098..d3d5ff3a6019 100644
> --- a/arch/riscv/kvm/Makefile
> +++ b/arch/riscv/kvm/Makefile
> @@ -23,4 +23,5 @@ kvm-y += vcpu_exit.o
>  kvm-y += vcpu_fp.o
>  kvm-y += vcpu_switch.o
>  kvm-y += vcpu_sbi.o
> +kvm-$(CONFIG_RISCV_SBI_V01) += vcpu_sbi_v01.o
>  kvm-y += vcpu_timer.o
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index 05cab5f27eee..06b42f6977e1 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -9,9 +9,7 @@
>  #include <linux/errno.h>
>  #include <linux/err.h>
>  #include <linux/kvm_host.h>
> -#include <asm/csr.h>
>  #include <asm/sbi.h>
> -#include <asm/kvm_vcpu_timer.h>
>  #include <asm/kvm_vcpu_sbi.h>
>
>  static int kvm_linux_err_map_sbi(int err)
> @@ -32,8 +30,21 @@ static int kvm_linux_err_map_sbi(int err)
>         };
>  }
>
> -static void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu,
> -                                      struct kvm_run *run)
> +#ifdef CONFIG_RISCV_SBI_V01
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01;
> +#else
> +static const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01 = {
> +       .extid_start = -1UL,
> +       .extid_end = -1UL,
> +       .handler = NULL,
> +};
> +#endif
> +
> +static const struct kvm_vcpu_sbi_extension *sbi_ext[] = {
> +       &vcpu_sbi_ext_v01,
> +};
> +
> +void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
>         struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
>
> @@ -71,126 +82,6 @@ int kvm_riscv_vcpu_sbi_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
>         return 0;
>  }
>
> -#ifdef CONFIG_RISCV_SBI_V01
> -
> -static void kvm_sbi_system_shutdown(struct kvm_vcpu *vcpu,
> -                                   struct kvm_run *run, u32 type)
> -{
> -       int i;
> -       struct kvm_vcpu *tmp;
> -
> -       kvm_for_each_vcpu(i, tmp, vcpu->kvm)
> -               tmp->arch.power_off = true;
> -       kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_SLEEP);
> -
> -       memset(&run->system_event, 0, sizeof(run->system_event));
> -       run->system_event.type = type;
> -       run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> -}
> -
> -static int kvm_sbi_ext_v01_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,
> -                                     unsigned long *out_val,
> -                                     struct kvm_cpu_trap *utrap,
> -                                     bool *exit)
> -{
> -       ulong hmask;
> -       int i, ret = 0;
> -       u64 next_cycle;
> -       struct kvm_vcpu *rvcpu;
> -       struct cpumask cm, hm;
> -       struct kvm *kvm = vcpu->kvm;
> -       struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
> -
> -       if (!cp)
> -               return -EINVAL;
> -
> -       switch (cp->a7) {
> -       case SBI_EXT_0_1_CONSOLE_GETCHAR:
> -       case SBI_EXT_0_1_CONSOLE_PUTCHAR:
> -               /*
> -                * The CONSOLE_GETCHAR/CONSOLE_PUTCHAR SBI calls cannot be
> -                * handled in kernel so we forward these to user-space
> -                */
> -               kvm_riscv_vcpu_sbi_forward(vcpu, run);
> -               *exit = true;
> -               break;
> -       case SBI_EXT_0_1_SET_TIMER:
> -#if __riscv_xlen == 32
> -               next_cycle = ((u64)cp->a1 << 32) | (u64)cp->a0;
> -#else
> -               next_cycle = (u64)cp->a0;
> -#endif
> -               ret = kvm_riscv_vcpu_timer_next_event(vcpu, next_cycle);
> -               break;
> -       case SBI_EXT_0_1_CLEAR_IPI:
> -               ret = kvm_riscv_vcpu_unset_interrupt(vcpu, IRQ_VS_SOFT);
> -               break;
> -       case SBI_EXT_0_1_SEND_IPI:
> -               if (cp->a0)
> -                       hmask = kvm_riscv_vcpu_unpriv_read(vcpu, false, cp->a0,
> -                                                          utrap);
> -               else
> -                       hmask = (1UL << atomic_read(&kvm->online_vcpus)) - 1;
> -               if (utrap->scause)
> -                       break;
> -
> -               for_each_set_bit(i, &hmask, BITS_PER_LONG) {
> -                       rvcpu = kvm_get_vcpu_by_id(vcpu->kvm, i);
> -                       ret = kvm_riscv_vcpu_set_interrupt(rvcpu, IRQ_VS_SOFT);
> -                       if (ret < 0)
> -                               break;
> -               }
> -               break;
> -       case SBI_EXT_0_1_SHUTDOWN:
> -               kvm_sbi_system_shutdown(vcpu, run, KVM_SYSTEM_EVENT_SHUTDOWN);
> -               *exit = true;
> -               break;
> -       case SBI_EXT_0_1_REMOTE_FENCE_I:
> -       case SBI_EXT_0_1_REMOTE_SFENCE_VMA:
> -       case SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID:
> -               if (cp->a0)
> -                       hmask = kvm_riscv_vcpu_unpriv_read(vcpu, false, cp->a0,
> -                                                          utrap);
> -               else
> -                       hmask = (1UL << atomic_read(&kvm->online_vcpus)) - 1;
> -               if (utrap->scause)
> -                       break;
> -
> -               cpumask_clear(&cm);
> -               for_each_set_bit(i, &hmask, BITS_PER_LONG) {
> -                       rvcpu = kvm_get_vcpu_by_id(vcpu->kvm, i);
> -                       if (rvcpu->cpu < 0)
> -                               continue;
> -                       cpumask_set_cpu(rvcpu->cpu, &cm);
> -               }
> -               riscv_cpuid_to_hartid_mask(&cm, &hm);
> -               if (cp->a7 == SBI_EXT_0_1_REMOTE_FENCE_I)
> -                       ret = sbi_remote_fence_i(cpumask_bits(&hm));
> -               else if (cp->a7 == SBI_EXT_0_1_REMOTE_SFENCE_VMA)
> -                       ret = sbi_remote_hfence_vvma(cpumask_bits(&hm),
> -                                               cp->a1, cp->a2);
> -               else
> -                       ret = sbi_remote_hfence_vvma_asid(cpumask_bits(&hm),
> -                                               cp->a1, cp->a2, cp->a3);
> -               break;
> -       default:
> -               ret = -EINVAL;
> -               break;
> -       }
> -
> -       return ret;
> -}
> -
> -const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01 = {
> -       .extid_start = SBI_EXT_0_1_SET_TIMER,
> -       .extid_end = SBI_EXT_0_1_SHUTDOWN,
> -       .handler = kvm_sbi_ext_v01_handler,
> -};
> -
> -static const struct kvm_vcpu_sbi_extension *sbi_ext[] = {
> -       &vcpu_sbi_ext_v01,
> -};
> -
>  const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(unsigned long extid)
>  {
>         int i = 0;
> @@ -220,9 +111,11 @@ int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
>         sbi_ext = kvm_vcpu_sbi_find_ext(cp->a7);
>         if (sbi_ext && sbi_ext->handler) {
> +#ifdef CONFIG_RISCV_SBI_V01
>                 if (cp->a7 >= SBI_EXT_0_1_SET_TIMER &&
>                     cp->a7 <= SBI_EXT_0_1_SHUTDOWN)
>                         ext_is_v01 = true;
> +#endif
>                 ret = sbi_ext->handler(vcpu, run, &out_val, &utrap, &userspace_exit);
>         } else {
>                 /* Return error for unsupported SBI calls */
> @@ -262,13 +155,3 @@ int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
>         return ret;
>  }
> -
> -#else
> -
> -int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run)
> -{
> -       kvm_riscv_vcpu_sbi_forward(vcpu, run);
> -       return 0;
> -}
> -
> -#endif
> diff --git a/arch/riscv/kvm/vcpu_sbi_v01.c b/arch/riscv/kvm/vcpu_sbi_v01.c
> new file mode 100644
> index 000000000000..5a8e2afc8a57
> --- /dev/null
> +++ b/arch/riscv/kvm/vcpu_sbi_v01.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021 Western Digital Corporation or its affiliates.
> + *
> + * Authors:
> + *     Atish Patra <atish.patra@wdc.com>
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/kvm_host.h>
> +#include <asm/csr.h>
> +#include <asm/sbi.h>
> +#include <asm/kvm_vcpu_timer.h>
> +#include <asm/kvm_vcpu_sbi.h>
> +
> +static void kvm_sbi_system_shutdown(struct kvm_vcpu *vcpu,
> +                                   struct kvm_run *run, u32 type)
> +{
> +       int i;
> +       struct kvm_vcpu *tmp;
> +
> +       kvm_for_each_vcpu(i, tmp, vcpu->kvm)
> +               tmp->arch.power_off = true;
> +       kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_SLEEP);
> +
> +       memset(&run->system_event, 0, sizeof(run->system_event));
> +       run->system_event.type = type;
> +       run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +}
> +
> +static int kvm_sbi_ext_v01_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,
> +                                     unsigned long *out_val,
> +                                     struct kvm_cpu_trap *utrap,
> +                                     bool *exit)
> +{
> +       ulong hmask;
> +       int i, ret = 0;
> +       u64 next_cycle;
> +       struct kvm_vcpu *rvcpu;
> +       struct cpumask cm, hm;
> +       struct kvm *kvm = vcpu->kvm;
> +       struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
> +
> +       if (!cp)
> +               return -EINVAL;

This check on "cp" is unnecessary.

> +
> +       switch (cp->a7) {
> +       case SBI_EXT_0_1_CONSOLE_GETCHAR:
> +       case SBI_EXT_0_1_CONSOLE_PUTCHAR:
> +               /*
> +                * The CONSOLE_GETCHAR/CONSOLE_PUTCHAR SBI calls cannot be
> +                * handled in kernel so we forward these to user-space
> +                */
> +               kvm_riscv_vcpu_sbi_forward(vcpu, run);
> +               *exit = true;
> +               break;
> +       case SBI_EXT_0_1_SET_TIMER:
> +#if __riscv_xlen == 32
> +               next_cycle = ((u64)cp->a1 << 32) | (u64)cp->a0;
> +#else
> +               next_cycle = (u64)cp->a0;
> +#endif
> +               ret = kvm_riscv_vcpu_timer_next_event(vcpu, next_cycle);
> +               break;
> +       case SBI_EXT_0_1_CLEAR_IPI:
> +               ret = kvm_riscv_vcpu_unset_interrupt(vcpu, IRQ_VS_SOFT);
> +               break;
> +       case SBI_EXT_0_1_SEND_IPI:
> +               if (cp->a0)
> +                       hmask = kvm_riscv_vcpu_unpriv_read(vcpu, false, cp->a0,
> +                                                          utrap);
> +               else
> +                       hmask = (1UL << atomic_read(&kvm->online_vcpus)) - 1;
> +               if (utrap->scause)
> +                       break;
> +
> +               for_each_set_bit(i, &hmask, BITS_PER_LONG) {
> +                       rvcpu = kvm_get_vcpu_by_id(vcpu->kvm, i);
> +                       ret = kvm_riscv_vcpu_set_interrupt(rvcpu, IRQ_VS_SOFT);
> +                       if (ret < 0)
> +                               break;
> +               }
> +               break;
> +       case SBI_EXT_0_1_SHUTDOWN:
> +               kvm_sbi_system_shutdown(vcpu, run, KVM_SYSTEM_EVENT_SHUTDOWN);
> +               *exit = true;
> +               break;
> +       case SBI_EXT_0_1_REMOTE_FENCE_I:
> +       case SBI_EXT_0_1_REMOTE_SFENCE_VMA:
> +       case SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID:
> +               if (cp->a0)
> +                       hmask = kvm_riscv_vcpu_unpriv_read(vcpu, false, cp->a0,
> +                                                          utrap);
> +               else
> +                       hmask = (1UL << atomic_read(&kvm->online_vcpus)) - 1;
> +               if (utrap->scause)
> +                       break;
> +
> +               cpumask_clear(&cm);
> +               for_each_set_bit(i, &hmask, BITS_PER_LONG) {
> +                       rvcpu = kvm_get_vcpu_by_id(vcpu->kvm, i);
> +                       if (rvcpu->cpu < 0)
> +                               continue;
> +                       cpumask_set_cpu(rvcpu->cpu, &cm);
> +               }
> +               riscv_cpuid_to_hartid_mask(&cm, &hm);
> +               if (cp->a7 == SBI_EXT_0_1_REMOTE_FENCE_I)
> +                       ret = sbi_remote_fence_i(cpumask_bits(&hm));
> +               else if (cp->a7 == SBI_EXT_0_1_REMOTE_SFENCE_VMA)
> +                       ret = sbi_remote_hfence_vvma(cpumask_bits(&hm),
> +                                               cp->a1, cp->a2);
> +               else
> +                       ret = sbi_remote_hfence_vvma_asid(cpumask_bits(&hm),
> +                                               cp->a1, cp->a2, cp->a3);
> +               break;
> +       default:
> +               ret = -EINVAL;
> +               break;
> +       };
> +
> +       return ret;
> +}
> +
> +const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01 = {
> +       .extid_start = SBI_EXT_0_1_SET_TIMER,
> +       .extid_end = SBI_EXT_0_1_SHUTDOWN,
> +       .handler = kvm_sbi_ext_v01_handler,
> +};
> --
> 2.31.1
>

Otherwise it looks good to me.

Reviewed-by: Anup Patel <anup.patel@wdc.com>

Regards,
Anup

WARNING: multiple messages have this Message-ID (diff)
From: Anup Patel <anup@brainfault.org>
To: Atish Patra <atish.patra@wdc.com>
Cc: "linux-kernel@vger.kernel.org List"
	<linux-kernel@vger.kernel.org>, Anup Patel <anup.patel@wdc.com>,
	 Heinrich Schuchardt <xypron.glpk@gmx.de>,
	kvm-riscv@lists.infradead.org,  KVM General <kvm@vger.kernel.org>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	 Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	 Vincent Chen <vincent.chen@sifive.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	 Sean Christopherson <seanjc@google.com>
Subject: Re: [PATCH v4 2/5] RISC-V: KVM: Reorganize SBI code by moving SBI v0.1 to its own file
Date: Sat, 6 Nov 2021 10:13:35 +0530	[thread overview]
Message-ID: <CAAhSdy12kq6AHwRkgAoPaSxSDp6LM3oFj03Zyt8_jkQtBZRLZA@mail.gmail.com> (raw)
In-Reply-To: <20211105235852.3011900-3-atish.patra@wdc.com>

On Sat, Nov 6, 2021 at 5:29 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> With SBI v0.2, there may be more SBI extensions in future. It makes more
> sense to group related extensions in separate files. Guest kernel will
> choose appropriate SBI version dynamically.
>
> Move the existing implementation to a separate file so that it can be
> removed in future without much conflict.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/include/asm/kvm_vcpu_sbi.h |   2 +
>  arch/riscv/kvm/Makefile               |   1 +
>  arch/riscv/kvm/vcpu_sbi.c             | 151 +++-----------------------
>  arch/riscv/kvm/vcpu_sbi_v01.c         | 129 ++++++++++++++++++++++
>  4 files changed, 149 insertions(+), 134 deletions(-)
>  create mode 100644 arch/riscv/kvm/vcpu_sbi_v01.c
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index 1a4cb0db2d0b..704151969ceb 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -25,5 +25,7 @@ struct kvm_vcpu_sbi_extension {
>                        bool *exit);
>  };
>
> +void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run);
>  const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(unsigned long extid);
> +
>  #endif /* __RISCV_KVM_VCPU_SBI_H__ */
> diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile
> index 30cdd1df0098..d3d5ff3a6019 100644
> --- a/arch/riscv/kvm/Makefile
> +++ b/arch/riscv/kvm/Makefile
> @@ -23,4 +23,5 @@ kvm-y += vcpu_exit.o
>  kvm-y += vcpu_fp.o
>  kvm-y += vcpu_switch.o
>  kvm-y += vcpu_sbi.o
> +kvm-$(CONFIG_RISCV_SBI_V01) += vcpu_sbi_v01.o
>  kvm-y += vcpu_timer.o
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index 05cab5f27eee..06b42f6977e1 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -9,9 +9,7 @@
>  #include <linux/errno.h>
>  #include <linux/err.h>
>  #include <linux/kvm_host.h>
> -#include <asm/csr.h>
>  #include <asm/sbi.h>
> -#include <asm/kvm_vcpu_timer.h>
>  #include <asm/kvm_vcpu_sbi.h>
>
>  static int kvm_linux_err_map_sbi(int err)
> @@ -32,8 +30,21 @@ static int kvm_linux_err_map_sbi(int err)
>         };
>  }
>
> -static void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu,
> -                                      struct kvm_run *run)
> +#ifdef CONFIG_RISCV_SBI_V01
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01;
> +#else
> +static const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01 = {
> +       .extid_start = -1UL,
> +       .extid_end = -1UL,
> +       .handler = NULL,
> +};
> +#endif
> +
> +static const struct kvm_vcpu_sbi_extension *sbi_ext[] = {
> +       &vcpu_sbi_ext_v01,
> +};
> +
> +void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
>         struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
>
> @@ -71,126 +82,6 @@ int kvm_riscv_vcpu_sbi_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
>         return 0;
>  }
>
> -#ifdef CONFIG_RISCV_SBI_V01
> -
> -static void kvm_sbi_system_shutdown(struct kvm_vcpu *vcpu,
> -                                   struct kvm_run *run, u32 type)
> -{
> -       int i;
> -       struct kvm_vcpu *tmp;
> -
> -       kvm_for_each_vcpu(i, tmp, vcpu->kvm)
> -               tmp->arch.power_off = true;
> -       kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_SLEEP);
> -
> -       memset(&run->system_event, 0, sizeof(run->system_event));
> -       run->system_event.type = type;
> -       run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> -}
> -
> -static int kvm_sbi_ext_v01_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,
> -                                     unsigned long *out_val,
> -                                     struct kvm_cpu_trap *utrap,
> -                                     bool *exit)
> -{
> -       ulong hmask;
> -       int i, ret = 0;
> -       u64 next_cycle;
> -       struct kvm_vcpu *rvcpu;
> -       struct cpumask cm, hm;
> -       struct kvm *kvm = vcpu->kvm;
> -       struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
> -
> -       if (!cp)
> -               return -EINVAL;
> -
> -       switch (cp->a7) {
> -       case SBI_EXT_0_1_CONSOLE_GETCHAR:
> -       case SBI_EXT_0_1_CONSOLE_PUTCHAR:
> -               /*
> -                * The CONSOLE_GETCHAR/CONSOLE_PUTCHAR SBI calls cannot be
> -                * handled in kernel so we forward these to user-space
> -                */
> -               kvm_riscv_vcpu_sbi_forward(vcpu, run);
> -               *exit = true;
> -               break;
> -       case SBI_EXT_0_1_SET_TIMER:
> -#if __riscv_xlen == 32
> -               next_cycle = ((u64)cp->a1 << 32) | (u64)cp->a0;
> -#else
> -               next_cycle = (u64)cp->a0;
> -#endif
> -               ret = kvm_riscv_vcpu_timer_next_event(vcpu, next_cycle);
> -               break;
> -       case SBI_EXT_0_1_CLEAR_IPI:
> -               ret = kvm_riscv_vcpu_unset_interrupt(vcpu, IRQ_VS_SOFT);
> -               break;
> -       case SBI_EXT_0_1_SEND_IPI:
> -               if (cp->a0)
> -                       hmask = kvm_riscv_vcpu_unpriv_read(vcpu, false, cp->a0,
> -                                                          utrap);
> -               else
> -                       hmask = (1UL << atomic_read(&kvm->online_vcpus)) - 1;
> -               if (utrap->scause)
> -                       break;
> -
> -               for_each_set_bit(i, &hmask, BITS_PER_LONG) {
> -                       rvcpu = kvm_get_vcpu_by_id(vcpu->kvm, i);
> -                       ret = kvm_riscv_vcpu_set_interrupt(rvcpu, IRQ_VS_SOFT);
> -                       if (ret < 0)
> -                               break;
> -               }
> -               break;
> -       case SBI_EXT_0_1_SHUTDOWN:
> -               kvm_sbi_system_shutdown(vcpu, run, KVM_SYSTEM_EVENT_SHUTDOWN);
> -               *exit = true;
> -               break;
> -       case SBI_EXT_0_1_REMOTE_FENCE_I:
> -       case SBI_EXT_0_1_REMOTE_SFENCE_VMA:
> -       case SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID:
> -               if (cp->a0)
> -                       hmask = kvm_riscv_vcpu_unpriv_read(vcpu, false, cp->a0,
> -                                                          utrap);
> -               else
> -                       hmask = (1UL << atomic_read(&kvm->online_vcpus)) - 1;
> -               if (utrap->scause)
> -                       break;
> -
> -               cpumask_clear(&cm);
> -               for_each_set_bit(i, &hmask, BITS_PER_LONG) {
> -                       rvcpu = kvm_get_vcpu_by_id(vcpu->kvm, i);
> -                       if (rvcpu->cpu < 0)
> -                               continue;
> -                       cpumask_set_cpu(rvcpu->cpu, &cm);
> -               }
> -               riscv_cpuid_to_hartid_mask(&cm, &hm);
> -               if (cp->a7 == SBI_EXT_0_1_REMOTE_FENCE_I)
> -                       ret = sbi_remote_fence_i(cpumask_bits(&hm));
> -               else if (cp->a7 == SBI_EXT_0_1_REMOTE_SFENCE_VMA)
> -                       ret = sbi_remote_hfence_vvma(cpumask_bits(&hm),
> -                                               cp->a1, cp->a2);
> -               else
> -                       ret = sbi_remote_hfence_vvma_asid(cpumask_bits(&hm),
> -                                               cp->a1, cp->a2, cp->a3);
> -               break;
> -       default:
> -               ret = -EINVAL;
> -               break;
> -       }
> -
> -       return ret;
> -}
> -
> -const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01 = {
> -       .extid_start = SBI_EXT_0_1_SET_TIMER,
> -       .extid_end = SBI_EXT_0_1_SHUTDOWN,
> -       .handler = kvm_sbi_ext_v01_handler,
> -};
> -
> -static const struct kvm_vcpu_sbi_extension *sbi_ext[] = {
> -       &vcpu_sbi_ext_v01,
> -};
> -
>  const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(unsigned long extid)
>  {
>         int i = 0;
> @@ -220,9 +111,11 @@ int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
>         sbi_ext = kvm_vcpu_sbi_find_ext(cp->a7);
>         if (sbi_ext && sbi_ext->handler) {
> +#ifdef CONFIG_RISCV_SBI_V01
>                 if (cp->a7 >= SBI_EXT_0_1_SET_TIMER &&
>                     cp->a7 <= SBI_EXT_0_1_SHUTDOWN)
>                         ext_is_v01 = true;
> +#endif
>                 ret = sbi_ext->handler(vcpu, run, &out_val, &utrap, &userspace_exit);
>         } else {
>                 /* Return error for unsupported SBI calls */
> @@ -262,13 +155,3 @@ int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
>         return ret;
>  }
> -
> -#else
> -
> -int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run)
> -{
> -       kvm_riscv_vcpu_sbi_forward(vcpu, run);
> -       return 0;
> -}
> -
> -#endif
> diff --git a/arch/riscv/kvm/vcpu_sbi_v01.c b/arch/riscv/kvm/vcpu_sbi_v01.c
> new file mode 100644
> index 000000000000..5a8e2afc8a57
> --- /dev/null
> +++ b/arch/riscv/kvm/vcpu_sbi_v01.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021 Western Digital Corporation or its affiliates.
> + *
> + * Authors:
> + *     Atish Patra <atish.patra@wdc.com>
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/kvm_host.h>
> +#include <asm/csr.h>
> +#include <asm/sbi.h>
> +#include <asm/kvm_vcpu_timer.h>
> +#include <asm/kvm_vcpu_sbi.h>
> +
> +static void kvm_sbi_system_shutdown(struct kvm_vcpu *vcpu,
> +                                   struct kvm_run *run, u32 type)
> +{
> +       int i;
> +       struct kvm_vcpu *tmp;
> +
> +       kvm_for_each_vcpu(i, tmp, vcpu->kvm)
> +               tmp->arch.power_off = true;
> +       kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_SLEEP);
> +
> +       memset(&run->system_event, 0, sizeof(run->system_event));
> +       run->system_event.type = type;
> +       run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +}
> +
> +static int kvm_sbi_ext_v01_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,
> +                                     unsigned long *out_val,
> +                                     struct kvm_cpu_trap *utrap,
> +                                     bool *exit)
> +{
> +       ulong hmask;
> +       int i, ret = 0;
> +       u64 next_cycle;
> +       struct kvm_vcpu *rvcpu;
> +       struct cpumask cm, hm;
> +       struct kvm *kvm = vcpu->kvm;
> +       struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
> +
> +       if (!cp)
> +               return -EINVAL;

This check on "cp" is unnecessary.

> +
> +       switch (cp->a7) {
> +       case SBI_EXT_0_1_CONSOLE_GETCHAR:
> +       case SBI_EXT_0_1_CONSOLE_PUTCHAR:
> +               /*
> +                * The CONSOLE_GETCHAR/CONSOLE_PUTCHAR SBI calls cannot be
> +                * handled in kernel so we forward these to user-space
> +                */
> +               kvm_riscv_vcpu_sbi_forward(vcpu, run);
> +               *exit = true;
> +               break;
> +       case SBI_EXT_0_1_SET_TIMER:
> +#if __riscv_xlen == 32
> +               next_cycle = ((u64)cp->a1 << 32) | (u64)cp->a0;
> +#else
> +               next_cycle = (u64)cp->a0;
> +#endif
> +               ret = kvm_riscv_vcpu_timer_next_event(vcpu, next_cycle);
> +               break;
> +       case SBI_EXT_0_1_CLEAR_IPI:
> +               ret = kvm_riscv_vcpu_unset_interrupt(vcpu, IRQ_VS_SOFT);
> +               break;
> +       case SBI_EXT_0_1_SEND_IPI:
> +               if (cp->a0)
> +                       hmask = kvm_riscv_vcpu_unpriv_read(vcpu, false, cp->a0,
> +                                                          utrap);
> +               else
> +                       hmask = (1UL << atomic_read(&kvm->online_vcpus)) - 1;
> +               if (utrap->scause)
> +                       break;
> +
> +               for_each_set_bit(i, &hmask, BITS_PER_LONG) {
> +                       rvcpu = kvm_get_vcpu_by_id(vcpu->kvm, i);
> +                       ret = kvm_riscv_vcpu_set_interrupt(rvcpu, IRQ_VS_SOFT);
> +                       if (ret < 0)
> +                               break;
> +               }
> +               break;
> +       case SBI_EXT_0_1_SHUTDOWN:
> +               kvm_sbi_system_shutdown(vcpu, run, KVM_SYSTEM_EVENT_SHUTDOWN);
> +               *exit = true;
> +               break;
> +       case SBI_EXT_0_1_REMOTE_FENCE_I:
> +       case SBI_EXT_0_1_REMOTE_SFENCE_VMA:
> +       case SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID:
> +               if (cp->a0)
> +                       hmask = kvm_riscv_vcpu_unpriv_read(vcpu, false, cp->a0,
> +                                                          utrap);
> +               else
> +                       hmask = (1UL << atomic_read(&kvm->online_vcpus)) - 1;
> +               if (utrap->scause)
> +                       break;
> +
> +               cpumask_clear(&cm);
> +               for_each_set_bit(i, &hmask, BITS_PER_LONG) {
> +                       rvcpu = kvm_get_vcpu_by_id(vcpu->kvm, i);
> +                       if (rvcpu->cpu < 0)
> +                               continue;
> +                       cpumask_set_cpu(rvcpu->cpu, &cm);
> +               }
> +               riscv_cpuid_to_hartid_mask(&cm, &hm);
> +               if (cp->a7 == SBI_EXT_0_1_REMOTE_FENCE_I)
> +                       ret = sbi_remote_fence_i(cpumask_bits(&hm));
> +               else if (cp->a7 == SBI_EXT_0_1_REMOTE_SFENCE_VMA)
> +                       ret = sbi_remote_hfence_vvma(cpumask_bits(&hm),
> +                                               cp->a1, cp->a2);
> +               else
> +                       ret = sbi_remote_hfence_vvma_asid(cpumask_bits(&hm),
> +                                               cp->a1, cp->a2, cp->a3);
> +               break;
> +       default:
> +               ret = -EINVAL;
> +               break;
> +       };
> +
> +       return ret;
> +}
> +
> +const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01 = {
> +       .extid_start = SBI_EXT_0_1_SET_TIMER,
> +       .extid_end = SBI_EXT_0_1_SHUTDOWN,
> +       .handler = kvm_sbi_ext_v01_handler,
> +};
> --
> 2.31.1
>

Otherwise it looks good to me.

Reviewed-by: Anup Patel <anup.patel@wdc.com>

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2021-11-06  4:43 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-05 23:58 [PATCH v4 0/5] Add SBI v0.2 support for KVM Atish Patra
2021-11-05 23:58 ` Atish Patra
2021-11-05 23:58 ` [PATCH v4 1/5] RISC-V: KVM: Mark the existing SBI implementation as v01 Atish Patra
2021-11-05 23:58   ` Atish Patra
2021-11-06  4:27   ` Heinrich Schuchardt
2021-11-06  4:27     ` Heinrich Schuchardt
2021-11-06  4:35     ` Anup Patel
2021-11-06  4:35       ` Anup Patel
2021-11-08 16:45     ` Atish Patra
2021-11-08 16:45       ` Atish Patra
2021-11-06  4:41   ` Anup Patel
2021-11-06  4:41     ` Anup Patel
2021-11-05 23:58 ` [PATCH v4 2/5] RISC-V: KVM: Reorganize SBI code by moving SBI v0.1 to its own file Atish Patra
2021-11-05 23:58   ` Atish Patra
2021-11-06  4:43   ` Anup Patel [this message]
2021-11-06  4:43     ` Anup Patel
2021-11-05 23:58 ` [PATCH v4 3/5] RISC-V: KVM: Add SBI v0.2 base extension Atish Patra
2021-11-05 23:58   ` Atish Patra
2021-11-06  4:45   ` Anup Patel
2021-11-06  4:45     ` Anup Patel
2021-11-05 23:58 ` [PATCH v4 4/5] RISC-V: KVM: Add v0.1 replacement SBI extensions defined in v02 Atish Patra
2021-11-05 23:58   ` Atish Patra
2021-11-06  4:46   ` Anup Patel
2021-11-06  4:46     ` Anup Patel
2021-11-05 23:58 ` [PATCH v4 5/5] RISC-V: KVM: Add SBI HSM extension in KVM Atish Patra
2021-11-05 23:58   ` Atish Patra
2021-11-06  4:50   ` Anup Patel
2021-11-06  4:50     ` Anup Patel

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=CAAhSdy12kq6AHwRkgAoPaSxSDp6LM3oFj03Zyt8_jkQtBZRLZA@mail.gmail.com \
    --to=anup@brainfault.org \
    --cc=anup.patel@wdc.com \
    --cc=atish.patra@wdc.com \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=vincent.chen@sifive.com \
    --cc=xypron.glpk@gmx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.