Cleanup commit 23ebdedc67e ("compiler-intel.h: Remove duplicate
definition") removed the double definition of __memory_barrier()
intrinsics.
However, in doing so, it also removed the preceding #undef barrier,
meaning, the actual barrier() macro from compiler-gcc.h with inline
asm is still in place when __GNUC__ is provided.
Subsequently, barrier() can never be defined as __memory_barrier()
from compiler.h since it already has a definition in place and if
we trust the comment in compiler-intel.h, ecc doesn't support gcc
specific asm statements. I don't have an ecc at hand, so a revert
of that cleanup would be the safest option, imho, as it has been
like this since pre-git times.
Fixes: 73679e508201 ("compiler-intel.h: Remove duplicate definition")
Signed-off-by: Daniel Borkmann <[email protected]>
Cc: Pranith Kumar <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: mancha security <[email protected]>
---
include/linux/compiler-intel.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index ba147a1..5529c52 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -13,9 +13,12 @@
/* Intel ECC compiler doesn't support gcc specific asm stmts.
* It uses intrinsics to do the equivalent things.
*/
+#undef barrier
#undef RELOC_HIDE
#undef OPTIMIZER_HIDE_VAR
+#define barrier() __memory_barrier()
+
#define RELOC_HIDE(ptr, off) \
({ unsigned long __ptr; \
__ptr = (unsigned long) (ptr); \
--
1.9.3
Hi Daniel,
On Wed, Apr 29, 2015 at 10:42 AM, Daniel Borkmann <[email protected]> wrote:
> Cleanup commit 23ebdedc67e ("compiler-intel.h: Remove duplicate
> definition") removed the double definition of __memory_barrier()
> intrinsics.
>
> However, in doing so, it also removed the preceding #undef barrier,
> meaning, the actual barrier() macro from compiler-gcc.h with inline
> asm is still in place when __GNUC__ is provided.
When you use the Intel compilers, the barrier() definition will come
from compiler.h and not compiler-gcc.h. That is what the commit
message says in 73679e508201(your commit message has the wrong hash).
I don't understand what problem you are seeing with this, can you
please explain?
Thanks!
On Wed, Apr 29, 2015 at 10:51:40AM -0400, Pranith Kumar wrote:
> Hi Daniel,
>
> On Wed, Apr 29, 2015 at 10:42 AM, Daniel Borkmann <[email protected]> wrote:
> > Cleanup commit 23ebdedc67e ("compiler-intel.h: Remove duplicate
> > definition") removed the double definition of __memory_barrier()
> > intrinsics.
> >
> > However, in doing so, it also removed the preceding #undef barrier,
> > meaning, the actual barrier() macro from compiler-gcc.h with inline
> > asm is still in place when __GNUC__ is provided.
>
> When you use the Intel compilers, the barrier() definition will come
> from compiler.h and not compiler-gcc.h. That is what the commit
> message says in 73679e508201(your commit message has the wrong hash).
> I don't understand what problem you are seeing with this, can you
> please explain?
>
> Thanks!
Hi Pranith.
The problem is that ICC defines __GNUC__ so barrier() gets defined
in compiler-gcc.h. Your commit removed an #undef from compiler-intel.h
so compiler.h will never define barrier to __memory_barrier().
--mancha
On 04/29/2015 04:51 PM, Pranith Kumar wrote:
...
> message says in 73679e508201(your commit message has the wrong hash).
Sorry for that, the Fixes tag actually got it right.
On Wed, Apr 29, 2015 at 10:59 AM, mancha security <[email protected]> wrote:
>
> The problem is that ICC defines __GNUC__ so barrier() gets defined
> in compiler-gcc.h. Your commit removed an #undef from compiler-intel.h
> so compiler.h will never define barrier to __memory_barrier().
>
OK, I see your point. But, ICC has support for GCC inline assembly. So
the change does not seem to be making any difference. We are using our
own asm barrier rather than the inbuilt one provided by ICC.
--
Pranith
On 04/29/2015 06:40 PM, Pranith Kumar wrote:
> On Wed, Apr 29, 2015 at 10:59 AM, mancha security <[email protected]> wrote:
>>
>> The problem is that ICC defines __GNUC__ so barrier() gets defined
>> in compiler-gcc.h. Your commit removed an #undef from compiler-intel.h
>> so compiler.h will never define barrier to __memory_barrier().
>
> OK, I see your point. But, ICC has support for GCC inline assembly. So
> the change does not seem to be making any difference. We are using our
> own asm barrier rather than the inbuilt one provided by ICC.
It does make a difference: gcc inline assembly is not supported by
/ecc/, see that it's wrapped within the ifdef __ECC part. I believe,
that should be for ia64 which we have under arch/, no?
On Wed, Apr 29, 2015 at 12:59 PM, Daniel Borkmann <[email protected]> wrote:
> On 04/29/2015 06:40 PM, Pranith Kumar wrote:
>>
>> On Wed, Apr 29, 2015 at 10:59 AM, mancha security <[email protected]>
>> wrote:
>>>
>>>
>>> The problem is that ICC defines __GNUC__ so barrier() gets defined
>>> in compiler-gcc.h. Your commit removed an #undef from compiler-intel.h
>>> so compiler.h will never define barrier to __memory_barrier().
>>
>>
>> OK, I see your point. But, ICC has support for GCC inline assembly. So
>> the change does not seem to be making any difference. We are using our
>> own asm barrier rather than the inbuilt one provided by ICC.
>
>
> It does make a difference: gcc inline assembly is not supported by
> /ecc/, see that it's wrapped within the ifdef __ECC part. I believe,
> that should be for ia64 which we have under arch/, no?
Yes, looks like this breaks building the kernel with ecc compiler on
IA64. Has anyone tried building it with ECC on ia64 lately(or ever)?
Reviewed-by: Pranith Kumar <[email protected]>
--
Pranith
On Wed, Apr 29, 2015 at 12:40:15PM -0400, Pranith Kumar wrote:
> On Wed, Apr 29, 2015 at 10:59 AM, mancha security <[email protected]>
> wrote:
> >
> > The problem is that ICC defines __GNUC__ so barrier() gets defined
> > in compiler-gcc.h. Your commit removed an #undef from
> > compiler-intel.h so compiler.h will never define barrier to
> > __memory_barrier().
> >
>
> OK, I see your point. But, ICC has support for GCC inline assembly. So
> the change does not seem to be making any difference. We are using our
> own asm barrier rather than the inbuilt one provided by ICC.
>
> -- Pranith
Yes, I misspoke earlier and meant to say ECC rather than ICC.
--mancha