All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Build fixes for clang 4.0
@ 2017-03-06 12:31 Roger Pau Monne
  2017-03-06 12:31 ` [PATCH 1/2] build/clang: remove the address-of-packed-member warning Roger Pau Monne
  2017-03-06 12:31 ` [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0 Roger Pau Monne
  0 siblings, 2 replies; 15+ messages in thread
From: Roger Pau Monne @ 2017-03-06 12:31 UTC (permalink / raw)
  To: xen-devel

Hello,

This series makes Xen (both hypervisor and tools) build with clang 4.0. Note
that patch #2 is well... unfortunate, but I couldn't find any other way to
solve this (although I'm open to suggestions). I'm also afraid that issues
similar to this one might come up in future compiler versions with XSM.

Thanks, Roger.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] build/clang: remove the address-of-packed-member warning
  2017-03-06 12:31 [PATCH 0/2] Build fixes for clang 4.0 Roger Pau Monne
@ 2017-03-06 12:31 ` Roger Pau Monne
  2017-03-06 12:47   ` Jan Beulich
                     ` (2 more replies)
  2017-03-06 12:31 ` [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0 Roger Pau Monne
  1 sibling, 3 replies; 15+ messages in thread
From: Roger Pau Monne @ 2017-03-06 12:31 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Jan Beulich, Roger Pau Monne

Clang 4.0 has added a new check that triggers when taking the address of a
field of a packed structure. I've explained our situation to the llvm/clang
folks, but nobody seem to care:

https://reviews.llvm.org/D20561

So simply disable that check if present.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 Config.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Config.mk b/Config.mk
index 81550a7..e9f60c7 100644
--- a/Config.mk
+++ b/Config.mk
@@ -216,6 +216,7 @@ $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
 $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
 $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
 $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
+$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member)
 
 LDFLAGS += $(foreach i, $(EXTRA_LIB), -L$(i)) 
 CFLAGS += $(foreach i, $(EXTRA_INCLUDES), -I$(i))
-- 
2.10.1 (Apple Git-78)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
  2017-03-06 12:31 [PATCH 0/2] Build fixes for clang 4.0 Roger Pau Monne
  2017-03-06 12:31 ` [PATCH 1/2] build/clang: remove the address-of-packed-member warning Roger Pau Monne
@ 2017-03-06 12:31 ` Roger Pau Monne
  2017-03-06 12:52   ` Jan Beulich
  2017-03-11  0:42   ` Daniel De Graaf
  1 sibling, 2 replies; 15+ messages in thread
From: Roger Pau Monne @ 2017-03-06 12:31 UTC (permalink / raw)
  To: xen-devel; +Cc: Daniel De Graaf, Roger Pau Monne

There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
working as expected, and vpmu.o ends up with a reference to
__xsm_action_mismatch_detected which makes the build fail:

[...]
ld    -melf_x86_64_fbsd  -T xen.lds -N prelink.o  \
    xen/common/symbols-dummy.o -o xen/.xen-syms.0
prelink.o: In function `xsm_default_action':
xen/include/xsm/dummy.h:80: undefined reference to `__xsm_action_mismatch_detected'
xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `__xsm_action_mismatch_detected'
ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' isn't defined

Then doing a search in the objects files:

# find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
  'for filename; do nm "$filename" | \
  grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
xen/arch/x86/prelink.o
xen/arch/x86/cpu/vpmu.o
xen/arch/x86/cpu/built_in.o
xen/arch/x86/built_in.o

The current patch is the only way I've found to fix this so far, by simply
moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
XENPMU_* operations, instead of -EPERM when called by a privileged domain.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/include/xsm/dummy.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 4b27ae7..ff73039 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -682,18 +682,13 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int
     XSM_ASSERT_ACTION(XSM_OTHER);
     switch ( op )
     {
-    case XENPMU_mode_set:
-    case XENPMU_mode_get:
-    case XENPMU_feature_set:
-    case XENPMU_feature_get:
-        return xsm_default_action(XSM_PRIV, d, current->domain);
     case XENPMU_init:
     case XENPMU_finish:
     case XENPMU_lvtpc_set:
     case XENPMU_flush:
         return xsm_default_action(XSM_HOOK, d, current->domain);
     default:
-        return -EPERM;
+        return xsm_default_action(XSM_PRIV, d, current->domain);
     }
 }
 
-- 
2.10.1 (Apple Git-78)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] build/clang: remove the address-of-packed-member warning
  2017-03-06 12:31 ` [PATCH 1/2] build/clang: remove the address-of-packed-member warning Roger Pau Monne
@ 2017-03-06 12:47   ` Jan Beulich
  2017-03-06 13:41   ` Andrew Cooper
  2017-03-06 13:58   ` Jan Beulich
  2 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2017-03-06 12:47 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	IanJackson, Tim Deegan, xen-devel

>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
> Clang 4.0 has added a new check that triggers when taking the address of a
> field of a packed structure. I've explained our situation to the llvm/clang
> folks, but nobody seem to care:
> 
> https://reviews.llvm.org/D20561 
> 
> So simply disable that check if present.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
  2017-03-06 12:31 ` [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0 Roger Pau Monne
@ 2017-03-06 12:52   ` Jan Beulich
  2017-03-06 14:42     ` Boris Ostrovsky
  2017-03-06 15:21     ` Roger Pau Monne
  2017-03-11  0:42   ` Daniel De Graaf
  1 sibling, 2 replies; 15+ messages in thread
From: Jan Beulich @ 2017-03-06 12:52 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel, Boris Ostrovsky, Daniel De Graaf

>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
> There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
> working as expected, and vpmu.o ends up with a reference to
> __xsm_action_mismatch_detected which makes the build fail:
> 
> [...]
> ld    -melf_x86_64_fbsd  -T xen.lds -N prelink.o  \
>     xen/common/symbols-dummy.o -o xen/.xen-syms.0
> prelink.o: In function `xsm_default_action':
> xen/include/xsm/dummy.h:80: undefined reference to 
> `__xsm_action_mismatch_detected'
> xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 
> against undefined symbol `__xsm_action_mismatch_detected'
> ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' 
> isn't defined
> 
> Then doing a search in the objects files:
> 
> # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
>   'for filename; do nm "$filename" | \
>   grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
> xen/arch/x86/prelink.o
> xen/arch/x86/cpu/vpmu.o
> xen/arch/x86/cpu/built_in.o
> xen/arch/x86/built_in.o
> 
> The current patch is the only way I've found to fix this so far, by simply
> moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
> the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
> XENPMU_* operations, instead of -EPERM when called by a privileged domain.

Especially from this perspective I think the patch is fine (but also
Cc-ing Boris), yet I still think the compilation aspect needs to be got
to the bottom of, to have a complete picture in case the problem
shows up in a slightly different way elsewhere. Did you report this
to clang folks? Did they have any explanation of what's going on?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] build/clang: remove the address-of-packed-member warning
  2017-03-06 12:31 ` [PATCH 1/2] build/clang: remove the address-of-packed-member warning Roger Pau Monne
  2017-03-06 12:47   ` Jan Beulich
@ 2017-03-06 13:41   ` Andrew Cooper
  2017-03-06 13:58   ` Jan Beulich
  2 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2017-03-06 13:41 UTC (permalink / raw)
  To: Roger Pau Monne, xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Ian Jackson,
	Tim Deegan, Jan Beulich

On 06/03/17 12:31, Roger Pau Monne wrote:
> Clang 4.0 has added a new check that triggers when taking the address of a
> field of a packed structure. I've explained our situation to the llvm/clang
> folks, but nobody seem to care:
>
> https://reviews.llvm.org/D20561
>
> So simply disable that check if present.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

In this case, I'd prefer that we drop the unnecessary packed on
segment_attributes and segment_register, perhaps adding some
BUILD_BUG_ON()'s in the SVM code to check that the structure still
matches hardwares representation.

Or were there other structures which ended up in a similar situation?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] build/clang: remove the address-of-packed-member warning
  2017-03-06 12:31 ` [PATCH 1/2] build/clang: remove the address-of-packed-member warning Roger Pau Monne
  2017-03-06 12:47   ` Jan Beulich
  2017-03-06 13:41   ` Andrew Cooper
@ 2017-03-06 13:58   ` Jan Beulich
  2017-03-06 14:36     ` George Dunlap
  2 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2017-03-06 13:58 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	IanJackson, Tim Deegan, xen-devel

>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
> --- a/Config.mk
> +++ b/Config.mk
> @@ -216,6 +216,7 @@ $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
>  $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
>  $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
>  $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
> +$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member)

Actually, having thought some more about this, the warning
should be suppressed only for x86 imo. ARM wants aligned
accesses after all.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] build/clang: remove the address-of-packed-member warning
  2017-03-06 13:58   ` Jan Beulich
@ 2017-03-06 14:36     ` George Dunlap
  2017-03-06 15:16       ` Tim Deegan
  0 siblings, 1 reply; 15+ messages in thread
From: George Dunlap @ 2017-03-06 14:36 UTC (permalink / raw)
  To: Jan Beulich, Roger Pau Monne
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	IanJackson, Tim Deegan, xen-devel

On 06/03/17 13:58, Jan Beulich wrote:
>>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
>> --- a/Config.mk
>> +++ b/Config.mk
>> @@ -216,6 +216,7 @@ $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
>>  $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
>>  $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
>>  $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
>> +$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member)
> 
> Actually, having thought some more about this, the warning
> should be suppressed only for x86 imo. ARM wants aligned
> accesses after all.

Looking at Roger's complaint, it appears that the warning is issued even
if the member actually is aligned, if *on some unknown system*, it might
someday be un-aligned.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
  2017-03-06 12:52   ` Jan Beulich
