On Thu, Nov 03, 2022 at 09:41:44PM +0800,
Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx> wrote:
>
> On 2022/10/30 14:22, isaku.yamahata@xxxxxxxxx wrote:
> > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> >
> > To support TDX, KVM is enhanced to operate with #VE. For TDX, KVM programs
> > to inject #VE conditionally and set #VE suppress bit in EPT entry. For VMX
> > case, #VE isn't used. If #VE happens for VMX, it's a bug. To be
> > defensive (test that VMX case isn't broken), introduce option
> > ept_violation_ve_test and when it's set, set error.
> >
> > Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> > ---
> > arch/x86/include/asm/vmx.h | 12 +++++++
> > arch/x86/kvm/vmx/vmcs.h | 5 +++
> > arch/x86/kvm/vmx/vmx.c | 69 +++++++++++++++++++++++++++++++++++++-
> > arch/x86/kvm/vmx/vmx.h | 6 +++-
> > 4 files changed, 90 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> > index cdbf12c1a83c..752d53652007 100644
> > --- a/arch/x86/include/asm/vmx.h
> > +++ b/arch/x86/include/asm/vmx.h
> > @@ -68,6 +68,7 @@
> > #define SECONDARY_EXEC_ENCLS_EXITING VMCS_CONTROL_BIT(ENCLS_EXITING)
> > #define SECONDARY_EXEC_RDSEED_EXITING VMCS_CONTROL_BIT(RDSEED_EXITING)
> > #define SECONDARY_EXEC_ENABLE_PML VMCS_CONTROL_BIT(PAGE_MOD_LOGGING)
> > +#define SECONDARY_EXEC_EPT_VIOLATION_VE VMCS_CONTROL_BIT(EPT_VIOLATION_VE)
> > #define SECONDARY_EXEC_PT_CONCEAL_VMX VMCS_CONTROL_BIT(PT_CONCEAL_VMX)
> > #define SECONDARY_EXEC_XSAVES VMCS_CONTROL_BIT(XSAVES)
> > #define SECONDARY_EXEC_MODE_BASED_EPT_EXEC VMCS_CONTROL_BIT(MODE_BASED_EPT_EXEC)
> > @@ -223,6 +224,8 @@ enum vmcs_field {
> > VMREAD_BITMAP_HIGH = 0x00002027,
> > VMWRITE_BITMAP = 0x00002028,
> > VMWRITE_BITMAP_HIGH = 0x00002029,
> > + VE_INFORMATION_ADDRESS = 0x0000202A,
> > + VE_INFORMATION_ADDRESS_HIGH = 0x0000202B,
> > XSS_EXIT_BITMAP = 0x0000202C,
> > XSS_EXIT_BITMAP_HIGH = 0x0000202D,
> > ENCLS_EXITING_BITMAP = 0x0000202E,
> > @@ -628,4 +631,13 @@ enum vmx_l1d_flush_state {
> > extern enum vmx_l1d_flush_state l1tf_vmx_mitigation;
> > +struct vmx_ve_information {
> > + u32 exit_reason;
> > + u32 delivery;
> > + u64 exit_qualification;
> > + u64 guest_linear_address;
> > + u64 guest_physical_address;
> > + u16 eptp_index;
> > +};
> > +
> > #endif
> > diff --git a/arch/x86/kvm/vmx/vmcs.h b/arch/x86/kvm/vmx/vmcs.h
> > index ac290a44a693..9277676057a7 100644
> > --- a/arch/x86/kvm/vmx/vmcs.h
> > +++ b/arch/x86/kvm/vmx/vmcs.h
> > @@ -140,6 +140,11 @@ static inline bool is_nm_fault(u32 intr_info)
> > return is_exception_n(intr_info, NM_VECTOR);
> > }
> > +static inline bool is_ve_fault(u32 intr_info)
> > +{
> > + return is_exception_n(intr_info, VE_VECTOR);
> > +}
> > +
> > /* Undocumented: icebp/int1 */
> > static inline bool is_icebp(u32 intr_info)
> > {
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index f890191e8580..dd3fde9d3c32 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -126,6 +126,9 @@ module_param(error_on_inconsistent_vmcs_config, bool, 0444);
> > static bool __read_mostly dump_invalid_vmcs = 0;
> > module_param(dump_invalid_vmcs, bool, 0644);
> > +static bool __read_mostly ept_violation_ve_test;
> > +module_param(ept_violation_ve_test, bool, 0444);
> > +
> > #define MSR_BITMAP_MODE_X2APIC 1
> > #define MSR_BITMAP_MODE_X2APIC_APICV 2
> > @@ -783,6 +786,13 @@ void vmx_update_exception_bitmap(struct kvm_vcpu *vcpu)
> > eb = (1u << PF_VECTOR) | (1u << UD_VECTOR) | (1u << MC_VECTOR) |
> > (1u << DB_VECTOR) | (1u << AC_VECTOR);
> > + /*
> > + * #VE isn't used for VMX, but for TDX. To test against unexpected
> > + * change related to #VE for VMX, intercept unexpected #VE and warn on
> > + * it.
> > + */
> > + if (ept_violation_ve_test)
> > + eb |= 1u << VE_VECTOR;
> > /*
> > * Guest access to VMware backdoor ports could legitimately
> > * trigger #GP because of TSS I/O permission bitmap.
> > @@ -2644,6 +2654,9 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
> > &_cpu_based_2nd_exec_control))
> > return -EIO;
> > }
> > + if (!ept_violation_ve_test)
> > + _cpu_based_exec_control &= ~SECONDARY_EXEC_EPT_VIOLATION_VE;
>
> should be _cpu_based_2nd_exec_control
Oops, thanks.
--
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
