* [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls @ 2022-02-23 16:23 Sean Christopherson 2022-02-23 17:27 ` Nathan Chancellor ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Sean Christopherson @ 2022-02-23 16:23 UTC (permalink / raw) To: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel, Like Xu Cast kvm_x86_ops.func to 'void *' when updating KVM static calls that are conditionally patched to __static_call_return0(). clang complains about using mismatching pointers in the ternary operator, which breaks the build when compiling with CONFIG_KVM_WERROR=y. >> arch/x86/include/asm/kvm-x86-ops.h:82:1: warning: pointer type mismatch ('bool (*)(struct kvm_vcpu *)' and 'void *') [-Wpointer-type-mismatch] Fixes: 5be2226f417d ("KVM: x86: allow defining return-0 static calls") Reported-by: Like Xu <like.xu.linux@gmail.com> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/include/asm/kvm_host.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 713e08f62385..f285ddb8b66b 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1547,8 +1547,8 @@ static inline void kvm_ops_static_call_update(void) WARN_ON(!kvm_x86_ops.func); __KVM_X86_OP(func) #define KVM_X86_OP_OPTIONAL __KVM_X86_OP #define KVM_X86_OP_OPTIONAL_RET0(func) \ - static_call_update(kvm_x86_##func, kvm_x86_ops.func ? : \ - (void *) __static_call_return0); + static_call_update(kvm_x86_##func, (void *)kvm_x86_ops.func ? : \ + (void *)__static_call_return0); #include <asm/kvm-x86-ops.h> #undef __KVM_X86_OP } base-commit: f4bc051fc91ab9f1d5225d94e52d369ef58bec58 -- 2.35.1.473.g83b2b277ed-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls 2022-02-23 16:23 [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls Sean Christopherson @ 2022-02-23 17:27 ` Nathan Chancellor 2022-02-23 17:59 ` Sean Christopherson 2022-02-23 22:23 ` David Dunn 2022-02-24 14:23 ` Paolo Bonzini 2 siblings, 1 reply; 7+ messages in thread From: Nathan Chancellor @ 2022-02-23 17:27 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Nick Desaulniers, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel, Like Xu Hi Sean, On Wed, Feb 23, 2022 at 04:23:55PM +0000, Sean Christopherson wrote: > Cast kvm_x86_ops.func to 'void *' when updating KVM static calls that are > conditionally patched to __static_call_return0(). clang complains about > using mismatching pointers in the ternary operator, which breaks the > build when compiling with CONFIG_KVM_WERROR=y. > > >> arch/x86/include/asm/kvm-x86-ops.h:82:1: warning: pointer type mismatch > ('bool (*)(struct kvm_vcpu *)' and 'void *') [-Wpointer-type-mismatch] > > Fixes: 5be2226f417d ("KVM: x86: allow defining return-0 static calls") > Reported-by: Like Xu <like.xu.linux@gmail.com> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> Thank you for the patch! Is this a bug in clang? I ended up creating a small reproducer in case we need to file a bug with clang upstream: https://godbolt.org/z/P7nEdzejE This does shut up the warning and I can still spawn guests on my AMD and Intel systems so: Reviewed-by: Nathan Chancellor <nathan@kernel.org> Tested-by: Nathan Chancellor <nathan@kernel.org> > --- > arch/x86/include/asm/kvm_host.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 713e08f62385..f285ddb8b66b 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1547,8 +1547,8 @@ static inline void kvm_ops_static_call_update(void) > WARN_ON(!kvm_x86_ops.func); __KVM_X86_OP(func) > #define KVM_X86_OP_OPTIONAL __KVM_X86_OP > #define KVM_X86_OP_OPTIONAL_RET0(func) \ > - static_call_update(kvm_x86_##func, kvm_x86_ops.func ? : \ > - (void *) __static_call_return0); > + static_call_update(kvm_x86_##func, (void *)kvm_x86_ops.func ? : \ > + (void *)__static_call_return0); > #include <asm/kvm-x86-ops.h> > #undef __KVM_X86_OP > } > > base-commit: f4bc051fc91ab9f1d5225d94e52d369ef58bec58 > -- > 2.35.1.473.g83b2b277ed-goog > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls 2022-02-23 17:27 ` Nathan Chancellor @ 2022-02-23 17:59 ` Sean Christopherson 2022-02-23 18:13 ` Nathan Chancellor 0 siblings, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2022-02-23 17:59 UTC (permalink / raw) To: Nathan Chancellor Cc: Paolo Bonzini, Nick Desaulniers, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel, Like Xu On Wed, Feb 23, 2022, Nathan Chancellor wrote: > Hi Sean, > > On Wed, Feb 23, 2022 at 04:23:55PM +0000, Sean Christopherson wrote: > > Cast kvm_x86_ops.func to 'void *' when updating KVM static calls that are > > conditionally patched to __static_call_return0(). clang complains about > > using mismatching pointers in the ternary operator, which breaks the > > build when compiling with CONFIG_KVM_WERROR=y. > > > > >> arch/x86/include/asm/kvm-x86-ops.h:82:1: warning: pointer type mismatch > > ('bool (*)(struct kvm_vcpu *)' and 'void *') [-Wpointer-type-mismatch] > > > > Fixes: 5be2226f417d ("KVM: x86: allow defining return-0 static calls") > > Reported-by: Like Xu <like.xu.linux@gmail.com> > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > Thank you for the patch! Is this a bug in clang? IMO, no. I think it's completely reasonable for the compiler to complain that KVM is generating two different pointer types out of a ternary operator. clang is somewhat inconsistent, though it may be deliberate. clang doesn't complain about implicitly casting a 'void *' to another data type, e.g. this complies clean, where "data" is a 'void *' struct kvm_vcpu *x = vcpu ? : data; But changing it to a function on the lhs triggers the warn: typeof(kvm_x86_ops.vcpu_run) x = kvm_x86_ops.vcpu_run ? : data; Again, complaining in the function pointer case seems reasonable. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls 2022-02-23 17:59 ` Sean Christopherson @ 2022-02-23 18:13 ` Nathan Chancellor 2022-02-23 18:56 ` Sean Christopherson 0 siblings, 1 reply; 7+ messages in thread From: Nathan Chancellor @ 2022-02-23 18:13 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Nick Desaulniers, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel, Like Xu On Wed, Feb 23, 2022 at 05:59:05PM +0000, Sean Christopherson wrote: > On Wed, Feb 23, 2022, Nathan Chancellor wrote: > > Hi Sean, > > > > On Wed, Feb 23, 2022 at 04:23:55PM +0000, Sean Christopherson wrote: > > > Cast kvm_x86_ops.func to 'void *' when updating KVM static calls that are > > > conditionally patched to __static_call_return0(). clang complains about > > > using mismatching pointers in the ternary operator, which breaks the > > > build when compiling with CONFIG_KVM_WERROR=y. > > > > > > >> arch/x86/include/asm/kvm-x86-ops.h:82:1: warning: pointer type mismatch > > > ('bool (*)(struct kvm_vcpu *)' and 'void *') [-Wpointer-type-mismatch] > > > > > > Fixes: 5be2226f417d ("KVM: x86: allow defining return-0 static calls") > > > Reported-by: Like Xu <like.xu.linux@gmail.com> > > > Reported-by: kernel test robot <lkp@intel.com> > > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > > > Thank you for the patch! Is this a bug in clang? > > IMO, no. I think it's completely reasonable for the compiler to complain that KVM > is generating two different pointer types out of a ternary operator. > > clang is somewhat inconsistent, though it may be deliberate. clang doesn't complain > about implicitly casting a 'void *' to another data type, e.g. this complies clean, > where "data" is a 'void *' > > struct kvm_vcpu *x = vcpu ? : data; Right, I would assume this is deliberate. I think warning in this case might be quite noisy, as the kernel implicitly converts 'void *' to typed pointers for certain function pointer callbacks (although this particular case is probably pretty rare). > But changing it to a function on the lhs triggers the warn: > > typeof(kvm_x86_ops.vcpu_run) x = kvm_x86_ops.vcpu_run ? : data; > > Again, complaining in the function pointer case seems reasonable. Ack, thank you for the clarification and explanation! Cheers, Nathan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls 2022-02-23 18:13 ` Nathan Chancellor @ 2022-02-23 18:56 ` Sean Christopherson 0 siblings, 0 replies; 7+ messages in thread From: Sean Christopherson @ 2022-02-23 18:56 UTC (permalink / raw) To: Nathan Chancellor Cc: Paolo Bonzini, Nick Desaulniers, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel, Like Xu On Wed, Feb 23, 2022, Nathan Chancellor wrote: > On Wed, Feb 23, 2022 at 05:59:05PM +0000, Sean Christopherson wrote: > > On Wed, Feb 23, 2022, Nathan Chancellor wrote: > > > Hi Sean, > > > > > > On Wed, Feb 23, 2022 at 04:23:55PM +0000, Sean Christopherson wrote: > > > > Cast kvm_x86_ops.func to 'void *' when updating KVM static calls that are > > > > conditionally patched to __static_call_return0(). clang complains about > > > > using mismatching pointers in the ternary operator, which breaks the > > > > build when compiling with CONFIG_KVM_WERROR=y. > > > > > > > > >> arch/x86/include/asm/kvm-x86-ops.h:82:1: warning: pointer type mismatch > > > > ('bool (*)(struct kvm_vcpu *)' and 'void *') [-Wpointer-type-mismatch] > > > > > > > > Fixes: 5be2226f417d ("KVM: x86: allow defining return-0 static calls") > > > > Reported-by: Like Xu <like.xu.linux@gmail.com> > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > > > > > Thank you for the patch! Is this a bug in clang? > > > > IMO, no. I think it's completely reasonable for the compiler to complain that KVM > > is generating two different pointer types out of a ternary operator. > > > > clang is somewhat inconsistent, though it may be deliberate. clang doesn't complain > > about implicitly casting a 'void *' to another data type, e.g. this complies clean, > > where "data" is a 'void *' > > > > struct kvm_vcpu *x = vcpu ? : data; > > Right, I would assume this is deliberate. I think warning in this case > might be quite noisy, as the kernel implicitly converts 'void *' to > typed pointers for certain function pointer callbacks (although this > particular case is probably pretty rare). Aha! Looks like clang's behavior is correct, assuming a function is not considered an "object". From C99 "6.5.15 Conditional operator": One of the following shall hold for the second and third operands: — both operands have arithmetic type; — both operands have the same structure or union type; — both operands have void type; — both operands are pointers to qualified or unqualified versions of compatible types; — one operand is a pointer and the other is a null pointer constant; or — one operand is a pointer to an object or incomplete type and the other is a pointer to a qualified or unqualified version of void. That last case would explain why clang warns about a function pointer but not a object pointer when the third operand is a 'void *'. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls 2022-02-23 16:23 [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls Sean Christopherson 2022-02-23 17:27 ` Nathan Chancellor @ 2022-02-23 22:23 ` David Dunn 2022-02-24 14:23 ` Paolo Bonzini 2 siblings, 0 replies; 7+ messages in thread From: David Dunn @ 2022-02-23 22:23 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel, Like Xu Reviewed-by: David Dunn <daviddunn@google.com> On Wed, Feb 23, 2022 at 8:24 AM Sean Christopherson <seanjc@google.com> wrote: > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 713e08f62385..f285ddb8b66b 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1547,8 +1547,8 @@ static inline void kvm_ops_static_call_update(void) > WARN_ON(!kvm_x86_ops.func); __KVM_X86_OP(func) > #define KVM_X86_OP_OPTIONAL __KVM_X86_OP > #define KVM_X86_OP_OPTIONAL_RET0(func) \ > - static_call_update(kvm_x86_##func, kvm_x86_ops.func ? : \ > - (void *) __static_call_return0); > + static_call_update(kvm_x86_##func, (void *)kvm_x86_ops.func ? : \ > + (void *)__static_call_return0); > #include <asm/kvm-x86-ops.h> > #undef __KVM_X86_OP > } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls 2022-02-23 16:23 [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls Sean Christopherson 2022-02-23 17:27 ` Nathan Chancellor 2022-02-23 22:23 ` David Dunn @ 2022-02-24 14:23 ` Paolo Bonzini 2 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2022-02-24 14:23 UTC (permalink / raw) To: Sean Christopherson, Nathan Chancellor, Nick Desaulniers Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel, Like Xu On 2/23/22 17:23, Sean Christopherson wrote: > Cast kvm_x86_ops.func to 'void *' when updating KVM static calls that are > conditionally patched to __static_call_return0(). clang complains about > using mismatching pointers in the ternary operator, which breaks the > build when compiling with CONFIG_KVM_WERROR=y. > > >> arch/x86/include/asm/kvm-x86-ops.h:82:1: warning: pointer type mismatch > ('bool (*)(struct kvm_vcpu *)' and 'void *') [-Wpointer-type-mismatch] > > Fixes: 5be2226f417d ("KVM: x86: allow defining return-0 static calls") > Reported-by: Like Xu <like.xu.linux@gmail.com> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/include/asm/kvm_host.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 713e08f62385..f285ddb8b66b 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1547,8 +1547,8 @@ static inline void kvm_ops_static_call_update(void) > WARN_ON(!kvm_x86_ops.func); __KVM_X86_OP(func) > #define KVM_X86_OP_OPTIONAL __KVM_X86_OP > #define KVM_X86_OP_OPTIONAL_RET0(func) \ > - static_call_update(kvm_x86_##func, kvm_x86_ops.func ? : \ > - (void *) __static_call_return0); > + static_call_update(kvm_x86_##func, (void *)kvm_x86_ops.func ? : \ > + (void *)__static_call_return0); > #include <asm/kvm-x86-ops.h> > #undef __KVM_X86_OP > } > > base-commit: f4bc051fc91ab9f1d5225d94e52d369ef58bec58 Queued, thanks. Paolo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-24 14:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-23 16:23 [PATCH] KVM: x86: Fix pointer mistmatch warning when patching RET0 static calls Sean Christopherson 2022-02-23 17:27 ` Nathan Chancellor 2022-02-23 17:59 ` Sean Christopherson 2022-02-23 18:13 ` Nathan Chancellor 2022-02-23 18:56 ` Sean Christopherson 2022-02-23 22:23 ` David Dunn 2022-02-24 14:23 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).