Re: [Xen-devel] [PATCH] xstate: make use_xsave non-init

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jul 01, 2019 at 11:39:16AM +0000, Jan Beulich wrote:
> On 01.07.2019 12:49, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/xstate.c
> > +++ b/xen/arch/x86/xstate.c
> > @@ -577,7 +577,7 @@ unsigned int xstate_ctxt_size(u64 xcr0)
> >   /* Collect the information of processor's extended state */
> >   void xstate_init(struct cpuinfo_x86 *c)
> >   {
> > -    static bool __initdata use_xsave = true;
> > +    static bool use_xsave = true;
> 
> Please attach at least a brief comment here, such that people
> won't consider the __initdata missing.

Sure.

> 
> Of course I'd actually prefer the annotation to stay here in
> the gcc case. Iirc there was one other case where there was
> such an issue; I don't recall whether there it too got dealt
> with by removing an annotation.

Yes, in that other case the annotation was just removed, it's 43fa95ae [0]

> How about we introduce an
> annotation that expands to nothing in the clang case, but
> continues to provide the same functionality for gcc? That
> would then also clarify the reason for it being in any
> particular place (I guess there are going to be more) without
> the need for further commentary.

IMO that's a little bit dangerous, from the LLVM bug report it seems
like LLVM behaviour is not a bug, and hence I wouldn't be surprised if
newer versions of gcc also exhibit the same issue.

Thanks, Roger.

[0] 
http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=43fa95ae6a64132b8ebe3025bd187ab9df68677b

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel