Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] KVM: nVMX: cleanup and fix host 64-bit mode checks
To:     Krish Sadhukhan <krish.sadhukhan@oracle.com>,
        Jim Mattson <jmattson@google.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        kvm list <kvm@vger.kernel.org>,
        Sean Christopherson <sean.j.christopherson@intel.com>
References: <1569429286-35157-1-git-send-email-pbonzini@redhat.com>
 <CALMp9eTBPTnsRDipdGDgmugWgfFEjQ2wd_9-JY0ZeM9YG2fBjg@mail.gmail.com>
 <3460bd57-6fdd-f73c-9ce0-c97d4cc85f63@oracle.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
Openpgp: preference=signencrypt
Message-ID: <1543003a-9a2f-2a52-444a-d55bde6b8e2f@redhat.com>
Date:   Thu, 26 Sep 2019 10:42:38 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <3460bd57-6fdd-f73c-9ce0-c97d4cc85f63@oracle.com>
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 26/09/19 01:55, Krish Sadhukhan wrote:
>=20
>=20
> On 09/25/2019 09:47 AM, Jim Mattson wrote:
>> On Wed, Sep 25, 2019 at 9:34 AM Paolo Bonzini <pbonzini@redhat.com>
>> wrote:
>>> KVM was incorrectly checking vmcs12->host_ia32_efer even if the "load
>>> IA32_EFER" exit control was reset.=C2=A0 Also, some checks were not usi=
ng
>>> the new CC macro for tracing.
>>>
>>> Cleanup everything so that the vCPU's 64-bit mode is determined
>>> directly from EFER_LMA and the VMCS checks are based on that, which
>>> matches section 26.2.4 of the SDM.
>>>
>>> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
>>> Cc: Jim Mattson <jmattson@google.com>
>>> Cc: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>>> Fixes: 5845038c111db27902bc220a4f70070fe945871c
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>> =C2=A0 arch/x86/kvm/vmx/nested.c | 53
>>> ++++++++++++++++++++---------------------------
>>> =C2=A0 1 file changed, 22 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>>> index 70d59d9304f2..e108847f6cf8 100644
>>> --- a/arch/x86/kvm/vmx/nested.c
>>> +++ b/arch/x86/kvm/vmx/nested.c
>>> @@ -2664,8 +2664,26 @@ static int nested_vmx_check_host_state(struct
>>> kvm_vcpu *vcpu,
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 CC(!kvm_pat_valid(vmcs12->host_ia32_pat)))
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 return -EINVAL;
>>>
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ia32e =3D (vmcs12->vm_exit_contro=
ls &
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 VM_EXIT_HOST_ADDR_SPACE_SIZE) !=3D 0;
>>> +#ifdef CONFIG_X86_64
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ia32e =3D !!(vcpu->arch.efer & EF=
ER_LMA);
>>> +#else
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (CC(vmcs12->vm_entry_controls =
& VM_ENTRY_IA32E_MODE))
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 return -EINVAL;
>> This check is redundant, since it is checked in the else block below.
>=20
> Should we be re-using is_long_mode() instead of duplicating the code ?

Of course!  I have already pushed the patch, but I will send a follow up.

Paolo