On 3/29/2023 7:33 AM, Huang, Kai wrote:
On Tue, 2023-03-21 at 14:35 -0700, Sean Christopherson wrote:
On Mon, Mar 20, 2023, Chao Gao wrote:
On Sun, Mar 19, 2023 at 04:49:22PM +0800, Binbin Wu wrote:
get_vmx_mem_address() and sgx_get_encls_gva() use is_long_mode()
to check 64-bit mode. Should use is_64_bit_mode() instead.
Fixes: f9eb4af67c9d ("KVM: nVMX: VMX instructions: add checks for #GP/#SS exceptions")
Fixes: 70210c044b4e ("KVM: VMX: Add SGX ENCLS[ECREATE] handler to enforce CPUID restrictions")
It is better to split this patch into two: one for nested and one for
SGX.
It is possible that there is a kernel release which has just one of
above two flawed commits, then this fix patch cannot be applied cleanly
to the release.
The nVMX code isn't buggy, VMX instructions #UD in compatibility mode, and except
for VMCALL, that #UD has higher priority than VM-Exit interception. So I'd say
just drop the nVMX side of things.
But it looks the old code doesn't unconditionally inject #UD when in
compatibility mode?
I think Sean means VMX instructions is not valid in compatibility mode
and it triggers #UD, which has higher priority than VM-Exit, by the
processor in non-root mode.
So if there is a VM-Exit due to VMX instruction , it is in 64-bit mode
for sure if it is in long mode.
/* Checks for #GP/#SS exceptions. */
exn = false;
if (is_long_mode(vcpu)) {
/*
* The virtual/linear address is never truncated in 64-bit
* mode, e.g. a 32-bit address size can yield a 64-bit virtual
* address when using FS/GS with a non-zero base.
*/
if (seg_reg == VCPU_SREG_FS || seg_reg == VCPU_SREG_GS)
*ret = s.base + off;
else
*ret = off;
/* Long mode: #GP(0)/#SS(0) if the memory address is in a
* non-canonical form. This is the only check on the memory
* destination for long mode!
*/
exn = is_noncanonical_address(*ret, vcpu);
}
...
The logic of only adding seg.base for FS/GS to linear address (and ignoring
seg.base for all other segs) only applies to 64 bit mode, but the code only
checks _long_ mode.
Am I missing something?
I could have sworn ENCLS had the same behavior, but the SDM disagrees. Though why
on earth ENCLS is allowed in compatibility mode is beyond me. ENCLU I can kinda
sorta understand, but ENCLS?!?!!
I can reach out to Intel guys to (try to) find the answer if you want me to? :)
