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::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (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 5B09A21168203 for ; Thu, 18 Oct 2018 21:43:04 -0700 (PDT) Received: by mail-it1-x144.google.com with SMTP id q70-v6so2990212itb.3 for ; Thu, 18 Oct 2018 21:43:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=y9zIJENIUzDBIA+bvYZ22Ou/vYK5wPlgTDafRf0QpSQ=; b=EHwTE9+KFNzTzGiy8DmY1v5LpB5Q+2NllMak3V0KN5VjkMd17YRoALMYYLSdL2zSRs wl8mU3vntvSOJrDHUl+fKN67m7/3ORxycS8IkgcjFThfaY7u/U3ERL+7hvUwfP7cI07z wDV5o4i9uJFL82ehHKh1x3EVH3l/lwlI4PCmQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=y9zIJENIUzDBIA+bvYZ22Ou/vYK5wPlgTDafRf0QpSQ=; b=K2Hz1bBCGCao9OM9DQutvs1sKQJyzpASaiyEvZ+hl8Q3YisBQvF/YkjteKVqX+BWCg Xo8wu+x+NGI7f3J9ZGYmCpI3Tqj/QoEnCB+Ofi7GxwLYlsQdXVZ+2g84eu5cX2tJdeYb oB03DTBfoawuGNXhZ0nllFxuznxKKdVJ8a6nxjfmQqfiLaOw180FVt7yYBIqZX01ZmsO nK0FV7wHQ284q6hxqsloOYToZQBCQ6DIEPGymNTKxOzOijNvqn6zbjHvwyNDnbEt8dGM QeeAcXakcxSnnoNeiHAF+wx3f1lprz47Ib1a0Mn8PuwnCy7U57Fl/g6PbZ+tbymo5Jny pM6A== X-Gm-Message-State: ABuFfoikrKqI6/cuZVk0dgL5D1IyAHyezmEo/U73yAHJVQeVlodd578L kJNglC7MOdI93r8Qr+wp/SZl9YhvZJeZI14u41Fp/A== X-Google-Smtp-Source: ACcGV625c9E+3BjMvwD+oJEy7dxLPZnkbQqQxd1mP8H/iRrT0bwG8gLRjp2ido+2eXQ5AxMo9wIK09TwBZSkAJUeo8s= X-Received: by 2002:a24:e08f:: with SMTP id c137-v6mr2282169ith.71.1539924183308; Thu, 18 Oct 2018 21:43:03 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:5910:0:0:0:0:0 with HTTP; Thu, 18 Oct 2018 21:43:02 -0700 (PDT) In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BC2357A@shsmsx102.ccr.corp.intel.com> References: <20181019025418.3037-1-ard.biesheuvel@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103BC2352B@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103BC2357A@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Fri, 19 Oct 2018 12:43:02 +0800 Message-ID: To: "Zeng, Star" Cc: Peter Jones , "edk2-devel@lists.01.org" , "Dong, Eric" , Leif Lindholm , "Kinney, Michael D" , "Yao, Jiewen" Subject: Re: [PATCH] MdeModulePkg/EsrtDxe: allocate ESRT table from RtServicesData memory 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: Fri, 19 Oct 2018 04:43:04 -0000 Content-Type: text/plain; charset="UTF-8" On 19 October 2018 at 12:39, Zeng, Star wrote: > Ard, > > What is the real case you met that has firmware to access ESRT configuration table at runtime? > When called from the OS, UpdateCapsule() crashes when IsFmpCapsule() is invoked, because it attempts to access the ESRT. The driver uses ConvertPointer() for the address and does not check the return value, so it does not notice the failure. > I am thinking about a possible solution with current situation. > The consumer can cache ESRT configuration table by a gEfiSystemResourceTableGuid even group notification. > Yes, so you are saying DxeCapsuleLibVirtualAddressChangeEvent () in DxeCapsuleLibFmp/DxeCapsuleRuntime.c should cache the entire table rather than get the address? I suppose that should work. > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Friday, October 19, 2018 11:48 AM > To: Zeng, Star ; Peter Jones > Cc: edk2-devel@lists.01.org; Dong, Eric ; Leif Lindholm ; Kinney, Michael D ; Yao, Jiewen > Subject: Re: [PATCH] MdeModulePkg/EsrtDxe: allocate ESRT table from RtServicesData memory > > (+ Peter) > > On 19 October 2018 at 11:46, Zeng, Star wrote: >> Hi Ard, >> >> Thanks for the patch. >> >> Cc more people who knows ESRT. >> >> UEFI 2.7 chapter 23.3: >> The ESRT shall be stored in memory of type EfiBootServicesData. >> >> Seeming, we need update UEFI spec if firmware really needs access ESRT configuration table at runtime. >> >> >> Thanks, >> Star >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> Sent: Friday, October 19, 2018 11:25 AM >> To: edk2-devel@lists.01.org >> Cc: Zeng, Star ; Dong, Eric >> ; Leif Lindholm ; Ard >> Biesheuvel >> Subject: Re: [PATCH] MdeModulePkg/EsrtDxe: allocate ESRT table from >> RtServicesData memory >> >> On 19 October 2018 at 10:54, Ard Biesheuvel wrote: >>> Given that the firmware itself may access the ESRT table when the OS >>> invokes the UpdateCapsule () boot service, >> >> *runtime* service >> >>> it requires a virtual mapping >>> and so it needs to be allocated from RtServicesData memory. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ard Biesheuvel >>> --- >>> MdeModulePkg/Universal/EsrtDxe/EsrtDxe.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/MdeModulePkg/Universal/EsrtDxe/EsrtDxe.c >>> b/MdeModulePkg/Universal/EsrtDxe/EsrtDxe.c >>> index cab8d69e35ad..66266a44cec9 100644 >>> --- a/MdeModulePkg/Universal/EsrtDxe/EsrtDxe.c >>> +++ b/MdeModulePkg/Universal/EsrtDxe/EsrtDxe.c >>> @@ -577,7 +577,8 @@ EsrtReadyToBootEventNotify ( >>> goto EXIT; >>> } >>> >>> - EsrtTable = AllocatePool(sizeof(EFI_SYSTEM_RESOURCE_TABLE) + >>> NonFmpRepositorySize + FmpRepositorySize); >>> + EsrtTable = AllocateRuntimePool (sizeof(EFI_SYSTEM_RESOURCE_TABLE) + >>> + NonFmpRepositorySize + >>> + FmpRepositorySize); >>> if (EsrtTable == NULL) { >>> DEBUG ((EFI_D_ERROR, "Esrt table memory allocation failure\n")); >>> goto EXIT; >>> -- >>> 2.17.1 >>>