[edk2-devel] [PATCH v1 1/1] UefipayloadPkg: Protect coreboot tables

Tue Oct 6 08:25:00 UTC 2020

Hi Marcello,

On 10/05/20 17:34, Marcello Sylvester Bauer wrote:
> On Thu, Oct 1, 2020 at 12:24 PM Laszlo Ersek <lersek at redhat.com> wrote:
> 
>> On 09/14/20 19:32, Guo Dong wrote:
>>>
>>> OK. Let me merge this patch firstly. /Guo
>>
>> The PR at <https://github.com/tianocore/edk2/pull/924> failed 17 days
>> ago and there have been no updates since, as far as I can tell. I've
>> closed the PR for now.
>>
>> Thanks
>> Laszlo
>>
> 
> "Mergify / Rule: Automatically merge a PR when all required checks pass and
> 'push' label is present (merge)" is
> the only that failed, because mergify thinks it has no rights to update the
> base branch (tianocore/master).
> I don't understand why. What should I do?

I don't know -- I've seen this issue before, and it seems like a bug in
github / mergify. In some cases, when all of the other checks pass, this
check suddenly flips to "OK" as well, and then the series is merged.

I'm now actually tempted to merge this patch for you (in Guo Dong's
absence). However, Guo Dong never posted an Acked-by or Reviewed-by to
the list, while reviewing this patch. So I cannot merge the patch for you.

... In fact, the commit that Guo Dong tried to merge:

https://github.com/tianocore/edk2/pull/924/commits/3f61e8ba9750d66430835fe5812c4329a1f4c2ec

doesn't even have any kind of R-b or A-b feedback tag in the commit
message. Guo Dong only edited the PR description -- that's where he
added his R-b. That is a complete failure of the development process,
and it's *just as well* that the PR failed -- for whatever reason.

One of the UefiPayloadPkg maintainers (Maurice Ma, Guo Dong, Benjamin
You) needs to approve the patch ON THE LIST, and then any maintainer
with push access can merge the patch for you.

Sorry about this mess,
Laszlo

