2016-03-31 07:44:18

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH] compiler-gcc: disable -ftracer for __noclone functions

-ftracer can duplicate asm blocks causing compilation to fail in
noclone functions. For example, KVM declares a global variable
in an asm like

asm("2: ... \n
.pushsection data \n
.global vmx_return \n
vmx_return: .long 2b");

and -ftracer causes a double declaration.

Cc: Andrew Morton <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: [email protected]
Cc: [email protected]
Reported-by: Linda Walsh <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 22ab246feed3..eeae401a2412 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -199,7 +199,7 @@
#define unreachable() __builtin_unreachable()

/* Mark a function definition as prohibited from being cloned. */
-#define __noclone __attribute__((__noclone__))
+#define __noclone __attribute__((__noclone__, __optimize__("no-tracer")))

#endif /* GCC_VERSION >= 40500 */

--
2.5.5


2016-03-31 08:37:09

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] compiler-gcc: disable -ftracer for __noclone functions

Hi Paolo,

[auto build test WARNING on v4.6-rc1]
[also build test WARNING on next-20160331]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url: https://github.com/0day-ci/linux/commits/Paolo-Bonzini/compiler-gcc-disable-ftracer-for-__noclone-functions/20160331-154556
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

include/linux/compiler.h:232:8: sparse: attribute 'no_sanitize_address': unknown attribute
>> arch/x86/kvm/vmx.c:8610:13: sparse: attribute '__optimize__': unknown attribute

vim +/__optimize__ +8610 arch/x86/kvm/vmx.c

d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8594 int i, nr_msrs;
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8595 struct perf_guest_switch_msr *msrs;
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8596
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8597 msrs = perf_guest_get_msrs(&nr_msrs);
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8598
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8599 if (!msrs)
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8600 return;
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8601
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8602 for (i = 0; i < nr_msrs; i++)
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8603 if (msrs[i].host == msrs[i].guest)
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8604 clear_atomic_switch_msr(vmx, msrs[i].msr);
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8605 else
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8606 add_atomic_switch_msr(vmx, msrs[i].msr, msrs[i].guest,
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8607 msrs[i].host);
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8608 }
d7cd9796 arch/x86/kvm/vmx.c Gleb Natapov 2011-10-05 8609
a3b5ba49 arch/x86/kvm/vmx.c Lai Jiangshan 2011-02-11 @8610 static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
6aa8b732 drivers/kvm/vmx.c Avi Kivity 2006-12-10 8611 {
a2fa3e9f drivers/kvm/vmx.c Gregory Haskins 2007-07-27 8612 struct vcpu_vmx *vmx = to_vmx(vcpu);
d974baa3 arch/x86/kvm/vmx.c Andy Lutomirski 2014-10-08 8613 unsigned long debugctlmsr, cr4;
104f226b arch/x86/kvm/vmx.c Avi Kivity 2010-11-18 8614
104f226b arch/x86/kvm/vmx.c Avi Kivity 2010-11-18 8615 /* Record the guest's net vcpu time for enforced NMI injections. */
104f226b arch/x86/kvm/vmx.c Avi Kivity 2010-11-18 8616 if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked))
104f226b arch/x86/kvm/vmx.c Avi Kivity 2010-11-18 8617 vmx->entry_time = ktime_get();
104f226b arch/x86/kvm/vmx.c Avi Kivity 2010-11-18 8618

:::::: The code at line 8610 was first introduced by commit
:::::: a3b5ba49a8c58d9a578e016523b047467a41e047 KVM: VMX: add the __noclone attribute to vmx_vcpu_run

:::::: TO: Lai Jiangshan <[email protected]>
:::::: CC: Marcelo Tosatti <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

2018-05-05 17:23:48

by Nadav Amit

[permalink] [raw]
Subject: Re: [PATCH] compiler-gcc: disable -ftracer for __noclone functions

Paolo Bonzini <[email protected]> wrote:

