linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, kys@microsoft.com,  haiyangz@microsoft.com,
	wei.liu@kernel.org, decui@microsoft.com,  tglx@linutronix.de,
	mingo@redhat.com, bp@alien8.de,  dave.hansen@linux.intel.com,
	pbonzini@redhat.com, ardb@kernel.org,  kees@kernel.org,
	Arnd Bergmann <arnd@arndb.de>,
	gregkh@linuxfoundation.org,  jpoimboe@kernel.org,
	linux-hyperv@vger.kernel.org,  linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, linux-efi@vger.kernel.org,
	 samitolvanen@google.com, ojeda@kernel.org, xin@zytor.com
Subject: Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
Date: Thu, 1 May 2025 11:30:18 -0700	[thread overview]
Message-ID: <aBO9uoLnxCSD0UwT@google.com> (raw)
In-Reply-To: <20250501153844.GD4356@noisy.programming.kicks-ass.net>

On Thu, May 01, 2025, Peter Zijlstra wrote:
> On Thu, May 01, 2025 at 12:30:38PM +0200, Peter Zijlstra wrote:
> > On Wed, Apr 30, 2025 at 09:06:00PM +0200, Peter Zijlstra wrote:
> > > On Wed, Apr 30, 2025 at 07:24:15AM -0700, H. Peter Anvin wrote:
> > > 
> > > > >KVM has another; the VMX interrupt injection stuff calls the IDT handler
> > > > >directly.  Is there an alternative? Can we keep a table of Linux functions
> > > > >slighly higher up the call stack (asm_\cfunc ?) and add CFI to those?
> > > 
> > > > We do have a table of handlers higher up in the stack in the form of
> > > > the dispatch tables for FRED. They don't in general even need the
> > > > assembly entry stubs, either.
> > > 
> > > Oh, right. I'll go have a look at those.
> > 
> > Right, so perhaps the easiest way around this is to setup the FRED entry
> > tables unconditionally, have VMX mandate CONFIG_FRED and then have it
> > always use the FRED entry points.
> > 
> > Let me see how ugly that gets.
> 
> Something like so... except this is broken. Its reporting spurious
> interrupts on vector 0x00, so something is buggered passing that vector
> along.

Uh, aren't you making this way more complex than it needs to be?  IIUC, KVM never
uses the FRED hardware entry points, i.e. the FRED entry tables don't need to be
in place because they'll never be used.  The only bits of code KVM needs is the
__fred_entry_from_kvm() glue.

Lightly tested, but this combo works for IRQs and NMIs on non-FRED hardware.

--
From 664468143109ab7c525c0babeba62195fa4c657e Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Thu, 1 May 2025 11:20:29 -0700
Subject: [PATCH 1/2] x86/fred: Play nice with invoking
 asm_fred_entry_from_kvm() on non-FRED hardware

Modify asm_fred_entry_from_kvm() to allow it to be invoked by KVM even
when FRED isn't fully enabled, e.g. when running with CONFIG_X86_FRED=y
on non-FRED hardware.  This will allow forcing KVM to always use the FRED
entry points for 64-bit kernels, which in turn will eliminate a rather
gross non-CFI indirect call that KVM uses to trampoline IRQs by doing IDT
lookups.

When FRED isn't enabled, simply skip ERETS and restore RBP and RSP from
the stack frame prior to doing a "regular" RET back to KVM (in quotes
because of all the RET mitigation horrors).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/entry/entry_64_fred.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index 29c5c32c16c3..7aff2f0a285f 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S
@@ -116,7 +116,8 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
 	movq %rsp, %rdi				/* %rdi -> pt_regs */
 	call __fred_entry_from_kvm		/* Call the C entry point */
 	POP_REGS
-	ERETS
+
+	ALTERNATIVE "", __stringify(ERETS), X86_FEATURE_FRED
 1:
 	/*
 	 * Objtool doesn't understand what ERETS does, this hint tells it that
@@ -124,7 +125,7 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
 	 * isn't strictly needed, but it's the simplest form.
 	 */
 	UNWIND_HINT_RESTORE
-	pop %rbp
+	leave
 	RET
 
 SYM_FUNC_END(asm_fred_entry_from_kvm)

base-commit: 45eb29140e68ffe8e93a5471006858a018480a45
-- 
2.49.0.906.g1f30a19c02-goog

From c50fb5a8a46058bbcfdcac0a100c2aa0f7f68f1c Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Thu, 1 May 2025 11:10:39 -0700
Subject: [PATCH 2/2] x86/fred: KVM: VMX: Always use FRED for IRQ+NMI when
 CONFIG_X86_FRED=y

Now that FRED provides C-code entry points for handling IRQ and NMI exits,
use the FRED infrastructure for forwarding all such events even if FRED
isn't supported in hardware.  Avoiding the non-FRED assembly trampolines
into the IDT handlers for IRQs eliminates the associated non-CFI indirect
call (KVM performs a CALL by doing a lookup on the IDT using the IRQ
vector).