> 
> Thanks
> Marcello
> 
>>>
>>> From: Marcello Sylvester Bauer <marcello.bauer at 9elements.com>
>>> Sent: Monday, September 14, 2020 2:00 AM
>>> To: Dong, Guo <guo.dong at intel.com>
>>> Cc: devel at edk2.groups.io; Ma, Maurice <maurice.ma at intel.com>; Desimone,
>> Nathaniel L <nathaniel.l.desimone at intel.com>; Zeng, Star <
>> star.zeng at intel.com>
>>> Subject: Re: [edk2-devel] [PATCH v1 1/1] UefipayloadPkg: Protect
>> coreboot tables
>>>
>>> Hi Guo,
>>>
>>> Sounds like a good proposal, but it would be great to merge this change
>> temporarily.
>>> In some cases of the current implementation edk2 does override the
>> memory area, where the coreboot table pointer is located.
>>> Therefore the kernel and cbmem tool is not able to locate the tables
>> anymore.
>>>
>>> Thanks,
>>> Marcello
>>>
>>> On Tue, Sep 8, 2020 at 11:40 PM Dong, Guo <guo.dong at intel.com<mailto:
>> guo.dong at intel.com>> wrote:
>>>
>>> Hi Marcello,
>>>
>>> In the UEFI payload, we should not hardcoded any memory usage. It means
>> UEFI payload should use the memory map whatever reported from the
>> bootloader. I plan to remove this hardcoded memory usage soon.
>>> Before that, it is OK for me to merge this change if you want.
>>> BTW, did you see any issue with current implement?
>>>
>>> Thanks,
>>> Guo
>>>
>>>> -----Original Message-----
>>>> From: devel at edk2.groups.io<mailto:devel at edk2.groups.io> <
>> devel at edk2.groups.io<mailto:devel at edk2.groups.io>> On Behalf Of Marcello
>>>> Sylvester Bauer
>>>> Sent: Wednesday, July 8, 2020 5:01 AM
>>>> To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>
>>>> Cc: Ma, Maurice <maurice.ma at intel.com<mailto:maurice.ma at intel.com>>;
>> Desimone, Nathaniel L
>>>> <nathaniel.l.desimone at intel.com<mailto:nathaniel.l.desimone at intel.com>>;
>> Zeng, Star <star.zeng at intel.com<mailto:star.zeng at intel.com>>
>>>> Subject: [edk2-devel] [PATCH v1 1/1] UefipayloadPkg: Protect coreboot
>> tables
>>>>
>>>> From: Patrick Rudolph <patrick.rudolph at 9elements.com<mailto:
>> patrick.rudolph at 9elements.com>>
>>>>
>>>> Signed-off-by: Patrick Rudolph <patrick.rudolph at 9elements.com<mailto:
>> patrick.rudolph at 9elements.com>>
>>>> Signed-off-by: Marcello Sylvester Bauer <marcello.bauer at 9elements.com
>> <mailto:marcello.bauer at 9elements.com>>
>>>> Cc: Maurice Ma <maurice.ma at intel.com<mailto:maurice.ma at intel.com>>
>>>> Cc: Nate DeSimone <nathaniel.l.desimone at intel.com<mailto:
>> nathaniel.l.desimone at intel.com>>
>>>> Cc: Star Zeng <star.zeng at intel.com<mailto:star.zeng at intel.com>>
>>>> ---
>>>>  UefiPayloadPkg/BlSupportPei/BlSupportPei.c | 26 ++++++++++++++------
>>>>  1 file changed, 19 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
>>>> b/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
>>>> index 22972453117a..b3ff065a537e 100644
>>>> --- a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
>>>> +++ b/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
>>>> @@ -390,24 +390,36 @@ BlPeiEntryPoint (
>>>>    EFI_PEI_GRAPHICS_DEVICE_INFO_HOB GfxDeviceInfo;
>>>>
>>>>    EFI_PEI_GRAPHICS_DEVICE_INFO_HOB *NewGfxDeviceInfo;
>>>>
>>>>
>>>>
>>>> -
>>>>
>>>> -  //
>>>>
>>>> -  // Report lower 640KB of RAM. Attribute EFI_RESOURCE_ATTRIBUTE_TESTED
>>>>
>>>> -  // is intentionally omitted to prevent erasing of the coreboot header
>>>>
>>>> -  // record before it is processed by ParseMemoryInfo.
>>>>
>>>> +  // Report lower 640KB of RAM.
>>>>
>>>> +  // Mark memory as reserved to keep coreboot header in place.
>>>>
>>>>    //
>>>>
>>>>    BuildResourceDescriptorHob (
>>>>
>>>> -    EFI_RESOURCE_SYSTEM_MEMORY,
>>>>
>>>> +    EFI_RESOURCE_MEMORY_RESERVED,
>>>>
>>>>      (
>>>>
>>>>      EFI_RESOURCE_ATTRIBUTE_PRESENT |
>>>>
>>>>      EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
>>>>
>>>> +    EFI_RESOURCE_ATTRIBUTE_TESTED |
>>>>
>>>>      EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
>>>>
>>>>      EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
>>>>
>>>>      EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
>>>>
>>>>      EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
>>>>
>>>>      ),
>>>>
>>>>      (EFI_PHYSICAL_ADDRESS)(0),
>>>>
>>>> -    (UINT64)(0xA0000)
>>>>
>>>> +    (UINT64)(0x1000)
>>>>
>>>> +    );
>>>>
>>>> +
>>>>
>>>> +  BuildResourceDescriptorHob (
>>>>
>>>> +    EFI_RESOURCE_SYSTEM_MEMORY,
>>>>
>>>> +    (
>>>>
>>>> +    EFI_RESOURCE_ATTRIBUTE_PRESENT |
>>>>
>>>> +    EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
>>>>
>>>> +    EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
>>>>
>>>> +    EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
>>>>
>>>> +    EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
>>>>
>>>> +    EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
>>>>
>>>> +    ),
>>>>
>>>> +    (EFI_PHYSICAL_ADDRESS)(0x1000),
>>>>
>>>> +    (UINT64)(0x9F000)
>>>>
>>>>      );
>>>>
>>>>
>>>>
>>>>    BuildResourceDescriptorHob (
>>>>
>>>> --
>>>> 2.27.0
>>>>
>>>>
>>>> -=-=-=-=-=-=
>>>> Groups.io Links: You receive all messages sent to this group.
>>>>
>>>> View/Reply Online (#62229):
>> https://edk2.groups.io/g/devel/message/62229
>>>> Mute This Topic: https://groups.io/mt/75374752/1781375
>>>> Group Owner: devel+owner at edk2.groups.io<mailto:
>> devel%2Bowner at edk2.groups.io>
>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [guo.dong at intel.com
>> <mailto:guo.dong at intel.com>]
>>>> -=-=-=-=-=-=
>>>
>>>
>>> --
>>> [Marcello Sylvester Bauer]
>>>
>>> [http://static.9elements.com/logo-signature.png]
>>> 9elements Agency GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
>>> Email:  [DEINE EMAIL ADDRESSE]<
>> https://static.9elements.com/email_signatur.html>
>>> Phone:  +49 234 68 94 188<tel:+492346894188>
>>> Mobile:  +49 1722847618<tel:+491722847618>
>>>
>>> Sitz der Gesellschaft: Bochum
>>> Handelsregister: Amtsgericht Bochum, HRB 17519
>>> Geschäftsführung: Sebastian Deutsch, Eray Basar
>>>
>>> Datenschutzhinweise nach Art. 13 DSGVO<https://9elements.com/privacy>
>>>
>>> 
>>>
>>
>>
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65914): https://edk2.groups.io/g/devel/message/65914
Mute This Topic: https://groups.io/mt/75374752/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-