mbox series

[GIT,PULL] Final KVM fix for Linux 6.7

Message ID 20240104154844.129586-1-pbonzini@redhat.com
State New
Headers
Series [GIT,PULL] Final KVM fix for Linux 6.7 |

Pull-request

https://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus

Message

Paolo Bonzini Jan. 4, 2024, 3:48 p.m. UTC 
  Linus,

The following changes since commit ac865f00af293d081356bec56eea90815094a60e:

  Merge tag 'pci-v6.7-fixes-2' of git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci (2024-01-03 14:18:57 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus

for you to fetch changes up to 971079464001c6856186ca137778e534d983174a:

  KVM: x86/pmu: fix masking logic for MSR_CORE_PERF_GLOBAL_CTRL (2024-01-04 16:31:27 +0100)

This is technically not a KVM patch, but I am sending it anyway considering that:

- the patch is doing nothing but undoing a Boolean logic brain fart, restoring
  the pre-6.0 logic

- the function is literally called intel_guest_get_msrs and I am touching only
  the "guest" field, so the non-KVM effects are clearly nil

- this part of the file is often marked as "KVM" in the commit summaries, and
  sent via the KVM tree with Acked-by (usually from PeterZ, whom I am CCing)

- we are pretty close to the release but many people are still in Christmas
  vacation mode/mood

- the bug is not theoretical, Paul described the reproducer as triggering
  "rarely but intolerably often"

- writing this explanation almost took more time than writing the patch,
  thus proving that I reaallyy would like this in 6.7

Thanks,

Paolo

----------------------------------------------------------------
* Fix Boolean logic in intel_guest_get_msrs

----------------------------------------------------------------
Paolo Bonzini (1):
      KVM: x86/pmu: fix masking logic for MSR_CORE_PERF_GLOBAL_CTRL

 arch/x86/events/intel/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Linus Torvalds Jan. 5, 2024, 5:21 p.m. UTC | #1 
On Thu, 4 Jan 2024 at 07:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> * Fix Boolean logic in intel_guest_get_msrs

I think the intention of the original was to write this as

        .guest = intel_ctrl & ~(cpuc->intel_ctrl_host_mask | pebs_mask),

but your version certainly works too.

Pulled.

           Linus
Sean Christopherson Jan. 5, 2024, 5:29 p.m. UTC | #2 
On Fri, Jan 05, 2024, Linus Torvalds wrote:
> On Thu, 4 Jan 2024 at 07:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > * Fix Boolean logic in intel_guest_get_msrs
> 
> I think the intention of the original was to write this as
> 
>         .guest = intel_ctrl & ~(cpuc->intel_ctrl_host_mask | pebs_mask),
> 
> but your version certainly works too.

Ha!  That's what I suggested too, clearly Paolo is the weird one :-)
Paolo Bonzini Jan. 5, 2024, 5:35 p.m. UTC | #3 
On Fri, Jan 5, 2024 at 6:29 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Jan 05, 2024, Linus Torvalds wrote:
> > On Thu, 4 Jan 2024 at 07:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >
> > > * Fix Boolean logic in intel_guest_get_msrs
> >
> > I think the intention of the original was to write this as
> >
> >         .guest = intel_ctrl & ~(cpuc->intel_ctrl_host_mask | pebs_mask),
> >
> > but your version certainly works too.
>
> Ha!  That's what I suggested too, clearly Paolo is the weird one :-)

Not that I don't like your version; but since you hadn't provided
(yet) a Signed-off-by I had not looked at the patch until now.

In the end it's a coin toss between "fix what was clearly intended"
and "restore the logic before the broken commit". I picked the latter
because when trying to reverse engineer Linus's brain processes I tend
to overthink these things. :) That counts as being weird, I guess.

Paolo
Linus Torvalds Jan. 5, 2024, 5:38 p.m. UTC | #4 
On Fri, 5 Jan 2024 at 09:29, Sean Christopherson <seanjc@google.com> wrote:
>
> Ha!  That's what I suggested too, clearly Paolo is the weird one :-)

Well, it's technically one fewer operation to do it our way, but
Paolo's version is

 (a) textually one character shorter

 (b) something the compiler can (and likely will) munge anyway, since
boolean operation optimizations are common

 (c) with the 'andn' instruction, the "fewer operations" isn't
necessarily fewer instructions

Of course, we can't currently use 'andn' in kernel code due to it
being much too new and requires BMI1. Plus the memory op version is
the wrong way around (ie the "not" part of the op only works on
register inputs), but _evenbtually_ that might have been an argument.

            Linus
pr-tracker-bot@kernel.org Jan. 5, 2024, 6:10 p.m. UTC | #5 
The pull request you sent on Thu,  4 Jan 2024 16:48:44 +0100:

> https://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/7987b8b75f1b0d00483629a0ba006dac81e227c8

Thank you!