Re: [coreboot] Problem with tianocore compilation in coreboot-sdk:1.50

[Patrick Georgi via coreboot]

Am Fr., 6. Apr. 2018 um 17:15 Uhr schrieb Piotr Król :

> Can you tell what is this "one system"?
>
Sorry, "any one system" - just the system that the developer who last
touched the payload integration happened to test with.

As said, there's little rigor around the payload integration. All
professional deployments I know use separately built payload binaries.
And as said, if you want to take this on, we can certainly tighten up the
rules going forward.


Regards,
Patrick
-- 
coreboot mailing list: coreboot@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Problem with tianocore compilation in coreboot-sdk:1.50

[Piotr Król]

On 04/06/2018 12:20 PM, Patrick Georgi wrote:
> Am Mi., 4. Apr. 2018 um 18:10 Uhr schrieb Aaron Durbin via coreboot
> >:
> 
> > Agree, but coreboot use old commit from edk2. Is it fine to push
> vUDK2018?
> 
> I guess? I'm not really sure who uses edk2. I guess tianocore payload?
> Patrick is on holiday right now, but he would probably the best person
> to answer that.
> 
> It's used for the tianocore payload, indeed.
> And yes, updating to a newer commit is fine.
>  
> 
> > I believe coreboot should stick to edk2 releases not to random commits
> > in the tree.
> 
> That sounds like a reasonable guideline.

Ok, I will push vUDK2018.

> However if a release isn't fit for use as a coreboot payload, I'd go for
> (not so) random commits that fix problems rather than stick to another
> project's schedule and keep things broken.
> 
> > Can anyone tell what tests were made against tianocore payload? There
> > are patches in payload/external/tianocore/patches and I'm not sure
> > against what hardware those should be validated.
> 
> The payload integration stuff is provided on a best-effort basis,
> so right now "it builds and it boots on one system" is good enough.
> If you want to support the default payload selection for your board(s),
> I'll appreciate your effort, and we could tighten up the policy around
> it (so that you, and others who volunteer to test, would have to sign
> off on updates that might affect their supported systems).

Can you tell what is this "one system"?

I did ECC2017 presentation about support of tianocore payload on PC
Engines apu{2,3,4,5}. With couple patches I was able to port that
support on top of vUDK2018.

https://github.com/3mdeb/edk2/pull/2/files

Truly I just need one patch
payloads/external/tianocore/1_CorebootPayloadPkg_pcinoenum.patch - other
patches are not exactly needed for apu{2,3,4,5} support. My modification
probably should lend somewhere in edk2, but I'm not sure how much they
are interested.

I just wonder how to solve situation where edk2 customization is needed
to boot given platform and in the same case customization may affect
behavior of other platforms.

