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=K96Ppo5e; spf=pass (domain: linaro.org, ip: 209.85.166.195, mailfrom: ard.biesheuvel@linaro.org) Received: from mail-it1-f195.google.com (mail-it1-f195.google.com [209.85.166.195]) by groups.io with SMTP; Fri, 19 Apr 2019 11:38:48 -0700 Received: by mail-it1-f195.google.com with SMTP id s3so9559333itk.1 for ; Fri, 19 Apr 2019 11:38:48 -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=jHpQjfbyVkYRFNRP09+IXMg0BSll3PGnncz2Or96tys=; b=K96Ppo5eKzHtDJmN6wVzlK4o613OPPKIj+9XqlTyOhXX9estjArDBVAlHBHbpp2rFV gz/P8NXiMY8JQMG0GRqbickbjGIqqe0UhuBnvm+Xh3rltaMulajwQsy7zCFtBx5CPYBN JqtYWZe/hgrY54IQTE96odiVGsZ2GAiJ+hmjowS2mCQU7/bbjaE51VsyciVSVjOHvbBW OS+CL97TllG2eyYBTSwj+766tZy2K63+5oCF7WKzv+n7C9jvVDpo3clZZQyw63s6MIaA 4AKlAMR/gqEm0Laz2VdSLp075r6DwOCXzmGii0cSJFvMJsBv/bj1vohCXLRhx2+CGL8U kPmQ== 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=jHpQjfbyVkYRFNRP09+IXMg0BSll3PGnncz2Or96tys=; b=MwRbyCQ4jTKlNWv/0rQ1P9OY6fM/ZB8xAP8pRRqHyc6dWJqK0FRgWv98IqjldlMbjc ma1kCAmZk2UlZ0vASjQonen9UDzhx+P8vEhFsJyMHF+oq3iligx/EnQzuKIUW8wq8gi0 3z6v9IlwrfRSUcijF63Ts9cJE5RUH7iIyJr+IIOcmgsHt8ijv54jIQv7lnCvpfxYm65v SEBLOpjwE8X8yVvtpNBIQmyZZnL+Gl+K3GcXDfCIuVxtmcSCJKBm09GWt8k5vdZDC9fb wPIPy/XK/ydjJzLGjEH8CjN0+sa9Ra3I6e4v/BaCKvR4rWJKZ16YWX2rHM0CwxOm5Uix RkEA== X-Gm-Message-State: APjAAAVA4KAIftMVDuOhNmQtDy9PL6qFrN+vi4n6HpAMpV/IMu3aJWM8 +OZc3cp58XNg9yqlDgvIYzhC2DgESz3H+m8l1QQG6Q== X-Google-Smtp-Source: APXvYqy66y1LuwUjJDW2DKWjRLj3qPxRYHTc57/BdpwJ+Y2wFkPz7zjRvryYQwat5mlEBVcd1X+krVMr0YOYQodYFyI= X-Received: by 2002:a24:59c1:: with SMTP id p184mr4333272itb.158.1555699127368; Fri, 19 Apr 2019 11:38:47 -0700 (PDT) MIME-Version: 1.0 References: <20190419141319.11084-1-ard.biesheuvel@linaro.org> In-Reply-To: From: "Ard Biesheuvel" Date: Fri, 19 Apr 2019 20:38:34 +0200 Message-ID: Subject: Re: [edk2-devel] [PATCH resend] MdeModulePkg/EsrtDxe: allocate ESRT table from RtServicesData memory To: "Kinney, Michael D" Cc: "devel@edk2.groups.io" , "ming.huang@linaro.org" , "Wu, Hao A" , "Wang, Jian J" Content-Type: text/plain; charset="UTF-8" On Fri, 19 Apr 2019 at 20:23, Kinney, Michael D wrote: > > Ard, > > Let's see if we can remove the ESRT access from those > paths. That would be the better fix. > I am not that familiar with this code, but it seems that the only reason we access the ESRT at runtime is to check whether a capsule is a nested FMP capsule, where the outer GUID is cross referenced against the ESRT before checking whether the inner GUID is the FMP guid. Could we relax this check? FMP capsule can only be dispatched across a reboot anyway, and so the runtime capsule handling could accept any capsule that has an inner FMP guid. If not, it means we do need to read the ESRT at runtime, in which case my patch is the simplest solution. We would have to clarify the spec though. > > -----Original Message----- > > From: devel@edk2.groups.io > > [mailto:devel@edk2.groups.io] On Behalf Of Ard > > Biesheuvel > > Sent: Friday, April 19, 2019 11:11 AM > > To: Kinney, Michael D > > Cc: devel@edk2.groups.io; ming.huang@linaro.org; Wu, > > Hao A ; Wang, Jian J > > > > Subject: Re: [edk2-devel] [PATCH resend] > > MdeModulePkg/EsrtDxe: allocate ESRT table from > > RtServicesData memory > > > > On Fri, 19 Apr 2019 at 20:08, Kinney, Michael D > > wrote: > > > > > > Ard, > > > > > > Where is the use of ESRT at runtime? > > > > > > > DxeCapsuleLibFmp is invoked at runtime by the > > UpdateCapsule() and > > QueryCapsuleCapabilities() code. > > > > The Canonical FWTS crashes on this runtime service on > > some ARM and x86 > > systems due to this issue, hence the patch. > > > > > > > The SetVa conversion of the ESRT table may be an old > > artifact > > > that we should consider removing. > > > > > > > No, it is definitely live code. > > > > > > > > > > > -----Original Message----- > > > > From: devel@edk2.groups.io > > > > [mailto:devel@edk2.groups.io] On Behalf Of Ard > > > > Biesheuvel > > > > Sent: Friday, April 19, 2019 10:54 AM > > > > To: edk2-devel-groups-io ; > > > > Kinney, Michael D > > > > Cc: ming.huang@linaro.org; Wu, Hao A > > > > ; Wang, Jian J > > > > > > > > Subject: Re: [edk2-devel] [PATCH resend] > > > > MdeModulePkg/EsrtDxe: allocate ESRT table from > > > > RtServicesData memory > > > > > > > > On Fri, 19 Apr 2019 at 19:42, Ard Biesheuvel > > > > wrote: > > > > > > > > > > On Fri, 19 Apr 2019 at 19:36, Michael D Kinney > > > > > wrote: > > > > > > > > > > > > Hi Ard, > > > > > > > > > > > > The UEFI Specification Section 23.3 says: > > > > > > > > > > > > "The ESRT shall be stored in memory of > > type > > > > EfiBootServicesData." > > > > > > > > > > > > If an RT driver needs ESRT info after > > > > ExitBootServices(), then the > > > > > > RT driver should collect that information > > before > > > > ExitBootServices(). > > > > > > > > > > > > > > > > But that means that before EBS(), we will have to > > > > make a copy of the > > > > > ESRT into runtime services data, and we will end > > up > > > > using twice as > > > > > much memory - one copy in boot services data for > > the > > > > OS, and one copy > > > > > in runtime services data for the firmware itself. > > I > > > > understand the > > > > > reasoning behind preferring boot over runtime, so > > > > that the OS is not > > > > > burdened with it if it does not care. But it this > > > > case, the RT > > > > > services data will be allocated regardless, and > > so we > > > > end up using > > > > > less memory if we expose that same copy to the > > OS. > > > > > > > > > > > > > Actually, DxeCapsuleLibVirtualAddressChangeEvent() > > in > > > > DxeCapsuleLibFmp > > > > updates the ESRT pointer to virtual, so this code > > is > > > > clearly based on > > > > the assumption that the ESRT resides in memory that > > is > > > > covered by a > > > > runtime VA mapping. > > > > > > > > Could we instead clarify the UEFI spec to something > > > > like > > > > > > > > "The ESRT shall be stored in memory of type > > > > EfiBootServicesData unless > > > > the runtime services implementations themselves > > need to > > > > access it at > > > > OS runtime, in which case it may be stored in > > > > EfiRuntimeServicesData." > > > > > > > > > > > > > > > >