2020-02-12 16:48:30

by Paolo Bonzini

[permalink] [raw]
Subject: [GIT PULL] KVM changes for Linux 5.6-rc2

Linus,

The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:

Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)

are available in the Git repository at:

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

for you to fetch changes up to c4c3fdc9c43b77f1b99c71d6edda8e20d00c3e2d:

docs: virt: guest-halt-polling.txt convert to ReST (2020-02-12 14:33:07 +0100)

----------------------------------------------------------------
A mix of cleanups and bugfixes, some of them slightly more invasive than usual
but still not worth waiting for 5.7. On top of this, Mauro converted the
KVM documentation to rst format, which was very welcome, and Eric ported
the selftests infrastructure to nested AMD virtualization (which will come
in handy when adding nested live migration support, as it did for Intel).

----------------------------------------------------------------
Eric Auger (4):
selftests: KVM: Replace get_{gdt,idt}_base() by get_{gdt,idt}()
selftests: KVM: AMD Nested test infrastructure
selftests: KVM: SVM: Add vmcall test
selftests: KVM: Remove unused x86_register enum

Marc Zyngier (1):
KVM: Disable preemption in kvm_get_running_vcpu()

Mauro Carvalho Chehab (28):
docs: kvm: add arm/pvtime.rst to index.rst
docs: virt: convert UML documentation to ReST
docs: virt: user_mode_linux.rst: update compiling instructions
docs: virt: user_mode_linux.rst: fix URL references
docs: virt: convert halt-polling.txt to ReST format
docs: virt: Convert msr.txt to ReST format
docs: kvm: devices/arm-vgic-its.txt to ReST format
docs: kvm: devices/arm-vgit-v3.txt to ReST
docs: kvm: convert devices/arm-vgit.txt to ReST
docs: kvm: convert devices/mpic.txt to ReST
docs: kvm: convert devices/s390_flic.txt to ReST
docs: kvm: convert devices/vcpu.txt to ReST
docs: kvm: convert devices/vfio.txt to ReST
docs: kvm: convert devices/vm.txt to ReST
docs: kvm: convert devices/xics.txt to ReST
docs: kvm: convert devices/xive.txt to ReST
docs: kvm: Convert api.txt to ReST format
docs: kvm: convert arm/hyp-abi.txt to ReST
docs: kvm: arm/psci.txt: convert to ReST
docs: kvm: Convert hypercalls.txt to ReST format
docs: kvm: Convert locking.txt to ReST format
docs: kvm: Convert mmu.txt to ReST format
docs: kvm: Convert nested-vmx.txt to ReST format
docs: kvm: Convert ppc-pv.txt to ReST format
docs: kvm: Convert s390-diag.txt to ReST format
docs: kvm: Convert timekeeping.txt to ReST format
docs: kvm: review-checklist.txt: rename to ReST
docs: virt: guest-halt-polling.txt convert to ReST

Miaohe Lin (3):
KVM: x86: remove duplicated KVM_REQ_EVENT request
KVM: apic: reuse smp_wmb() in kvm_make_request()
KVM: nVMX: Fix some comment typos and coding style

Oliver Upton (4):
KVM: x86: Mask off reserved bit from #DB exception payload
KVM: nVMX: Handle pending #DB when injecting INIT VM-exit
KVM: x86: Deliver exception payload on KVM_GET_VCPU_EVENTS
KVM: nVMX: Emulate MTF when performing instruction emulation

Paolo Bonzini (2):
KVM: x86: do not reset microcode version on INIT or RESET
KVM: x86: fix WARN_ON check of an unsigned less than zero

Peter Xu (4):
KVM: Provide kvm_flush_remote_tlbs_common()
KVM: MIPS: Drop flush_shadow_memslot() callback
KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references
KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs()

Sean Christopherson (6):
KVM: x86/mmu: Avoid retpoline on ->page_fault() with TDP
KVM: nVMX: Use correct root level for nested EPT shadow page tables
KVM: x86/mmu: Fix struct guest_walker arrays for 5-level paging
KVM: nVMX: Rename nested_ept_get_cr3() to nested_ept_get_eptp()
KVM: nVMX: Rename EPTP validity helper and associated variables
KVM: nVMX: Drop unnecessary check on ept caps for execute-only