> Ideally we'd have automated hardware testing, but that's a sore point
> for years now. :-(

Internally we use RTE (Remote Test Environment) it is open hardware:
https://github.com/3mdeb/rte-schematics

Our plan is to create commercial service on top of that platform with
firmware development, validation and remote debugging services. We are
in preparation of marketing materials, so some offer should appear soon.
I already had email discussion with Patrick, but we didn't moved forward.

Some blog posts about RTE:
https://3mdeb.com/firmware/minnowboard-turbot-remote-firmware-flashing-with-rte-remote-testing-environment/
https://3mdeb.com/firmware/pc-engines-apu2-python-robot-framework-validation-automation/


Best Regards,
-- 
Piotr Król
Embedded Systems Consultant
http://3mdeb.com | @3mdeb_com

-- 
coreboot mailing list: coreboot@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Problem with tianocore compilation in coreboot-sdk:1.50

[Patrick Georgi via coreboot]

Am Mi., 4. Apr. 2018 um 18:10 Uhr schrieb Aaron Durbin via coreboot <
coreboot@coreboot.org>:

> > Agree, but coreboot use old commit from edk2. Is it fine to push
> vUDK2018?
>
> I guess? I'm not really sure who uses edk2. I guess tianocore payload?
> Patrick is on holiday right now, but he would probably the best person
> to answer that.
>
It's used for the tianocore payload, indeed.
And yes, updating to a newer commit is fine.


> > I believe coreboot should stick to edk2 releases not to random commits
> > in the tree.
>
That sounds like a reasonable guideline.
However if a release isn't fit for use as a coreboot payload, I'd go for
(not so) random commits that fix problems rather than stick to another
project's schedule and keep things broken.

> Can anyone tell what tests were made against tianocore payload? There
> > are patches in payload/external/tianocore/patches and I'm not sure
> > against what hardware those should be validated.
>
The payload integration stuff is provided on a best-effort basis, so right
now "it builds and it boots on one system" is good enough.
If you want to support the default payload selection for your board(s),
I'll appreciate your effort, and we could tighten up the policy around it
(so that you, and others who volunteer to test, would have to sign off on
updates that might affect their supported systems).
Ideally we'd have automated hardware testing, but that's a sore point for
years now. :-(


Patrick
-- 
Google Germany GmbH, ABC-Str. 19, 20354 Hamburg
Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft:
Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
-- 
coreboot mailing list: coreboot@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Problem with tianocore compilation in coreboot-sdk:1.50

[Aaron Durbin via coreboot]

On Wed, Apr 4, 2018 at 8:40 AM, Piotr Król  wrote:
>
>
> On 04/04/2018 05:31 PM, Aaron Durbin wrote:
>> On Wed, Apr 4, 2018 at 4:55 AM, Piotr Król  wrote:
>>> Hi all,
>>> I can't compile tianocore payload using coreboot-sdk:1.50 and coreboot 4.7.
>>>
>>> VfrUtilityLib.cpp: In member function 'CHAR8*
>>> CVfrStringDB::GetVarStoreNameFormStringId(EFI_STRING_ID)':
>>>
>>> VfrUtilityLib.cpp:3375:26: error: ISO C++ forbids comparison between
>>> pointer and integer [-fpermissive]
>>>if (mStringFileName == '\0' ) {
>>
>> That's just an incorrect check. i.e. bad code. Current master seems to
>> not have that error.
>>
>> https://github.com/tianocore/edk2/blob/master/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
>>
>> FWIW, comparing a char (integer) to a pointer is indeed incorrect.
>
> Agree, but coreboot use old commit from edk2. Is it fine to push vUDK2018?

I guess? I'm not really sure who uses edk2. I guess tianocore payload?
Patrick is on holiday right now, but he would probably the best person
to answer that.

>
> I believe coreboot should stick to edk2 releases not to random commits
> in the tree.
>
> Can anyone tell what tests were made against tianocore payload? There
> are patches in payload/external/tianocore/patches and I'm not sure
> against what hardware those should be validated.
>
> Best Regards,
> --
> Piotr Król
> Embedded Systems Consultant
> http://3mdeb.com | @3mdeb_com

-- 
coreboot mailing list: coreboot@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Problem with tianocore compilation in coreboot-sdk:1.50

[Piotr Król]

On 04/04/2018 05:31 PM, Aaron Durbin wrote:
> On Wed, Apr 4, 2018 at 4:55 AM, Piotr Król  wrote:
>> Hi all,
>> I can't compile tianocore payload using coreboot-sdk:1.50 and coreboot 4.7.
>>
>> VfrUtilityLib.cpp: In member function 'CHAR8*
>> CVfrStringDB::GetVarStoreNameFormStringId(EFI_STRING_ID)':
>>
>> VfrUtilityLib.cpp:3375:26: error: ISO C++ forbids comparison between
>> pointer and integer [-fpermissive]
>>if (mStringFileName == '\0' ) {
> 
> That's just an incorrect check. i.e. bad code. Current master seems to
> not have that error.
> 
> https://github.com/tianocore/edk2/blob/master/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> 
> FWIW, comparing a char (integer) to a pointer is indeed incorrect.

Agree, but coreboot use old commit from edk2. Is it fine to push vUDK2018?

I believe coreboot should stick to edk2 releases not to random commits
in the tree.

Can anyone tell what tests were made against tianocore payload? There
are patches in payload/external/tianocore/patches and I'm not sure
against what hardware those should be validated.

Best Regards,
-- 
Piotr Król
Embedded Systems Consultant
http://3mdeb.com | @3mdeb_com

-- 
coreboot mailing list: coreboot@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Problem with tianocore compilation in coreboot-sdk:1.50

[Aaron Durbin via coreboot]

On Wed, Apr 4, 2018 at 4:55 AM, Piotr Król  wrote:
> Hi all,
> I can't compile tianocore payload using coreboot-sdk:1.50 and coreboot 4.7.
>
> VfrUtilityLib.cpp: In member function 'CHAR8*
> CVfrStringDB::GetVarStoreNameFormStringId(EFI_STRING_ID)':
>
> VfrUtilityLib.cpp:3375:26: error: ISO C++ forbids comparison between
> pointer and integer [-fpermissive]
>if (mStringFileName == '\0' ) {

That's just an incorrect check. i.e. bad code. Current master seems to
not have that error.

https://github.com/tianocore/edk2/blob/master/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp

FWIW, comparing a char (integer) to a pointer is indeed incorrect.

>
> I think this is not a single issue. Key problem is that tianocore do not
> support modern toolchains - it requires GCC5. I have no problem when
> compiling with 3mdeb/edk2-docker container which use GCC5.
>
> I can workaround that problem by including payload as elf file, but if
> we have integration we should make sure it works.
>
> I'm not sure how coexistence of 3 toolchain should be solved to serve
> stable build environment. In coreboot-sdk we have 2 toolchains native
> Debian GCC7.2 and coreboot toolchain GCC6.3.
>
> If we want stable build environment for tianocore payload we should have
> 3rd - GCC5.
>
> Options to solve that problem:
> 1. Add another toolchain, which would be selected somehow through
> Makefile logic when tianocore payload selected
> 2. Add patches in coreboot to support GCC7 or GCC6 - this may cause
> unexpected behavior and maintaining this support out of edk2 can be
> problematic
> 3. Add patches in edk2 to support GCC7 or GCC6 - I think this is long
> and hard road
> 4. Any other that I'm missing ?
>
> In light of coreboot-sdk I have one question that bugs me for long time.
> Why we use Debian sid? Each build using moving target can be different
> what means it is hard to build 2 the same coreboot-sdk containers.
>
> Best Regards,
> --
> Piotr Król
> Embedded Systems Consultant
> http://3mdeb.com | @3mdeb_com
>
> --
> coreboot mailing list: coreboot@coreboot.org
> https://mail.coreboot.org/mailman/listinfo/coreboot

-- 
coreboot mailing list: coreboot@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot