From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] xen/arch/x86: Fix early boot command line parsing Date: Tue, 3 Dec 2013 10:47:58 +0000 Message-ID: <529DB6DE.7040308@citrix.com> References: <1386011720-29451-1-git-send-email-daniel.kiper@oracle.com> <529D3E7A.7010800@citrix.com> <529DC1DA0200007800109468@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VnnWN-0005Aq-0R for xen-devel@lists.xenproject.org; Tue, 03 Dec 2013 10:48:03 +0000 In-Reply-To: <529DC1DA0200007800109468@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Daniel Kiper , keir@xen.org List-Id: xen-devel@lists.xenproject.org On 03/12/13 10:34, Jan Beulich wrote: >>>> On 03.12.13 at 03:14, 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 >>> --- >>> 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 80 3b 20 cmpb $0x20,(%ebx) 74 05 je 8028c2b1 80 3b 3d cmpb $0x3d,(%ebx) 75 c2 jne 8028c273 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