.../guest-halt-polling.rst} | 12 +-
Documentation/virt/index.rst | 2 +
Documentation/virt/kvm/{api.txt => api.rst} | 3350 ++++++++++++--------
.../virt/kvm/arm/{hyp-abi.txt => hyp-abi.rst} | 28 +-
Documentation/virt/kvm/arm/index.rst | 12 +
Documentation/virt/kvm/arm/{psci.txt => psci.rst} | 46 +-
.../devices/{arm-vgic-its.txt => arm-vgic-its.rst} | 106 +-
.../devices/{arm-vgic-v3.txt => arm-vgic-v3.rst} | 132 +-
.../kvm/devices/{arm-vgic.txt => arm-vgic.rst} | 89 +-
Documentation/virt/kvm/devices/index.rst | 19 +
.../virt/kvm/devices/{mpic.txt => mpic.rst} | 11 +-
.../kvm/devices/{s390_flic.txt => s390_flic.rst} | 70 +-
Documentation/virt/kvm/devices/vcpu.rst | 114 +
Documentation/virt/kvm/devices/vcpu.txt | 76 -
.../virt/kvm/devices/{vfio.txt => vfio.rst} | 25 +-
Documentation/virt/kvm/devices/{vm.txt => vm.rst} | 206 +-
.../virt/kvm/devices/{xics.txt => xics.rst} | 28 +-
.../virt/kvm/devices/{xive.txt => xive.rst} | 152 +-
.../kvm/{halt-polling.txt => halt-polling.rst} | 90 +-
.../virt/kvm/{hypercalls.txt => hypercalls.rst} | 129 +-
Documentation/virt/kvm/index.rst | 16 +
Documentation/virt/kvm/locking.rst | 243 ++
Documentation/virt/kvm/locking.txt | 215 --
Documentation/virt/kvm/{mmu.txt => mmu.rst} | 62 +-
Documentation/virt/kvm/{msr.txt => msr.rst} | 147 +-
.../virt/kvm/{nested-vmx.txt => nested-vmx.rst} | 37 +-
Documentation/virt/kvm/{ppc-pv.txt => ppc-pv.rst} | 26 +-
.../{review-checklist.txt => review-checklist.rst} | 3 +
.../virt/kvm/{s390-diag.txt => s390-diag.rst} | 13 +-
.../virt/kvm/{timekeeping.txt => timekeeping.rst} | 223 +-
...UserModeLinux-HOWTO.txt => user_mode_linux.rst} | 1810 +++++------
arch/mips/include/asm/kvm_host.h | 7 -
arch/mips/kvm/Kconfig | 1 +
arch/mips/kvm/mips.c | 22 +-
arch/mips/kvm/trap_emul.c | 15 +-
arch/mips/kvm/vz.c | 14 +-
arch/x86/include/asm/kvm_host.h | 17 +-
arch/x86/include/uapi/asm/kvm.h | 1 +
arch/x86/kvm/lapic.c | 3 -
arch/x86/kvm/mmu.h | 13 +
arch/x86/kvm/mmu/mmu.c | 11 +-
arch/x86/kvm/mmu/paging_tmpl.h | 2 +-
arch/x86/kvm/svm.c | 3 +-
arch/x86/kvm/vmx/nested.c | 104 +-
arch/x86/kvm/vmx/nested.h | 9 +-
arch/x86/kvm/vmx/vmx.c | 42 +-
arch/x86/kvm/vmx/vmx.h | 3 +
arch/x86/kvm/x86.c | 44 +-
include/linux/kvm_host.h | 11 +-
tools/testing/selftests/kvm/Makefile | 3 +-
.../selftests/kvm/include/x86_64/processor.h | 44 +-
tools/testing/selftests/kvm/include/x86_64/svm.h | 297 ++
.../selftests/kvm/include/x86_64/svm_util.h | 38 +
tools/testing/selftests/kvm/lib/x86_64/svm.c | 161 +
tools/testing/selftests/kvm/lib/x86_64/vmx.c | 6 +-
.../testing/selftests/kvm/x86_64/svm_vmcall_test.c | 79 +
virt/kvm/arm/vgic/vgic-mmio.c | 12 -
virt/kvm/kvm_main.c | 22 +-
58 files changed, 5036 insertions(+), 3440 deletions(-)
rename Documentation/{virtual/guest-halt-polling.txt => virt/guest-halt-polling.rst} (91%)
rename Documentation/virt/kvm/{api.txt => api.rst} (71%)
rename Documentation/virt/kvm/arm/{hyp-abi.txt => hyp-abi.rst} (79%)
create mode 100644 Documentation/virt/kvm/arm/index.rst
rename Documentation/virt/kvm/arm/{psci.txt => psci.rst} (60%)
rename Documentation/virt/kvm/devices/{arm-vgic-its.txt => arm-vgic-its.rst} (71%)
rename Documentation/virt/kvm/devices/{arm-vgic-v3.txt => arm-vgic-v3.rst} (77%)
rename Documentation/virt/kvm/devices/{arm-vgic.txt => arm-vgic.rst} (66%)
create mode 100644 Documentation/virt/kvm/devices/index.rst
rename Documentation/virt/kvm/devices/{mpic.txt => mpic.rst} (91%)
rename Documentation/virt/kvm/devices/{s390_flic.txt => s390_flic.rst} (87%)
create mode 100644 Documentation/virt/kvm/devices/vcpu.rst
delete mode 100644 Documentation/virt/kvm/devices/vcpu.txt
rename Documentation/virt/kvm/devices/{vfio.txt => vfio.rst} (72%)
rename Documentation/virt/kvm/devices/{vm.txt => vm.rst} (61%)
rename Documentation/virt/kvm/devices/{xics.txt => xics.rst} (84%)
rename Documentation/virt/kvm/devices/{xive.txt => xive.rst} (62%)
rename Documentation/virt/kvm/{halt-polling.txt => halt-polling.rst} (64%)
rename Documentation/virt/kvm/{hypercalls.txt => hypercalls.rst} (55%)
create mode 100644 Documentation/virt/kvm/locking.rst
delete mode 100644 Documentation/virt/kvm/locking.txt
rename Documentation/virt/kvm/{mmu.txt => mmu.rst} (94%)
rename Documentation/virt/kvm/{msr.txt => msr.rst} (74%)
rename Documentation/virt/kvm/{nested-vmx.txt => nested-vmx.rst} (90%)
rename Documentation/virt/kvm/{ppc-pv.txt => ppc-pv.rst} (91%)
rename Documentation/virt/kvm/{review-checklist.txt => review-checklist.rst} (95%)
rename Documentation/virt/kvm/{s390-diag.txt => s390-diag.rst} (90%)
rename Documentation/virt/kvm/{timekeeping.txt => timekeeping.rst} (85%)
rename Documentation/virt/uml/{UserModeLinux-HOWTO.txt => user_mode_linux.rst} (74%)
create mode 100644 tools/testing/selftests/kvm/include/x86_64/svm.h
create mode 100644 tools/testing/selftests/kvm/include/x86_64/svm_util.h
create mode 100644 tools/testing/selftests/kvm/lib/x86_64/svm.c
create mode 100644 tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c


