| Message ID | 20221215170046.2010255-4-atishp@rivosinc.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | KVM perf support | expand |
On Thu, Dec 15, 2022 at 09:00:38AM -0800, Atish Patra wrote: > According to the SBI specification, the stop function can only > return error code SBI_ERR_FAILED. However, currently it returns > -EINVAL which will be mapped SBI_ERR_INVALID_PARAM. I presume the mapping referred to here is kvm_linux_err_map_sbi(). If so, then -EPERM isn't correct either. That maps to SBI_ERR_DENIED. The only thing that will ensure we get SBI_ERR_FAILURE (-1) is anything not handled by the kvm_linux_err_map_sbi switch, as we need to use the default. Thanks, drew > > Return the appropriate linux error code. > > Signed-off-by: Atish Patra <atishp@rivosinc.com> > --- > arch/riscv/kvm/vcpu_sbi_hsm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/kvm/vcpu_sbi_hsm.c b/arch/riscv/kvm/vcpu_sbi_hsm.c > index 2e915ca..0f8d9fe 100644 > --- a/arch/riscv/kvm/vcpu_sbi_hsm.c > +++ b/arch/riscv/kvm/vcpu_sbi_hsm.c > @@ -42,7 +42,7 @@ static int kvm_sbi_hsm_vcpu_start(struct kvm_vcpu *vcpu) > static int kvm_sbi_hsm_vcpu_stop(struct kvm_vcpu *vcpu) > { > if (vcpu->arch.power_off) > - return -EINVAL; > + return -EPERM; > > kvm_riscv_vcpu_power_off(vcpu); > > -- > 2.25.1 >
On Thu, Jan 12, 2023 at 2:28 AM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Thu, Dec 15, 2022 at 09:00:38AM -0800, Atish Patra wrote: > > According to the SBI specification, the stop function can only > > return error code SBI_ERR_FAILED. However, currently it returns > > -EINVAL which will be mapped SBI_ERR_INVALID_PARAM. > > I presume the mapping referred to here is kvm_linux_err_map_sbi(). > If so, then -EPERM isn't correct either. That maps to SBI_ERR_DENIED. > The only thing that will ensure we get SBI_ERR_FAILURE (-1) is > anything not handled by the kvm_linux_err_map_sbi switch, as we > need to use the default. > It returns SBI_ERR_FAILURE in the next patch when kvm_linux_err_map_sbi is removed. Maybe I should drop this patch. The next patch does the correct thing anyways. > Thanks, > drew > > > > > Return the appropriate linux error code. > > > > Signed-off-by: Atish Patra <atishp@rivosinc.com> > > --- > > arch/riscv/kvm/vcpu_sbi_hsm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/kvm/vcpu_sbi_hsm.c b/arch/riscv/kvm/vcpu_sbi_hsm.c > > index 2e915ca..0f8d9fe 100644 > > --- a/arch/riscv/kvm/vcpu_sbi_hsm.c > > +++ b/arch/riscv/kvm/vcpu_sbi_hsm.c > > @@ -42,7 +42,7 @@ static int kvm_sbi_hsm_vcpu_start(struct kvm_vcpu *vcpu) > > static int kvm_sbi_hsm_vcpu_stop(struct kvm_vcpu *vcpu) > > { > > if (vcpu->arch.power_off) > > - return -EINVAL; > > + return -EPERM; > > > > kvm_riscv_vcpu_power_off(vcpu); > > > > -- > > 2.25.1 > >
On Thu, Jan 12, 2023 at 10:25:06AM -0800, Atish Kumar Patra wrote: > On Thu, Jan 12, 2023 at 2:28 AM Andrew Jones <ajones@ventanamicro.com> wrote: > > > > On Thu, Dec 15, 2022 at 09:00:38AM -0800, Atish Patra wrote: > > > According to the SBI specification, the stop function can only > > > return error code SBI_ERR_FAILED. However, currently it returns > > > -EINVAL which will be mapped SBI_ERR_INVALID_PARAM. > > > > I presume the mapping referred to here is kvm_linux_err_map_sbi(). > > If so, then -EPERM isn't correct either. That maps to SBI_ERR_DENIED. > > The only thing that will ensure we get SBI_ERR_FAILURE (-1) is > > anything not handled by the kvm_linux_err_map_sbi switch, as we > > need to use the default. > > > > It returns SBI_ERR_FAILURE in the next patch when kvm_linux_err_map_sbi > is removed. Maybe I should drop this patch. The next patch does the > correct thing anyways. Yeah, I saw that, but then we silently fix a bug in the next patch. I like that this is a separate patch, but it should do what it says it's doing :-) Thanks, drew
diff --git a/arch/riscv/kvm/vcpu_sbi_hsm.c b/arch/riscv/kvm/vcpu_sbi_hsm.c index 2e915ca..0f8d9fe 100644 --- a/arch/riscv/kvm/vcpu_sbi_hsm.c +++ b/arch/riscv/kvm/vcpu_sbi_hsm.c @@ -42,7 +42,7 @@ static int kvm_sbi_hsm_vcpu_start(struct kvm_vcpu *vcpu) static int kvm_sbi_hsm_vcpu_stop(struct kvm_vcpu *vcpu) { if (vcpu->arch.power_off) - return -EINVAL; + return -EPERM; kvm_riscv_vcpu_power_off(vcpu);
According to the SBI specification, the stop function can only return error code SBI_ERR_FAILED. However, currently it returns -EINVAL which will be mapped SBI_ERR_INVALID_PARAM. Return the appropriate linux error code. Signed-off-by: Atish Patra <atishp@rivosinc.com> --- arch/riscv/kvm/vcpu_sbi_hsm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)