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);
[-- Attachment #1.1.1: Type: text/plain, Size: 367 bytes --] 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 [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --]
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
[-- Attachment #1.1.1: Type: text/plain, Size: 630 bytes --] 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 [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --]
[-- Attachment #1.1.1: Type: text/plain, Size: 1030 bytes --] 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 [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --]
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
[-- Attachment #1.1.1: Type: text/plain, Size: 1172 bytes --] 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 [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --]
[-- Attachment #1.1.1: Type: text/plain, Size: 362 bytes --] 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 [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --]
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