@ 2017-03-06 14:42     ` Boris Ostrovsky
  2017-03-06 14:54       ` Jan Beulich
  2017-03-06 15:21     ` Roger Pau Monne
  1 sibling, 1 reply; 15+ messages in thread
From: Boris Ostrovsky @ 2017-03-06 14:42 UTC (permalink / raw)
  To: Jan Beulich, Roger Pau Monne; +Cc: xen-devel, Daniel De Graaf

On 03/06/2017 07:52 AM, Jan Beulich wrote:
>>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
>> There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
>> working as expected, and vpmu.o ends up with a reference to
>> __xsm_action_mismatch_detected which makes the build fail:
>>
>> [...]
>> ld    -melf_x86_64_fbsd  -T xen.lds -N prelink.o  \
>>     xen/common/symbols-dummy.o -o xen/.xen-syms.0
>> prelink.o: In function `xsm_default_action':
>> xen/include/xsm/dummy.h:80: undefined reference to 
>> `__xsm_action_mismatch_detected'
>> xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 
>> against undefined symbol `__xsm_action_mismatch_detected'
>> ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' 
>> isn't defined
>>
>> Then doing a search in the objects files:
>>
>> # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
>>   'for filename; do nm "$filename" | \
>>   grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
>> xen/arch/x86/prelink.o
>> xen/arch/x86/cpu/vpmu.o
>> xen/arch/x86/cpu/built_in.o
>> xen/arch/x86/built_in.o
>>
>> The current patch is the only way I've found to fix this so far, by simply
>> moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
>> the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
>> XENPMU_* operations, instead of -EPERM when called by a privileged domain.

