Re: [Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

2017-04-10 Thread Daniel De Graaf 

On 04/10/2017 04:41 AM, Roger Pau Monne wrote:

On Fri, Apr 07, 2017 at 08:54:59AM -0600, Jan Beulich wrote:

On 07.04.17 at 16:34,  wrote:

The changes introduced on c47d1d broke the clang build due to undefined
references to __xsm_action_mismatch_detected, because clang hasn't optimized
the code properly.


This starts to become annoying.


Yes, tell me about it... I don't think we can blame clang for those failures
anyway. Xen is relying on optimizations to do compile time validations, and
AFAIK this is a grey area. There's nothing in the C spec that guarantees you
that those calls will be removed.

Anyway, will send a new version shortly.


If this problem expands more, I think it would be best to restrict the check to
a particular compiler or #define (as long as it's one used in the build bot);
there's no need to do this kind of check on every build as long as it's done on
occasional builds.  Alternatively, it could be done by a static analysis tool,
but I've not looked into how to do that with Coverity.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

2017-04-10 Thread Roger Pau Monne 
On Fri, Apr 07, 2017 at 08:54:59AM -0600, Jan Beulich wrote:
> >>> On 07.04.17 at 16:34,  wrote:
> > The changes introduced on c47d1d broke the clang build due to undefined
> > references to __xsm_action_mismatch_detected, because clang hasn't optimized
> > the code properly.
> 
> This starts to become annoying.

Yes, tell me about it... I don't think we can blame clang for those failures
anyway. Xen is relying on optimizations to do compile time validations, and
AFAIK this is a grey area. There's nothing in the C spec that guarantees you
that those calls will be removed.

Anyway, will send a new version shortly.

Roger.

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

Re: [Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

2017-04-10 Thread Jan Beulich 
>>> On 09.04.17 at 21:01,  wrote:
> On 04/07/2017 03:54 PM, Jan Beulich wrote:
> On 07.04.17 at 16:34,  wrote:
>>> The changes introduced on c47d1d broke the clang build due to undefined
>>> references to __xsm_action_mismatch_detected, because clang hasn't optimized
>>> the code properly.
>> 
>> This starts to become annoying.
> 
> This is also breaking ARM32 build with GCC (4.9 and 5.4) (see [1]):

That's certainly unexpected, as we're building with -O1 even for
debug builds (for this very reason).

Jan


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

Re: [Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

2017-04-09 Thread Julien Grall 

Hi Roger,

On 04/07/2017 03:34 PM, Roger Pau Monne wrote:

The changes introduced on c47d1d broke the clang build due to undefined
references to __xsm_action_mismatch_detected, because clang hasn't optimized
the code properly. The following patch allows the clang build to work again,
while keeping the same functionality.

Signed-off-by: Roger Pau Monné 


FWIW: this is fixing ARM32 build with XSM (blocks osstest).

Tested-by: Julien Grall 

Cheers,


---
Cc: Daniel De Graaf 
Cc: Julien Grall 
Cc: Tamas K Lengyel 
---
NB: this fixes travis build: https://travis-ci.org/royger/xen/builds/219697038
---
 xen/include/xsm/dummy.h | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 56a8814d82..7d35744afe 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -557,25 +557,23 @@ static XSM_INLINE int 
xsm_hvm_param_altp2mhvm(XSM_DEFAULT_ARG struct domain *d)

 static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d, 
uint64_t mode, uint32_t op)
 {
-xsm_default_t a;
 XSM_ASSERT_ACTION(XSM_OTHER);

 switch ( mode )
 {
 case XEN_ALTP2M_mixed:
-a = XSM_TARGET;
-break;
+return xsm_default_action(XSM_TARGET, current->domain, d);
 case XEN_ALTP2M_external:
-a = XSM_DM_PRIV;
+return xsm_default_action(XSM_DM_PRIV, current->domain, d);
 break;
 case XEN_ALTP2M_limited:
-a = (HVMOP_altp2m_vcpu_enable_notify == op) ? XSM_TARGET : XSM_DM_PRIV;
-break;
+if ( HVMOP_altp2m_vcpu_enable_notify == op )
+return xsm_default_action(XSM_TARGET, current->domain, d);
+else
+return xsm_default_action(XSM_DM_PRIV, current->domain, d);
 default:
 return -EPERM;
-};
-
-return xsm_default_action(a, current->domain, d);
+}
 }

 static XSM_INLINE int xsm_vm_event_control(XSM_DEFAULT_ARG struct domain *d, 
int mode, int op)



--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

2017-04-09 Thread Julien Grall 
Hi,

On 04/07/2017 03:54 PM, Jan Beulich wrote:
 On 07.04.17 at 16:34,  wrote:
>> The changes introduced on c47d1d broke the clang build due to undefined
>> references to __xsm_action_mismatch_detected, because clang hasn't optimized
>> the code properly.
> 
> This starts to become annoying.

This is also breaking ARM32 build with GCC (4.9 and 5.4) (see [1]):

ld-EL  -T xen.lds -N prelink.o \
/home/osstest/build.107275.build-armhf-xsm/xen/xen/common/symbols-dummy.o 
-o /home/osstest/build.107275.build-armhf-xsm/xen/xen/.xen-syms.0
prelink.o: In function `xsm_default_action':
/home/osstest/build.107275.build-armhf-xsm/xen/xen/include/xsm/dummy.h:80: 
undefined reference to `__xsm_action_mismatch_detected'
gcc  -DPIC -marm -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall 
-Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable 
-Wno-unused-local-typedefs   -g3 -O0 -fno-omit-frame-pointer 
-D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF 
.xc_dom_boot.opic.d -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-D_LARGEFILE64_SOURCE -D_GNU_SOURCE  -I../../xen/common/libelf -Werror 
-Wmissing-prototypes -I. -I./include 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/include
 -D__XEN_TOOLS__ -pthread 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/libs/toollog/include
 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/include
 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/libs/evtchn/include
 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/include
 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/libs/devicemodel/include
 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/include
 -include 
/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/config.h 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/libs/call/include
 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/include
 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/libs/foreignmemory/include
 
-I/home/osstest/build.107275.build-armhf-xsm/xen/tools/libxc/../../tools/include
  -fPIC -c -o xc_dom_boot.opic xc_dom_boot.c 
ld: /home/osstest/build.107275.build-armhf-xsm/xen/xen/.xen-syms.0: hidden 
symbol `__xsm_action_mismatch_detected' isn't defined
ld: final link failed: Bad value
make[3]: *** [/home/osstest/build.107275.build-armhf-xsm/xen/xen/xen-syms] 
Error 1
Makefile:96: recipe for target 
'/home/osstest/build.107275.build-armhf-xsm/xen/xen/xen-syms' failed
make[3]: Leaving directory 
'/home/osstest/build.107275.build-armhf-xsm/xen/xen/arch/arm'
make[2]: *** [/home/osstest/build.107275.build-armhf-xsm/xen/xen/xen] Error 2
Makefile:136: recipe for target 
'/home/osstest/build.107275.build-armhf-xsm/xen/xen/xen' failed
make[2]: Leaving directory '/home/osstest/build.107275.build-armhf-xsm/xen/xen'
Makefile:45: recipe for target 'install' failed
make[1]: Leaving directory '/home/osstest/build.107275.build-armhf-xsm/xen/xen'
Makefile:97: recipe for target 'install-xen' failed
make[1]: *** [install] Error 2
make: *** [install-xen] Error 2
make: *** Waiting for unfinished jobs

[1] 
http://logs.test-lab.xenproject.org/osstest/logs/107275/build-armhf-xsm/5.ts-xen-build.log

-- 
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

2017-04-07 Thread Jan Beulich 
>>> On 07.04.17 at 16:34,  wrote:
> The changes introduced on c47d1d broke the clang build due to undefined
> references to __xsm_action_mismatch_detected, because clang hasn't optimized
> the code properly.

This starts to become annoying.

> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -557,25 +557,23 @@ static XSM_INLINE int 
> xsm_hvm_param_altp2mhvm(XSM_DEFAULT_ARG struct domain *d)
>  
>  static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d, 
> uint64_t mode, uint32_t op)
>  {
> -xsm_default_t a;
>  XSM_ASSERT_ACTION(XSM_OTHER);
>  
>  switch ( mode )
>  {
>  case XEN_ALTP2M_mixed:
> -a = XSM_TARGET;
> -break;
> +return xsm_default_action(XSM_TARGET, current->domain, d);
>  case XEN_ALTP2M_external:
> -a = XSM_DM_PRIV;
> +return xsm_default_action(XSM_DM_PRIV, current->domain, d);
>  break;

Just like above, this break wants to be eliminated.

>  case XEN_ALTP2M_limited:
> -a = (HVMOP_altp2m_vcpu_enable_notify == op) ? XSM_TARGET : 
> XSM_DM_PRIV;
> -break;
> +if ( HVMOP_altp2m_vcpu_enable_notify == op )
> +return xsm_default_action(XSM_TARGET, current->domain, d);
> +else
> +return xsm_default_action(XSM_DM_PRIV, current->domain, d);

Could you either keep the conditional expression at least (as a
direct function parameter), or if that still doesn't work at the
minimum drop the pointless "else"?

Jan


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

Re: [Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

2017-04-07 Thread M A Young 
On Fri, 7 Apr 2017, Roger Pau Monne wrote:

...
> +return xsm_default_action(XSM_TARGET, current->domain, d);
>  case XEN_ALTP2M_external:
> -a = XSM_DM_PRIV;
> +return xsm_default_action(XSM_DM_PRIV, current->domain, d);
>  break;
>  case XEN_ALTP2M_limited:

It looks like you left a break; in there which will never be reached.

Michael Young

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

[Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

2017-04-07 Thread Roger Pau Monne 
The changes introduced on c47d1d broke the clang build due to undefined
references to __xsm_action_mismatch_detected, because clang hasn't optimized
the code properly. The following patch allows the clang build to work again,
while keeping the same functionality.

Signed-off-by: Roger Pau Monné 
---
Cc: Daniel De Graaf 
Cc: Julien Grall 
Cc: Tamas K Lengyel 
---
NB: this fixes travis build: https://travis-ci.org/royger/xen/builds/219697038
---
 xen/include/xsm/dummy.h | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 56a8814d82..7d35744afe 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -557,25 +557,23 @@ static XSM_INLINE int 
xsm_hvm_param_altp2mhvm(XSM_DEFAULT_ARG struct domain *d)
 
 static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d, 
uint64_t mode, uint32_t op)
 {
-xsm_default_t a;
 XSM_ASSERT_ACTION(XSM_OTHER);
 
 switch ( mode )
 {
 case XEN_ALTP2M_mixed:
-a = XSM_TARGET;
-break;
+return xsm_default_action(XSM_TARGET, current->domain, d);
 case XEN_ALTP2M_external:
-a = XSM_DM_PRIV;
+return xsm_default_action(XSM_DM_PRIV, current->domain, d);
 break;
 case XEN_ALTP2M_limited:
-a = (HVMOP_altp2m_vcpu_enable_notify == op) ? XSM_TARGET : XSM_DM_PRIV;
-break;
+if ( HVMOP_altp2m_vcpu_enable_notify == op )
+return xsm_default_action(XSM_TARGET, current->domain, d);
+else
+return xsm_default_action(XSM_DM_PRIV, current->domain, d);
 default:
 return -EPERM;
-};
-
-return xsm_default_action(a, current->domain, d);
+}
 }
 
 static XSM_INLINE int xsm_vm_event_control(XSM_DEFAULT_ARG struct domain *d, 
int mode, int op)
-- 
2.11.0 (Apple Git-81)


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