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