How will these changes make do_xenpmu_op() return -EINVAL?

-boris

> Especially from this perspective I think the patch is fine (but also
> Cc-ing Boris), yet I still think the compilation aspect needs to be got
> to the bottom of, to have a complete picture in case the problem
> shows up in a slightly different way elsewhere. Did you report this
> to clang folks? Did they have any explanation of what's going on?
>
> Jan
>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
  2017-03-06 14:42     ` Boris Ostrovsky
@ 2017-03-06 14:54       ` Jan Beulich
  2017-03-06 15:07         ` Boris Ostrovsky
  0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2017-03-06 14:54 UTC (permalink / raw)
  To: Boris Ostrovsky; +Cc: xen-devel, Daniel De Graaf, Roger Pau Monne

>>> On 06.03.17 at 15:42, <boris.ostrovsky@oracle.com> wrote:
> On 03/06/2017 07:52 AM, Jan Beulich wrote:
>>>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
>>> There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
>>> working as expected, and vpmu.o ends up with a reference to
>>> __xsm_action_mismatch_detected which makes the build fail:
>>>
>>> [...]
>>> ld    -melf_x86_64_fbsd  -T xen.lds -N prelink.o  \
>>>     xen/common/symbols-dummy.o -o xen/.xen-syms.0
>>> prelink.o: In function `xsm_default_action':
>>> xen/include/xsm/dummy.h:80: undefined reference to 
>>> `__xsm_action_mismatch_detected'
>>> xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 
>>> against undefined symbol `__xsm_action_mismatch_detected'
>>> ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' 
>>> isn't defined
>>>
>>> Then doing a search in the objects files:
>>>
>>> # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
>>>   'for filename; do nm "$filename" | \
>>>   grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
>>> xen/arch/x86/prelink.o
>>> xen/arch/x86/cpu/vpmu.o
>>> xen/arch/x86/cpu/built_in.o
>>> xen/arch/x86/built_in.o
>>>
>>> The current patch is the only way I've found to fix this so far, by simply
>>> moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
>>> the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
>>> XENPMU_* operations, instead of -EPERM when called by a privileged domain.
> 
> How will these changes make do_xenpmu_op() return -EINVAL?

