On 17/01/25 09:15, Sean Christopherson wrote:
On Fri, Jan 17, 2025, Valentin Schneider wrote:
On 14/01/25 13:13, Sean Christopherson wrote:
On Tue, Jan 14, 2025, Valentin Schneider wrote:
+/**
- is_kernel_noinstr_text - checks if the pointer address is located in the
.noinstr section
- @addr: address to check
- Returns: true if the address is located in .noinstr, false otherwise.
- */
+static inline bool is_kernel_noinstr_text(unsigned long addr) +{
- return addr >= (unsigned long)__noinstr_text_start &&
addr < (unsigned long)__noinstr_text_end;+}
This doesn't do the right thing for modules, which matters because KVM can be built as a module on x86, and because context tracking understands transitions to GUEST mode, i.e. CPUs that are running in a KVM guest will be treated as not being in the kernel, and thus will have IPIs deferred. If KVM uses a static key or branch between guest_state_enter_irqoff() and guest_state_exit_irqoff(), the patching code won't wait for CPUs to exit guest mode, i.e. KVM could theoretically use the wrong static path.
AFAICT guest_state_{enter,exit}_irqoff() are only used in noinstr functions and thus such a static key usage should at the very least be caught and warned about by objtool - when this isn't built as a module.
That doesn't magically do the right thing though. If KVM is built as a module, is_kernel_noinstr_text() will get false negatives even for static keys/branches that are annotaed as NOINSTR.
Quite so.
I've been looking at mod_mem_type & friends, I'm thinking adding a MOD_NOINSTR_TEXT type might be overkill considering modules really shouldn't be involved with early entry, KVM being the one exception.
Your suggestion to have a KVM-module-specific noinstr section sounds good to me, I'll have a look at that.