2020-02-12 18:54:38

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] KVM changes for Linux 5.6-rc2

On Wed, Feb 12, 2020 at 8:47 AM Paolo Bonzini <[email protected]> wrote:
>
> A mix of cleanups and bugfixes, some of them slightly more invasive than usual
> but still not worth waiting for 5.7.

What? No.

> Oliver Upton (4):
> KVM: nVMX: Emulate MTF when performing instruction emulation

This was committed today, and it's complete and utter garbage:

CommitDate: 7 hours ago

It doesn't even compile. Just in the patch itself - so this is not a
merge issue, I see this:

int (*skip_emulated_instruction)(struct kvm_vcpu *vcpu);
..
@@ -1599,6 +1599,40 @@ static int skip_emulated_instruction(struct
kvm_vcpu *vcpu)
..
+static void vmx_skip_emulated_instruction(struct kvm_vcpu *vcpu)
+ return skip_emulated_instruction(vcpu);
..
- .skip_emulated_instruction = skip_emulated_instruction,
+ .skip_emulated_instruction = vmx_skip_emulated_instruction,

ie note how that vmx_skip_emulated_instruction() is a void function,
and then you have

return skip_emulated_instruction(vcpu);

in it, and you assign that garbage to ".skip_emulated_instruction"
which is supposed to be returning 'int'.

So this clearly never even got a _whiff_ of build-testing. The thing
is completely broken.

Stop sending me garbage.

You're now on my shit-list, which means that I want to see only (a)
pure fixes and (b) well-tested such. Nothing else will be pulled.

Linus

2020-02-12 19:21:16

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [GIT PULL] KVM changes for Linux 5.6-rc2

On 12/02/20 19:53, Linus Torvalds wrote:
> It doesn't even compile. Just in the patch itself - so this is not a
> merge issue, I see this:
>
> int (*skip_emulated_instruction)(struct kvm_vcpu *vcpu);
> ..
> @@ -1599,6 +1599,40 @@ static int skip_emulated_instruction(struct
> kvm_vcpu *vcpu)
> ..
> +static void vmx_skip_emulated_instruction(struct kvm_vcpu *vcpu)
> + return skip_emulated_instruction(vcpu);
> ..
> - .skip_emulated_instruction = skip_emulated_instruction,
> + .skip_emulated_instruction = vmx_skip_emulated_instruction,
>
> ie note how that vmx_skip_emulated_instruction() is a void function,
> and then you have
>
> return skip_emulated_instruction(vcpu);
>
> in it, and you assign that garbage to ".skip_emulated_instruction"
> which is supposed to be returning 'int'.

