| Message ID | 20230404130923.27749-5-binbin.wu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Linear Address Masking (LAM) KVM Enabling | expand |
On Tue, 2023-04-04 at 21:09 +0800, Binbin Wu wrote: > case VMX_VPID_EXTENT_INDIVIDUAL_ADDR: > + /* invvpid is not valid in compatibility mode */ > + if (is_long_mode(vcpu)) > + operand.gla = vmx_untag_addr(vcpu, operand.gla, 0); This comment doesn't make sense. The code does nothing to distinguish the compatibility mode and the 64-bit mode. Now although we are all clear that here is_long_mode() basically equals to is_64_bit_mode(), but I do think we need a comment or WARN() _SOMEWHERE_ to indicate that compatibility mode is not possible when handling VMEXIT for VMX instructions (except VMCALL). Not everyone will be able to notice this small thing in the SDM. Then you can just delete this comment here. Alternatively, for better readability actually I am thinking maybe we should just use is_64_bit_mode(), because those segments are cached by KVM anyway so I don't think there's measurable performance difference between is_long_mode() and is_64_bit_mode(). Sean, any comments?
On 4/6/2023 9:20 PM, Huang, Kai wrote: > On Tue, 2023-04-04 at 21:09 +0800, Binbin Wu wrote: >> case VMX_VPID_EXTENT_INDIVIDUAL_ADDR: >> + /* invvpid is not valid in compatibility mode */ >> + if (is_long_mode(vcpu)) >> + operand.gla = vmx_untag_addr(vcpu, operand.gla, 0); > This comment doesn't make sense. The code does nothing to distinguish the > compatibility mode and the 64-bit mode. I was also hesitant when added the comment. > > Now although we are all clear that here is_long_mode() basically equals to > is_64_bit_mode(), but I do think we need a comment or WARN() _SOMEWHERE_ to > indicate that compatibility mode is not possible when handling VMEXIT for VMX > instructions (except VMCALL). Not everyone will be able to notice this small > thing in the SDM. If the WARN() is preferred, IMO, it can be added to nested_vmx_check_permission() because it is called by all handlers "need" the check except for handle_vmxon(). handle_vmxon() can be added separately. > > Then you can just delete this comment here. > > Alternatively, for better readability actually I am thinking maybe we should > just use is_64_bit_mode(), because those segments are cached by KVM anyway so I > don't think there's measurable performance difference between is_long_mode() and > is_64_bit_mode(). Agree. > > Sean, any comments?
On 4/4/2023 9:09 PM, Binbin Wu wrote: > Untag address for 64-bit memory/mmio operand in instruction emulations > and vmexit handlers when LAM is applicable. > > For instruction emulation, untag address in __linearize() before > canonical check. LAM doesn't apply to instruction fetch and invlpg, > use KVM_X86_UNTAG_ADDR_SKIP_LAM to skip LAM untag. > > For vmexit handlings related to 64-bit linear address: > - Cases need to untag address > Operand(s) of VMX instructions and INVPCID > Operand(s) of SGX ENCLS > Linear address in INVVPID descriptor. > - Cases LAM doesn't apply to (no change needed) > Operand of INVLPG > Linear address in INVPCID descriptor > > Co-developed-by: Robert Hoo <robert.hu@linux.intel.com> > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com> > Tested-by: Xuelian Guo <xuelian.guo@intel.com> > --- > arch/x86/kvm/emulate.c | 23 ++++++++++++++++++----- > arch/x86/kvm/kvm_emulate.h | 2 ++ > arch/x86/kvm/vmx/nested.c | 4 ++++ > arch/x86/kvm/vmx/sgx.c | 1 + > arch/x86/kvm/x86.c | 10 ++++++++++ > 5 files changed, 35 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index a20bec931764..b7df465eccf2 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -688,7 +688,8 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > struct segmented_address addr, > unsigned *max_size, unsigned size, > bool write, bool fetch, > - enum x86emul_mode mode, ulong *linear) > + enum x86emul_mode mode, ulong *linear, > + u64 untag_flags) IMO, here should be "u64 flags" instead of "u64 untag_flags". Emulator can use it as flag combination for other purpose. > { > struct desc_struct desc; > bool usable; > @@ -701,6 +702,7 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > *max_size = 0; > switch (mode) { > case X86EMUL_MODE_PROT64: > + la = ctxt->ops->untag_addr(ctxt, la, untag_flags); > *linear = la; > va_bits = ctxt_virt_addr_bits(ctxt); > if (!__is_canonical_address(la, va_bits)) > @@ -758,7 +760,7 @@ static int linearize(struct x86_emulate_ctxt *ctxt, > { > unsigned max_size; > return __linearize(ctxt, addr, &max_size, size, write, false, > - ctxt->mode, linear); > + ctxt->mode, linear, 0); > } > > static inline int assign_eip(struct x86_emulate_ctxt *ctxt, ulong dst) > @@ -771,7 +773,12 @@ static inline int assign_eip(struct x86_emulate_ctxt *ctxt, ulong dst) > > if (ctxt->op_bytes != sizeof(unsigned long)) > addr.ea = dst & ((1UL << (ctxt->op_bytes << 3)) - 1); > - rc = __linearize(ctxt, addr, &max_size, 1, false, true, ctxt->mode, &linear); > + /* > + * LAM does not apply to addresses used for instruction fetches > + * or to those that specify the targets of jump and call instructions > + */ This api handles the target address of branch and call instructions. I think it enough to only explain the exact case. > + rc = __linearize(ctxt, addr, &max_size, 1, false, true, ctxt->mode, > + &linear, KVM_X86_UNTAG_ADDR_SKIP_LAM); > if (rc == X86EMUL_CONTINUE) > ctxt->_eip = addr.ea; > return rc; > @@ -906,9 +913,12 @@ static int __do_insn_fetch_bytes(struct x86_emulate_ctxt *ctxt, int op_size) > * __linearize is called with size 0 so that it does not do any > * boundary check itself. Instead, we use max_size to check > * against op_size. > + * > + * LAM does not apply to addresses used for instruction fetches > + * or to those that specify the targets of jump and call instructions Ditto. > */ > rc = __linearize(ctxt, addr, &max_size, 0, false, true, ctxt->mode, > - &linear); > + &linear, KVM_X86_UNTAG_ADDR_SKIP_LAM); > if (unlikely(rc != X86EMUL_CONTINUE)) > return rc; > >
On 4/18/2023 11:28 AM, Zeng Guang wrote: > > On 4/4/2023 9:09 PM, Binbin Wu wrote: >> Untag address for 64-bit memory/mmio operand in instruction emulations >> and vmexit handlers when LAM is applicable. >> >> For instruction emulation, untag address in __linearize() before >> canonical check. LAM doesn't apply to instruction fetch and invlpg, >> use KVM_X86_UNTAG_ADDR_SKIP_LAM to skip LAM untag. >> >> For vmexit handlings related to 64-bit linear address: >> - Cases need to untag address >> Operand(s) of VMX instructions and INVPCID >> Operand(s) of SGX ENCLS >> Linear address in INVVPID descriptor. >> - Cases LAM doesn't apply to (no change needed) >> Operand of INVLPG >> Linear address in INVPCID descriptor >> >> Co-developed-by: Robert Hoo <robert.hu@linux.intel.com> >> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> >> Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com> >> Tested-by: Xuelian Guo <xuelian.guo@intel.com> >> --- >> arch/x86/kvm/emulate.c | 23 ++++++++++++++++++----- >> arch/x86/kvm/kvm_emulate.h | 2 ++ >> arch/x86/kvm/vmx/nested.c | 4 ++++ >> arch/x86/kvm/vmx/sgx.c | 1 + >> arch/x86/kvm/x86.c | 10 ++++++++++ >> 5 files changed, 35 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index a20bec931764..b7df465eccf2 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -688,7 +688,8 @@ static __always_inline int __linearize(struct >> x86_emulate_ctxt *ctxt, >> struct segmented_address addr, >> unsigned *max_size, unsigned size, >> bool write, bool fetch, >> - enum x86emul_mode mode, ulong *linear) >> + enum x86emul_mode mode, ulong *linear, >> + u64 untag_flags) > > IMO, here should be "u64 flags" instead of "u64 untag_flags". Emulator > can > use it as flag combination for other purpose. yes, make sense with the advise you suggested in patch 3. > > >> { >> struct desc_struct desc; >> bool usable; >> @@ -701,6 +702,7 @@ static __always_inline int __linearize(struct >> x86_emulate_ctxt *ctxt, >> *max_size = 0; >> switch (mode) { >> case X86EMUL_MODE_PROT64: >> + la = ctxt->ops->untag_addr(ctxt, la, untag_flags); >> *linear = la; >> va_bits = ctxt_virt_addr_bits(ctxt); >> if (!__is_canonical_address(la, va_bits)) >> @@ -758,7 +760,7 @@ static int linearize(struct x86_emulate_ctxt *ctxt, >> { >> unsigned max_size; >> return __linearize(ctxt, addr, &max_size, size, write, false, >> - ctxt->mode, linear); >> + ctxt->mode, linear, 0); >> } >> static inline int assign_eip(struct x86_emulate_ctxt *ctxt, ulong >> dst) >> @@ -771,7 +773,12 @@ static inline int assign_eip(struct >> x86_emulate_ctxt *ctxt, ulong dst) >> if (ctxt->op_bytes != sizeof(unsigned long)) >> addr.ea = dst & ((1UL << (ctxt->op_bytes << 3)) - 1); >> - rc = __linearize(ctxt, addr, &max_size, 1, false, true, >> ctxt->mode, &linear); >> + /* >> + * LAM does not apply to addresses used for instruction fetches >> + * or to those that specify the targets of jump and call >> instructions >> + */ > > This api handles the target address of branch and call instructions. I > think it enough to only explain the exact case. OK, will make it specific. > >> + rc = __linearize(ctxt, addr, &max_size, 1, false, true, ctxt->mode, >> + &linear, KVM_X86_UNTAG_ADDR_SKIP_LAM); >> if (rc == X86EMUL_CONTINUE) >> ctxt->_eip = addr.ea; >> return rc; >> @@ -906,9 +913,12 @@ static int __do_insn_fetch_bytes(struct >> x86_emulate_ctxt *ctxt, int op_size) >> * __linearize is called with size 0 so that it does not do any >> * boundary check itself. Instead, we use max_size to check >> * against op_size. >> + * >> + * LAM does not apply to addresses used for instruction fetches >> + * or to those that specify the targets of jump and call >> instructions > > Ditto. > >> */ >> rc = __linearize(ctxt, addr, &max_size, 0, false, true, >> ctxt->mode, >> - &linear); >> + &linear, KVM_X86_UNTAG_ADDR_SKIP_LAM); >> if (unlikely(rc != X86EMUL_CONTINUE)) >> return rc; >>
On Tue, Apr 04, 2023 at 09:09:22PM +0800, Binbin Wu wrote: >Untag address for 64-bit memory/mmio operand in instruction emulations >and vmexit handlers when LAM is applicable. > >For instruction emulation, untag address in __linearize() before >canonical check. LAM doesn't apply to instruction fetch and invlpg, >use KVM_X86_UNTAG_ADDR_SKIP_LAM to skip LAM untag. > >For vmexit handlings related to 64-bit linear address: >- Cases need to untag address > Operand(s) of VMX instructions and INVPCID > Operand(s) of SGX ENCLS > Linear address in INVVPID descriptor. >- Cases LAM doesn't apply to (no change needed) > Operand of INVLPG > Linear address in INVPCID descriptor > >Co-developed-by: Robert Hoo <robert.hu@linux.intel.com> >Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> >Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com> >Tested-by: Xuelian Guo <xuelian.guo@intel.com> >--- > arch/x86/kvm/emulate.c | 23 ++++++++++++++++++----- > arch/x86/kvm/kvm_emulate.h | 2 ++ > arch/x86/kvm/vmx/nested.c | 4 ++++ > arch/x86/kvm/vmx/sgx.c | 1 + > arch/x86/kvm/x86.c | 10 ++++++++++ > 5 files changed, 35 insertions(+), 5 deletions(-) > >diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >index a20bec931764..b7df465eccf2 100644 >--- a/arch/x86/kvm/emulate.c >+++ b/arch/x86/kvm/emulate.c >@@ -688,7 +688,8 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > struct segmented_address addr, > unsigned *max_size, unsigned size, > bool write, bool fetch, >- enum x86emul_mode mode, ulong *linear) >+ enum x86emul_mode mode, ulong *linear, >+ u64 untag_flags) @write and @fetch are like flags. I think we can consolidate them into the @flags first as a cleanup patch and then add a flag for LAM.
On 4/19/2023 2:43 PM, Chao Gao wrote: > On Tue, Apr 04, 2023 at 09:09:22PM +0800, Binbin Wu wrote: >> Untag address for 64-bit memory/mmio operand in instruction emulations >> and vmexit handlers when LAM is applicable. >> >> For instruction emulation, untag address in __linearize() before >> canonical check. LAM doesn't apply to instruction fetch and invlpg, >> use KVM_X86_UNTAG_ADDR_SKIP_LAM to skip LAM untag. >> >> For vmexit handlings related to 64-bit linear address: >> - Cases need to untag address >> Operand(s) of VMX instructions and INVPCID >> Operand(s) of SGX ENCLS >> Linear address in INVVPID descriptor. >> - Cases LAM doesn't apply to (no change needed) >> Operand of INVLPG >> Linear address in INVPCID descriptor >> >> Co-developed-by: Robert Hoo <robert.hu@linux.intel.com> >> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> >> Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com> >> Tested-by: Xuelian Guo <xuelian.guo@intel.com> >> --- >> arch/x86/kvm/emulate.c | 23 ++++++++++++++++++----- >> arch/x86/kvm/kvm_emulate.h | 2 ++ >> arch/x86/kvm/vmx/nested.c | 4 ++++ >> arch/x86/kvm/vmx/sgx.c | 1 + >> arch/x86/kvm/x86.c | 10 ++++++++++ >> 5 files changed, 35 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index a20bec931764..b7df465eccf2 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -688,7 +688,8 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, >> struct segmented_address addr, >> unsigned *max_size, unsigned size, >> bool write, bool fetch, >> - enum x86emul_mode mode, ulong *linear) >> + enum x86emul_mode mode, ulong *linear, >> + u64 untag_flags) > @write and @fetch are like flags. I think we can consolidate them into > the @flags first as a cleanup patch and then add a flag for LAM. OK. Here is the proposed cleanup patch: --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -687,8 +687,8 @@ static unsigned insn_alignment(struct x86_emulate_ctxt *ctxt, unsigned size) static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, struct segmented_address addr, unsigned *max_size, unsigned size, - bool write, bool fetch, - enum x86emul_mode mode, ulong *linear) + u64 flags, enum x86emul_mode mode, + ulong *linear) { struct desc_struct desc; bool usable; @@ -696,6 +696,8 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, u32 lim; u16 sel; u8 va_bits; + bool fetch = !!(flags & KVM_X86_EMULFLAG_FETCH); + bool write = !!(flags & KVM_X86_EMULFLAG_WRITE); la = seg_base(ctxt, addr.seg) + addr.ea; *max_size = 0; @@ -757,7 +759,12 @@ static int linearize(struct x86_emulate_ctxt *ctxt, ulong *linear) { unsigned max_size; - return __linearize(ctxt, addr, &max_size, size, write, false, + u64 flags = 0; + + if (write) + flags |= KVM_X86_EMULFLAG_WRITE; + + return __linearize(ctxt, addr, &max_size, size, flags, ctxt->mode, linear); } @@ -768,10 +775,11 @@ static inline int assign_eip(struct x86_emulate_ctxt *ctxt, ulong dst) unsigned max_size; struct segmented_address addr = { .seg = VCPU_SREG_CS, .ea = dst }; + u64 flags = KVM_X86_EMULFLAG_FETCH; if (ctxt->op_bytes != sizeof(unsigned long)) addr.ea = dst & ((1UL << (ctxt->op_bytes << 3)) - 1); - rc = __linearize(ctxt, addr, &max_size, 1, false, true, ctxt->mode, &linear); + rc = __linearize(ctxt, addr, &max_size, 1, flags, ctxt->mode, &linear); if (rc == X86EMUL_CONTINUE) ctxt->_eip = addr.ea; return rc; @@ -896,6 +904,7 @@ static int __do_insn_fetch_bytes(struct x86_emulate_ctxt *ctxt, int op_size) int cur_size = ctxt->fetch.end - ctxt->fetch.data; struct segmented_address addr = { .seg = VCPU_SREG_CS, .ea = ctxt->eip + cur_size }; + u64 flags = KVM_X86_EMULFLAG_FETCH; /* * We do not know exactly how many bytes will be needed, and @@ -907,8 +916,7 @@ static int __do_insn_fetch_bytes(struct x86_emulate_ctxt *ctxt, int op_size) * boundary check itself. Instead, we use max_size to check * against op_size. */ - rc = __linearize(ctxt, addr, &max_size, 0, false, true, ctxt->mode, - &linear); + rc = __linearize(ctxt, addr, &max_size, 0, flags, ctxt->mode, &linear); if (unlikely(rc != X86EMUL_CONTINUE)) return rc; diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index a8167b47b8c8..8076e013ff9f 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -48,6 +48,15 @@ void kvm_spurious_fault(void); #define KVM_SVM_DEFAULT_PLE_WINDOW_MAX USHRT_MAX #define KVM_SVM_DEFAULT_PLE_WINDOW 3000 +/* x86-specific emulation flags */ +#define KVM_X86_EMULFLAG_FETCH _BITULL(0) +#define KVM_X86_EMULFLAG_WRITE _BITULL(1) And the following two will be defined for untag: #define KVM_X86_EMULFLAG_SKIP_UNTAG_VMX _BITULL(2) #define KVM_X86_EMULFLAG_SKIP_UNTAG_SVM _BITULL(3) /* reserved for SVM */
On Fri, Apr 21, 2023 at 03:57:15PM +0800, Binbin Wu wrote: > >> > --- a/arch/x86/kvm/emulate.c >> > +++ b/arch/x86/kvm/emulate.c >> > @@ -688,7 +688,8 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, >> > struct segmented_address addr, >> > unsigned *max_size, unsigned size, >> > bool write, bool fetch, >> > - enum x86emul_mode mode, ulong *linear) >> > + enum x86emul_mode mode, ulong *linear, >> > + u64 untag_flags) >> @write and @fetch are like flags. I think we can consolidate them into >> the @flags first as a cleanup patch and then add a flag for LAM. > >OK. Here is the proposed cleanup patch: looks good to me > > >--- a/arch/x86/kvm/x86.h >+++ b/arch/x86/kvm/x86.h >@@ -48,6 +48,15 @@ void kvm_spurious_fault(void); > #define KVM_SVM_DEFAULT_PLE_WINDOW_MAX USHRT_MAX > #define KVM_SVM_DEFAULT_PLE_WINDOW 3000 > >+/* x86-specific emulation flags */ >+#define KVM_X86_EMULFLAG_FETCH _BITULL(0) >+#define KVM_X86_EMULFLAG_WRITE _BITULL(1) Can we move the definitions to arch/x86/kvm/kvm_emulate.h? > > >And the following two will be defined for untag: > >#define KVM_X86_EMULFLAG_SKIP_UNTAG_VMX _BITULL(2) >#define KVM_X86_EMULFLAG_SKIP_UNTAG_SVM _BITULL(3) /* reserved for SVM */ > >
On 4/21/2023 4:36 PM, Chao Gao wrote: > On Fri, Apr 21, 2023 at 03:57:15PM +0800, Binbin Wu wrote: >>>> --- a/arch/x86/kvm/emulate.c >>>> +++ b/arch/x86/kvm/emulate.c >>>> @@ -688,7 +688,8 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, >>>> struct segmented_address addr, >>>> unsigned *max_size, unsigned size, >>>> bool write, bool fetch, >>>> - enum x86emul_mode mode, ulong *linear) >>>> + enum x86emul_mode mode, ulong *linear, >>>> + u64 untag_flags) >>> @write and @fetch are like flags. I think we can consolidate them into >>> the @flags first as a cleanup patch and then add a flag for LAM. >> OK. Here is the proposed cleanup patch: > looks good to me > >> >> --- a/arch/x86/kvm/x86.h >> +++ b/arch/x86/kvm/x86.h >> @@ -48,6 +48,15 @@ void kvm_spurious_fault(void); >> #define KVM_SVM_DEFAULT_PLE_WINDOW_MAX USHRT_MAX >> #define KVM_SVM_DEFAULT_PLE_WINDOW 3000 >> >> +/* x86-specific emulation flags */ >> +#define KVM_X86_EMULFLAG_FETCH _BITULL(0) >> +#define KVM_X86_EMULFLAG_WRITE _BITULL(1) > Can we move the definitions to arch/x86/kvm/kvm_emulate.h? Then, the flags needs to be removed from .untag_addr() interface since currently KVM_X86_EMULFLAG_SKIP_UNTAG_VMX is used in vmx. :( > >> >> And the following two will be defined for untag: >> >> #define KVM_X86_EMULFLAG_SKIP_UNTAG_VMX _BITULL(2) >> #define KVM_X86_EMULFLAG_SKIP_UNTAG_SVM _BITULL(3) /* reserved for SVM */ >> >>
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index a20bec931764..b7df465eccf2 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -688,7 +688,8 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, struct segmented_address addr, unsigned *max_size, unsigned size, bool write, bool fetch, - enum x86emul_mode mode, ulong *linear) + enum x86emul_mode mode, ulong *linear, + u64 untag_flags) { struct desc_struct desc; bool usable; @@ -701,6 +702,7 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, *max_size = 0; switch (mode) { case X86EMUL_MODE_PROT64: + la = ctxt->ops->untag_addr(ctxt, la, untag_flags); *linear = la; va_bits = ctxt_virt_addr_bits(ctxt); if (!__is_canonical_address(la, va_bits)) @@ -758,7 +760,7 @@ static int linearize(struct x86_emulate_ctxt *ctxt, { unsigned max_size; return __linearize(ctxt, addr, &max_size, size, write, false, - ctxt->mode, linear); + ctxt->mode, linear, 0); } static inline int assign_eip(struct x86_emulate_ctxt *ctxt, ulong dst) @@ -771,7 +773,12 @@ static inline int assign_eip(struct x86_emulate_ctxt *ctxt, ulong dst) if (ctxt->op_bytes != sizeof(unsigned long)) addr.ea = dst & ((1UL << (ctxt->op_bytes << 3)) - 1); - rc = __linearize(ctxt, addr, &max_size, 1, false, true, ctxt->mode, &linear); + /* + * LAM does not apply to addresses used for instruction fetches + * or to those that specify the targets of jump and call instructions + */ + rc = __linearize(ctxt, addr, &max_size, 1, false, true, ctxt->mode, + &linear, KVM_X86_UNTAG_ADDR_SKIP_LAM); if (rc == X86EMUL_CONTINUE) ctxt->_eip = addr.ea; return rc; @@ -906,9 +913,12 @@ static int __do_insn_fetch_bytes(struct x86_emulate_ctxt *ctxt, int op_size) * __linearize is called with size 0 so that it does not do any * boundary check itself. Instead, we use max_size to check * against op_size. + * + * LAM does not apply to addresses used for instruction fetches + * or to those that specify the targets of jump and call instructions */ rc = __linearize(ctxt, addr, &max_size, 0, false, true, ctxt->mode, - &linear); + &linear, KVM_X86_UNTAG_ADDR_SKIP_LAM); if (unlikely(rc != X86EMUL_CONTINUE)) return rc; @@ -3433,8 +3443,11 @@ static int em_invlpg(struct x86_emulate_ctxt *ctxt) { int rc; ulong linear; + unsigned max_size; - rc = linearize(ctxt, ctxt->src.addr.mem, 1, false, &linear); + /* skip untag for invlpg since LAM is not applied to invlpg */ + rc = __linearize(ctxt, ctxt->src.addr.mem, &max_size, 1, false, false, + ctxt->mode, &linear, KVM_X86_UNTAG_ADDR_SKIP_LAM); if (rc == X86EMUL_CONTINUE) ctxt->ops->invlpg(ctxt, linear); /* Disable writeback. */ diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h index ab65f3a47dfd..8d9f782adccb 100644 --- a/arch/x86/kvm/kvm_emulate.h +++ b/arch/x86/kvm/kvm_emulate.h @@ -225,6 +225,8 @@ struct x86_emulate_ops { int (*leave_smm)(struct x86_emulate_ctxt *ctxt); void (*triple_fault)(struct x86_emulate_ctxt *ctxt); int (*set_xcr)(struct x86_emulate_ctxt *ctxt, u32 index, u64 xcr); + + u64 (*untag_addr)(struct x86_emulate_ctxt *ctxt, u64 addr, u64 flags); }; /* Type, address-of, and value of an instruction's operand. */ diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index d35bda9610e2..48cca88bfd37 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -4970,6 +4970,7 @@ int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification, else *ret = off; + *ret = vmx_untag_addr(vcpu, *ret, 0); /* 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! @@ -5787,6 +5788,9 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) vpid02 = nested_get_vpid02(vcpu); switch (type) { case VMX_VPID_EXTENT_INDIVIDUAL_ADDR: + /* invvpid is not valid in compatibility mode */ + if (is_long_mode(vcpu)) + operand.gla = vmx_untag_addr(vcpu, operand.gla, 0); if (!operand.vpid || is_noncanonical_address(operand.gla, vcpu)) return nested_vmx_fail(vcpu, diff --git a/arch/x86/kvm/vmx/sgx.c b/arch/x86/kvm/vmx/sgx.c index 0574030b071f..527f1a902c65 100644 --- a/arch/x86/kvm/vmx/sgx.c +++ b/arch/x86/kvm/vmx/sgx.c @@ -37,6 +37,7 @@ static int sgx_get_encls_gva(struct kvm_vcpu *vcpu, unsigned long offset, if (!IS_ALIGNED(*gva, alignment)) { fault = true; } else if (likely(is_64_bit_mode(vcpu))) { + *gva = vmx_untag_addr(vcpu, *gva, 0); fault = is_noncanonical_address(*gva, vcpu); } else { *gva &= 0xffffffff; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index aca255e69d0d..18ad38649714 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -8218,6 +8218,11 @@ static void emulator_vm_bugged(struct x86_emulate_ctxt *ctxt) kvm_vm_bugged(kvm); } +static u64 emulator_untag_addr(struct x86_emulate_ctxt *ctxt, u64 addr, u64 flags) +{ + return static_call(kvm_x86_untag_addr)(emul_to_vcpu(ctxt), addr, flags); +} + static const struct x86_emulate_ops emulate_ops = { .vm_bugged = emulator_vm_bugged, .read_gpr = emulator_read_gpr, @@ -8263,6 +8268,7 @@ static const struct x86_emulate_ops emulate_ops = { .leave_smm = emulator_leave_smm, .triple_fault = emulator_triple_fault, .set_xcr = emulator_set_xcr, + .untag_addr = emulator_untag_addr, }; static void toggle_interruptibility(struct kvm_vcpu *vcpu, u32 mask) @@ -13260,6 +13266,10 @@ int kvm_handle_invpcid(struct kvm_vcpu *vcpu, unsigned long type, gva_t gva) switch (type) { case INVPCID_TYPE_INDIV_ADDR: + /* + * LAM doesn't apply to the linear address in the descriptor, + * still need to be canonical + */ if ((!pcid_enabled && (operand.pcid != 0)) || is_noncanonical_address(operand.gla, vcpu)) { kvm_inject_gp(vcpu, 0);