diff mbox

[2/2] build/clang: fix XSM dummy policy when using clang 4.0

Message ID 20170306123150.99386-3-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roger Pau Monné March 6, 2017, 12:31 p.m. UTC
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(-)

Comments

Jan Beulich March 6, 2017, 12:52 p.m. UTC | #1
>>> 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
Boris Ostrovsky March 6, 2017, 2:42 p.m. UTC | #2
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
>
Jan Beulich March 6, 2017, 2:54 p.m. UTC | #3
>>> 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
Boris Ostrovsky March 6, 2017, 3:07 p.m. UTC | #4
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
Roger Pau Monné March 6, 2017, 3:21 p.m. UTC | #5
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.
Daniel De Graaf March 11, 2017, 12:42 a.m. UTC | #6
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.
diff mbox

Patch

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);
     }
 }