Re: [Xen-devel] [PATCH] x86/vmx: fix build with clang 3.8.0

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/02/17 11:33, Roger Pau Monne wrote:
> The usage of the __transparent__ attribute in 991033fa introduces some issues
> when compiled with clang 3.8.0:
>
> xen/include/asm/hvm/vmx/vmx.h:605:15: error: transparent_union attribute can 
> only be
>       applied to a union definition; attribute ignored 
> [-Werror,-Wignored-attributes]
> typedef union __transparent__ ept_qual {
>               ^
> xen/include/xen/compiler.h:50:44: note: expanded from macro '__transparent__'
> #define __transparent__     __attribute__((__transparent_union__))
>
> This can be easily fixed by moving the attribute to the end of the definition,
> but then the following error triggers:
>
> xen/include/asm/hvm/vmx/vmx.h:607:5: error: size of field '' (16 bits) does 
> not
>       match the size of the first field in transparent union; 
> transparent_union attribute ignored
>       [-Werror,-Wignored-attributes]
>     struct {
>     ^
> xen/include/asm/hvm/vmx/vmx.h:606:19: note: size of first field is 64 bits
>     unsigned long raw;
>                   ^
>
> Which can be fixed by introducing a new field in the nested structure that
> contains the padding in order to match the size of an unsigned long.
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Sorry about that.  I could have sworn I build tested with clang, but it
appears I didn't.

> ---
> Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
>  xen/include/asm-x86/hvm/vmx/vmx.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h 
> b/xen/include/asm-x86/hvm/vmx/vmx.h
> index 5f7512b..52646b7 100644
> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -602,15 +602,16 @@ void vmx_pi_hooks_assign(struct domain *d);
>  void vmx_pi_hooks_deassign(struct domain *d);
>  
>  /* EPT violation qualifications definitions */
> -typedef union __transparent__ ept_qual {
> +typedef union ept_qual {

Please can we use

typedef __transparent__ union ept_qual {

which clang is happy with, and will help avoid problems such as the
cper_mce_record issue in c/s f8be76e2fe

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> and I
can fix this up on commit.

~Andrew

>      unsigned long raw;
>      struct {
>          bool read:1, write:1, fetch:1,
>              eff_read:1, eff_write:1, eff_exec:1, /* eff_user_exec */:1,
>              gla_valid:1,
>              gla_fault:1; /* Valid iff gla_valid. */
> +        unsigned long /* pad */:55;
>      };
> -} ept_qual_t;
> +} ept_qual_t __transparent__;
>  
>  #define EPT_L4_PAGETABLE_SHIFT      39
>  #define EPT_PAGETABLE_ENTRIES       512


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