| Message ID | 20240712235701.1458888-9-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GIT,PULL,(sort,of)] KVM: x86: Static call changes for 6.11 | expand |
On 7/13/24 01:56, Sean Christopherson wrote: > Here's a massage pull request for the static_call() changes, just in case you > want to go this route instead of applying patches directly after merging > everything else for 6.11 (it was easy to generate this). If you want to go the > patches route, I'll post 'em next week. > > The following changes since commit c1c8a908a5f4c372f8a8dca0501b56ffc8d260fe: > > Merge branch 'vmx' (2024-06-28 22:22:53 +0000) > > are available in the Git repository at: > > https://github.com/kvm-x86/linux.git tags/kvm-x86-static_calls-6.11 > > for you to fetch changes up to b528de209c858f61953023b405a4abbf9a9933da: > > KVM: x86/pmu: Add kvm_pmu_call() to simplify static calls of kvm_pmu_ops (2024-06-28 15:23:49 -0700) Thanks, indeed there was no straggler static_call() after applying this. However, there might be a problem: static_call_cond() is equal to static_call() only if CONFIG_HAVE_STATIC_CALL_INLINE, and arch/x86 has this: select HAVE_STATIC_CALL_INLINE if HAVE_OBJTOOL select HAVE_OBJTOOL if X86_64 And indeed if I apply diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index 80e5afde69f4..d20159d4a37a 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -21,6 +21,8 @@ config KVM tristate "Kernel-based Virtual Machine (KVM) support" depends on HIGH_RES_TIMERS depends on X86_LOCAL_APIC + # KVM relies on static_call_cond() being the same as static_call() + depends on HAVE_STATIC_CALL_INLINE select KVM_COMMON select KVM_GENERIC_MMU_NOTIFIER select HAVE_KVM_IRQCHIP KVM disappears from 32-bit kernels. :) So I haven't checked but I suspect this breaks 32-bit? Paolo
On Tue, Jul 16, 2024, Paolo Bonzini wrote: > On 7/13/24 01:56, Sean Christopherson wrote: > > Here's a massage pull request for the static_call() changes, just in case you > > want to go this route instead of applying patches directly after merging > > everything else for 6.11 (it was easy to generate this). If you want to go the > > patches route, I'll post 'em next week. > > > > The following changes since commit c1c8a908a5f4c372f8a8dca0501b56ffc8d260fe: > > > > Merge branch 'vmx' (2024-06-28 22:22:53 +0000) > > > > are available in the Git repository at: > > > > https://github.com/kvm-x86/linux.git tags/kvm-x86-static_calls-6.11 > > > > for you to fetch changes up to b528de209c858f61953023b405a4abbf9a9933da: > > > > KVM: x86/pmu: Add kvm_pmu_call() to simplify static calls of kvm_pmu_ops (2024-06-28 15:23:49 -0700) > > Thanks, indeed there was no straggler static_call() after applying > this. However, there might be a problem: static_call_cond() is equal > to static_call() only if CONFIG_HAVE_STATIC_CALL_INLINE, No, I think you misread the #if-#elif-#else. It's only the !HAVE_STATIC_CALL case that requires use of static_call_cond(). From include/linux/static_call.h: #ifdef CONFIG_HAVE_STATIC_CALL_INLINE #define static_call_cond(name) (void)__static_call(name) #elif defined(CONFIG_HAVE_STATIC_CALL) #define static_call_cond(name) (void)__static_call(name) #else #define static_call_cond(name) (void)__static_call_cond(name) #endif And per Josh, from an old RFC[*] to yank out static_call_cond(): : Static calling a NULL pointer is a NOP, unless you're one of those poor : souls running on an arch (or backported x86 monstrosity) with : CONFIG_HAVE_STATIC_CALL=n, then it's a panic. I double checked that 32-bit KVM works on Intel (which is guaranteed to have a NULL guest_memory_reclaimed()). I also verified that the generated code is identical for both static_call() and static_call_cond(), i.e. the READ_ONCE() of the func at runtime that's present in __static_call_cond() isn't showing up. Dump of assembler code for function kvm_arch_guest_memory_reclaimed: 0xc1042094 <+0>: call 0xc10ce650 <__fentry__> 0xc1042099 <+5>: push %ebp 0xc104209a <+6>: mov %esp,%ebp 0xc104209c <+8>: call 0xc1932d8c <__SCT__kvm_x86_guest_memory_reclaimed> 0xc10420a1 <+13>: pop %ebp 0xc10420a2 <+14>: ret End of assembler dump. Dump of assembler code for function __SCT__kvm_x86_guest_memory_reclaimed: 0xc1932d8c <+0>: ret 0xc1932d8d <+1>: int3 0xc1932d8e <+2>: nop 0xc1932d8f <+3>: nop 0xc1932d90 <+4>: nop 0xc1932d91 <+5>: ud1 %esp,%ecx End of assembler dump. [*] https://lore.kernel.org/all/cover.1678474914.git.jpoimboe@kernel.org
On 7/16/24 17:46, Sean Christopherson wrote: > No, I think you misread the #if-#elif-#else. It's only the !HAVE_STATIC_CALL > case that requires use of static_call_cond(). Oh, of course - the "select HAVE_STATIC_CALL" is right above the "select HAVE_STATIC_CALL_INLINE if HAVE_OBJTOOL" line. I was definitely overthinking it. Paolo
Here's a massage pull request for the static_call() changes, just in case you want to go this route instead of applying patches directly after merging everything else for 6.11 (it was easy to generate this). If you want to go the patches route, I'll post 'em next week. The following changes since commit c1c8a908a5f4c372f8a8dca0501b56ffc8d260fe: Merge branch 'vmx' (2024-06-28 22:22:53 +0000) are available in the Git repository at: https://github.com/kvm-x86/linux.git tags/kvm-x86-static_calls-6.11 for you to fetch changes up to b528de209c858f61953023b405a4abbf9a9933da: KVM: x86/pmu: Add kvm_pmu_call() to simplify static calls of kvm_pmu_ops (2024-06-28 15:23:49 -0700) ---------------------------------------------------------------- KVM x86 static_call() cleanup for 6.11 Add kvm_x86_call() and kvm_pmu_call() wrappers for KVM's static_call() usage to improve readability and make it easier to connect the calls to the vendor implementations. ---------------------------------------------------------------- Wei Wang (3): KVM: x86: Replace static_call_cond() with static_call() KVM: x86: Introduce kvm_x86_call() to simplify static calls of kvm_x86_ops KVM: x86/pmu: Add kvm_pmu_call() to simplify static calls of kvm_pmu_ops arch/x86/include/asm/kvm_host.h | 11 +++-- arch/x86/kvm/cpuid.c | 2 +- arch/x86/kvm/hyperv.c | 6 +-- arch/x86/kvm/irq.c | 2 +- arch/x86/kvm/kvm_cache_regs.h | 10 ++--- arch/x86/kvm/lapic.c | 42 +++++++++--------- arch/x86/kvm/lapic.h | 2 +- arch/x86/kvm/mmu.h | 6 +-- arch/x86/kvm/mmu/mmu.c | 6 +-- arch/x86/kvm/mmu/spte.c | 4 +- arch/x86/kvm/pmu.c | 29 ++++++------ arch/x86/kvm/smm.c | 44 +++++++++--------- arch/x86/kvm/trace.h | 15 ++++--- arch/x86/kvm/x86.c | 324 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------- arch/x86/kvm/x86.h | 2 +- arch/x86/kvm/xen.c | 4 +- 16 files changed, 261 insertions(+), 248 deletions(-)