Re: [PATCH v8 2/6] KVM: x86: Virtualize CR4.LAM_SUP

[Binbin Wu]

On 5/11/2023 8:50 PM, Huang, Kai wrote:
> On Wed, 2023-05-10 at 14:06 +0800, Binbin Wu wrote:
> > From: Robert Hoo <robert.hu@xxxxxxxxxxxxxxx>
> >
> > Add support to allow guests to set the new CR4 control bit for guests to enable
> > the new Intel CPU feature Linear Address Masking (LAM) on supervisor pointers.
> >
> > LAM modifies the checking that is applied to 64-bit linear addresses, allowing
> > software to use of the untranslated address bits for metadata and masks the
> > metadata bits before using them as linear addresses to access memory. LAM uses
> > CR4.LAM_SUP (bit 28) to configure LAM for supervisor pointers. LAM also changes
> > VMENTER to allow the bit to be set in VMCS's HOST_CR4 and GUEST_CR4 for
> > virtualization.
> >
> > Move CR4.LAM_SUP out of CR4_RESERVED_BITS and its reservation depends on vcpu
> > supporting LAM feature or not. Leave the bit intercepted to avoid vmread every
> > time when KVM fetches its value, with the expectation that guest won't toggle
> > the bit frequently.
> >
> > Set CR4.LAM_SUP bit in the emulated IA32_VMX_CR4_FIXED1 MSR for guests to allow
> > guests to enable LAM for supervisor pointers in nested VMX operation.
> >
> > Hardware is not required to do TLB flush when CR4.LAM_SUP toggled, KVM doesn't
> > need to emulate TLB flush based on it.
> > There's no other features/vmx_exec_controls connection, no other code needed in
> > {kvm,vmx}_set_cr4().
> >
> > Signed-off-by: Robert Hoo <robert.hu@xxxxxxxxxxxxxxx>
> > Co-developed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> > Signed-off-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> > Reviewed-by: Chao Gao <chao.gao@xxxxxxxxx>
> > Tested-by: Xuelian Guo <xuelian.guo@xxxxxxxxx>
> > ---
> >   arch/x86/include/asm/kvm_host.h | 3 ++-
> >   arch/x86/kvm/vmx/vmx.c          | 3 +++
> >   arch/x86/kvm/x86.h              | 2 ++
> >   3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index fb9d1f2d6136..c6f03d151c31 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -125,7 +125,8 @@
> >   			  | X86_CR4_PGE | X86_CR4_PCE | X86_CR4_OSFXSR | X86_CR4_PCIDE \
> >   			  | X86_CR4_OSXSAVE | X86_CR4_SMEP | X86_CR4_FSGSBASE \
> >   			  | X86_CR4_OSXMMEXCPT | X86_CR4_LA57 | X86_CR4_VMXE \
> > -			  | X86_CR4_SMAP | X86_CR4_PKE | X86_CR4_UMIP))
> > +			  | X86_CR4_SMAP | X86_CR4_PKE | X86_CR4_UMIP \
> > +			  | X86_CR4_LAM_SUP))
> >   
> >   #define CR8_RESERVED_BITS (~(unsigned long)X86_CR8_TPR)
> >   
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 44fb619803b8..362b2dce7661 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -7603,6 +7603,9 @@ static void nested_vmx_cr_fixed1_bits_update(struct kvm_vcpu *vcpu)
> >   	cr4_fixed1_update(X86_CR4_UMIP,       ecx, feature_bit(UMIP));
> >   	cr4_fixed1_update(X86_CR4_LA57,       ecx, feature_bit(LA57));
> >   
> > +	entry = kvm_find_cpuid_entry_index(vcpu, 0x7, 1);
> > +	cr4_fixed1_update(X86_CR4_LAM_SUP,    eax, feature_bit(LAM));
> > +
> >   #undef cr4_fixed1_update
> >   }
> >   
> > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > index c544602d07a3..fe67b641cce4 100644
> > --- a/arch/x86/kvm/x86.h
> > +++ b/arch/x86/kvm/x86.h
> > @@ -529,6 +529,8 @@ bool kvm_msr_allowed(struct kvm_vcpu *vcpu, u32 index, u32 type);
> >   		__reserved_bits |= X86_CR4_VMXE;        \
> >   	if (!__cpu_has(__c, X86_FEATURE_PCID))          \
> >   		__reserved_bits |= X86_CR4_PCIDE;       \
> > +	if (!__cpu_has(__c, X86_FEATURE_LAM))           \
> > +		__reserved_bits |= X86_CR4_LAM_SUP;     \
> >   	__reserved_bits;                                \
> >   })
> >   
> LAM only applies to 64-bit linear address, which means LAM can only be enabled
> when CPU is in 64-bit mode with either 4-level or 5-level paging enabled.
>
> What's the hardware behaviour if we set CR4.LAM_SUP when CPU isn't in 64-bit
> mode?  And how does VMENTRY check GUEST_CR4.LAM_SUP and 64-bit mode?
>
> Looks they are not clear in the spec you pasted in the cover letter:
>
> https://cdrdv2.intel.com/v1/dl/getContent/671368
>
> Or I am missing something?
Yes, it is not clearly described in LAM spec.
Had some internal discussions and also did some tests in host,
if the processor supports LAM, CR4.LAM_SUP is allowed to be set even 
when cpu isn't in 64bit mode.

There was a statement in commit message of the last version, but I 
missed it in this version. I'll add it back.
"CR4.LAM_SUP is allowed to be set even not in 64-bit mode, but it will not
take effect since LAM only applies to 64-bit linear address."

Also, I will try to ask Intel guys if it's possible to update the document.