From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::142; helo=mail-it1-x142.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x142.google.com (mail-it1-x142.google.com [IPv6:2607:f8b0:4864:20::142]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6D583201B042E for ; Wed, 20 Feb 2019 00:59:47 -0800 (PST) Received: by mail-it1-x142.google.com with SMTP id i2so13718475ite.5 for ; Wed, 20 Feb 2019 00:59:47 -0800 (PST) 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=tZBieDg7h4YbE4fCm2+xXwIYMpR384TcOnB5sJciQNk=; b=SRBp3DPLUYHxDCdVIQRWo11mAWhvtRrEelCQGRUsKrPpO0oLu4chIuOIR+ncNYwQlg Z56KT0lseYWZ0P6V8i/tm96YAFvr0FGJyd2McL0y2GQI69+1lER0V7vsNjF0eyIU6+rq +Ie0uQGXsC1by73uUBLNedSSiD5kysOSA3EyhjXSu+OLVWvnmPP2KYbTPH6E2QsqqEzS 1yEFuc/rCuhh4EixpUZUeJlSwkb9l9O+LSbxbc4764eAju/Lk/DH65It1DJFSezu3NqZ dNSEYRUBdYKe3RMKXeJvUGoRmLUtKebf3xxsBAbnaPs8NTX096vZPABlU8w6ukfk+UYz tPxA== 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=tZBieDg7h4YbE4fCm2+xXwIYMpR384TcOnB5sJciQNk=; b=I6vfHnO9EH7zx7GGqMKYP7DQ94OKCuUzEUZqqA9sYNlJSq3TYqhSPwulNb+betBL40 cPiZXRaifv5tGvZ3UvVMT3hWUncOBt7QAOj80vHlxK2Vj6xcCFamuJZDrqxXIQQGsBFV LLJy+Pe8MD4HBF5e/MZzs0//tspwJS2yvGkGHqqWvWFEzJ0lHjL4/bbYXhUNH7VOuzg9 dEkZYcMJKpzj0DVZdgWEjq1vmzsJGr44aH3j76TH81JM+AFJ4bsfdTu4u66NOb3Q5tr3 KwmvoGkTPsbxgdCqvJYEoWCs37A5CeWht5FWDYT9B9WfxaClyXkjWA/c4vQagrMpGVdq OaQQ== X-Gm-Message-State: AHQUAuZfXq6B8tXw4UfyZH3Sla88DplpXFj88c0dLQFYSpZM5SlYTSuI bb22XteVwtT4egOP8dbs5Xx9Usinxx1YwcnDr9WUIDtv X-Google-Smtp-Source: AHgI3IZ/XKjpA8hK7vbn7kDnEZU+0QG+iRc+wbwtTmAX8nbFgEGC33cJdReHqz7wJ2GLRfQuQY+dRlKsj4LFceVuxyY= X-Received: by 2002:a5e:9704:: with SMTP id w4mr21109260ioj.60.1550653186412; Wed, 20 Feb 2019 00:59:46 -0800 (PST) MIME-Version: 1.0 References: <20190218041141.21363-1-jordan.l.justen@intel.com> <20190218041141.21363-7-jordan.l.justen@intel.com> <155048090465.22654.1079629797155553207@jljusten-skl> <155065273839.12518.3314562596426152677@jljusten-skl> In-Reply-To: <155065273839.12518.3314562596426152677@jljusten-skl> From: Ard Biesheuvel Date: Wed, 20 Feb 2019 09:59:35 +0100 Message-ID: To: Jordan Justen Cc: "edk2-devel@lists.01.org" , Laszlo Ersek , Anthony Perard , Julien Grall Subject: Re: [PATCH 06/10] OvmfPkg/Sec: Disable optimizations for TemporaryRamMigration X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2019 08:59:47 -0000 Content-Type: text/plain; charset="UTF-8" On Wed, 20 Feb 2019 at 09:52, Jordan Justen wrote: > > On 2019-02-18 01:32:53, Ard Biesheuvel wrote: > > On Mon, 18 Feb 2019 at 10:08, Jordan Justen wrote: > > > > > > On 2019-02-17 23:53:01, Ard Biesheuvel wrote: > > > > On Mon, 18 Feb 2019 at 05:12, Jordan Justen wrote: > > > > > > > > > > > > > This needs an explanation why optimization needs to be disabled. > > > > > > I'm not sure this is required. The reason I added these patches is to > > > hopefully prevent the compiler from removing the frame pointer. We > > > adjust the frame pointer in the code, and that is a little sketchy if > > > the frame pointer isn't being used. > > > > > > Unfortunately, it can reasonably be argued that the > > > TemporaryRamSupport PPI definition ultimately makes it unsafe to write > > > the migration code in C. > > > > > > I tried reverting both the EmulatorPkg and OvmfPkg patches for > > > disabling the optimizations, and with my setup there was no impact. I > > > think there is a good change that we'd be pretty safe to just drop > > > these two patches to wait and see if someone encounters a situation > > > that requires it. > > > > > > Ok, so based on this explanation, do you think I should add info to > > > the commit message and keep the patches, or just drop them? > > > > > > > I think 'little sketchy' is an understatement here (as is > > setjmp/longjmp in general), but it is the reality we have to deal with > > when writing startup code in C. Looking at the code, I agree that the > > fact that [re]bp is assigned directly implies that we should not > > permit it to be used as a general purpose register, especially when > > you throw LTO into the mix, which could produce all kinds of > > surprising results when it decides to inline functions being called > > from here. > > > > For GCC/Clang, I don't think it is correct to assume that changing the > > optimization level will result in -fno-omit-frame-pointer to be set, > > so I'd prefer setting that option directly, either via the pragma, or > > for the whole file. > > Based on: https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html > > It appears that -O0 will not have -fomit-frame-pointer, since that is > added in -O1. > For current versions of GCC, perhaps. But what about older versions? What about future versions? What about Clang? > For both gcc and MSVC, I think we could be more targeted: > > #ifdef __GNUC__ > #pragma GCC push_options > #pragma GCC optimize ("no-omit-frame-pointer") > #else > #pragma optimize ("y", off) > #endif > > Do you prefer this version? > Assuming that "y" affects frame pointer generation, yes.