Indeed I missed the warning. Of course the return value is in %rax so,
despite the patch being shitty (it is), it is also true that it
*happens* to pass the corresponding unit test.

Not a particularly high bar to clear I admit, but enough to explain the
mistake and ensure it doesn't happen again; I have now added "ccflags-y
+= -Werror" to the KVM makefile.

> So this clearly never even got a _whiff_ of build-testing.

Oh come on.

> You're now on my shit-list, which means that I want to see only (a)
> pure fixes and (b) well-tested such. Nothing else will be pulled.

Fair enough, I removed the following patches from the pull request and
will resend:

KVM: nVMX: Emulate MTF when performing instruction emulation
KVM: nVMX: Rename nested_ept_get_cr3() to nested_ept_get_eptp()
KVM: nVMX: Rename EPTP validity helper and associated variables
KVM: nVMX: Drop unnecessary check on ept caps for execute-only
KVM: Provide kvm_flush_remote_tlbs_common()
KVM: MIPS: Drop flush_shadow_memslot() callback
KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references
KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs()

The first one is a bug fix, but since it's the one that caused all the
mess I guess it's not really a good idea to argue about it.

Paolo

2020-02-12 19:39:06

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] KVM changes for Linux 5.6-rc2

On Wed, Feb 12, 2020 at 11:19 AM Paolo Bonzini <[email protected]> wrote:
>
> > So this clearly never even got a _whiff_ of build-testing.
>
> Oh come on.

Seriously - if you don't even _look_ at the warnings the build
generates, then it hasn't been build-tested.

I don't want to hear "Oh come on". I'm 100% serious.

Build-testing is not just "building". It's the "testing" of the build
too. You clearly never did _any_ testing of the build, since the build
had huge warnings.

Without the checking of the result, "build-testing" is just
"building", and completely irrelevant.

If you have problems seeing the warnings, add a "-Werror" to your scripts.

I do not want to see a _single_ warning in the kernel build. Yes, we
have one in the samples code, and even that annoys the hell out of me.

And exactly because we don't have any warnings in the default build,
it should be really really easy to check for new ones - it's not like
you have to wade through pages of warnings to see if any of them are
your new ones.

So no "Oh come on". You did *zero* testing of this crap, and you need
to own that fact instead of making excuses about it.

Linus

2020-02-12 19:53:38

by Oliver Upton

[permalink] [raw]
Subject: Re: [GIT PULL] KVM changes for Linux 5.6-rc2

On Wed, Feb 12, 2020 at 11:38:20AM -0800, Linus Torvalds wrote:
> On Wed, Feb 12, 2020 at 11:19 AM Paolo Bonzini <[email protected]> wrote:
> >
> > > So this clearly never even got a _whiff_ of build-testing.
> >
> > Oh come on.
>
> Seriously - if you don't even _look_ at the warnings the build
> generates, then it hasn't been build-tested.
>
> I don't want to hear "Oh come on". I'm 100% serious.
>
> Build-testing is not just "building". It's the "testing" of the build
> too. You clearly never did _any_ testing of the build, since the build
> had huge warnings.
>
> Without the checking of the result, "build-testing" is just
> "building", and completely irrelevant.
>
> If you have problems seeing the warnings, add a "-Werror" to your scripts.
>
> I do not want to see a _single_ warning in the kernel build. Yes, we
> have one in the samples code, and even that annoys the hell out of me.
>
> And exactly because we don't have any warnings in the default build,
> it should be really really easy to check for new ones - it's not like
> you have to wade through pages of warnings to see if any of them are
> your new ones.
>
> So no "Oh come on". You did *zero* testing of this crap, and you need
> to own that fact instead of making excuses about it.
>
> Linus

I should've caught this before even a build test, let alone sending it
out. My apologies for such an obvious + crap mistake.

--
Oliver

2020-02-12 20:04:08

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [GIT PULL] KVM changes for Linux 5.6-rc2

On 12/02/20 20:38, Linus Torvalds wrote:
> On Wed, Feb 12, 2020 at 11:19 AM Paolo Bonzini <[email protected]> wrote:
>>
>>> So this clearly never even got a _whiff_ of build-testing.
>>
>> Oh come on.
>
> Seriously - if you don't even _look_ at the warnings the build
> generates, then it hasn't been build-tested.
>
> I don't want to hear "Oh come on". I'm 100% serious.