Since the XSM check no longer returns -EPERM, the handling inside
do_vpmu_op() will now reach the default case there.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
  2017-03-06 14:54       ` Jan Beulich
@ 2017-03-06 15:07         ` Boris Ostrovsky
  0 siblings, 0 replies; 15+ messages in thread
From: Boris Ostrovsky @ 2017-03-06 15:07 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Daniel De Graaf, Roger Pau Monne

On 03/06/2017 09:54 AM, Jan Beulich wrote:
>>>> On 06.03.17 at 15:42, <boris.ostrovsky@oracle.com> wrote:
>> On 03/06/2017 07:52 AM, Jan Beulich wrote:
>>>>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
>>>> There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
>>>> working as expected, and vpmu.o ends up with a reference to
>>>> __xsm_action_mismatch_detected which makes the build fail:
>>>>
>>>> [...]
>>>> ld    -melf_x86_64_fbsd  -T xen.lds -N prelink.o  \
>>>>     xen/common/symbols-dummy.o -o xen/.xen-syms.0
>>>> prelink.o: In function `xsm_default_action':
>>>> xen/include/xsm/dummy.h:80: undefined reference to 
>>>> `__xsm_action_mismatch_detected'
>>>> xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 
>>>> against undefined symbol `__xsm_action_mismatch_detected'
>>>> ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' 
>>>> isn't defined
>>>>
>>>> Then doing a search in the objects files:
>>>>
>>>> # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
>>>>   'for filename; do nm "$filename" | \
>>>>   grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
>>>> xen/arch/x86/prelink.o
>>>> xen/arch/x86/cpu/vpmu.o
>>>> xen/arch/x86/cpu/built_in.o
>>>> xen/arch/x86/built_in.o
>>>>
>>>> The current patch is the only way I've found to fix this so far, by simply
>>>> moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
>>>> the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
>>>> XENPMU_* operations, instead of -EPERM when called by a privileged domain.
>> How will these changes make do_xenpmu_op() return -EINVAL?
> Since the XSM check no longer returns -EPERM, the handling inside
> do_vpmu_op() will now reach the default case there.

Oh, I was staring at xsm_default_action() and trying to see where
-EINVAL would be coming from.

Sorry for the noise.

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] build/clang: remove the address-of-packed-member warning
  2017-03-06 14:36     ` George Dunlap
@ 2017-03-06 15:16       ` Tim Deegan
  2017-03-06 15:33         ` Andrew Cooper
  0 siblings, 1 reply; 15+ messages in thread
