Re: [PATCH v5 11/22] RISC-V: KVM: No need to exit to the user space if perf event failed

Andrew Jones
2024-04-04T12:19:33Z

On Wed, Apr 03, 2024 at 01:04:40AM -0700, Atish Patra wrote:
> Currently, we return a linux error code if creating a perf event failed
> in kvm. That shouldn't be necessary as guest can continue to operate
> without perf profiling or profiling with firmware counters.
>
> Return appropriate SBI error code to indicate that PMU configuration
> failed. An error message in kvm already describes the reason for failure.
>
> Fixes: 0cb74b65d2e5 ("RISC-V: KVM: Implement perf support without sampling")
> Reviewed-by: Anup Patel <anup@brainfault.org>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
> arch/riscv/kvm/vcpu_pmu.c | 14 +++++++++-----
> arch/riscv/kvm/vcpu_sbi_pmu.c | 6 +++---
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
> index b5159ce4592d..2d9929bbc2c8 100644
> --- a/arch/riscv/kvm/vcpu_pmu.c
> +++ b/arch/riscv/kvm/vcpu_pmu.c
> @@ -229,8 +229,9 @@ static int kvm_pmu_validate_counter_mask(struct kvm_pmu *kvpmu, unsigned long ct
> return 0;
> }
>
> -static int kvm_pmu_create_perf_event(struct kvm_pmc *pmc, struct perf_event_attr *attr,
> - unsigned long flags, unsigned long eidx, unsigned long evtdata)
> +static long kvm_pmu_create_perf_event(struct kvm_pmc *pmc, struct perf_event_attr *attr,
> + unsigned long flags, unsigned long eidx,
> + unsigned long evtdata)
> {
> struct perf_event *event;
>
> @@ -454,7 +455,8 @@ int kvm_riscv_vcpu_pmu_ctr_cfg_match(struct kvm_vcpu *vcpu, unsigned long ctr_ba
> unsigned long eidx, u64 evtdata,
> struct kvm_vcpu_sbi_return *retdata)
> {
> - int ctr_idx, ret, sbiret = 0;
> + int ctr_idx, sbiret = 0;
> + long ret;
> bool is_fevent;
> unsigned long event_code;
> u32 etype = kvm_pmu_get_perf_event_type(eidx);
> @@ -513,8 +515,10 @@ int kvm_riscv_vcpu_pmu_ctr_cfg_match(struct kvm_vcpu *vcpu, unsigned long ctr_ba
> kvpmu->fw_event[event_code].started = true;
> } else {
> ret = kvm_pmu_create_perf_event(pmc, &attr, flags, eidx, evtdata);
> - if (ret)
> - return ret;
> + if (ret) {
> + sbiret = SBI_ERR_NOT_SUPPORTED;

I'm still not sure about this. I replied in the v4 thread about it.

Thanks,
drew

> + goto out;
> + }
> }
>
> set_bit(ctr_idx, kvpmu->pmc_in_use);
> diff --git a/arch/riscv/kvm/vcpu_sbi_pmu.c b/arch/riscv/kvm/vcpu_sbi_pmu.c
> index 7eca72df2cbd..e1633606c98b 100644
> --- a/arch/riscv/kvm/vcpu_sbi_pmu.c
> +++ b/arch/riscv/kvm/vcpu_sbi_pmu.c
> @@ -42,9 +42,9 @@ static int kvm_sbi_ext_pmu_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,
> #endif
> /*
> * This can fail if perf core framework fails to create an event.
> - * Forward the error to userspace because it's an error which
> - * happened within the host kernel. The other option would be
> - * to convert to an SBI error and forward to the guest.
> + * No need to forward the error to userspace and exit the guest.
> + * The operation can continue without profiling. Forward the
> + * appropriate SBI error to the guest.
> */
> ret = kvm_riscv_vcpu_pmu_ctr_cfg_match(vcpu, cp->a0, cp->a1,
> cp->a2, cp->a3, temp, retdata);
> --
> 2.34.1
>