Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
Use explicit value of the entry instead of using GDT_ENTRY() macro.
Signed-off-by: Boris Ostrovsky boris.ostrovsky@oracle.com Cc: stable@vger.kernel.org --- arch/x86/xen/xen-pvh.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S index e1a5fbe..934f7d4 100644 --- a/arch/x86/xen/xen-pvh.S +++ b/arch/x86/xen/xen-pvh.S @@ -145,11 +145,11 @@ gdt_start: .quad 0x0000000000000000 /* NULL descriptor */ .quad 0x0000000000000000 /* reserved */ #ifdef CONFIG_X86_64 - .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* __KERNEL_CS */ + .quad 0x00af9a000000ffff /* __BOOT_CS */ #else - .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */ + .quad 0x00cf9a000000ffff /* __BOOT_CS */ #endif - .quad GDT_ENTRY(0xc092, 0, 0xfffff) /* __KERNEL_DS */ + .quad 0x00cf92000000ffff /* __BOOT_DS */ gdt_end:
.balign 4
On Mon, Apr 30, 2018 at 12:23:36PM -0400, Boris Ostrovsky wrote:
Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
Use explicit value of the entry instead of using GDT_ENTRY() macro.
Signed-off-by: Boris Ostrovsky boris.ostrovsky@oracle.com Cc: stable@vger.kernel.org
arch/x86/xen/xen-pvh.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S index e1a5fbe..934f7d4 100644 --- a/arch/x86/xen/xen-pvh.S +++ b/arch/x86/xen/xen-pvh.S @@ -145,11 +145,11 @@ gdt_start: .quad 0x0000000000000000 /* NULL descriptor */ .quad 0x0000000000000000 /* reserved */ #ifdef CONFIG_X86_64
- .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00af9a000000ffff /* __BOOT_CS */
#else
- .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00cf9a000000ffff /* __BOOT_CS */
Maybe it would be cleaner to use something like:
.word 0xffff /* limit */ .word 0 /* base */ .byte 0 /* base */ .byte 0x9a /* access */ #ifdef CONFIG_X86_64 .byte 0xaf /* flags plus limit */ #else .byte 0xcf /* flags plus limit */ #endif .byte 0 /* base */
Or try to fix the GDT_ENTRY macro, maybe if you prepend extra 0's to make the values 64bit that would prevent the warnings?
Or declare the GDT in enlighten_pvh in C and use it here?
Also, IIRC the base/limit values are ignored in long mode.
Thanks, Roger.
On 04/30/2018 12:57 PM, Roger Pau Monné wrote:
On Mon, Apr 30, 2018 at 12:23:36PM -0400, Boris Ostrovsky wrote:
Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
Use explicit value of the entry instead of using GDT_ENTRY() macro.
Signed-off-by: Boris Ostrovsky boris.ostrovsky@oracle.com Cc: stable@vger.kernel.org
arch/x86/xen/xen-pvh.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S index e1a5fbe..934f7d4 100644 --- a/arch/x86/xen/xen-pvh.S +++ b/arch/x86/xen/xen-pvh.S @@ -145,11 +145,11 @@ gdt_start: .quad 0x0000000000000000 /* NULL descriptor */ .quad 0x0000000000000000 /* reserved */ #ifdef CONFIG_X86_64
- .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00af9a000000ffff /* __BOOT_CS */
#else
- .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00cf9a000000ffff /* __BOOT_CS */
Maybe it would be cleaner to use something like:
I actually considered all of these and ended up with a raw number because it seems to be a convention in kernel (and Xen too, apparently) to use raw values in .S files.
Kernel is using now GDT_ENTRY_INIT() which is a C macro. There is one other location where GDT_INIT() is used (arch/x86/boot/pm.c) and, incidentally, it also generates this warning IIRC.
I really don't want to move definition to C code just to use a macro --- I don't think C code needs to be exposed to this GDT.
.word 0xffff /* limit */ .word 0 /* base */ .byte 0 /* base */ .byte 0x9a /* access */ #ifdef CONFIG_X86_64 .byte 0xaf /* flags plus limit */ #else .byte 0xcf /* flags plus limit */ #endif .byte 0 /* base */
I, in fact, started with something like this. But if you repeat this 4 times you will probably see why I decided against it ;-)
-boris
Or try to fix the GDT_ENTRY macro, maybe if you prepend extra 0's to make the values 64bit that would prevent the warnings?
Or declare the GDT in enlighten_pvh in C and use it here?
Also, IIRC the base/limit values are ignored in long mode.
Thanks, Roger.
On Mon, Apr 30, 2018 at 02:07:43PM -0400, Boris Ostrovsky wrote:
On 04/30/2018 12:57 PM, Roger Pau Monné wrote:
On Mon, Apr 30, 2018 at 12:23:36PM -0400, Boris Ostrovsky wrote:
Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
Use explicit value of the entry instead of using GDT_ENTRY() macro.
Signed-off-by: Boris Ostrovsky boris.ostrovsky@oracle.com Cc: stable@vger.kernel.org
arch/x86/xen/xen-pvh.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S index e1a5fbe..934f7d4 100644 --- a/arch/x86/xen/xen-pvh.S +++ b/arch/x86/xen/xen-pvh.S @@ -145,11 +145,11 @@ gdt_start: .quad 0x0000000000000000 /* NULL descriptor */ .quad 0x0000000000000000 /* reserved */ #ifdef CONFIG_X86_64
- .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00af9a000000ffff /* __BOOT_CS */
#else
- .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00cf9a000000ffff /* __BOOT_CS */
Maybe it would be cleaner to use something like:
I actually considered all of these and ended up with a raw number because it seems to be a convention in kernel (and Xen too, apparently) to use raw values in .S files.
Kernel is using now GDT_ENTRY_INIT() which is a C macro. There is one other location where GDT_INIT() is used (arch/x86/boot/pm.c) and, incidentally, it also generates this warning IIRC.
I really don't want to move definition to C code just to use a macro --- I don't think C code needs to be exposed to this GDT.
.word 0xffff /* limit */ .word 0 /* base */ .byte 0 /* base */ .byte 0x9a /* access */ #ifdef CONFIG_X86_64 .byte 0xaf /* flags plus limit */ #else .byte 0xcf /* flags plus limit */ #endif .byte 0 /* base */
I, in fact, started with something like this. But if you repeat this 4 times you will probably see why I decided against it ;-)
Heh, right. Maybe a .macro to generate those? Or this is all too much for just a couple of GDT entries anyway...
For long mode however you could use simpler values, AFAICT the code segment in long mode could be simplified to:
0x00209a0000000000
Because the base/limit have no effect.
In any case I'm not specially inclined either way, and maybe using similar values for 32 and 64bit modes makes this easier to understand (and decode if needed).
Roger.
On 05/01/2018 03:53 AM, Roger Pau Monné wrote:
On Mon, Apr 30, 2018 at 02:07:43PM -0400, Boris Ostrovsky wrote:
On 04/30/2018 12:57 PM, Roger Pau Monné wrote:
On Mon, Apr 30, 2018 at 12:23:36PM -0400, Boris Ostrovsky wrote:
Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
Use explicit value of the entry instead of using GDT_ENTRY() macro.
Signed-off-by: Boris Ostrovsky boris.ostrovsky@oracle.com Cc: stable@vger.kernel.org
arch/x86/xen/xen-pvh.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S index e1a5fbe..934f7d4 100644 --- a/arch/x86/xen/xen-pvh.S +++ b/arch/x86/xen/xen-pvh.S @@ -145,11 +145,11 @@ gdt_start: .quad 0x0000000000000000 /* NULL descriptor */ .quad 0x0000000000000000 /* reserved */ #ifdef CONFIG_X86_64
- .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00af9a000000ffff /* __BOOT_CS */ #else
- .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00cf9a000000ffff /* __BOOT_CS */
Maybe it would be cleaner to use something like:
I actually considered all of these and ended up with a raw number because it seems to be a convention in kernel (and Xen too, apparently) to use raw values in .S files.
Kernel is using now GDT_ENTRY_INIT() which is a C macro. There is one other location where GDT_INIT() is used (arch/x86/boot/pm.c) and, incidentally, it also generates this warning IIRC.
I really don't want to move definition to C code just to use a macro --- I don't think C code needs to be exposed to this GDT.
.word 0xffff /* limit */ .word 0 /* base */ .byte 0 /* base */ .byte 0x9a /* access */ #ifdef CONFIG_X86_64 .byte 0xaf /* flags plus limit */ #else .byte 0xcf /* flags plus limit */ #endif .byte 0 /* base */
I, in fact, started with something like this. But if you repeat this 4 times you will probably see why I decided against it ;-)
Heh, right. Maybe a .macro to generate those? Or this is all too much for just a couple of GDT entries anyway...
That's what I thought. Especially given that assembly code seems to be using raw values.
For long mode however you could use simpler values, AFAICT the code segment in long mode could be simplified to:
0x00209a0000000000
Because the base/limit have no effect.
True. However, we are sharing the DS (and later GS) descriptors between 32- and 64-it modes. I can separate them if you think it makes sense.
-boris
In any case I'm not specially inclined either way, and maybe using similar values for 32 and 64bit modes makes this easier to understand (and decode if needed).
Roger.
From: Boris Ostrovsky
Sent: 30 April 2018 17:24 To: linux-kernel@vger.kernel.org; xen-devel@lists.xenproject.org Cc: jgross@suse.com; Boris Ostrovsky; stable@vger.kernel.org Subject: [PATCH 1/4] xen/PVH: Replace GDT_ENTRY with explicit constant
Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
Use explicit value of the entry instead of using GDT_ENTRY() macro.
...
#ifdef CONFIG_X86_64
- .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00af9a000000ffff /* __BOOT_CS */
#else
- .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00cf9a000000ffff /* __BOOT_CS */
#endif
- .quad GDT_ENTRY(0xc092, 0, 0xfffff) /* __KERNEL_DS */
- .quad 0x00cf92000000ffff /* __BOOT_DS */
gdt_end:
It has to be possible to fix the GDT_ENTRY() macro. Even if you end up with one that generates two 32bit values.
You've also changed the name in the comments.
David
On 05/01/2018 07:31 AM, David Laight wrote:
From: Boris Ostrovsky
Sent: 30 April 2018 17:24 To: linux-kernel@vger.kernel.org; xen-devel@lists.xenproject.org Cc: jgross@suse.com; Boris Ostrovsky; stable@vger.kernel.org Subject: [PATCH 1/4] xen/PVH: Replace GDT_ENTRY with explicit constant
Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
Use explicit value of the entry instead of using GDT_ENTRY() macro.
...
#ifdef CONFIG_X86_64
- .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00af9a000000ffff /* __BOOT_CS */ #else
- .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */
- .quad 0x00cf9a000000ffff /* __BOOT_CS */ #endif
- .quad GDT_ENTRY(0xc092, 0, 0xfffff) /* __KERNEL_DS */
- .quad 0x00cf92000000ffff /* __BOOT_DS */ gdt_end:
It has to be possible to fix the GDT_ENTRY() macro. Even if you end up with one that generates two 32bit values.
Is it worth it though? We seem to be using GDT_ENTRY_INIT() everywhere and the only other reference that I see is in pm.c and it also probably ought to use GDT_ENTRY_INIT().
You've also changed the name in the comments.
Yes, I should mention this in the commit message.
-boris
On 30.04.18 at 18:23, boris.ostrovsky@oracle.com wrote:
Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
I think this is a mis-configured binutils build - even if targeting 32-bit only, it should allow 64-bit arithmetic (i.e. be configured with --enable-64-bit-bfd). Note how, for example, this
.long 1 << 32, 0x10000 * 0x10000 .quad 1 << 32, 0x10000 * 0x10000
assembles consistently with a warning on _both_ values on the first line with what I'd call a properly configured binutils build, but errors only on the shift expressions on each line for an (imo) improperly configured one. The only viable alternative would imo be to simply disallow .quad without --enable-64-bit-bfd, but I guess that would break a number of consumers.
In any event I'd like to suggest to drop this patch.
Jan
On 05/02/2018 04:00 AM, Jan Beulich wrote:
On 30.04.18 at 18:23, boris.ostrovsky@oracle.com wrote:
Latest binutils release (2.29.1) will no longer allow proper computation of GDT entries on 32-bits, with warning:
arch/x86/xen/xen-pvh.S: Assembler messages: arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:150: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (40 is not between 0 and 31) arch/x86/xen/xen-pvh.S:152: Warning: shift count out of range (32 is not between 0 and 31)
I think this is a mis-configured binutils build - even if targeting 32-bit only, it should allow 64-bit arithmetic (i.e. be configured with --enable-64-bit-bfd). Note how, for example, this
.long 1 << 32, 0x10000 * 0x10000 .quad 1 << 32, 0x10000 * 0x10000
assembles consistently with a warning on _both_ values on the first line with what I'd call a properly configured binutils build, but errors only on the shift expressions on each line for an (imo) improperly configured one. The only viable alternative would imo be to simply disallow .quad without --enable-64-bit-bfd, but I guess that would break a number of consumers.
In any event I'd like to suggest to drop this patch.
Let me see go back and see how I built my binutils. And maybe rebuild them on something > fedora13.
-boris
linux-stable-mirror@lists.linaro.org
-
Boris Ostrovsky -
David Laight -
Jan Beulich -
Roger Pau Monné