From: Tim Deegan @ 2017-03-06 15:16 UTC (permalink / raw)
  To: George Dunlap
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	IanJackson, Jan Beulich, xen-devel, Roger Pau Monne

At 14:36 +0000 on 06 Mar (1488811016), George Dunlap wrote:
> On 06/03/17 13:58, Jan Beulich wrote:
> >>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
> >> --- a/Config.mk
> >> +++ b/Config.mk
> >> @@ -216,6 +216,7 @@ $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
> >>  $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
> >>  $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
> >>  $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
> >> +$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member)
> > 
> > Actually, having thought some more about this, the warning
> > should be suppressed only for x86 imo. ARM wants aligned
> > accesses after all.
> 
> Looking at Roger's complaint, it appears that the warning is issued even
> if the member actually is aligned, if *on some unknown system*, it might
> someday be un-aligned.

AIUI the complaint is (based on the simplified example from the ticket):

    struct __attribute__((__packed__)) bar {
        uint16_t x1;
        uint16_t x2;
    } b;
    
    &b.x2;

Because the struct is packed, it has alignment 1, and so do its
fields.   &b.x2 is a pointer to a uint16_t, but it _isn't_ 16-bit
aligned (because the whole struct is only byte-aligned).

So I guess that one fix would be to declare that the struct has
appropriate alignment?  I don't know whether that would suppress the
warning, but the clang devs might be more receptive to seeing it as
a false positive.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
  2017-03-06 12:52   ` Jan Beulich
  2017-03-06 14:42     ` Boris Ostrovsky
@ 2017-03-06 15:21     ` Roger Pau Monne
  1 sibling, 0 replies; 15+ messages in thread
From: Roger Pau Monne @ 2017-03-06 15:21 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Boris Ostrovsky, Daniel De Graaf

On Mon, Mar 06, 2017 at 05:52:14AM -0700, Jan Beulich wrote:
> >>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
> > There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
> > working as expected, and vpmu.o ends up with a reference to
> > __xsm_action_mismatch_detected which makes the build fail:
> > 
> > [...]
> > ld    -melf_x86_64_fbsd  -T xen.lds -N prelink.o  \
> >     xen/common/symbols-dummy.o -o xen/.xen-syms.0
> > prelink.o: In function `xsm_default_action':
> > xen/include/xsm/dummy.h:80: undefined reference to 
> > `__xsm_action_mismatch_detected'
> > xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 
> > against undefined symbol `__xsm_action_mismatch_detected'
> > ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' 
> > isn't defined
> > 
> > Then doing a search in the objects files:
> > 
> > # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
> >   'for filename; do nm "$filename" | \
> >   grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
> > xen/arch/x86/prelink.o
> > xen/arch/x86/cpu/vpmu.o
> > xen/arch/x86/cpu/built_in.o
> > xen/arch/x86/built_in.o
> > 
> > The current patch is the only way I've found to fix this so far, by simply
> > moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
> > the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
> > XENPMU_* operations, instead of -EPERM when called by a privileged domain.
> 
> Especially from this perspective I think the patch is fine (but also
> Cc-ing Boris), yet I still think the compilation aspect needs to be got
> to the bottom of, to have a complete picture in case the problem
> shows up in a slightly different way elsewhere. Did you report this
> to clang folks? Did they have any explanation of what's going on?

I've just reported this upstream, for the record the ticket is at:

http://bugs.llvm.org/show_bug.cgi?id=32150

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] build/clang: remove the address-of-packed-member warning
  2017-03-06 15:16       ` Tim Deegan
@ 2017-03-06 15:33         ` Andrew Cooper
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2017-03-06 15:33 UTC (permalink / raw)
  To: Tim Deegan, George Dunlap
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, IanJackson,
	Jan Beulich, xen-devel, Roger Pau Monne

