2019-11-27 03:09:34

by Haiwei Li

Subject: [PATCH] KVM: SVM: Fix "error" isn't initialized

From d32ebcf6f426385942fe6c469255e73188cd7d38 Mon Sep 17 00:00:00 2001
From: Haiwei Li <[email protected]>
Date: Wed, 27 Nov 2019 11:03:21 +0800
Subject: [PATCH] initialize 'error'

There are a bunch of error paths were "error" isn't initialized.

Signed-off-by: Haiwei Li <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
---
arch/x86/kvm/svm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 362e874..0b3d49c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -6294,7 +6294,8 @@ static int enable_smi_window(struct kvm_vcpu *vcpu)

static int sev_flush_asids(void)
{
- int ret, error;
+ int ret;
+ int error = 0;

/*
* DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,
--
1.8.3.1

2019-11-27 03:36:05

by Miaohe Lin

Subject: Re: [PATCH] KVM: SVM: Fix "error" isn't initialized

> From: Haiwei Li <[email protected]>
> Subject: [PATCH] initialize 'error'
>
> There are a bunch of error paths were "error" isn't initialized.
Hi,
In case error case, sev_guest_df_flush() do not set the error.
Can you set the value of error to reflect what error happened
in sev_guest_df_flush()?
The current fix may looks confused when print "DF_FLUSH failed" with
error = 0.
Thanks.

PS: This is just my personal point.
>
> Signed-off-by: Haiwei Li <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
>--- a/arch/x86/kvm/svm.c
>+++ b/arch/x86/kvm/svm.c
>@@ -6294,7 +6294,8 @@ static int enable_smi_window(struct kvm_vcpu *vcpu)
>
> static int sev_flush_asids(void)
> {
>- int ret, error;
>+ int ret;
>+ int error = 0;
>
> /*
> * DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,

2019-11-27 03:46:58

by Sean Christopherson

Subject: Re: [PATCH] KVM: SVM: Fix "error" isn't initialized

On Wed, Nov 27, 2019 at 03:30:06AM +0000, linmiaohe wrote:
>
> > From: Haiwei Li <[email protected]>
> > Subject: [PATCH] initialize 'error'
> >
> > There are a bunch of error paths were "error" isn't initialized.
> Hi,
> In case error case, sev_guest_df_flush() do not set the error.
> Can you set the value of error to reflect what error happened
> in sev_guest_df_flush()?
> The current fix may looks confused when print "DF_FLUSH failed" with
> error = 0.
> Thanks.
>
> PS: This is just my personal point.

Disclaimer: not my world at all...

Based on the prototype for __sev_do_cmd_locked(), @error is intended to be
filled only if there's an actual response from the PSP, which is a 16-bit
value. So maybe init @psp_ret at the beginning of __sev_do_cmd_locked() to
-1 to indicate the command was never sent to the PSP? And update the
pr_err() in sev_asid_flush() to explicitly state it's the PSP return?

2019-11-27 05:42:21

by Haiwei Li

Subject: Re: [PATCH] KVM: SVM: Fix "error" isn't initialized

On 2019/11/27 11:44, Sean Christopherson wrote:
> On Wed, Nov 27, 2019 at 03:30:06AM +0000, linmiaohe wrote:
>>
>>> From: Haiwei Li <[email protected]>
>>> Subject: [PATCH] initialize 'error'
>>>
>>> There are a bunch of error paths were "error" isn't initialized.
>> Hi,
>> In case error case, sev_guest_df_flush() do not set the error.
>> Can you set the value of error to reflect what error happened
>> in sev_guest_df_flush()?
>> The current fix may looks confused when print "DF_FLUSH failed" with
>> error = 0.
>> Thanks.
>>
>> PS: This is just my personal point.
>
> Disclaimer: not my world at all...
>
> Based on the prototype for __sev_do_cmd_locked(), @error is intended to be
> filled only if there's an actual response from the PSP, which is a 16-bit
> value. So maybe init @psp_ret at the beginning of __sev_do_cmd_locked() to
> -1 to indicate the command was never sent to the PSP? And update the
> pr_err() in sev_asid_flush() to explicitly state it's the PSP return?
>

Thanks for your advise. Good point. I will send a new patch.

2019-11-27 06:30:10

by Miaohe Lin

Subject: Re: [PATCH] KVM: SVM: Fix "error" isn't initialized

On 2019/11/27 11:44, Sean Christopherson wrote:
> On Wed, Nov 27, 2019 at 03:30:06AM +0000, linmiaohe wrote:
>>
>>> From: Haiwei Li <[email protected]>
>>> Subject: [PATCH] initialize 'error'
>>>
>>> There are a bunch of error paths were "error" isn't initialized.
>> Hi,
>> In case error case, sev_guest_df_flush() do not set the error.
>> Can you set the value of error to reflect what error happened in
>> sev_guest_df_flush()?
>> The current fix may looks confused when print "DF_FLUSH failed" with
>> error = 0.
>> Thanks.
>>
>> PS: This is just my personal point.
>
> Disclaimer: not my world at all...
>
> Based on the prototype for __sev_do_cmd_locked(), @error is intended
> to be filled only if there's an actual response from the PSP, which is
> a 16-bit value. So maybe init @psp_ret at the beginning of
> __sev_do_cmd_locked() to
> -1 to indicate the command was never sent to the PSP? And update the
> pr_err() in sev_asid_flush() to explicitly state it's the PSP return?
>

I think it's a good alternative. Many Thanks.