xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables.
For this to work when NX is not available, x86_configure_nx() needs to
be called first.
Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1273,16 +1273,16 @@ asmlinkage __visible void __init xen_sta
/* Get mfn list */
xen_build_dynamic_phys_to_machine();
+ /* Work out if we support NX */
+ get_cpu_cap(&boot_cpu_data);
+ x86_configure_nx();
+
/*
* Set up kernel GDT and segment registers, mainly so that
* -fstack-protector code can be executed.
*/
xen_setup_gdt(0);
- /* Work out if we support NX */
- get_cpu_cap(&boot_cpu_data);
- x86_configure_nx();
-
/* Determine virtual and physical address sizes */
get_cpu_address_sizes(&boot_cpu_data);
On 20.05.21 13:42, Jan Beulich wrote: > xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. > For this to work when NX is not available, x86_configure_nx() needs to > be called first. > > Reported-by: Olaf Hering <olaf@aepfle.de> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On 20.05.2021 13:57, Juergen Gross wrote: > On 20.05.21 13:42, Jan Beulich wrote: >> xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. >> For this to work when NX is not available, x86_configure_nx() needs to >> be called first. >> >> Reported-by: Olaf Hering <olaf@aepfle.de> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Juergen Gross <jgross@suse.com> Thanks. I guess I forgot Cc: stable@vger.kernel.org If you agree, can you please add this before pushing to Linus? Jan
On 20.05.21 14:08, Jan Beulich wrote: > On 20.05.2021 13:57, Juergen Gross wrote: >> On 20.05.21 13:42, Jan Beulich wrote: >>> xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. >>> For this to work when NX is not available, x86_configure_nx() needs to >>> be called first. >>> >>> Reported-by: Olaf Hering <olaf@aepfle.de> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> Reviewed-by: Juergen Gross <jgross@suse.com> > > Thanks. I guess I forgot > > Cc: stable@vger.kernel.org > > If you agree, can you please add this before pushing to Linus? Uh, just had a look why x86_configure_nx() was called after xen_setup_gdt(). Upstream your patch will be fine, but before kernel 5.9 it will break running as 32-bit PV guest (see commit 36104cb9012a82e7). So I will take your patch as is, but for kernels 5.8 and older I recommend a different approach by directly setting the NX capability after checking the cpuid bit instead of letting that do get_cpu_cap(). Juergen
On 21.05.2021 09:18, Juergen Gross wrote: > On 20.05.21 14:08, Jan Beulich wrote: >> On 20.05.2021 13:57, Juergen Gross wrote: >>> On 20.05.21 13:42, Jan Beulich wrote: >>>> xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. >>>> For this to work when NX is not available, x86_configure_nx() needs to >>>> be called first. >>>> >>>> Reported-by: Olaf Hering <olaf@aepfle.de> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> >>> Reviewed-by: Juergen Gross <jgross@suse.com> >> >> Thanks. I guess I forgot >> >> Cc: stable@vger.kernel.org >> >> If you agree, can you please add this before pushing to Linus? > > Uh, just had a look why x86_configure_nx() was called after > xen_setup_gdt(). > > Upstream your patch will be fine, but before kernel 5.9 it will > break running as 32-bit PV guest (see commit 36104cb9012a82e7). Oh, indeed. That commit then actually introduced the issue here, and hence a Fixes: tag may be warranted. > So I will take your patch as is, but for kernels 5.8 and older I > recommend a different approach by directly setting the NX > capability after checking the cpuid bit instead of letting that > do get_cpu_cap(). Right - perhaps the only halfway viable option. 64-bit kernels predating 4f277295e54c may then also need that one, but perhaps all stable ones already have it because it was tagged for stable. Jan
On 21.05.21 09:26, Jan Beulich wrote: > On 21.05.2021 09:18, Juergen Gross wrote: >> On 20.05.21 14:08, Jan Beulich wrote: >>> On 20.05.2021 13:57, Juergen Gross wrote: >>>> On 20.05.21 13:42, Jan Beulich wrote: >>>>> xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. >>>>> For this to work when NX is not available, x86_configure_nx() needs to >>>>> be called first. >>>>> >>>>> Reported-by: Olaf Hering <olaf@aepfle.de> >>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>>> >>>> Reviewed-by: Juergen Gross <jgross@suse.com> >>> >>> Thanks. I guess I forgot >>> >>> Cc: stable@vger.kernel.org >>> >>> If you agree, can you please add this before pushing to Linus? >> >> Uh, just had a look why x86_configure_nx() was called after >> xen_setup_gdt(). >> >> Upstream your patch will be fine, but before kernel 5.9 it will >> break running as 32-bit PV guest (see commit 36104cb9012a82e7). > > Oh, indeed. That commit then actually introduced the issue here, > and hence a Fixes: tag may be warranted. Added it already. :-) And I've limited the backport to happen not for 5.8 and older, of course. Juergen
On 5/21/21 3:45 AM, Juergen Gross wrote: > On 21.05.21 09:26, Jan Beulich wrote: >> On 21.05.2021 09:18, Juergen Gross wrote: >>> On 20.05.21 14:08, Jan Beulich wrote: >>>> On 20.05.2021 13:57, Juergen Gross wrote: >>>>> On 20.05.21 13:42, Jan Beulich wrote: >>>>>> xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. >>>>>> For this to work when NX is not available, x86_configure_nx() needs > to >>>>>> be called first. >>>>>> >>>>>> Reported-by: Olaf Hering <olaf@aepfle.de> >>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>>>> >>>>> Reviewed-by: Juergen Gross <jgross@suse.com> >>>> >>>> Thanks. I guess I forgot >>>> >>>> Cc: stable@vger.kernel.org >>>> >>>> If you agree, can you please add this before pushing to Linus? >>> >>> Uh, just had a look why x86_configure_nx() was called after >>> xen_setup_gdt(). >>> >>> Upstream your patch will be fine, but before kernel 5.9 it will >>> break running as 32-bit PV guest (see commit 36104cb9012a82e7). >> >> Oh, indeed. That commit then actually introduced the issue here, >> and hence a Fixes: tag may be warranted. > > Added it already. :-) > > And I've limited the backport to happen not for 5.8 and older, of > course. Did something changed recently that this became a problem? That commit has been there for 3 years. Didn't Olaf report this to be a problem only on SLES kernels? -boris
On 21.05.2021 15:12, Boris Ostrovsky wrote: > > On 5/21/21 3:45 AM, Juergen Gross wrote: >> On 21.05.21 09:26, Jan Beulich wrote: >>> On 21.05.2021 09:18, Juergen Gross wrote: >>>> On 20.05.21 14:08, Jan Beulich wrote: >>>>> On 20.05.2021 13:57, Juergen Gross wrote: >>>>>> On 20.05.21 13:42, Jan Beulich wrote: >>>>>>> xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. >>>>>>> For this to work when NX is not available, x86_configure_nx() needs >> to >>>>>>> be called first. >>>>>>> >>>>>>> Reported-by: Olaf Hering <olaf@aepfle.de> >>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>>>>> >>>>>> Reviewed-by: Juergen Gross <jgross@suse.com> >>>>> >>>>> Thanks. I guess I forgot >>>>> >>>>> Cc: stable@vger.kernel.org >>>>> >>>>> If you agree, can you please add this before pushing to Linus? >>>> >>>> Uh, just had a look why x86_configure_nx() was called after >>>> xen_setup_gdt(). >>>> >>>> Upstream your patch will be fine, but before kernel 5.9 it will >>>> break running as 32-bit PV guest (see commit 36104cb9012a82e7). >>> >>> Oh, indeed. That commit then actually introduced the issue here, >>> and hence a Fixes: tag may be warranted. >> >> Added it already. :-) >> >> And I've limited the backport to happen not for 5.8 and older, of >> course. > > > Did something changed recently that this became a problem? That commit has been there for 3 years. He happened to try on a system where NX was turned off in the BIOS. That's not a setting you would usually find in use. > Didn't Olaf report this to be a problem only on SLES kernels? Which I assume have backports of the problematic change. Jan
On 5/21/21 9:15 AM, Jan Beulich wrote: > On 21.05.2021 15:12, Boris Ostrovsky wrote: >> >> Did something changed recently that this became a problem? That commit has been there for 3 years. > He happened to try on a system where NX was turned off in the BIOS. Yes, I missed that part. -boris
On 20.05.21 14:08, Jan Beulich wrote: > On 20.05.2021 13:57, Juergen Gross wrote: >> On 20.05.21 13:42, Jan Beulich wrote: >>> xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. >>> For this to work when NX is not available, x86_configure_nx() needs to >>> be called first. >>> >>> Reported-by: Olaf Hering <olaf@aepfle.de> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> Reviewed-by: Juergen Gross <jgross@suse.com> > > Thanks. I guess I forgot > > Cc: stable@vger.kernel.org > > If you agree, can you please add this before pushing to Linus? Yes, will do that. Juergen
On 20.05.21 13:42, Jan Beulich wrote: > xen_setup_gdt(), via xen_load_gdt_boot(), wants to adjust page tables. > For this to work when NX is not available, x86_configure_nx() needs to > be called first. > > Reported-by: Olaf Hering <olaf@aepfle.de> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Pushed to xen/tip.git for-linus-5.13b Juergen
© 2016 - 2024 Red Hat, Inc.