[{"id":1772472,"web_url":"http://patchwork.ozlabs.org/comment/1772472/","msgid":"<150597752087.58302.550313766472017587@achrid.arch.suse.de>","list_archive_url":null,"date":"2017-09-21T07:05:21","subject":"Re: [U-Boot] [U-Boot,v3,14/21] efi_loader: efi variable support","submitter":{"id":1212,"url":"http://patchwork.ozlabs.org/api/people/1212/","name":"Alexander Graf","email":"agraf@suse.de"},"content":"> Add EFI variable support, mapping to u-boot environment variables.\n> Variables are pretty important for setting up boot order, among other\n> things. If the board supports saveenv, then it will be called in\n> ExitBootServices() to persist variables set by the efi payload. (For\n> example, fallback.efi configuring BootOrder and BootXXXX load-option\n> variables.)\n> \n> Variables are *not* currently exposed at runtime, post ExitBootServices.\n> On boards without a dedicated device for storage, which the loaded OS\n> is not trying to also use, this is rather tricky. One idea, at least\n> for boards that can persist RAM across reboot, is to keep a \"journal\"\n> of modified variables in RAM, and then turn halt into a reboot into\n> u-boot, plus store variables, plus halt. Whatever the solution, it\n> likely involves some per-board support.\n> \n> Mapping between EFI variables and u-boot variables:\n> \n> efi_$guid_$varname = {attributes}(type)value\n> \n> For example:\n> \n> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported=\n> \"{ro,boot,run}(blob)0000000000000000\"\n> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder=\n> \"(blob)00010000\"\n> \n> The attributes are a comma separated list of these possible\n> attributes:\n> \n> + ro - read-only\n> + boot - boot-services access\n> + run - runtime access\n> \n> NOTE: with current implementation, no variables are available after\n> ExitBootServices, and all are persisted (if possible).\n> \n> If not specified, the attributes default to \"{boot}\".\n> \n> The required type is one of:\n> \n> + utf8 - raw utf8 string\n> + blob - arbitrary length hex string\n> \n> Signed-off-by: Rob Clark \n\nThanks, applied to efi-next\n\nAlex","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xySVt03DKz9s7h\n\tfor ;\n\tThu, 21 Sep 2017 17:12:53 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid B684FC21F56; Thu, 21 Sep 2017 07:08:12 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 3F7CAC21F3F;\n\tThu, 21 Sep 2017 07:07:25 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 650B2C21F3C; Thu, 21 Sep 2017 07:05:25 +0000 (UTC)","from mx1.suse.de (mx2.suse.de [195.135.220.15])\n\tby lists.denx.de (Postfix) with ESMTPS id 4D98FC21EC7\n\tfor ; Thu, 21 Sep 2017 07:05:22 +0000 (UTC)","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 1D92EABDB;\n\tThu, 21 Sep 2017 07:05:22 +0000 (UTC)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED\n\tautolearn=unavailable autolearn_force=no version=3.4.0","X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","From":"Alexander Graf ","To":"Rob Clark ","Date":"Thu, 21 Sep 2017 09:05:21 +0200","Message-Id":"<150597752087.58302.550313766472017587@achrid.arch.suse.de>","X-Mailer":"git-send-email 2.12.3","In-Reply-To":"<20170913220546.19560-15-robdclark@gmail.com>","References":"<20170913220546.19560-15-robdclark@gmail.com>","MIME-Version":": 1.0","Cc":"U-Boot Mailing List ","Subject":"Re: [U-Boot] [U-Boot,v3,14/21] efi_loader: efi variable support","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1790898,"web_url":"http://patchwork.ozlabs.org/comment/1790898/","msgid":"<210ca741-9bed-fa49-345a-d1211623cee2@gmx.de>","list_archive_url":null,"date":"2017-10-19T20:40:47","subject":"[U-Boot] Unexpected saving of all environment variables during EFI\n\tboot","submitter":{"id":61270,"url":"http://patchwork.ozlabs.org/api/people/61270/","name":"Heinrich Schuchardt","email":"xypron.glpk@gmx.de"},"content":"This was merged as\nad644e7c18238026fecc647f62584d87d2c5b0a3\nefi_loader: efi variable support\n\n> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c\n> index ec40f41bcb..c406ff82ff 100644\n> --- a/lib/efi_loader/efi_boottime.c\n> +++ b/lib/efi_loader/efi_boottime.c\n> @@ -8,6 +8,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -942,6 +943,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,\n> {\n> \tEFI_ENTRY(\"%p, %ld\", image_handle, map_key);\n> \n> +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)\n> +\t/* save any EFI variables that have been written: */\n> +\tenv_save();\n\nYou save all changed variables and not specifically those bound to EFI.\nThis is completely unexpected behavior for the user and not covered by\nthe commit message.\n\nFurthermore you call env_save even if no EFI variable has been touched\nat all.\n\nThis leads to writing to flash on every boot via EFI. Depending on\ntechnology this might ruin the flash after a few hundred reboots.\n\nPlease, revert this part of the change.\n\nBest regards\n\nHeinrich Schuchardt","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3yJ16X0Zmrz9t5x\n\tfor ;\n\tFri, 20 Oct 2017 07:41:06 +1100 (AEDT)","by lists.denx.de (Postfix, from userid 105)\n\tid 7A7A1C21F9E; Thu, 19 Oct 2017 20:40:59 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 973A2C21DF3;\n\tThu, 19 Oct 2017 20:40:57 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 4309DC21DF3; Thu, 19 Oct 2017 20:40:56 +0000 (UTC)","from mout.gmx.net (mout.gmx.net [212.227.17.20])\n\tby lists.denx.de (Postfix) with ESMTPS id E0430C21C60\n\tfor ; Thu, 19 Oct 2017 20:40:55 +0000 (UTC)","from [192.168.123.82] ([94.114.42.150]) by mail.gmx.com (mrgmx103\n\t[212.227.17.168]) with ESMTPSA (Nemesis) id\n\t0MHH3P-1e1D9j3z1P-00E9ZX; Thu, 19 Oct 2017 22:40:53 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=FREEMAIL_FROM\n\tautolearn=unavailable autolearn_force=no version=3.4.0","To":"Rob Clark ,\n\tU-Boot Mailing List ,\n\tAlexander Graf , Tom Rini ","References":"<20170913220546.19560-1-robdclark@gmail.com>\n\t<20170913220546.19560-15-robdclark@gmail.com>","From":"Heinrich Schuchardt ","Message-ID":"<210ca741-9bed-fa49-345a-d1211623cee2@gmx.de>","Date":"Thu, 19 Oct 2017 22:40:47 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170913220546.19560-15-robdclark@gmail.com>","Content-Language":"en-US","X-Provags-ID":"V03:K0:7BTRJZFtzPfu4qRdgSB0viklolb1riECjhUcPj2k+CRT2yAs3Ty\n\tCxch+QutGjGbY9vAIB42LcSaIoYsS6UBSyc7mZ8l53goFea1QhZvqR/DvrfeM9+kY9XGpe4\n\tpEy1/2+5ISjMvKTagyB23gpquZrmmAbsWV7OlSQgrgHtjrkUegsX9KTBsDX7eV4wQMupHyT\n\truuC5d85IOE0buK7KEcBQ==","X-UI-Out-Filterresults":"notjunk:1; V01:K0:sDwBB508nmY=:DdtVwaajrsWDsWXABeYzgQ\n\tkiNbYt38HpTTpRGB2JhX1l7cSysWESaZCsPw60GQ+vbWYQCCCeACrhOZQ41NS6m7a/fpddc7f\n\t6eN6CIQylzkDdVINvBppq5AQ43NLsxPmvdIMoOSrW5MfS5d1OSbiipnB7MqS4x7mXObOBxSVz\n\tD5nJiwr5sTNuNY1rxbWfJvyLirloiNdM/iYgAqzSZC8G5U86qcS/c/hHSrCugvBB2SmDOjdj3\n\tImSIxuh++RXe3evcIY+Xbic4rBsGQnlX7DfFYLtyIMaBdu2esjdkAfHUqCrdhwF1eOfDiXziy\n\tmybY68ZdUTr64V1yNIsZRjdC3iCu5pt7XW8TN+yufx09nG5NIoAod2kGv+jbGs0hUcrdZsO0D\n\tH6KDxr8OOYtgpSdTxrIpe6UYweFuTgnV1BjXfq6EEBr/d/NNUR0KYxIGPBf5GuL9ie3t8A0rL\n\tv9olLvQngScNfJUYrb65AAwx9gS4/S0NTwM7nGwurCa/vadePMNhp02TtU03HJtZLpixejYr3\n\t6u+UnhEC3cG3NPY90YNceB+xj5qwqtjXLAeFsOR9y5lwM7PrwbWd4lMjytNTj5ij31ZelbxqQ\n\t00nMzPvY4f09flPNY7jHu86WJNFe+1SgLfVNhtXBLC8oTGEL1T5egHuhOlY8AC2bXXx6k452J\n\tAjDNjHK6ybv8fPV+Kk4KjW5/1PY9iIMj9ULm36voD6/1e7nY0Pzrnkg8tB0XYT5y/usZRfoSA\n\t8VJh5hQAjTLVOv+uspLsVOu2S5IYrkHDVkjxxCEBUkhl9f0qvvhFIhRjiegsAbxzL0yZ/Zkc3\n\tuuzE+kSyxHou3PzccGgDbtyWHXPVPTgRWziCllcbd9dhM6hKhA=","Subject":"[U-Boot] Unexpected saving of all environment variables during EFI\n\tboot","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1790923,"web_url":"http://patchwork.ozlabs.org/comment/1790923/","msgid":"","list_archive_url":null,"date":"2017-10-19T21:05:04","subject":"Re: [U-Boot] Unexpected saving of all environment variables during\n\tEFI boot","submitter":{"id":1212,"url":"http://patchwork.ozlabs.org/api/people/1212/","name":"Alexander Graf","email":"agraf@suse.de"},"content":"On 19.10.17 22:40, Heinrich Schuchardt wrote:\n> This was merged as\n> ad644e7c18238026fecc647f62584d87d2c5b0a3\n> efi_loader: efi variable support\n> \n>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c\n>> index ec40f41bcb..c406ff82ff 100644\n>> --- a/lib/efi_loader/efi_boottime.c\n>> +++ b/lib/efi_loader/efi_boottime.c\n>> @@ -8,6 +8,7 @@\n>> \n>> #include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> #include \n>> @@ -942,6 +943,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,\n>> {\n>> \tEFI_ENTRY(\"%p, %ld\", image_handle, map_key);\n>> \n>> +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)\n>> +\t/* save any EFI variables that have been written: */\n>> +\tenv_save();\n> \n> You save all changed variables and not specifically those bound to EFI.\n> This is completely unexpected behavior for the user and not covered by\n> the commit message.\n> \n> Furthermore you call env_save even if no EFI variable has been touched\n> at all.\n> \n> This leads to writing to flash on every boot via EFI. Depending on\n> technology this might ruin the flash after a few hundred reboots.\n\nI agree that overwriting flash on every boot isn't a terribly smart idea.\n\nAlso saving random environment variables that are set while we are in a\ndistro loop to persistent storage isn't great either.\n\nI agree that we should probably improve this code to only save efi\nvariables.\n\nRob, how bad would it be for Fedora if we remove the persistent variable\nstore call (just the env_save() line) for 2017.11 to not kill people's\nstorage devices? Then for the next version tackle it for real and only\nsave efi variable changes here and only if anything really did change.\n\n\nAlex","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3yJ1fN1jWnz9t5x\n\tfor ;\n\tFri, 20 Oct 2017 08:05:16 +1100 (AEDT)","by lists.denx.de (Postfix, from userid 105)\n\tid ED8CBC21F90; Thu, 19 Oct 2017 21:05:12 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id EE4EDC21DF3;\n\tThu, 19 Oct 2017 21:05:09 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 3CF04C21DF3; Thu, 19 Oct 2017 21:05:08 +0000 (UTC)","from mx2.suse.de (mx2.suse.de [195.135.220.15])\n\tby lists.denx.de (Postfix) with ESMTPS id C55EFC21C60\n\tfor ; Thu, 19 Oct 2017 21:05:07 +0000 (UTC)","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx2.suse.de (Postfix) with ESMTP id 3E91BAAF1;\n\tThu, 19 Oct 2017 21:05:07 +0000 (UTC)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=none autolearn=unavailable\n\tautolearn_force=no version=3.4.0","X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","To":"Heinrich Schuchardt , Rob Clark\n\t, U-Boot Mailing List ,\n\tTom Rini ","References":"<20170913220546.19560-1-robdclark@gmail.com>\n\t<20170913220546.19560-15-robdclark@gmail.com>\n\t<210ca741-9bed-fa49-345a-d1211623cee2@gmx.de>","From":"Alexander Graf ","Message-ID":"","Date":"Thu, 19 Oct 2017 23:05:04 +0200","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<210ca741-9bed-fa49-345a-d1211623cee2@gmx.de>","Content-Language":"en-US","Subject":"Re: [U-Boot] Unexpected saving of all environment variables during\n\tEFI boot","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1790931,"web_url":"http://patchwork.ozlabs.org/comment/1790931/","msgid":"","list_archive_url":null,"date":"2017-10-19T21:15:50","subject":"Re: [U-Boot] Unexpected saving of all environment variables during\n\tEFI boot","submitter":{"id":18760,"url":"http://patchwork.ozlabs.org/api/people/18760/","name":"Rob Clark","email":"robdclark@gmail.com"},"content":"On Thu, Oct 19, 2017 at 5:05 PM, Alexander Graf wrote:\n>\n>\n> On 19.10.17 22:40, Heinrich Schuchardt wrote:\n>> This was merged as\n>> ad644e7c18238026fecc647f62584d87d2c5b0a3\n>> efi_loader: efi variable support\n>>\n>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c\n>>> index ec40f41bcb..c406ff82ff 100644\n>>> --- a/lib/efi_loader/efi_boottime.c\n>>> +++ b/lib/efi_loader/efi_boottime.c\n>>> @@ -8,6 +8,7 @@\n>>>\n>>> #include \n>>> #include \n>>> +#include \n>>> #include \n>>> #include \n>>> #include \n>>> @@ -942,6 +943,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,\n>>> {\n>>> EFI_ENTRY(\"%p, %ld\", image_handle, map_key);\n>>>\n>>> +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)\n>>> + /* save any EFI variables that have been written: */\n>>> + env_save();\n>>\n>> You save all changed variables and not specifically those bound to EFI.\n>> This is completely unexpected behavior for the user and not covered by\n>> the commit message.\n>>\n>> Furthermore you call env_save even if no EFI variable has been touched\n>> at all.\n>>\n>> This leads to writing to flash on every boot via EFI. Depending on\n>> technology this might ruin the flash after a few hundred reboots.\n>\n> I agree that overwriting flash on every boot isn't a terribly smart idea.\n>\n> Also saving random environment variables that are set while we are in a\n> distro loop to persistent storage isn't great either.\n>\n> I agree that we should probably improve this code to only save efi\n> variables.\n>\n> Rob, how bad would it be for Fedora if we remove the persistent variable\n> store call (just the env_save() line) for 2017.11 to not kill people's\n> storage devices? Then for the next version tackle it for real and only\n> save efi variable changes here and only if anything really did change.\n>\n\nAs I mentioned on #u-boot, I think we can just comment the\nenv_save().. or perhaps make it a config option. It will be mildly\nannoying, since it means every boot is \"first boot\", ie. falling back\nto fallback.efi to populate BootOrder/BootNNNN variables, and never\nusing bootmgr. But since we can't set EFI variables from userspace\nyet, it isn't really a regression (yet).\n\nLonger term, maybe we need to split out EFI variables from u-boot env\nvars.. I thought it would be nice to store them together, since it\ngives a convenient way to set EFI variables from script or cmdline.\nBut at least a subset of the EFI vars should be persisted to reach\nEBBR, and if that breaks expectations about u-boot env vars, then I\nguess we need to split them.\n\nSide note, devices that can't repeatedly save env (even if we split\nout the EFI variables to be stored separately) might be problematic\nwith EFI_LOADER. Maybe we need a build option they can set to have a\nmore crippled version of EFI_LOADER, so as to not penalize more\ncapable devices.\n\nBR,\n-R","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"h/HJnet2\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3yJ1tl5Qz2z9t63\n\tfor ;\n\tFri, 20 Oct 2017 08:15:58 +1100 (AEDT)","by lists.denx.de (Postfix, from userid 105)\n\tid 1AE36C21F96; Thu, 19 Oct 2017 21:15:56 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id A0FB2C21DF3;\n\tThu, 19 Oct 2017 21:15:53 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 3614FC21DF3; Thu, 19 Oct 2017 21:15:52 +0000 (UTC)","from mail-lf0-f50.google.com (mail-lf0-f50.google.com\n\t[209.85.215.50])\n\tby lists.denx.de (Postfix) with ESMTPS id C3D99C21C60\n\tfor ; Thu, 19 Oct 2017 21:15:51 +0000 (UTC)","by mail-lf0-f50.google.com with SMTP id n69so11085700lfn.2\n\tfor ; Thu, 19 Oct 2017 14:15:51 -0700 (PDT)","by 10.46.127.12 with HTTP; Thu, 19 Oct 2017 14:15:50 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=FREEMAIL_FROM,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=arYrDYL5GofKMH9Hsi7fm8zIq0vPeWK5CXUO/oQZZVY=;\n\tb=h/HJnet2ViiFdWLwzb8WEwwRX+1HdVO53+7+7JsZfApSXe4A6xgQj91r/hSrlZW1rz\n\tkZ1w52h0desLWO8v1lNZwHKvYXtnsH4/FN5xck7DJeGYzh+M53TEapYKdi8IylBEWC9y\n\t8wdnrMfdNSDfL/A9x+odLM8S7UQ6QLReVks15wwfARw4HgYxxxp0xGALOThh8NlgIUcQ\n\tpnit/gPojIeCf9VH61CQlhxwuiYoY3agLR6bQgV9hNONqPZ1OC8kDD8iuQN+6pcF7n+q\n\tyB5CVPPakbkeDhU+kcIoHB2BirKl6fNozJS2XND7sieTCkdlWDHg0DgI0IM08/+KH0nm\n\taH5A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=arYrDYL5GofKMH9Hsi7fm8zIq0vPeWK5CXUO/oQZZVY=;\n\tb=m7DRLO9x2LBUbEL/zriwfYM96ccX4UT+No9oxIJ8QAJ3AVe5oVrTPhifoWUDeFGzDs\n\tXCdmL6ImigMrlmad/BqAlZJSn9TH7kdeuHoCVp7S4CRDQsDOFMXGz3zn46lqwBhUmiTS\n\tFEwJ1NcDz/Lo30HvfzNlMrIg0XLrLopT+pR3NoVNjdabaTr6STP3bbqEBIFVEzhuAQLI\n\ttMPWUDFmmuiKHjYKuSejTtECq4fpEWgu7Y7zOLWAmkru6RGAFXT0OKRZmClYg54iPefe\n\tow8pRvN+n0CMKTWewEv7ZgkkoBGW+8ncpzlMSjVqU4c3Bj2E4vYZSmXZuXhZ7AFRbNN+\n\tEt9w==","X-Gm-Message-State":"AMCzsaUOXH4vVMcMWMFEz2rvs1fHfqH4bvvkWl6Am3GXw416GEq5fbma\n\tKKHtep/BqZNHRhYHpfc73BhGlTZs+16ItNLx/sE=","X-Google-Smtp-Source":"ABhQp+TPS5FcGGxHugCZSyKDlu55t1Jn9afhDeohYBDuzxw/q3IkUt1zfaagTGC7PWiuyohiQW3v5503S0j++9aKXJs=","X-Received":"by 10.46.22.88 with SMTP id 24mr1220634ljw.20.1508447751205; Thu,\n\t19 Oct 2017 14:15:51 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"","References":"<20170913220546.19560-1-robdclark@gmail.com>\n\t<20170913220546.19560-15-robdclark@gmail.com>\n\t<210ca741-9bed-fa49-345a-d1211623cee2@gmx.de>\n\t","From":"Rob Clark ","Date":"Thu, 19 Oct 2017 17:15:50 -0400","Message-ID":"","To":"Alexander Graf ","Cc":"Heinrich Schuchardt , Tom Rini , \n\tU-Boot Mailing List ","Subject":"Re: [U-Boot] Unexpected saving of all environment variables during\n\tEFI boot","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1790941,"web_url":"http://patchwork.ozlabs.org/comment/1790941/","msgid":"","list_archive_url":null,"date":"2017-10-19T21:28:43","subject":"Re: [U-Boot] Unexpected saving of all environment variables during\n\tEFI boot","submitter":{"id":1212,"url":"http://patchwork.ozlabs.org/api/people/1212/","name":"Alexander Graf","email":"agraf@suse.de"},"content":"On 19.10.17 23:15, Rob Clark wrote:\n> On Thu, Oct 19, 2017 at 5:05 PM, Alexander Graf wrote:\n>>\n>>\n>> On 19.10.17 22:40, Heinrich Schuchardt wrote:\n>>> This was merged as\n>>> ad644e7c18238026fecc647f62584d87d2c5b0a3\n>>> efi_loader: efi variable support\n>>>\n>>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c\n>>>> index ec40f41bcb..c406ff82ff 100644\n>>>> --- a/lib/efi_loader/efi_boottime.c\n>>>> +++ b/lib/efi_loader/efi_boottime.c\n>>>> @@ -8,6 +8,7 @@\n>>>>\n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> #include \n>>>> #include \n>>>> #include \n>>>> @@ -942,6 +943,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,\n>>>> {\n>>>> EFI_ENTRY(\"%p, %ld\", image_handle, map_key);\n>>>>\n>>>> +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)\n>>>> + /* save any EFI variables that have been written: */\n>>>> + env_save();\n>>>\n>>> You save all changed variables and not specifically those bound to EFI.\n>>> This is completely unexpected behavior for the user and not covered by\n>>> the commit message.\n>>>\n>>> Furthermore you call env_save even if no EFI variable has been touched\n>>> at all.\n>>>\n>>> This leads to writing to flash on every boot via EFI. Depending on\n>>> technology this might ruin the flash after a few hundred reboots.\n>>\n>> I agree that overwriting flash on every boot isn't a terribly smart idea.\n>>\n>> Also saving random environment variables that are set while we are in a\n>> distro loop to persistent storage isn't great either.\n>>\n>> I agree that we should probably improve this code to only save efi\n>> variables.\n>>\n>> Rob, how bad would it be for Fedora if we remove the persistent variable\n>> store call (just the env_save() line) for 2017.11 to not kill people's\n>> storage devices? Then for the next version tackle it for real and only\n>> save efi variable changes here and only if anything really did change.\n>>\n> \n> As I mentioned on #u-boot, I think we can just comment the\n> env_save().. or perhaps make it a config option. It will be mildly\n> annoying, since it means every boot is \"first boot\", ie. falling back\n> to fallback.efi to populate BootOrder/BootNNNN variables, and never\n> using bootmgr. But since we can't set EFI variables from userspace\n> yet, it isn't really a regression (yet).\n\nOk, I sent a patch to remove the env_save() line.\n\n> Longer term, maybe we need to split out EFI variables from u-boot env\n> vars.. I thought it would be nice to store them together, since it\n> gives a convenient way to set EFI variables from script or cmdline.\n> But at least a subset of the EFI vars should be persisted to reach\n> EBBR, and if that breaks expectations about u-boot env vars, then I\n> guess we need to split them.\n\nI really like the sharing of the env space. We just need to have a\ndifferent write mechanism for EFI variables: Instead of writing the\ncurrent env, we could for example reread the original env, copy all\ncurrent efi variables into that and write it back?\n\n> \n> Side note, devices that can't repeatedly save env (even if we split\n> out the EFI variables to be stored separately) might be problematic\n> with EFI_LOADER. Maybe we need a build option they can set to have a\n> more crippled version of EFI_LOADER, so as to not penalize more\n> capable devices.\n\nEvery flash device self-destroys :).\n\nReally, it boils down to the amount of saves. Writing to flash on every\nboot is a pretty bad idea. If we simply only write when a variable write\nactually occurred (which in most cases won't), then we should be ok on\nall devices I think.\n\n\nAlex","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3yJ29d5Btdz9t5x\n\tfor ;\n\tFri, 20 Oct 2017 08:28:53 +1100 (AEDT)","by lists.denx.de (Postfix, from userid 105)\n\tid 7796FC21F81; Thu, 19 Oct 2017 21:28:50 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id CD218C21DF3;\n\tThu, 19 Oct 2017 21:28:47 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid A0B41C21DF3; Thu, 19 Oct 2017 21:28:46 +0000 (UTC)","from mx2.suse.de (mx2.suse.de [195.135.220.15])\n\tby lists.denx.de (Postfix) with ESMTPS id 4E6FAC21C60\n\tfor ; Thu, 19 Oct 2017 21:28:46 +0000 (UTC)","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx2.suse.de (Postfix) with ESMTP id 03B6BAC27;\n\tThu, 19 Oct 2017 21:28:46 +0000 (UTC)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=none autolearn=unavailable\n\tautolearn_force=no version=3.4.0","X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","To":"Rob Clark ","References":"<20170913220546.19560-1-robdclark@gmail.com>\n\t<20170913220546.19560-15-robdclark@gmail.com>\n\t<210ca741-9bed-fa49-345a-d1211623cee2@gmx.de>\n\t\n\t","From":"Alexander Graf ","Message-ID":"","Date":"Thu, 19 Oct 2017 23:28:43 +0200","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","Cc":"Heinrich Schuchardt , Tom Rini , \n\tU-Boot Mailing List ","Subject":"Re: [U-Boot] Unexpected saving of all environment variables during\n\tEFI boot","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1790995,"web_url":"http://patchwork.ozlabs.org/comment/1790995/","msgid":"<835b1e40-afcc-056c-f765-f71eac948944@gmx.de>","list_archive_url":null,"date":"2017-10-19T22:25:55","subject":"Re: [U-Boot] Unexpected saving of all environment variables during\n\tEFI boot","submitter":{"id":61270,"url":"http://patchwork.ozlabs.org/api/people/61270/","name":"Heinrich Schuchardt","email":"xypron.glpk@gmx.de"},"content":"On 10/19/2017 11:28 PM, Alexander Graf wrote:\n> \n> \n> On 19.10.17 23:15, Rob Clark wrote:\n>> On Thu, Oct 19, 2017 at 5:05 PM, Alexander Graf wrote:\n>>>\n>>>\n>>> On 19.10.17 22:40, Heinrich Schuchardt wrote:\n>>>> This was merged as\n>>>> ad644e7c18238026fecc647f62584d87d2c5b0a3\n>>>> efi_loader: efi variable support\n>>>>\n>>>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c\n>>>>> index ec40f41bcb..c406ff82ff 100644\n>>>>> --- a/lib/efi_loader/efi_boottime.c\n>>>>> +++ b/lib/efi_loader/efi_boottime.c\n>>>>> @@ -8,6 +8,7 @@\n>>>>>\n>>>>> #include \n>>>>> #include \n>>>>> +#include \n>>>>> #include \n>>>>> #include \n>>>>> #include \n>>>>> @@ -942,6 +943,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,\n>>>>> {\n>>>>> EFI_ENTRY(\"%p, %ld\", image_handle, map_key);\n>>>>>\n>>>>> +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)\n>>>>> + /* save any EFI variables that have been written: */\n>>>>> + env_save();\n>>>>\n>>>> You save all changed variables and not specifically those bound to EFI.\n>>>> This is completely unexpected behavior for the user and not covered by\n>>>> the commit message.\n>>>>\n>>>> Furthermore you call env_save even if no EFI variable has been touched\n>>>> at all.\n>>>>\n>>>> This leads to writing to flash on every boot via EFI. Depending on\n>>>> technology this might ruin the flash after a few hundred reboots.\n>>>\n>>> I agree that overwriting flash on every boot isn't a terribly smart idea.\n>>>\n>>> Also saving random environment variables that are set while we are in a\n>>> distro loop to persistent storage isn't great either.\n>>>\n>>> I agree that we should probably improve this code to only save efi\n>>> variables.\n>>>\n>>> Rob, how bad would it be for Fedora if we remove the persistent variable\n>>> store call (just the env_save() line) for 2017.11 to not kill people's\n>>> storage devices? Then for the next version tackle it for real and only\n>>> save efi variable changes here and only if anything really did change.\n>>>\n>>\n>> As I mentioned on #u-boot, I think we can just comment the\n>> env_save().. or perhaps make it a config option. It will be mildly\n>> annoying, since it means every boot is \"first boot\", ie. falling back\n>> to fallback.efi to populate BootOrder/BootNNNN variables, and never\n>> using bootmgr. But since we can't set EFI variables from userspace\n>> yet, it isn't really a regression (yet).\n> \n> Ok, I sent a patch to remove the env_save() line.\n\nSeems we worked in parallel :)\n\n> \n>> Longer term, maybe we need to split out EFI variables from u-boot env\n>> vars.. I thought it would be nice to store them together, since it\n>> gives a convenient way to set EFI variables from script or cmdline.\n>> But at least a subset of the EFI vars should be persisted to reach\n>> EBBR, and if that breaks expectations about u-boot env vars, then I\n>> guess we need to split them.\n> \n> I really like the sharing of the env space. We just need to have a\n> different write mechanism for EFI variables: Instead of writing the\n> current env, we could for example reread the original env, copy all\n> current efi variables into that and write it back?\n\nThat is why I said we have to add read and write methods in struct\nenv_driver.\n\n> \n>>\n>> Side note, devices that can't repeatedly save env (even if we split\n>> out the EFI variables to be stored separately) might be problematic\n>> with EFI_LOADER. Maybe we need a build option they can set to have a\n>> more crippled version of EFI_LOADER, so as to not penalize more\n>> capable devices.\n> \n> Every flash device self-destroys :).\n\nThere are also env implementations in U-Boot writing to disk:\n- env/ext4.c\n- env/fat.c\n\n> \n> Really, it boils down to the amount of saves. Writing to flash on every\n> boot is a pretty bad idea. If we simply only write when a variable write\n> actually occurred (which in most cases won't), then we should be ok on\n> all devices I think.\n\nAccording to the UEFI standard we should save the monotonic counter on\nevery boot.\n\nRegards\n\nHeinrich","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3yJ3Rl5WT8z9t7V\n\tfor ;\n\tFri, 20 Oct 2017 09:26:10 +1100 (AEDT)","by lists.denx.de (Postfix, from userid 105)\n\tid 121C9C21FB8; Thu, 19 Oct 2017 22:26:02 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 357D5C21D78;\n\tThu, 19 Oct 2017 22:26:00 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 33F05C21D78; Thu, 19 Oct 2017 22:25:59 +0000 (UTC)","from mout.gmx.net (mout.gmx.net [212.227.15.19])\n\tby lists.denx.de (Postfix) with ESMTPS id CEDE4C21C60\n\tfor ; Thu, 19 Oct 2017 22:25:58 +0000 (UTC)","from [192.168.123.82] ([94.114.42.150]) by mail.gmx.com (mrgmx001\n\t[212.227.17.190]) with ESMTPSA (Nemesis) id\n\t0McVGq-1dnmNQ2ayu-00He5X; Fri, 20 Oct 2017 00:25:56 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=FREEMAIL_FROM,\n\tRCVD_IN_MSPIKE_H2 autolearn=unavailable autolearn_force=no\n\tversion=3.4.0","To":"Alexander Graf , Rob Clark ","References":"<20170913220546.19560-1-robdclark@gmail.com>\n\t<20170913220546.19560-15-robdclark@gmail.com>\n\t<210ca741-9bed-fa49-345a-d1211623cee2@gmx.de>\n\t\n\t\n\t","From":"Heinrich Schuchardt ","Message-ID":"<835b1e40-afcc-056c-f765-f71eac948944@gmx.de>","Date":"Fri, 20 Oct 2017 00:25:55 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","X-Provags-ID":"V03:K0:JvknrHLHmSPCQFcPFGOlbb48VJ64BWxxOKAUg9B+yQrFoo//KCz\n\t296szI64675vvNHX1cBXvuUcNHIv2wxTjCogEKqCfxxL7/9w4zYorwYqNaJIFNccGJzdAVW\n\tpZPyuYTxKvuqIyLL4G+fckE5qnNt19DRa0k+C/e9gWudwP6/Uw7VUTymmH8PCPQnfIlhnLT\n\tGEZYtZ37FmUYM9dXVtWvw==","X-UI-Out-Filterresults":"notjunk:1; V01:K0:TSdoQ5EP9Ms=:yE339uNFa1/NAc+E2PLz8F\n\tXtwpUNYfbFCquDdan3TCWeRWNGKw88Yb3ds+UweSGvUn3JKyxFVmdzsBjvuaUHzu/m/nyV6B2\n\tyVq4GJwvkWmqOIYJ0gyxBFje+u2Xbfb2KnDZ2VfL6BpXlnYqTzEYc8oHNZs9ggeH8bM5UWenq\n\t2+rqrYv1HSp3sXUYUMvbPuAT9k0LCeHq6AmGhDdu6VKX32owp6kvz9JVzJ/QZbS9X7hthu/1r\n\tA5rNp7tbuxFjg5SqymL78v5nQV/oTt+dCuQVlCBU43vryNPrEsan8jLMPZQkOtAWtf26FsNsw\n\tkwF8FIjvpNz+96dkTT2HmNEjI2kvSkSb1sNV3qZM6scJvUGE0h1rRRRid8oxWGsHJ+/TaPKSB\n\tSm2O/oCZ67i3tDjYPQSFd0IQH6UvuRZeSnsv4Q280XHMy1HVZ3GcI6ZMX9Uz2WB9PXFSvtbNA\n\tZwBqellqXBm3d1wOe/8JVXjwQikfHEk56XUkKXL2k1bBHdwwRYlfBOqXH0nXrtqFxUgkqgFaG\n\tAmjwdij3pW/iKPslDz5tDDqUZHJFfPO36RMdlC4WvJlAExidFM1s1WdFfVbwOn7KE6K/x5acF\n\tS5iNzvYkSUjfHJuT8UTOtI0OJBToCjAi9iYDPiHf8WuPInoMQx5xvJhssrDGJlfXvDfsOU4NZ\n\tGK/YC3phBNk5zwfnBgCDeqUYhOwo//93ONiHTqXWn/0blqT3wzZweOLlMzRLgTzDLZPo9o1Vw\n\t6ZE2L2xYGmz94+nqsPSso7c1IfzXP2N9RKFL7w0H3dANDou6iMxGrNnKAmkgjieOD/nDnIiBB\n\txwXpdn8TH+4HwbAAJb0Tm4BjtWIZDhHJQMKaHCxWbnNDq6gTh18N2T7rHrDX5CsD9REw8bOHL\n\tQ8xqm0qk6/Zm0MjPyTvfh1DgTXnaQMIGDVEt8upMLI2IxniSk7v63+eRv37Bst","Cc":"U-Boot Mailing List , Tom Rini ","Subject":"Re: [U-Boot] Unexpected saving of all environment variables during\n\tEFI boot","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]