Re: [PATCH v4 25/30] context_tracking,x86: Defer kernel text patching IPIs

[Sean Christopherson <seanjc@xxxxxxxxxx>]

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.