Daniel Kiper
2013-Dec-02 19:15 UTC
[PATCH] xen/arch/x86: Fix early boot command line parsing
There is no reliable way to encode nul character as a character so encode
it as a number. Read: http://sourceware.org/binutils/docs/as/Characters.html.
Octal and hex encoding does not work on at least my system (GNU assembler
version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian)
2.22).
Without this fix e.g. no-real-mode option at the end of xen.gz command line
is not detected. Additionally, encode other characters accordingly to
the gas documentation.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/arch/x86/boot/cmdline.S | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S
index 05ffb94..4ea56b3 100644
--- a/xen/arch/x86/boot/cmdline.S
+++ b/xen/arch/x86/boot/cmdline.S
@@ -138,11 +138,11 @@
call .Lstrlen
add $4,%esp
xadd %eax,%ebx
- cmpb $''\0'',(%ebx)
+ cmpb $0,(%ebx)
je 3f
- cmpb $'' '',(%ebx)
+ cmpb $'' ,(%ebx)
je 3f
- cmpb $''='',(%ebx)
+ cmpb $''=,(%ebx)
jne 1b
3: pop %ebx
ret
--
1.7.10.4
Andrew Cooper
2013-Dec-03 02:14 UTC
Re: [PATCH] xen/arch/x86: Fix early boot command line parsing
On 02/12/2013 19:15, Daniel Kiper wrote:> There is no reliable way to encode nul character as a character so encode > it as a number. Read: http://sourceware.org/binutils/docs/as/Characters.html. > Octal and hex encoding does not work on at least my system (GNU assembler > version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian) 2.22). > Without this fix e.g. no-real-mode option at the end of xen.gz command line > is not detected. Additionally, encode other characters accordingly to > the gas documentation. > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > --- > xen/arch/x86/boot/cmdline.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S > index 05ffb94..4ea56b3 100644 > --- a/xen/arch/x86/boot/cmdline.S > +++ b/xen/arch/x86/boot/cmdline.S > @@ -138,11 +138,11 @@ > call .Lstrlen > add $4,%esp > xadd %eax,%ebx > - cmpb $''\0'',(%ebx) > + cmpb $0,(%ebx) > je 3f > - cmpb $'' '',(%ebx) > + cmpb $'' ,(%ebx) > je 3f > - cmpb $''='',(%ebx) > + cmpb $''=,(%ebx) > jne 1b > 3: pop %ebx > retMy version of binutils 2.22 for Debian Wheezy does perfectly well with these constants as-are. What does the disassembly from your toolchain look like? I find it quite hard to believe that the above syntax has been broken by a recent version of binutils. ~Andrew
Jan Beulich
2013-Dec-03 10:34 UTC
Re: [PATCH] xen/arch/x86: Fix early boot command line parsing
>>> On 03.12.13 at 03:14, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > On 02/12/2013 19:15, Daniel Kiper wrote: >> There is no reliable way to encode nul character as a character so encode >> it as a number. Read: > http://sourceware.org/binutils/docs/as/Characters.html. >> Octal and hex encoding does not work on at least my system (GNU assembler >> version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian) > 2.22). >> Without this fix e.g. no-real-mode option at the end of xen.gz command line >> is not detected. Additionally, encode other characters accordingly to >> the gas documentation. >> >> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> >> --- >> xen/arch/x86/boot/cmdline.S | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S >> index 05ffb94..4ea56b3 100644 >> --- a/xen/arch/x86/boot/cmdline.S >> +++ b/xen/arch/x86/boot/cmdline.S >> @@ -138,11 +138,11 @@ >> call .Lstrlen >> add $4,%esp >> xadd %eax,%ebx >> - cmpb $''\0'',(%ebx) >> + cmpb $0,(%ebx) >> je 3f >> - cmpb $'' '',(%ebx) >> + cmpb $'' ,(%ebx) >> je 3f >> - cmpb $''='',(%ebx) >> + cmpb $''=,(%ebx) >> jne 1b >> 3: pop %ebx >> ret > > My version of binutils 2.22 for Debian Wheezy does perfectly well with > these constants as-are.Does it? The ''\0'' is consistently being translated as ''0'' rather than the intended NIL for every gas from 2.18 to 2.23.2. I''m not eager to take the other two adjustments though - the code is better readable/understandable the with a closing quote in place. Jan
Andrew Cooper
2013-Dec-03 10:47 UTC
Re: [PATCH] xen/arch/x86: Fix early boot command line parsing
On 03/12/13 10:34, Jan Beulich wrote:>>>> On 03.12.13 at 03:14, Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> On 02/12/2013 19:15, Daniel Kiper wrote: >>> There is no reliable way to encode nul character as a character so encode >>> it as a number. Read: >> http://sourceware.org/binutils/docs/as/Characters.html. >>> Octal and hex encoding does not work on at least my system (GNU assembler >>> version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian) >> 2.22). >>> Without this fix e.g. no-real-mode option at the end of xen.gz command line >>> is not detected. Additionally, encode other characters accordingly to >>> the gas documentation. >>> >>> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> >>> --- >>> xen/arch/x86/boot/cmdline.S | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S >>> index 05ffb94..4ea56b3 100644 >>> --- a/xen/arch/x86/boot/cmdline.S >>> +++ b/xen/arch/x86/boot/cmdline.S >>> @@ -138,11 +138,11 @@ >>> call .Lstrlen >>> add $4,%esp >>> xadd %eax,%ebx >>> - cmpb $''\0'',(%ebx) >>> + cmpb $0,(%ebx) >>> je 3f >>> - cmpb $'' '',(%ebx) >>> + cmpb $'' ,(%ebx) >>> je 3f >>> - cmpb $''='',(%ebx) >>> + cmpb $''=,(%ebx) >>> jne 1b >>> 3: pop %ebx >>> ret >> My version of binutils 2.22 for Debian Wheezy does perfectly well with >> these constants as-are. > Does it? The ''\0'' is consistently being translated as ''0'' rather than > the intended NIL for every gas from 2.18 to 2.23.2. > > I''m not eager to take the other two adjustments though - the > code is better readable/understandable the with a closing quote > in place. > > Jan >Ah - I missed the qualification of "at the end of the command line" - my appologies. My "perfectly well" was based on my recent use of "no-real-mode" working fine, but it wasn''t at the end of the command line. Looking at the current disassembly, 0f c1 c3 xadd %eax,%ebx 80 3b 30 cmpb $0x30,(%ebx) 74 0a je 8028c2b1 <idx+0x8028c0b1> 80 3b 20 cmpb $0x20,(%ebx) 74 05 je 8028c2b1 <idx+0x8028c0b1> 80 3b 3d cmpb $0x3d,(%ebx) 75 c2 jne 8028c273 <idx+0x8028c073> 5b pop %ebx c3 ret The NULL check is indeed being mistranslated, but the space and = are fine. I would suggest switching it to $0 and leaving a line comment indicating that this is a null check? It is sad that ''\0'' doesn''t work, as it is far more informative of the intent of the code. ~Andrew
Daniel Kiper
2013-Dec-03 10:59 UTC
Re: [PATCH] xen/arch/x86: Fix early boot command line parsing
On Tue, Dec 03, 2013 at 02:14:18AM +0000, Andrew Cooper wrote:> On 02/12/2013 19:15, Daniel Kiper wrote: > > There is no reliable way to encode nul character as a character so encode > > it as a number. Read: http://sourceware.org/binutils/docs/as/Characters.html. > > Octal and hex encoding does not work on at least my system (GNU assembler > > version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian) 2.22). > > Without this fix e.g. no-real-mode option at the end of xen.gz command line > > is not detected. Additionally, encode other characters accordingly to > > the gas documentation. > > > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > > --- > > xen/arch/x86/boot/cmdline.S | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S > > index 05ffb94..4ea56b3 100644 > > --- a/xen/arch/x86/boot/cmdline.S > > +++ b/xen/arch/x86/boot/cmdline.S > > @@ -138,11 +138,11 @@ > > call .Lstrlen > > add $4,%esp > > xadd %eax,%ebx > > - cmpb $''\0'',(%ebx) > > + cmpb $0,(%ebx) > > je 3f > > - cmpb $'' '',(%ebx) > > + cmpb $'' ,(%ebx) > > je 3f > > - cmpb $''='',(%ebx) > > + cmpb $''=,(%ebx) > > jne 1b > > 3: pop %ebx > > ret > > My version of binutils 2.22 for Debian Wheezy does perfectly well with > these constants as-are. > > What does the disassembly from your toolchain look like? I find itobjdump -dm i386 xen/arch/x86/boot/head.o [...] 29f: 0f c1 c3 xadd %eax,%ebx 2a2: 80 3b 30 cmpb $0x30,(%ebx) ^^^^^ == ''0'' != NUL 2a5: 74 0a je 2b1 <__start+0x246> 2a7: 80 3b 20 cmpb $0x20,(%ebx) 2aa: 74 05 je 2b1 <__start+0x246> 2ac: 80 3b 3d cmpb $0x3d,(%ebx) 2af: 75 c2 jne 273 <__start+0x208>> quite hard to believe that the above syntax has been broken by a recent > version of binutils.I see the same issue on older machines too. If you show me how to encode NUL in gas assembly file I am happy to do that. Current notation does not work. Please test with no-real-mode (or other option checked at early boot stage) at the end of Xen boot command line. It is completely ignored. Once I found description of that issue somewhere but I do not remember where. Later I was hurt by it during EFI work. Daniel