I know, but still I consider it. There is no reason why the "build
test" should be anything more than "make && echo yes i am build-tested".
It shouldn't involve any grep or script magic, it's already hard enough
to script all the functional part of the testing.

My "Oh come on" was because "it never got a whiff of build-testing"
hides how "the default build testing configuration is crap". Of course
I don't want warnings either, but unless there is -Werror somewhere
mistakes _will_ happen. Get new commits from you, or make mrproper to
build another architecture? Everything gets rebuilt and the warnings
scroll by. During next-release development I will catch it of course,
but for rc I usually don't even need to build more than once before
applying a pull request. I am surprised it took a few years for the
wrong set of factors to occur at the same time.

Did I screw up? Yes. Could we do better to avoid someone else doing
the same mistake? Hell yes.

I'm not making excuses. I'm just saying that the *root* cause is not my
mistake or even Oliver's. The root cause is that our (shared!)
definition of "good" does not match the exit code of "make". We all
want zero warnings, but we don't enable -Werror. Let's add at least a
Kconfig to enable it for every architecture you build-test (is it only
x86 or anything else?).

Paolo

> Build-testing is not just "building". It's the "testing" of the build
> too. You clearly never did _any_ testing of the build, since the build
> had huge warnings.
>
> Without the checking of the result, "build-testing" is just
> "building", and completely irrelevant.
>
> If you have problems seeing the warnings, add a "-Werror" to your scripts.
>
> I do not want to see a _single_ warning in the kernel build. Yes, we
> have one in the samples code, and even that annoys the hell out of me.
>
> And exactly because we don't have any warnings in the default build,
> it should be really really easy to check for new ones - it's not like
> you have to wade through pages of warnings to see if any of them are
> your new ones.
>
> So no "Oh come on". You did *zero* testing of this crap, and you need
> to own that fact instead of making excuses about it.

2020-02-12 20:15:52

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] KVM changes for Linux 5.6-rc2

On Wed, Feb 12, 2020 at 12:02 PM Paolo Bonzini <[email protected]> wrote:
>
> I know, but still I consider it. There is no reason why the "build
> test" should be anything more than "make && echo yes i am build-tested".

It damn well should check for warnings.

And if you can't bother eye-balling it or scripting it, then simply use

make KCFLAGS=-Werror

but sadly I can't enforce that in general for all kernel builds simply
because some people use compilers that cause new warnings (compiler
updates etc commonly result in them, for example).

So I can't add -Werror in general, but developers can certainly use it
trivially.

No grep or other scripting required (although the above may cause
problems for that one sample file that does cause warnings - I didn't
check).

Linus

2020-02-13 07:14:34

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [GIT PULL] KVM changes for Linux 5.6-rc2

On 12/02/20 21:13, Linus Torvalds wrote:
> On Wed, Feb 12, 2020 at 12:02 PM Paolo Bonzini <[email protected]> wrote:
>>
>> I know, but still I consider it. There is no reason why the "build
>> test" should be anything more than "make && echo yes i am build-tested".
>
> It damn well should check for warnings.
>
> And if you can't bother eye-balling it or scripting it, then simply use
>
> make KCFLAGS=-Werror
>
> but sadly I can't enforce that in general for all kernel builds simply
> because some people use compilers that cause new warnings (compiler
> updates etc commonly result in them, for example).

Shouldn't we _try_? Compilers are not adding or triggering as many
warnings as they were a few years ago, when clang came out or GCC 4
rewrote their middle end. Compiling the 10-year-old 2.6.32 these days
results in a couple warnings for the RHEL6 configuration. Sometimes
there are even hard errors making -Wno-error moot. We can fix them in
stable kernels. For master, distro people and build bots would catch
that early and we can fix everything quickly.

For the odd case such as a bisection on old trees, _that_ is when you
add -Wno-error. Special cases deserve special options, general cases
don't. The issue percolates all the way down to the developers, Oliver
could have specified KCFLAGS too and he wouldn't have sent the bad
patch, but honestly I can't blame him. You can blame me, :) but again
that doesn't mean Linux as a whole can't do better.

Anyway---sorry again for the screwup. I'll send a revised pull request
soon. Thanks,

Paolo

> So I can't add -Werror in general, but developers can certainly use it
> trivially.
>
> No grep or other scripting required (although the above may cause
> problems for that one sample file that does cause warnings - I didn't
> check).
>
> Linus
>