linux-kernel - Re: [GIT PULL] First batch of KVM changes for 5.6 merge window

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 31 Jan 2020 10:01:37 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Borislav Petkov <bp@...e.de>, Xiaoyao Li <xiaoyao.li@...el.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        KVM list <kvm@...r.kernel.org>
Subject: Re: [GIT PULL] First batch of KVM changes for 5.6 merge window

On Thu, Jan 30, 2020 at 10:20 AM Paolo Bonzini <pbonzini@...hat.com> wrote:
>
> Xiaoyao Li (3):
>       KVM: VMX: Rename INTERRUPT_PENDING to INTERRUPT_WINDOW
>       KVM: VMX: Rename NMI_PENDING to NMI_WINDOW
>       KVM: VMX: Fix the spelling of CPU_BASED_USE_TSC_OFFSETTING

So in the meantime, on the x86 merge window side, we have this:

  b39033f504a7 ("KVM: VMX: Use VMX_FEATURE_* flags to define VMCS control bits")

and while the above results in a conflict, that's not a problem. The
conflict was trivial to fix up.

HOWEVER.

It most definitely shows that the above renaming now means that the
names don't match. It didn't match 100% before either, but now the
differences are even bigger. The VMX_FEATURE_xyz bits have different
names than the CPU_BASED_xyz bits, and that seems a bit questionable.

So I'm not convinced about the renaming. The spelling fix is good: it
actually now more closely resembles the VMCS_FEATURE bit that already
had OFFSETTING with two T's.

But even that one isn't really the same even then. The CPU_BASED_xyz
thing has "USE_TSC_OFFSETTING", while the VMCS_FEATURE_xyz bit doesn't
have the "USE" part.

And the actual renaming means that now we basically have

  CPU_BASED_INTR_WINDOW_EXITING
  VMX_FEATURE_VIRTUAL_INTR_PENDING

and

  CPU_BASED_NMI_WINDOW_EXITING
  VMX_FEATURE_VIRTUAL_NMI_PENDING

for the same bit definitions (yeah, the VMX_FEATURE bits obviously
have the offset in them, so it's not the same _value_, but it's a 1:1
relationship between them).

There are other (pre-existing) differences, but while fixing up the
merge conflict I really got the feeling that it's confusing and wrong
to basically use different naming for these things when they are about
the same bit.

I don't care much which way it goes (maybe the VMX_FATURE_xyz bits
should be renamed instead of the other way around?) and I wonder what
the official documentation names are? Is there some standard here or
are people just picking names at random?

The two commits both came from intel.com addresses, so hopefully there
can be some intel-sanctioned resolution on the naming? Please?

Hmm?

                  Linus