From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=aTvqX8sa; spf=pass (domain: linaro.org, ip: 209.85.128.68, mailfrom: ard.biesheuvel@linaro.org) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by groups.io with SMTP; Thu, 19 Sep 2019 07:44:34 -0700 Received: by mail-wm1-f68.google.com with SMTP id m18so1833049wmc.1 for ; Thu, 19 Sep 2019 07:44:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=sypsbm9EdgIEulxs1mp9UVw2rBnFfQ0VFlIPHe8VA6M=; b=aTvqX8sa1C4TX7qkYfcJZ0iiU4SyYGMvwPSR1/WYQUEsRnE0YKUBnFQIXMECEAe5b5 kKOh2c8WAGMUv1t+zfU+JLynz4b2caOj2VnML+187U1Styjpbegue+rWGbbTRL6yKkpX D+lpijzZUweAEbkmP2AUrmofkmMTui8TtdmB/+QzDxuAXK3N+bMU/08x0U01Hye/V/EC 27v0dAa+YYVO2n1x4kaGonqysXvMl83TgjqEUjmNTC5uTsC0vzxGMDRxu5K9y482z9n1 o7W0AE4ZKlBSrviDD/bfxaV6wH6cxFGkU+uKtSrqNRnjq05CkjEXvXG+2veUPZ0Jkore eSBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=sypsbm9EdgIEulxs1mp9UVw2rBnFfQ0VFlIPHe8VA6M=; b=pvrEMYomvBP/925wldYNU71o7OBTj6aVQFPd6V5K665qiipZX57xQlECIRfeKr6Uel oKW4LPYZBdVcg2p3TL/wMrm+jEfMEdV2xXdjBx8vx/52PmH7wMcFoKjorufT/U1OQJmf uwjyWFGvIyKZ8e27ORlbqP+G1ZZxEKsM8Ga3AHi9DtfQ9ez5AM/ziIJZKjZFlHbEJkjo 4xIUbn2am9KJRa5h/76kR35lTyGWdd1px7GBQ5E2n3s4VCr43HZ4mWL7RRQR7yFq2dbn huHAc+A1kOKLh0++m9ljjLd6uKgq3HU32mV4iR4JDFDmxT0GRAi+fOqi5qtdx3LxXRgJ XJog== X-Gm-Message-State: APjAAAXXScmVSVJQDhwr2Q7LKEMDpDU8lnfoPRrSMdsHJObk+UIRAnlW IM3TE11MXSgxikGgA3z/tHxJWfijr54QsGWUbEPuvA== X-Google-Smtp-Source: APXvYqzUr0A5a76/f93gdect1oxvK493jDqOBqugKrIIkKaM2yuIxfK9mpA1erc0u4H7NVm/pSH34uxN1sLU96Gfz7w= X-Received: by 2002:a1c:e906:: with SMTP id q6mr2930177wmc.136.1568904272488; Thu, 19 Sep 2019 07:44:32 -0700 (PDT) MIME-Version: 1.0 References: <20190919094846.GO28454@bivouac.eciton.net> <20190919100921.GR28454@bivouac.eciton.net> <20190919104711.GT28454@bivouac.eciton.net> <20190919112531.GU28454@bivouac.eciton.net> <20190919143154.GX28454@bivouac.eciton.net> In-Reply-To: <20190919143154.GX28454@bivouac.eciton.net> From: "Ard Biesheuvel" Date: Thu, 19 Sep 2019 17:44:04 +0300 Message-ID: Subject: Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT To: Leif Lindholm Cc: Baptiste Gerondeau , edk2-devel-groups-io , "Kinney, Michael D" , "Gao, Liming" , "Zhang, Shenglei" , Baptiste GERONDEAU Content-Type: text/plain; charset="UTF-8" On Thu, 19 Sep 2019 at 17:31, Leif Lindholm wrote: > > On Thu, Sep 19, 2019 at 03:36:50PM +0300, Ard Biesheuvel wrote: > > On Thu, 19 Sep 2019 at 14:25, Leif Lindholm wrote: > > > > The problem is that the first branch instruction is patched into the > > > > FV files by the BaseTools, and so the startup code is entered in ARM > > > > mode by default. > > > > > > > > So that means we'll either have to > > > > 1) switch to ARM mode > > > > 2) emit one branch instruction > > > > 3) switch back to Thumb mode > > > > > > I was thinking more like tying down the entry function (or as I said, > > > the whole file) as ARM, then letting the toolchain decide for the bits > > > where we don't have instuction-set dependent ABIs. > > > > > > > 4) fix up all the code so it assembles in Thumb mode > > > > > > Which is what Baptiste has done in this set. > > > > > > > 1) switch to ARM mode > > > > > > In all 48 files (+3 in edk2-platforms), or just the ones where not > > > doing so triggers build errors? Currently. > > > > > > I'm OK with restricting ourselves to just setting ARM in the > > > triggering files for simplicity, especially in order to streamline the > > > toolchain migration from RVCT to VS (and the subsequent purge of PVCT > > > support). I'm not seeing it as a solution. > > > > As long as we align the .asm files with the .S files, I am fine with > > that. And since we focus on v7+ only these days, I'm fine with > > changing existing ARM files to Thumb2. > > But I don't see us following this pattern today, for .S files. > > Frankly, looking at it, I can't quite manage to untangle how we end up > generating A32 for .asm in the first place. Is it just the toolchain > default? > > If we look for example at the post-build version of > Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.iii > , there is no .arm directive. > > That gets assembled into > Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.obj > by the step > --- > "arm-linux-gnueabihf-gcc" -march=armv7-a -c -x assembler -imacros > /work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/DEBUG/AutoGen.h > -mlittle-endian -o > /work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.obj > -I/work/git/edk2/ArmPkg/Library/ArmHvcLib/Arm > -I/work/git/edk2/ArmPkg/Library/ArmHvcLib > -I/work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/DEBUG > -I/work/git/edk2/MdePkg -I/work/git/edk2/MdePkg/Include > -I/work/git/edk2/MdePkg/Include/Arm -I/work/git/edk2/ArmPkg > -I/work/git/edk2/ArmPkg/Include /work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.iii > --- > (Adding hilarity to the situation is the bit where we, when > preprocessing the file, explicitly pass -mthumb on the command line - > which is obviously ignored at that point.) > > Fwiw, yes, it appears at least the Debian toolchain defaults to > assembling A32 if no .thumb directive is explicitly in place. > Yes, that appears to be what happens. Since we use GCC for both compiling and linker, but invoke the assembler directly (and don't pass the C flags in this case), we end up with binutils using ARM by default. Note that the BaseMemoryLibOptDxe code uses thumb explicitly, but my main concern is about the startup code we have in PrePi and PrePeiCore etc > > What I don't want is a situation where the toolchain's default decides > > how code is assembled, since that means you have to test all your code > > changes twice. > > And to me it looks like we avoid that problem today only by accident - > we are already relying on toolchain defaults for GCC. > Yup > Anyway, rewinding this about 27 steps ... > Are you OK with Baptiste going ahead and adding the ARM statement > *only* to the files requiring it to build[1] - and the one(s) requiring > it to operate properly (could you point it/them out please)? > The correctness concern applies to files called ModuleEntryPoint.S, but also to exception handling code that drops in and out of different modes, and may manipulate exception return addresses etc. If we happen to always compile that code to ARM today, I strongly suggest we keep doing that for VS as well. > [1] > Then the next question is whether he should also add it to the files > where that avoids the need for (some) refactoring of code, or whether > he should simply mirror the changes to .asm into .S? > I think all of our 32-bit ARM .S files should have an explicit .arm directive unless a .thumb directive is already in place. > The "we must tie down so that each .asm/.S file is only ever built to > a single instruction set" work is a substantial enough task, that I > hope we can undertake at a later date rather that rolling it into this > one. > See the above - I think it is fairly straight-forward.