On 19 October 2018 at 12:39, Zeng, Star <[email protected]> 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:[email protected]]
> Sent: Friday, October 19, 2018 11:48 AM
> To: Zeng, Star <[email protected]>; Peter Jones <[email protected]>
> Cc: [email protected]; Dong, Eric <[email protected]>; Leif Lindholm 
> <[email protected]>; Kinney, Michael D <[email protected]>; 
> Yao, Jiewen <[email protected]>
> Subject: Re: [PATCH] MdeModulePkg/EsrtDxe: allocate ESRT table from 
> RtServicesData memory
>
> (+ Peter)
>
> On 19 October 2018 at 11:46, Zeng, Star <[email protected]> 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:[email protected]]
>> Sent: Friday, October 19, 2018 11:25 AM
>> To: [email protected]
>> Cc: Zeng, Star <[email protected]>; Dong, Eric
>> <[email protected]>; Leif Lindholm <[email protected]>; Ard
>> Biesheuvel <[email protected]>
>> Subject: Re: [PATCH] MdeModulePkg/EsrtDxe: allocate ESRT table from
>> RtServicesData memory
>>
>> On 19 October 2018 at 10:54, Ard Biesheuvel <[email protected]> 
>> 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 <[email protected]>
>>> ---
>>>  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
>>>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to