lkml.org 
[lkml]   [2020]   [Feb]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [GIT PULL] KVM changes for Linux 5.6-rc2
From
Date
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

\
 
 \ /
  Last update: 2020-02-12 20:19    [W:0.080 / U:7.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site