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::143; helo=mail-it1-x143.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x143.google.com (mail-it1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) (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 6B01E21168208 for ; Thu, 18 Oct 2018 22:01:49 -0700 (PDT) Received: by mail-it1-x143.google.com with SMTP id m15so3153165itl.4 for ; Thu, 18 Oct 2018 22:01:48 -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=tqqFFvvF8VNPIEqP24zva54ptFNL6S75arnqrxOVE8U=; b=DM7ziXt7fKo22HX1GxJB0Kboz+CS/dUoM79NfwgvrEpbim4olGFa2jDHEV7YkjahnA vIWji2qlxk90ZCqN56IdrXIbirHxGC1PwpHqqqUhCZGd+T/kcUTLs//M8M4OooJSareL gycMUOpAJCkhVOhez8bU3AWRxBCVf7pdEbp6c= 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=tqqFFvvF8VNPIEqP24zva54ptFNL6S75arnqrxOVE8U=; b=GCf/NfNZ15rj18X/EeMZLt3EXMQH5T6KcAh7x/J6rT35Q120Ms3pyvRZaCv3eJfp9m CFiEfQbJT4W8lldXDkD+pdg5APeAiW/0xnGvAatGByP7SKlB5QRVM184UALAcO7GqxM0 Va+54RqnktlX67WJMJwkxsakUg2vfhHWI50xiNcmbnoWnIpWykNCAMJvkkJigQp305WT wnr4mXqy9rqn4rIMBd6pPLSRFLsNU34dF52hEwovoJCz7A3lwSUh6Z/jdD06GIMR9BFe fDFdL7sKPPJgyAvZUHtP6FEgxP965gpi42LWIMMiJt9MOlZuKn45gr4b9eni1vOme4CK TrMQ== X-Gm-Message-State: ABuFfoi1v8Ny1CIvszye+skVWZtOCOSg4VmXYOh8ygzTRz+tjprKq6Oq kqROCkgnNEBtJ6HyhMV3OvaRYMCCOe4m/gm8d/6JcA== X-Google-Smtp-Source: ACcGV62oKK0VEf5eK5injs3fUm+NFdlUQ1xsm3NS52hp+mZtE4GcihSBXxzSG+L4MJzy07Zt3nDBSnkirXTnAwMvNUM= X-Received: by 2002:a02:4f02:: with SMTP id c2-v6mr4120799jab.2.1539925308131; Thu, 18 Oct 2018 22:01:48 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:5910:0:0:0:0:0 with HTTP; Thu, 18 Oct 2018 22:01:47 -0700 (PDT) In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BC235A6@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> <0C09AFA07DD0434D9E2A0C6AEB0483103BC235A6@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Fri, 19 Oct 2018 13:01:47 +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 05:01:49 -0000 Content-Type: text/plain; charset="UTF-8" On 19 October 2018 at 12:48, Zeng, Star wrote: > Ok, thanks and got the case. > > DxeCapsuleLibVirtualAddressChangeEvent may be too late as it could not allocate pool to do caching. > I meant registering gEfiSystemResourceTableGuid event group notification(installconfigurationtable will trigger event group) and do caching in the notification function. > > OK, I will create a bugzilla for this. > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Friday, October 19, 2018 12:43 PM > 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 > > 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 >>>>