> -ftracer can duplicate asm blocks causing compilation to fail in
> noclone functions. For example, KVM declares a global variable
> in an asm like
>
> asm("2: ... \n
> .pushsection data \n
> .global vmx_return \n
> vmx_return: .long 2b");
>
> and -ftracer causes a double declaration.
>
> Cc: Andrew Morton <[email protected]>
> Cc: Michal Marek <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Reported-by: Linda Walsh <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> include/linux/compiler-gcc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 22ab246feed3..eeae401a2412 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -199,7 +199,7 @@
> #define unreachable() __builtin_unreachable()
>
> /* Mark a function definition as prohibited from being cloned. */
> -#define __noclone __attribute__((__noclone__))
> +#define __noclone __attribute__((__noclone__, __optimize__("no-tracer")))

[ Bringing the thread back from the dead for context ]

Setting different optimization attributes to certain functions apparently
prevents gcc from inlining functions with different “optimizations”. This
results in poor compilation - most notably of vmx_vcpu_run() - and causes
short functions such as to_vmx() not to be inlined.

Regards,
Nadav

2018-05-08 18:00:23

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH] compiler-gcc: disable -ftracer for __noclone functions

On 05/05/2018 19:22, Nadav Amit wrote:
> Paolo Bonzini <[email protected]> wrote:
>
>> -ftracer can duplicate asm blocks causing compilation to fail in
>> noclone functions. For example, KVM declares a global variable
>> in an asm like
>>
>> asm("2: ... \n
>> .pushsection data \n
>> .global vmx_return \n
>> vmx_return: .long 2b");
>>
>> and -ftracer causes a double declaration.
>>
>> Cc: Andrew Morton <[email protected]>
>> Cc: Michal Marek <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Reported-by: Linda Walsh <[email protected]>
>> Signed-off-by: Paolo Bonzini <[email protected]>
>> ---
>> include/linux/compiler-gcc.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> index 22ab246feed3..eeae401a2412 100644
>> --- a/include/linux/compiler-gcc.h
>> +++ b/include/linux/compiler-gcc.h
>> @@ -199,7 +199,7 @@
>> #define unreachable() __builtin_unreachable()
>>
>> /* Mark a function definition as prohibited from being cloned. */
>> -#define __noclone __attribute__((__noclone__))
>> +#define __noclone __attribute__((__noclone__, __optimize__("no-tracer")))
>
> [ Bringing the thread back from the dead for context ]
>
> Setting different optimization attributes to certain functions apparently
> prevents gcc from inlining functions with different “optimizations”. This
> results in poor compilation - most notably of vmx_vcpu_run() - and causes
> short functions such as to_vmx() not to be inlined.

Hmm, I suppose we need to disable -fno-tracer globally... Has this
changed in recent versions of GCC?

Paolo


2018-05-08 18:13:36

by Nadav Amit

[permalink] [raw]
Subject: Re: [PATCH] compiler-gcc: disable -ftracer for __noclone functions

Paolo Bonzini <[email protected]> wrote:

> On 05/05/2018 19:22, Nadav Amit wrote:
>> Paolo Bonzini <[email protected]> wrote:
>>
>>> -ftracer can duplicate asm blocks causing compilation to fail in
>>> noclone functions. For example, KVM declares a global variable
>>> in an asm like
>>>
>>> asm("2: ... \n
>>> .pushsection data \n
>>> .global vmx_return \n
>>> vmx_return: .long 2b");
>>>
>>> and -ftracer causes a double declaration.
>>>
>>> Cc: Andrew Morton <[email protected]>
>>> Cc: Michal Marek <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Reported-by: Linda Walsh <[email protected]>
>>> Signed-off-by: Paolo Bonzini <[email protected]>
>>> ---
>>> include/linux/compiler-gcc.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>>> index 22ab246feed3..eeae401a2412 100644
>>> --- a/include/linux/compiler-gcc.h
>>> +++ b/include/linux/compiler-gcc.h
>>> @@ -199,7 +199,7 @@
>>> #define unreachable() __builtin_unreachable()
>>>
>>> /* Mark a function definition as prohibited from being cloned. */
>>> -#define __noclone __attribute__((__noclone__))
>>> +#define __noclone __attribute__((__noclone__, __optimize__("no-tracer")))
>>
>> [ Bringing the thread back from the dead for context ]
>>
>> Setting different optimization attributes to certain functions apparently
>> prevents gcc from inlining functions with different “optimizations”. This
>> results in poor compilation - most notably of vmx_vcpu_run() - and causes
>> short functions such as to_vmx() not to be inlined.
>
> Hmm, I suppose we need to disable -fno-tracer globally... Has this
> changed in recent versions of GCC?

I don’t think so. I played with optimization attributes of gcc some time ago
and saw the same behavior. Unfortunately, I don’t easily find the place in
gcc that implements this behavior.