On Thu, Jul 28, 2022 at 08:11:59PM +0000, Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > On Thu, Jul 28, 2022, Kai Huang wrote: > > On Wed, 2022-07-27 at 16:39 -0700, Isaku Yamahata wrote: > > > On Wed, Jul 20, 2022 at 05:13:08PM +1200, > > > Kai Huang <kai.huang@xxxxxxxxx> wrote: > > > > > > > On Tue, 2022-07-19 at 07:49 -0700, Isaku Yamahata wrote: > > > > > On Fri, Jul 08, 2022 at 02:23:43PM +1200, > > > > > Kai Huang <kai.huang@xxxxxxxxx> wrote: > > > > > > > > > > > On Mon, 2022-06-27 at 14:53 -0700, 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. > > > > > > > > > > > > I don't see why we need this patch. It may be helpful during your test, but why > > > > > > do we need this patch for formal submission? > > > > > > > > > > > > And for a normal guest, what prevents one vcpu from sending #VE IPI to another > > > > > > vcpu? > > > > > > > > > > Paolo suggested it as follows. Maybe it should be kernel config. > > > > > (I forgot to add suggested-by. I'll add it) > > > > > > > > > > https://lore.kernel.org/lkml/84d56339-4a8a-6ddb-17cb-12074588ba9c@xxxxxxxxxx/ > > > > > > > > > > > > > > > > > > > OK. But can we assume a normal guest won't sending #VE IPI? > > > > > > Theoretically nothing prevents that. I wouldn't way "normal". > > > Anyway this is off by default. > > > > I don't think whether it is on or off by default matters. > > It matters in the sense that the module param is intended purely for testing, i.e. > there's zero reason to ever enable it in production. That changes what is and > wasn't isn't a reasonable response to an unexpected #VE. > > > If it can happen legitimately in the guest, it doesn't look right to print > > out something like below: > > > > pr_err("VMEXIT due to unexpected #VE.\n"); > > Agreed. In this particular case I think the right approach is to treat an > unexpected #VE as a fatal KVM bug. Yes, disabling EPT violation #VEs would likely > allow the guest to live, but as above the module param should never be enabled in > production. And if we get a #VE with the module param disabled, then KVM is truly > in the weeds and killing the VM is the safe option. > > E.g. something like Thanks, I finally ended up with the following. 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 881db80ceee9..c3e4c0d17b63 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5047,6 +5047,12 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu) if (is_invalid_opcode(intr_info)) return handle_ud(vcpu); + /* + * #VE isn't supposed to happen. Although vcpu can send + */ + if (KVM_BUG_ON(is_ve_fault(intr_info), vcpu->kvm)) + return -EIO; + error_code = 0; if (intr_info & INTR_INFO_DELIVER_CODE_MASK) error_code = vmcs_read32(VM_EXIT_INTR_ERROR_CODE); @@ -5167,14 +5173,7 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu) if (handle_guest_split_lock(kvm_rip_read(vcpu))) return 1; fallthrough; - case VE_VECTOR: default: - if (ept_violation_ve_test && ex_no == VE_VECTOR) { - pr_err("VMEXIT due to unexpected #VE.\n"); - secondary_exec_controls_clearbit( - vmx, SECONDARY_EXEC_EPT_VIOLATION_VE); - return 1; - } kvm_run->exit_reason = KVM_EXIT_EXCEPTION; kvm_run->ex.exception = ex_no; kvm_run->ex.error_code = error_code; -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
