| Message ID | 20230227084547.404871-5-robert.hu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Linear Address Masking (LAM) KVM Enabling | expand |
On 2/27/2023 4:45 PM, Robert Hoo wrote: > Emulate HW LAM masking when doing data access under 64-bit mode. > > kvm_lam_untag_addr() implements this: per CR4/CR3 LAM bits configuration, > firstly check the linear addr conforms LAM canonical, i.e. the highest > address bit matches bit 63. Then mask out meta data per LAM configuration. > If failed in above process, emulate #GP to guest. > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > --- > arch/x86/kvm/emulate.c | 13 ++++++++ > arch/x86/kvm/x86.h | 70 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 83 insertions(+) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 5cc3efa0e21c..77bd13f40711 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -700,6 +700,19 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > *max_size = 0; > switch (mode) { > case X86EMUL_MODE_PROT64: > + /* LAM applies only on data access */ > + if (!fetch && guest_cpuid_has(ctxt->vcpu, X86_FEATURE_LAM)) { > + enum lam_type type; > + > + type = kvm_vcpu_lam_type(la, ctxt->vcpu); > + if (type == LAM_ILLEGAL) { > + *linear = la; > + goto bad; > + } else { > + la = kvm_lam_untag_addr(la, type); > + } > + } > + __linearize is not the only path the modified LAM canonical check needed, also some vmexits path should be taken care of, like VMX, SGX ENCLS. Also the instruction INVLPG, INVPCID should have some special handling since LAM is not applied to the memory operand of the two instruction according to the LAM spec. > *linear = la; > va_bits = ctxt_virt_addr_bits(ctxt); > if (!__is_canonical_address(la, va_bits)) > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > index 6b6bfddc84e0..d992e5220602 100644 > --- a/arch/x86/kvm/x86.h > +++ b/arch/x86/kvm/x86.h > @@ -201,6 +201,76 @@ static inline bool is_noncanonical_address(u64 la, struct kvm_vcpu *vcpu) > return !__is_canonical_address(la, vcpu_virt_addr_bits(vcpu)); > } > > +enum lam_type { > + LAM_ILLEGAL = -1, > + LAM_U57, > + LAM_U48, > + LAM_S57, > + LAM_S48, > + LAM_NONE > +}; > + > +#ifdef CONFIG_X86_64 > +/* > + * LAM Canonical Rule: > + * LAM_U/S48 -- bit 63 == bit 47 > + * LAM_U/S57 -- bit 63 == bit 56 The modified LAM canonical check for LAM_U57 + 4-level paging is: bit 63, bit 56:47 should be all 0s. > + */ > +static inline bool lam_canonical(u64 addr, int effect_width) > +{ > + return (addr >> 63) == ((addr >> effect_width) & BIT(0)); > +} > + > +static inline enum lam_type kvm_vcpu_lam_type(u64 addr, struct kvm_vcpu *vcpu) > +{ > + WARN_ON_ONCE(!is_64_bit_mode(vcpu)); > + > + if (addr >> 63 == 0) { > + if (kvm_read_cr3(vcpu) & X86_CR3_LAM_U57) > + return lam_canonical(addr, 56) ? LAM_U57 : LAM_ILLEGAL; > + else if (kvm_read_cr3(vcpu) & X86_CR3_LAM_U48) > + return lam_canonical(addr, 47) ? LAM_U48 : LAM_ILLEGAL; > + } else if (kvm_read_cr4_bits(vcpu, X86_CR4_LAM_SUP)) { > + if (kvm_read_cr4_bits(vcpu, X86_CR4_LA57)) > + return lam_canonical(addr, 56) ? LAM_S57 : LAM_ILLEGAL; > + else > + return lam_canonical(addr, 47) ? LAM_S48 : LAM_ILLEGAL; > + } > + > + return LAM_NONE; > +} > + > +/* untag addr for guest, according to vCPU's LAM config */ > +static inline u64 kvm_lam_untag_addr(u64 addr, enum lam_type type) > +{ > + switch (type) { > + case LAM_U57: > + case LAM_S57: > + addr = __canonical_address(addr, 57); > + break; > + case LAM_U48: > + case LAM_S48: > + addr = __canonical_address(addr, 48); > + break; > + case LAM_NONE: > + default: > + break; > + } > + > + return addr; > +} > +#else > +static inline enum lam_type kvm_vcpu_lam_type(u64 addr, struct kvm_vcpu *vcpu) > +{ > + return LAM_NONE; > +} > + > +static inline u64 kvm_lam_untag_addr(u64 addr, enum lam_type type) > +{ > + return addr; > +} > +#endif > + > static inline void vcpu_cache_mmio_info(struct kvm_vcpu *vcpu, > gva_t gva, gfn_t gfn, unsigned access) > {
On Mon, Feb 27, 2023 at 04:45:46PM +0800, Robert Hoo wrote: >Emulate HW LAM masking when doing data access under 64-bit mode. > >kvm_lam_untag_addr() implements this: per CR4/CR3 LAM bits configuration, >firstly check the linear addr conforms LAM canonical, i.e. the highest >address bit matches bit 63. Then mask out meta data per LAM configuration. >If failed in above process, emulate #GP to guest. > >Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> >--- > arch/x86/kvm/emulate.c | 13 ++++++++ > arch/x86/kvm/x86.h | 70 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 83 insertions(+) > >diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >index 5cc3efa0e21c..77bd13f40711 100644 >--- a/arch/x86/kvm/emulate.c >+++ b/arch/x86/kvm/emulate.c >@@ -700,6 +700,19 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > *max_size = 0; > switch (mode) { > case X86EMUL_MODE_PROT64: >+ /* LAM applies only on data access */ >+ if (!fetch && guest_cpuid_has(ctxt->vcpu, X86_FEATURE_LAM)) { >+ enum lam_type type; >+ >+ type = kvm_vcpu_lam_type(la, ctxt->vcpu); >+ if (type == LAM_ILLEGAL) { >+ *linear = la; >+ goto bad; >+ } else { >+ la = kvm_lam_untag_addr(la, type); >+ } >+ } >+ > *linear = la; > va_bits = ctxt_virt_addr_bits(ctxt); > if (!__is_canonical_address(la, va_bits)) ... >+static inline u64 kvm_lam_untag_addr(u64 addr, enum lam_type type) >+{ >+ switch (type) { >+ case LAM_U57: >+ case LAM_S57: >+ addr = __canonical_address(addr, 57); >+ break; >+ case LAM_U48: >+ case LAM_S48: >+ addr = __canonical_address(addr, 48); >+ break; >+ case LAM_NONE: >+ default: >+ break; >+ } >+ >+ return addr; >+} LAM's change to canonicality check is: before performing the check, software metadata in pointers is masked by sign-extending the value of bit 56/47. so, to emulate this behavior, in kvm_lam_untag_addr(), we can simply: 1. determine which LAM configuration is enabled, LAM57 or LAM48. 2. mask software metadata by sign-extending the bit56/47, i.e., addr = (sign_extern64(addr, X) & ~BIT_ULL(63)) | (addr & BIT_ULL(63)); where X=56 for LAM57 and X=47 for LAM48. Note that this doesn't ensure the resulting @addr is canonical. It isn't a problem because the original canonicality check (__is_canonical_address() above) can identify non-canonical addresses and raise #GP/#SS to the guest.
On 3/2/2023 4:55 PM, Chao Gao wrote: > On Mon, Feb 27, 2023 at 04:45:46PM +0800, Robert Hoo wrote: >> Emulate HW LAM masking when doing data access under 64-bit mode. >> >> kvm_lam_untag_addr() implements this: per CR4/CR3 LAM bits configuration, >> firstly check the linear addr conforms LAM canonical, i.e. the highest >> address bit matches bit 63. Then mask out meta data per LAM configuration. >> If failed in above process, emulate #GP to guest. >> >> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> >> --- >> arch/x86/kvm/emulate.c | 13 ++++++++ >> arch/x86/kvm/x86.h | 70 ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 83 insertions(+) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index 5cc3efa0e21c..77bd13f40711 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -700,6 +700,19 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, >> *max_size = 0; >> switch (mode) { >> case X86EMUL_MODE_PROT64: >> + /* LAM applies only on data access */ >> + if (!fetch && guest_cpuid_has(ctxt->vcpu, X86_FEATURE_LAM)) { >> + enum lam_type type; >> + >> + type = kvm_vcpu_lam_type(la, ctxt->vcpu); >> + if (type == LAM_ILLEGAL) { >> + *linear = la; >> + goto bad; >> + } else { >> + la = kvm_lam_untag_addr(la, type); >> + } >> + } >> + >> *linear = la; >> va_bits = ctxt_virt_addr_bits(ctxt); >> if (!__is_canonical_address(la, va_bits)) > ... > >> +static inline u64 kvm_lam_untag_addr(u64 addr, enum lam_type type) >> +{ >> + switch (type) { >> + case LAM_U57: >> + case LAM_S57: >> + addr = __canonical_address(addr, 57); >> + break; >> + case LAM_U48: >> + case LAM_S48: >> + addr = __canonical_address(addr, 48); >> + break; >> + case LAM_NONE: >> + default: >> + break; >> + } >> + >> + return addr; >> +} > LAM's change to canonicality check is: > before performing the check, software metadata in pointers is masked by > sign-extending the value of bit 56/47. > > so, to emulate this behavior, in kvm_lam_untag_addr(), we can simply: > 1. determine which LAM configuration is enabled, LAM57 or LAM48. > 2. mask software metadata by sign-extending the bit56/47, i.e., > > addr = (sign_extern64(addr, X) & ~BIT_ULL(63)) | > (addr & BIT_ULL(63)); > > where X=56 for LAM57 and X=47 for LAM48. > > Note that this doesn't ensure the resulting @addr is canonical. It > isn't a problem because the original canonicality check > (__is_canonical_address() above) can identify non-canonical addresses > and raise #GP/#SS to the guest. Thanks for your suggestion. It's much simpler.
On Thu, 2023-03-02 at 14:41 +0800, Binbin Wu wrote: > __linearize is not the only path the modified LAM canonical check > needed, also some vmexits path should be taken care of, like VMX, > SGX > ENCLS. > SGX isn't in this version's implementation's scope, like nested LAM. > Also the instruction INVLPG, INVPCID should have some special > handling > since LAM is not applied to the memory operand of the two > instruction > according to the LAM spec. The spec's meaning on these 2 is: LAM masking doesn't apply to their operands (the address), so the behavior is like before LAM feature introduced. No change. > > > > +#ifdef CONFIG_X86_64 > > +/* > > + * LAM Canonical Rule: > > + * LAM_U/S48 -- bit 63 == bit 47 > > + * LAM_U/S57 -- bit 63 == bit 56 > > The modified LAM canonical check for LAM_U57 + 4-level paging is: > bit > 63, bit 56:47 should be all 0s. > Yes, this case was missed. Chao's suggestion on signed-extend + legacy canonical check can cover this.
On 3/2/2023 9:16 PM, Robert Hoo wrote: > On Thu, 2023-03-02 at 14:41 +0800, Binbin Wu wrote: >> __linearize is not the only path the modified LAM canonical check >> needed, also some vmexits path should be taken care of, like VMX, >> SGX >> ENCLS. >> > SGX isn't in this version's implementation's scope, like nested LAM. LAM in SGX enclave mode is not the scope of the this version. But I think since the capability is exposed to guest, need to cover the case if the guest use the supervisor mode pointer as the operand of SGX EENCS operations. > >> Also the instruction INVLPG, INVPCID should have some special >> handling >> since LAM is not applied to the memory operand of the two >> instruction >> according to the LAM spec. > The spec's meaning on these 2 is: LAM masking doesn't apply to their > operands (the address), so the behavior is like before LAM feature > introduced. No change. Yes, LAM are not applied to the 2 instrustions, but the __linearize is changed. For example, the emulation of invlpg (em_invpg) will also call it. So need to handle the case specificlly. Can add a flag as the input of linearize to indicate the LAM check and untag is needed or not. >> >>> +#ifdef CONFIG_X86_64 >>> +/* >>> + * LAM Canonical Rule: >>> + * LAM_U/S48 -- bit 63 == bit 47 >>> + * LAM_U/S57 -- bit 63 == bit 56 >> The modified LAM canonical check for LAM_U57 + 4-level paging is: >> bit >> 63, bit 56:47 should be all 0s. >> > Yes, this case was missed. Chao's suggestion on signed-extend + legacy > canonical check can cover this. >
On Fri, 2023-03-03 at 09:08 +0800, Binbin Wu wrote: > On 3/2/2023 9:16 PM, Robert Hoo wrote: > > On Thu, 2023-03-02 at 14:41 +0800, Binbin Wu wrote: > > > __linearize is not the only path the modified LAM canonical check > > > needed, also some vmexits path should be taken care of, like VMX, > > > SGX > > > ENCLS. > > > > > > > SGX isn't in this version's implementation's scope, like nested > > LAM. > > LAM in SGX enclave mode is not the scope of the this version. > But I think since the capability is exposed to guest, I think you can document this or other method to call out this to user. Even Kernel enabling doesn't include SGX interaction yet, I doubt if it's that urgent for KVM to do this at this phase. > need to cover the > case if the guest use the supervisor mode pointer No business with pointer mode here, I think. > as the operand of SGX > EENCS operations. > > > > > > > Also the instruction INVLPG, INVPCID should have some special > > > handling > > > since LAM is not applied to the memory operand of the two > > > instruction > > > according to the LAM spec. > > > > The spec's meaning on these 2 is: LAM masking doesn't apply to > > their > > operands (the address), so the behavior is like before LAM feature > > introduced. No change. > > Yes, LAM are not applied to the 2 instrustions, but the __linearize > is > changed. > For example, the emulation of invlpg (em_invpg) will also call it. > So > need to handle the case specificlly. > Can add a flag as the input of linearize to indicate the LAM check > and > untag is needed or not. > No need. "The INVLPG instruction ... LAM does not apply to the specified memory address. Thus, in 64-bit mode, ** if the memory address specified is in non-canonical form then the INVLPG is the same as a NOP. ** The INVPCID instruction ... LAM does not apply to the specified memory address, and in 64-bit mode ** if this memory address is in non-canonical form then the processor generates a #GP(0) exception. **" You can double confirm in SDM: Before-and-After LAM introduced, the behavior hasn't changed. Thus you don't need to worry about these 2 INS's emulations.
On 3/3/2023 11:16 AM, Robert Hoo wrote: > On Fri, 2023-03-03 at 09:08 +0800, Binbin Wu wrote: >> On 3/2/2023 9:16 PM, Robert Hoo wrote: >>> On Thu, 2023-03-02 at 14:41 +0800, Binbin Wu wrote: >>>> __linearize is not the only path the modified LAM canonical check >>>> needed, also some vmexits path should be taken care of, like VMX, >>>> SGX >>>> ENCLS. >>>> >>> SGX isn't in this version's implementation's scope, like nested >>> LAM. >> LAM in SGX enclave mode is not the scope of the this version. >> But I think since the capability is exposed to guest, > I think you can document this or other method to call out this to user. > Even Kernel enabling doesn't include SGX interaction yet, I doubt if > it's that urgent for KVM to do this at this phase. > >> need to cover the >> case if the guest use the supervisor mode pointer > No business with pointer mode here, I think. > >> as the operand of SGX >> EENCS operations. >> >> >>>> Also the instruction INVLPG, INVPCID should have some special >>>> handling >>>> since LAM is not applied to the memory operand of the two >>>> instruction >>>> according to the LAM spec. >>> The spec's meaning on these 2 is: LAM masking doesn't apply to >>> their >>> operands (the address), so the behavior is like before LAM feature >>> introduced. No change. >> Yes, LAM are not applied to the 2 instrustions, but the __linearize >> is >> changed. >> For example, the emulation of invlpg (em_invpg) will also call it. >> So >> need to handle the case specificlly. >> Can add a flag as the input of linearize to indicate the LAM check >> and >> untag is needed or not. >> > No need. > > "The INVLPG instruction ... > LAM does not apply to the specified memory address. Thus, in 64-bit > mode, ** if the memory address specified is in non-canonical form then > the INVLPG is the same as a NOP. ** Based on current patch, if the address of invlpg is non-canonical, it will be first checked and converted by the new LAM handling. After that, I will be canonical and do the invalidition, but not NOP. Maybe we can say do an additional TLB invalidation may be no big different as NOP, but it need to be documented/comment somewhere > > The INVPCID instruction ... > LAM does not apply to the specified memory address, and in 64-bit > mode ** if this memory address is in non-canonical form then the > processor generates a #GP(0) exception. **" > > You can double confirm in SDM: Before-and-After LAM introduced, the > behavior hasn't changed. Thus you don't need to worry about these 2 > INS's emulations. This is because currently, VMX vmexit handling is not considered yet. The linear address of guest is retrived from get_vmx_mem_address, which is also will be called by INVPCID. What arguable is that we need to cover all supervisor mode pointer cases in this phase. But IMO if thesel cases are not covered, CR4.LAM_SUP should be not allow to be set by guest.
On Fri, 2023-03-03 at 11:35 +0800, Binbin Wu wrote: > > > > > Also the instruction INVLPG, INVPCID should have some special > > > > > handling > > > > > since LAM is not applied to the memory operand of the two > > > > > instruction > > > > > according to the LAM spec. > > > > > > > > The spec's meaning on these 2 is: LAM masking doesn't apply to > > > > their > > > > operands (the address), so the behavior is like before LAM > > > > feature > > > > introduced. No change. > > > > > > Yes, LAM are not applied to the 2 instrustions, but the > > > __linearize > > > is > > > changed. > > > For example, the emulation of invlpg (em_invpg) will also call > > > it. Can you elaborate more on this? what emulation case of INVLPG? do you mean vm-exit handling due to Guest execute INVLPG when VMCS_control.INVLPG_exiting set? or other case? > > > So > > > need to handle the case specificlly. > > > Can add a flag as the input of linearize to indicate the LAM > > > check > > > and > > > untag is needed or not. > > > > > > > No need. > > > > "The INVLPG instruction ... > > LAM does not apply to the specified memory address. Thus, in 64-bit > > mode, ** if the memory address specified is in non-canonical form > > then > > the INVLPG is the same as a NOP. ** > > Based on current patch, if the address of invlpg is non-canonical, > it > will be first checked and converted by the new LAM handling. > After that, I will be canonical and do the invalidition, but not NOP. > Maybe we can say do an additional TLB invalidation may be no big > different as NOP, but it need to be documented/comment somewhere > > > > > > The INVPCID instruction ... > > LAM does not apply to the specified memory address, and in 64-bit > > mode ** if this memory address is in non-canonical form then the > > processor generates a #GP(0) exception. **" > > > > You can double confirm in SDM: Before-and-After LAM introduced, the > > behavior hasn't changed. Thus you don't need to worry about these 2 > > INS's emulations. > > This is because currently, VMX vmexit handling is not considered yet. > The linear address of guest is retrived from get_vmx_mem_address, > which > is also will be called by INVPCID. Again, nested LAM isn't in this patch set's scope. In terms of handle_invpcid() --> get_vmx_mem_address(), per Spec, no behavior changes, no changes needed. > > What arguable is that we need to cover all supervisor mode pointer > cases > in this phase. > But IMO if thesel cases are not covered, CR4.LAM_SUP should be not > allow > to be set by guest. >
On 3/3/2023 5:00 PM, Robert Hoo wrote: > On Fri, 2023-03-03 at 11:35 +0800, Binbin Wu wrote: >>>>>> Also the instruction INVLPG, INVPCID should have some special >>>>>> handling >>>>>> since LAM is not applied to the memory operand of the two >>>>>> instruction >>>>>> according to the LAM spec. >>>>> The spec's meaning on these 2 is: LAM masking doesn't apply to >>>>> their >>>>> operands (the address), so the behavior is like before LAM >>>>> feature >>>>> introduced. No change. >>>> Yes, LAM are not applied to the 2 instrustions, but the >>>> __linearize >>>> is >>>> changed. >>>> For example, the emulation of invlpg (em_invpg) will also call >>>> it. > Can you elaborate more on this? what emulation case of INVLPG? do you > mean vm-exit handling due to Guest execute INVLPG when > VMCS_control.INVLPG_exiting set? or other case? Not the vm-exit handling code. em_invpg() belongs to instruction emulation. But I am not sure this code is reachable or not. Let me double check. > >>>> So >>>> need to handle the case specificlly. >>>> Can add a flag as the input of linearize to indicate the LAM >>>> check >>>> and >>>> untag is needed or not. >>>> >>> No need. >>> >>> "The INVLPG instruction ... >>> LAM does not apply to the specified memory address. Thus, in 64-bit >>> mode, ** if the memory address specified is in non-canonical form >>> then >>> the INVLPG is the same as a NOP. ** >> Based on current patch, if the address of invlpg is non-canonical, >> it >> will be first checked and converted by the new LAM handling. >> After that, I will be canonical and do the invalidition, but not NOP. >> Maybe we can say do an additional TLB invalidation may be no big >> different as NOP, but it need to be documented/comment somewhere >> >> >>> The INVPCID instruction ... >>> LAM does not apply to the specified memory address, and in 64-bit >>> mode ** if this memory address is in non-canonical form then the >>> processor generates a #GP(0) exception. **" >>> >>> You can double confirm in SDM: Before-and-After LAM introduced, the >>> behavior hasn't changed. Thus you don't need to worry about these 2 >>> INS's emulations. >> This is because currently, VMX vmexit handling is not considered yet. >> The linear address of guest is retrived from get_vmx_mem_address, >> which >> is also will be called by INVPCID. > Again, nested LAM isn't in this patch set's scope. > In terms of handle_invpcid() --> get_vmx_mem_address(), per Spec, no > behavior changes, no changes needed. OK. >> What arguable is that we need to cover all supervisor mode pointer >> cases >> in this phase. >> But IMO if thesel cases are not covered, CR4.LAM_SUP should be not >> allow >> to be set by guest. >>
On Mon, Feb 27, 2023, Robert Hoo wrote: > Emulate HW LAM masking when doing data access under 64-bit mode. > > kvm_lam_untag_addr() implements this: per CR4/CR3 LAM bits configuration, > firstly check the linear addr conforms LAM canonical, i.e. the highest > address bit matches bit 63. Then mask out meta data per LAM configuration. > If failed in above process, emulate #GP to guest. > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > --- > arch/x86/kvm/emulate.c | 13 ++++++++ > arch/x86/kvm/x86.h | 70 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 83 insertions(+) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 5cc3efa0e21c..77bd13f40711 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -700,6 +700,19 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > *max_size = 0; > switch (mode) { > case X86EMUL_MODE_PROT64: > + /* LAM applies only on data access */ > + if (!fetch && guest_cpuid_has(ctxt->vcpu, X86_FEATURE_LAM)) { Derefencing ctxt->vcpu in the emulator is not allowed. > + enum lam_type type; > + > + type = kvm_vcpu_lam_type(la, ctxt->vcpu); > + if (type == LAM_ILLEGAL) { > + *linear = la; > + goto bad; > + } else { > + la = kvm_lam_untag_addr(la, type); > + } > + } This is wildly over-engineered. Just do the untagging and let __is_canonical_address() catch any non-canonical bits that weren't stripped.
On Fri, Mar 03, 2023, Binbin Wu wrote: > > On 3/2/2023 9:16 PM, Robert Hoo wrote: > > On Thu, 2023-03-02 at 14:41 +0800, Binbin Wu wrote: > > > __linearize is not the only path the modified LAM canonical check > > > needed, also some vmexits path should be taken care of, like VMX, > > > SGX > > > ENCLS. > > > > > SGX isn't in this version's implementation's scope, like nested LAM. > > LAM in SGX enclave mode is not the scope of the this version. I'm not merging half-baked support. Not supporting nested LAM _may_ be ok, because KVM can _prevent_ exposing LAM to L2. I say "may" because I would still _very_ strongly prefer that nested support be added in the initial series. But omitting architectural interactions just because is not going to happen.
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 5cc3efa0e21c..77bd13f40711 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -700,6 +700,19 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, *max_size = 0; switch (mode) { case X86EMUL_MODE_PROT64: + /* LAM applies only on data access */ + if (!fetch && guest_cpuid_has(ctxt->vcpu, X86_FEATURE_LAM)) { + enum lam_type type; + + type = kvm_vcpu_lam_type(la, ctxt->vcpu); + if (type == LAM_ILLEGAL) { + *linear = la; + goto bad; + } else { + la = kvm_lam_untag_addr(la, type); + } + } + *linear = la; va_bits = ctxt_virt_addr_bits(ctxt); if (!__is_canonical_address(la, va_bits)) diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index 6b6bfddc84e0..d992e5220602 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -201,6 +201,76 @@ static inline bool is_noncanonical_address(u64 la, struct kvm_vcpu *vcpu) return !__is_canonical_address(la, vcpu_virt_addr_bits(vcpu)); } +enum lam_type { + LAM_ILLEGAL = -1, + LAM_U57, + LAM_U48, + LAM_S57, + LAM_S48, + LAM_NONE +}; + +#ifdef CONFIG_X86_64 +/* + * LAM Canonical Rule: + * LAM_U/S48 -- bit 63 == bit 47 + * LAM_U/S57 -- bit 63 == bit 56 + */ +static inline bool lam_canonical(u64 addr, int effect_width) +{ + return (addr >> 63) == ((addr >> effect_width) & BIT(0)); +} + +static inline enum lam_type kvm_vcpu_lam_type(u64 addr, struct kvm_vcpu *vcpu) +{ + WARN_ON_ONCE(!is_64_bit_mode(vcpu)); + + if (addr >> 63 == 0) { + if (kvm_read_cr3(vcpu) & X86_CR3_LAM_U57) + return lam_canonical(addr, 56) ? LAM_U57 : LAM_ILLEGAL; + else if (kvm_read_cr3(vcpu) & X86_CR3_LAM_U48) + return lam_canonical(addr, 47) ? LAM_U48 : LAM_ILLEGAL; + } else if (kvm_read_cr4_bits(vcpu, X86_CR4_LAM_SUP)) { + if (kvm_read_cr4_bits(vcpu, X86_CR4_LA57)) + return lam_canonical(addr, 56) ? LAM_S57 : LAM_ILLEGAL; + else + return lam_canonical(addr, 47) ? LAM_S48 : LAM_ILLEGAL; + } + + return LAM_NONE; +} + +/* untag addr for guest, according to vCPU's LAM config */ +static inline u64 kvm_lam_untag_addr(u64 addr, enum lam_type type) +{ + switch (type) { + case LAM_U57: + case LAM_S57: + addr = __canonical_address(addr, 57); + break; + case LAM_U48: + case LAM_S48: + addr = __canonical_address(addr, 48); + break; + case LAM_NONE: + default: + break; + } + + return addr; +} +#else +static inline enum lam_type kvm_vcpu_lam_type(u64 addr, struct kvm_vcpu *vcpu) +{ + return LAM_NONE; +} + +static inline u64 kvm_lam_untag_addr(u64 addr, enum lam_type type) +{ + return addr; +} +#endif + static inline void vcpu_cache_mmio_info(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn, unsigned access) {
Emulate HW LAM masking when doing data access under 64-bit mode. kvm_lam_untag_addr() implements this: per CR4/CR3 LAM bits configuration, firstly check the linear addr conforms LAM canonical, i.e. the highest address bit matches bit 63. Then mask out meta data per LAM configuration. If failed in above process, emulate #GP to guest. Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> --- arch/x86/kvm/emulate.c | 13 ++++++++ arch/x86/kvm/x86.h | 70 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+)