On 06/03/17 15:16, Tim Deegan wrote:
> At 14:36 +0000 on 06 Mar (1488811016), George Dunlap wrote:
>> On 06/03/17 13:58, Jan Beulich wrote:
>>>>>> On 06.03.17 at 13:31, <roger.pau@citrix.com> wrote:
>>>> --- a/Config.mk
>>>> +++ b/Config.mk
>>>> @@ -216,6 +216,7 @@ $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
>>>>  $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
>>>>  $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
>>>>  $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
>>>> +$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member)
>>> Actually, having thought some more about this, the warning
>>> should be suppressed only for x86 imo. ARM wants aligned
>>> accesses after all.
>> Looking at Roger's complaint, it appears that the warning is issued even
>> if the member actually is aligned, if *on some unknown system*, it might
>> someday be un-aligned.
> AIUI the complaint is (based on the simplified example from the ticket):
>
>     struct __attribute__((__packed__)) bar {
>         uint16_t x1;
>         uint16_t x2;
>     } b;
>     
>     &b.x2;
>
> Because the struct is packed, it has alignment 1, and so do its
> fields.   &b.x2 is a pointer to a uint16_t, but it _isn't_ 16-bit
> aligned (because the whole struct is only byte-aligned).
>
> So I guess that one fix would be to declare that the struct has
> appropriate alignment?  I don't know whether that would suppress the
> warning, but the clang devs might be more receptive to seeing it as
> a false positive.

The structs in question (segment_attributes and segment_register) have
proper natural alignment with no padding anyway, so don't need to be
__packed__ to be correct.

It would be better to remove the unnecessary __packed__.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
  2017-03-06 12:31 ` [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0 Roger Pau Monne
  2017-03-06 12:52   ` Jan Beulich
@ 2017-03-11  0:42   ` Daniel De Graaf
  1 sibling, 0 replies; 15+ messages in thread
From: Daniel De Graaf @ 2017-03-11  0:42 UTC (permalink / raw)
  To: Roger Pau Monne, xen-devel

On 03/06/2017 07:31 AM, Roger Pau Monne wrote:
> There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
> working as expected, and vpmu.o ends up with a reference to
> __xsm_action_mismatch_detected which makes the build fail:
>
> [...]
> ld    -melf_x86_64_fbsd  -T xen.lds -N prelink.o  \
>     xen/common/symbols-dummy.o -o xen/.xen-syms.0
> prelink.o: In function `xsm_default_action':
> xen/include/xsm/dummy.h:80: undefined reference to `__xsm_action_mismatch_detected'
> xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `__xsm_action_mismatch_detected'
> ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' isn't defined
>
> Then doing a search in the objects files:
>
> # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
>   'for filename; do nm "$filename" | \
>   grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
> xen/arch/x86/prelink.o
> xen/arch/x86/cpu/vpmu.o
> xen/arch/x86/cpu/built_in.o
> xen/arch/x86/built_in.o
>
> The current patch is the only way I've found to fix this so far, by simply
> moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
> the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
> XENPMU_* operations, instead of -EPERM when called by a privileged domain.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

This also looks like a good patch for backporting.  The best alternative I
can think of is to disable the mismatch detection in non-DEBUG builds, but
I'd rather not do that unless this problem expands more than it has.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-03-11  0:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-06 12:31 [PATCH 0/2] Build fixes for clang 4.0 Roger Pau Monne
2017-03-06 12:31 ` [PATCH 1/2] build/clang: remove the address-of-packed-member warning Roger Pau Monne
2017-03-06 12:47   ` Jan Beulich
2017-03-06 13:41   ` Andrew Cooper
2017-03-06 13:58   ` Jan Beulich
2017-03-06 14:36     ` George Dunlap
2017-03-06 15:16       ` Tim Deegan
2017-03-06 15:33         ` Andrew Cooper
2017-03-06 12:31 ` [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0 Roger Pau Monne
2017-03-06 12:52   ` Jan Beulich
2017-03-06 14:42     ` Boris Ostrovsky
2017-03-06 14:54       ` Jan Beulich
2017-03-06 15:07         ` Boris Ostrovsky
2017-03-06 15:21     ` Roger Pau Monne
2017-03-11  0:42   ` Daniel De Graaf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.