From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by mx.groups.io with SMTP id smtpd.web11.6384.1573143297856949553 for ; Thu, 07 Nov 2019 08:14:58 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=VvJcG+eX; spf=pass (domain: linaro.org, ip: 209.85.221.67, mailfrom: leif.lindholm@linaro.org) Received: by mail-wr1-f67.google.com with SMTP id w30so3755729wra.0 for ; Thu, 07 Nov 2019 08:14:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=xF8K+2cXV7Ec5BXG3gnUqtnjslMcHNnZjVL9Zx7LaI4=; b=VvJcG+eX14DqJBIoZWEmJIF9dZet/pCeU+Ujina9o5oZ0E6UdsPdTPlevN2vt2ESA6 wjzJ8R59HLm7yGgQOd9tHfE79bfuLPGijRlc0kXnnrLSS1Ks82JbqAAStNLOcbuRfLe3 jgPpkEZ0NtPiN32VUMSvWHfUUCGesVo+eGNGGXNqhobL8zsweevybbJSBt6/3quq9X3+ eJU0ctd78kp1uBrTZ0eS2sGfbEebvYX+KbsP0Yr86OMuoc4XT7qgAxYbjhDa5B7s/XOs 2UHpkUbqYpwABphT3OBoLnly8oBNePiY79R0MqX9E8L8qRMe9a/wglfR/earihYm7KIY h+Nw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=xF8K+2cXV7Ec5BXG3gnUqtnjslMcHNnZjVL9Zx7LaI4=; b=WLWrHHx/JmyhjRgjCZNVAOLoTtRtcJDljoxqw0OHocxMM+az1PAI5L2Hh0YPmpEUTE 2e2/1TG/2YeDs6i1SPt2KTBN5/hd/XGXE++oL5ZZyxBRt7Zg+LSqrpYxJNDHR5kf0v/S L4c4MVWM/2wjnpScgTUkjJJ8qV0H6UTXeWc/9Uecetr4b1DPhvATjdEunZzBkOmJd0+5 nde1y9F4//wATShmj1psR4AMIvtm5aOXbNy7Of0gLFKIjlly9toUWVZhdCrdbuCLNFiC mCyaxkW1O8RajPjrWvd2/JCaoISyBuVi1goTmQLgAvEMRi07Na8ibdTJNrrUgwe8LV1u uC8Q== X-Gm-Message-State: APjAAAXojI8beiHKxLEwl8EMm7tMbANOJfI9uGNfysR9mcNdmPv/PiKA O++6gNemAhVMFCZYGbD/OIQFsQ== X-Google-Smtp-Source: APXvYqxUfCGzhurxRzZwwJTaU+kqkYwRzG4ARizR+lS5Xc+8o1zf9LJahFvIH/E+ibgXi1i2jXko/w== X-Received: by 2002:a5d:4dd2:: with SMTP id f18mr3556167wru.4.1573143296376; Thu, 07 Nov 2019 08:14:56 -0800 (PST) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id u203sm2744861wme.34.2019.11.07.08.14.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Nov 2019 08:14:55 -0800 (PST) Date: Thu, 7 Nov 2019 16:14:54 +0000 From: "Leif Lindholm" To: Ard Biesheuvel Cc: edk2-devel-groups-io , "Gao, Liming" , "Cohen, Eugene" Subject: Re: [PATCH 1/1] BaseTools/GenFw AARCH64: disregard ADRP instructions that are patched already Message-ID: <20191107161454.GP16820@bivouac.eciton.net> References: <20191107090618.15813-1-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline I *think* I understand what's going on (thank you for detailed commit message and comment block). And that looks like a good workaround. Thanks! Reviewed-by: Leif Lindholm On Thu, Nov 07, 2019 at 10:09:27AM +0100, Ard Biesheuvel wrote: > (+ Leif, Liming) > > On Thu, 7 Nov 2019 at 10:06, Ard Biesheuvel wrote: > > > > In order to permit the use of compilers that only implement the small > > code model [which involves the use of ADRP instructions that require > > 4 KB segment alignment] for generating PE/COFF binaries with a small > > footprint, we patch ADRP instructions into ADR instructions while doing > > the ELF to PE/COFF conversion. > > > > As it turns out, the linker may be doing the same, but for different > > reasons: there is a silicon erratum #843419 for ARM Cortex-A53 which > > affects ADRP instructions appearing at a certain offset in memory, and > > one of the mitigations for this erratum is to patch them into ADR > > instructions at link time if the symbol reference is within -/+ 1 MB. > > However, the LD linker fails to update the static relocation tables, and > > so we end up with an ADR instruction in the fully linked binary, but > > with a relocation entry in the RELA section identifying it as an ADRP > > instruction. > > > > Since the linker has already updated the symbol reference, there is no > > handling needed in GenFw for such instructions, and we can simply treat > > it as an ordinary ADR. However, since it is guaranteed to be accompanied > > by an add or load instruction with a LO12 relocation referencing the same > > symbol, the section offset check we apply to ADR instructions is going to > > take place anyway, so we can just disregard the ADR instruction entirely. > > > > Reported-by: Eugene Cohen > > Suggested-by: Eugene Cohen > > Signed-off-by: Ard Biesheuvel > > --- > > BaseTools/Source/C/GenFw/Elf64Convert.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c > > index d574300ac4fe..d623dce1f9da 100644 > > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c > > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c > > @@ -1044,6 +1044,19 @@ WriteSections64 ( > > /* fall through */ > > > > case R_AARCH64_ADR_PREL_PG_HI21: > > + // > > + // In order to handle Cortex-A53 erratum #843419, the LD linker may > > + // convert ADRP instructions into ADR instructions, but without > > + // updating the static relocation type, and so we may end up here > > + // while the instruction in question is actually ADR. So let's > > + // just disregard it: the section offset check we apply below to > > + // ADR instructions will trigger for its R_AARCH64_xxx_ABS_LO12_NC > > + // companion instruction as well, so it is safe to omit it here. > > + // > > + if ((*(UINT32 *)Targ & BIT31) == 0) { > > + break; > > + } > > + > > // > > // AArch64 PG_H21 relocations are typically paired with ABS_LO12 > > // relocations, where a PC-relative reference with +/- 4 GB range is > > -- > > 2.17.1 > >