Force FRED for 64-bit kernels if KVM_INTEL is enabled, as the benefits of
eliminating the IRQ trampoline usage far outwieghts the code overhead for
FRED.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/Kconfig   | 1 +
 arch/x86/kvm/vmx/vmx.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 2eeffcec5382..712a2ff28ce4 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -95,6 +95,7 @@ config KVM_SW_PROTECTED_VM
 config KVM_INTEL
 	tristate "KVM for Intel (and compatible) processors support"
 	depends on KVM && IA32_FEAT_CTL
+	select X86_FRED if X86_64
 	select KVM_GENERIC_PRIVATE_MEM if INTEL_TDX_HOST
 	select KVM_GENERIC_MEMORY_ATTRIBUTES if INTEL_TDX_HOST
 	help
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index ef2d7208dd20..2ea89985107d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6995,7 +6995,7 @@ static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu,
 		return;
 
 	kvm_before_interrupt(vcpu, KVM_HANDLING_IRQ);
-	if (cpu_feature_enabled(X86_FEATURE_FRED))
+	if (IS_ENABLED(CONFIG_X86_FRED))
 		fred_entry_from_kvm(EVENT_TYPE_EXTINT, vector);
 	else
 		vmx_do_interrupt_irqoff(gate_offset((gate_desc *)host_idt_base + vector));
@@ -7268,7 +7268,7 @@ noinstr void vmx_handle_nmi(struct kvm_vcpu *vcpu)
 		return;
 
 	kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
-	if (cpu_feature_enabled(X86_FEATURE_FRED))
+	if (IS_ENABLED(CONFIG_X86_FRED))
 		fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR);
 	else
 		vmx_do_nmi_irqoff();
-- 
2.49.0.906.g1f30a19c02-goog

  reply	other threads:[~2025-05-01 18:30 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-30 11:07 [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 01/13] x86/kvm/emulate: Implement test_cc() in C Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 02/13] x86/kvm/emulate: Introduce COP1 Peter Zijlstra
2025-04-30 16:19   ` Josh Poimboeuf
2025-04-30 19:05     ` Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 03/13] x86/kvm/emulate: Introduce COP2 Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 04/13] x86/kvm/emulate: Introduce COP2R Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 05/13] x86/kvm/emulate: Introduce COP2W Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 06/13] x86/kvm/emulate: Introduce COP2CL Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 07/13] x86/kvm/emulate: Introduce COP1SRC2 Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 08/13] x86/kvm/emulate: Introduce COP3WCL Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 09/13] x86/kvm/emulate: Convert em_salc() to C Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 10/13] x86/kvm/emulate: Remove fastops Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 11/13] x86,hyperv: Clean up hv_do_hypercall() Peter Zijlstra
2025-05-01  2:36   ` Michael Kelley
2025-04-30 11:07 ` [PATCH v2 12/13] x86_64,hyperv: Use direct call to hypercall-page Peter Zijlstra
2025-05-01  2:36   ` Michael Kelley
2025-05-01  8:59     ` Peter Zijlstra
2025-04-30 11:07 ` [PATCH v2 13/13] objtool: Validate kCFI calls Peter Zijlstra
2025-04-30 15:59   ` Josh Poimboeuf
2025-04-30 19:03     ` Peter Zijlstra
2025-05-01 15:56       ` Peter Zijlstra
2025-04-30 14:24 ` [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions H. Peter Anvin
2025-04-30 19:06   ` Peter Zijlstra
2025-05-01 10:30     ` Peter Zijlstra
2025-05-01 15:38       ` Peter Zijlstra
2025-05-01 18:30         ` Sean Christopherson [this message]
2025-05-01 18:42           ` H. Peter Anvin
2025-05-01 18:59             ` Sean Christopherson
2025-05-02  6:12               ` Xin Li
2025-05-02  5:46           ` Xin Li
2025-05-02  5:48           ` Xin Li
2025-05-02 19:43             ` H. Peter Anvin
2025-05-02  8:40           ` Peter Zijlstra
2025-05-02 19:53             ` Sean Christopherson
2025-05-03  9:50               ` Peter Zijlstra
2025-05-03 18:28                 ` Josh Poimboeuf
2025-05-06  7:31                   ` Peter Zijlstra
2025-05-06 13:32                     ` Peter Zijlstra
2025-05-06 19:18                       ` Josh Poimboeuf
2025-05-28  7:44                         ` Peter Zijlstra
2025-05-28 16:30                           ` Peter Zijlstra
2025-05-28 16:35                             ` Peter Zijlstra
2025-05-29  9:30                               ` Peter Zijlstra
2025-06-03  5:43                                 ` Josh Poimboeuf
2025-06-03 16:29                                   ` Josh Poimboeuf
2025-06-05 17:19                                     ` Josh Poimboeuf
2025-06-06 10:49                                       ` Peter Zijlstra
2025-06-06 13:15                                         ` Sean Christopherson
2025-06-06 13:20                                           ` Peter Zijlstra
2025-05-01 18:33 ` Paolo Bonzini
2025-05-02 11:08   ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aBO9uoLnxCSD0UwT@google.com \
    --to=seanjc@google.com \
    --cc=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=decui@microsoft.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haiyangz@microsoft.com \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@kernel.org \
    --cc=kees@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=kys@microsoft.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    --cc=tglx@linutronix.de \
    --cc=wei.liu@kernel.org \
    --cc=x86@kernel.org \
    --cc=xin@zytor.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).