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?
/* 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? :)
