| Message ID | 20170410090157.49501-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 10.04.17 at 11:01, <roger.pau@citrix.com> 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é <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Hi Roger, On 10/04/17 10:01, 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é <roger.pau@citrix.com> > --- > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Julien Grall <julien.grall@arm.com> > Cc: Tamas K Lengyel <tamas.lengyel@zentific.com> > --- > Changes since v1: > - Remove unused "break". > - Remove if condition. This new version does not fix the ARM32 build (though the previous one does). I still get: prelink.o: In function `xsm_default_action': /home/julieng/works/xen/xen/include/xsm/dummy.h:80: undefined reference to `__xsm_action_mismatch_detected' arm-linux-gnueabihf-ld: /home/julieng/works/xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' isn't defined I have just noticed this is also breaking ARM64 build (GCC 6.1). Cheers,
>>> On 10.04.17 at 12:39, <julien.grall@arm.com> wrote: > Hi Roger, > > On 10/04/17 10:01, 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é <roger.pau@citrix.com> >> --- >> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> >> Cc: Julien Grall <julien.grall@arm.com> >> Cc: Tamas K Lengyel <tamas.lengyel@zentific.com> >> --- >> Changes since v1: >> - Remove unused "break". >> - Remove if condition. > > This new version does not fix the ARM32 build (though the previous > one does). I still get: > > prelink.o: In function `xsm_default_action': > /home/julieng/works/xen/xen/include/xsm/dummy.h:80: undefined reference to > `__xsm_action_mismatch_detected' > arm-linux-gnueabihf-ld: /home/julieng/works/xen/xen/.xen-syms.0: hidden > symbol `__xsm_action_mismatch_detected' isn't defined I have to admit that I was afraid of this, after having seen this is an issue for ARM too. I guess the suggested use of a conditional expression (instead of if/else) needs to be undone, which is quite sad. Jan
On 10/04/17 11:50, Jan Beulich wrote: >>>> On 10.04.17 at 12:39, <julien.grall@arm.com> wrote: >> Hi Roger, >> >> On 10/04/17 10:01, 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é <roger.pau@citrix.com> >>> --- >>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> >>> Cc: Julien Grall <julien.grall@arm.com> >>> Cc: Tamas K Lengyel <tamas.lengyel@zentific.com> >>> --- >>> Changes since v1: >>> - Remove unused "break". >>> - Remove if condition. >> >> This new version does not fix the ARM32 build (though the previous >> one does). I still get: >> >> prelink.o: In function `xsm_default_action': >> /home/julieng/works/xen/xen/include/xsm/dummy.h:80: undefined reference to >> `__xsm_action_mismatch_detected' >> arm-linux-gnueabihf-ld: /home/julieng/works/xen/xen/.xen-syms.0: hidden >> symbol `__xsm_action_mismatch_detected' isn't defined > > I have to admit that I was afraid of this, after having seen this is > an issue for ARM too. I guess the suggested use of a conditional > expression (instead of if/else) needs to be undone, which is quite > sad. I think this would be the safest for now, I would like to cut an RC soon with what has been pushed before the code freeze. This would mean v1 + removing pointless break. Cheers,
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 56a8814d82..4cb901cb58 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -557,25 +557,21 @@ 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; - break; + return xsm_default_action(XSM_DM_PRIV, current->domain, d); case XEN_ALTP2M_limited: - a = (HVMOP_altp2m_vcpu_enable_notify == op) ? XSM_TARGET : XSM_DM_PRIV; - break; + return xsm_default_action(HVMOP_altp2m_vcpu_enable_notify == op ? + XSM_TARGET : 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)
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é <roger.pau@citrix.com> --- Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Julien Grall <julien.grall@arm.com> Cc: Tamas K Lengyel <tamas.lengyel@zentific.com> --- Changes since v1: - Remove unused "break". - Remove if condition. NB: this fixes travis build: https://travis-ci.org/royger/xen/builds/219697038 --- xen/include/xsm/dummy.h | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)