| Message ID | 20190905160424.6686-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] xstate: make use_xsave non-init | expand |
On 05.09.2019 18:04, Roger Pau Monne wrote: > --- a/xen/arch/x86/xstate.c > +++ b/xen/arch/x86/xstate.c > @@ -577,7 +577,11 @@ 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; > + /* > + * NB: use_xsave cannot live in initdata because llvm might optimize > + * reading it, see: https://bugs.llvm.org/show_bug.cgi?id=39707 > + */ > + static bool use_xsave = true; > boolean_param("xsave", use_xsave); > > bool bsp = c == &boot_cpu_data; I think we'd want to use __read_mostly then instead. Can be added while committing of course, if you agree. With the addition Acked-by: Jan Beulich <jbeulich@suse.com> Jan
On Fri, Sep 06, 2019 at 03:33:50PM +0200, Jan Beulich wrote: > On 05.09.2019 18:04, Roger Pau Monne wrote: > > --- a/xen/arch/x86/xstate.c > > +++ b/xen/arch/x86/xstate.c > > @@ -577,7 +577,11 @@ 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; > > + /* > > + * NB: use_xsave cannot live in initdata because llvm might optimize > > + * reading it, see: https://bugs.llvm.org/show_bug.cgi?id=39707 > > + */ > > + static bool use_xsave = true; > > boolean_param("xsave", use_xsave); > > > > bool bsp = c == &boot_cpu_data; > > I think we'd want to use __read_mostly then instead. Can be added > while committing of course, if you agree. With the addition > Acked-by: Jan Beulich <jbeulich@suse.com> Yes, that seems OK to me. Feel free to expand the commit message to mention the change to read_mostly if you want. Thanks, Roger.
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c index 3293ef834f..1dddda2818 100644 --- a/xen/arch/x86/xstate.c +++ b/xen/arch/x86/xstate.c @@ -577,7 +577,11 @@ 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; + /* + * NB: use_xsave cannot live in initdata because llvm might optimize + * reading it, see: https://bugs.llvm.org/show_bug.cgi?id=39707 + */ + static bool use_xsave = true; boolean_param("xsave", use_xsave); bool bsp = c == &boot_cpu_data;
LLVM code generation can attempt to load from a variable in the next condition of an expression under certain circumstances, thus attempting to load use_xsave regardless of the value of the bsp variable, which leads to a page fault when the init section has already been unmapped. Fix this by making use_xsave non-init, thus preventing the page fault. The LLVM bug with the discussion about this issue can be found at: https://bugs.llvm.org/show_bug.cgi?id=39707 Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/xstate.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)