|
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392Add FDT support in EDK2 by submodule 3rd party libfdt ( https://github.com/devicetree-org/pylibfdt/tree/main/libfdt) Cc: Michael D Kinney <michael.d.kinney@...> Cc: Liming Gao <gaoliming@...> Cc: Zhiguang Liu <zhiguang.liu@...> Cc: Sean Brogan <sean.brogan@...> Cc: Michael Kubacki <mikuback@...> Signed-off-by: Benny Lin <benny.lin@...> Benny Lin (2): MdePkg: Support FDT library. .pytool: Support FDT library. .gitmodules | 3 + .pytool/CISettings.py | 2 + MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++ MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++ MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 + MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++ MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++ MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++ MdePkg/Library/BaseFdtLib/libfdt | 1 + MdePkg/Library/BaseFdtLib/limits.h | 10 + MdePkg/Library/BaseFdtLib/stdbool.h | 10 + MdePkg/Library/BaseFdtLib/stddef.h | 10 + MdePkg/Library/BaseFdtLib/stdint.h | 10 + MdePkg/Library/BaseFdtLib/stdlib.h | 10 + MdePkg/Library/BaseFdtLib/string.h | 10 + MdePkg/MdePkg.ci.yaml | 17 +- MdePkg/MdePkg.dec | 4 + MdePkg/MdePkg.dsc | 2 + ReadMe.rst | 1 + 19 files changed, 988 insertions(+), 2 deletions(-) create mode 100644 MdePkg/Include/Library/FdtLib.h create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.inf create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c create mode 160000 MdePkg/Library/BaseFdtLib/libfdt create mode 100644 MdePkg/Library/BaseFdtLib/limits.h create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h create mode 100644 MdePkg/Library/BaseFdtLib/string.h -- 2.39.1.windows.1
|
|
|
On Thu, Mar 30, 2023 at 6:13 PM Benny Lin <benny.lin@...> wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392
Add FDT support in EDK2 by submodule 3rd party libfdt
(https://github.com/devicetree-org/pylibfdt/tree/main/libfdt)
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Sean Brogan <sean.brogan@...>
Cc: Michael Kubacki <mikuback@...>
Signed-off-by: Benny Lin <benny.lin@...>
Benny Lin (2):
MdePkg: Support FDT library.
.pytool: Support FDT library.
.gitmodules | 3 +
.pytool/CISettings.py | 2 +
MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 +
MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++
MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++
MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++
MdePkg/Library/BaseFdtLib/libfdt | 1 +
MdePkg/Library/BaseFdtLib/limits.h | 10 +
MdePkg/Library/BaseFdtLib/stdbool.h | 10 +
MdePkg/Library/BaseFdtLib/stddef.h | 10 +
MdePkg/Library/BaseFdtLib/stdint.h | 10 +
MdePkg/Library/BaseFdtLib/stdlib.h | 10 +
MdePkg/Library/BaseFdtLib/string.h | 10 +
MdePkg/MdePkg.ci.yaml | 17 +-
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 2 +
ReadMe.rst | 1 +
19 files changed, 988 insertions(+), 2 deletions(-)
create mode 100644 MdePkg/Include/Library/FdtLib.h
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.inf
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni
create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
create mode 160000 MdePkg/Library/BaseFdtLib/libfdt
create mode 100644 MdePkg/Library/BaseFdtLib/limits.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h
create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h
create mode 100644 MdePkg/Library/BaseFdtLib/string.h
--
2.39.1.windows.1
There's already a copy of libfdt plus "FdtLib" at https://github.com/tianocore/edk2/tree/master/EmbeddedPkg/Library/FdtLib. Please figure out what to do with it. It's an old copy and has been accidentally uncrustify'd so it's probably a good idea to at least ditch that specific copy for a git submodule. Also, NAK to Yet Another libc Implementation (and not a particularly good one at that). -- Pedro
|
|
|
toggle quoted message
Show quoted text
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Thursday, March 30, 2023 2:50 PM
To: devel@edk2.groups.io; Lin, Benny <benny.lin@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Gao, Liming <gaoliming@...>; Liu, Zhiguang
<zhiguang.liu@...>; Sean Brogan <sean.brogan@...>; Michael Kubacki <mikuback@...>
Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
On Thu, Mar 30, 2023 at 6:13 PM Benny Lin <benny.lin@...> wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392
Add FDT support in EDK2 by submodule 3rd party libfdt
(https://github.com/devicetree-org/pylibfdt/tree/main/libfdt)
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Sean Brogan <sean.brogan@...>
Cc: Michael Kubacki <mikuback@...>
Signed-off-by: Benny Lin <benny.lin@...>
Benny Lin (2):
MdePkg: Support FDT library.
.pytool: Support FDT library.
.gitmodules | 3 +
.pytool/CISettings.py | 2 +
MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 +
MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++
MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++
MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++
MdePkg/Library/BaseFdtLib/libfdt | 1 +
MdePkg/Library/BaseFdtLib/limits.h | 10 +
MdePkg/Library/BaseFdtLib/stdbool.h | 10 +
MdePkg/Library/BaseFdtLib/stddef.h | 10 +
MdePkg/Library/BaseFdtLib/stdint.h | 10 +
MdePkg/Library/BaseFdtLib/stdlib.h | 10 +
MdePkg/Library/BaseFdtLib/string.h | 10 +
MdePkg/MdePkg.ci.yaml | 17 +-
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 2 +
ReadMe.rst | 1 +
19 files changed, 988 insertions(+), 2 deletions(-)
create mode 100644 MdePkg/Include/Library/FdtLib.h
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.inf
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni
create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
create mode 160000 MdePkg/Library/BaseFdtLib/libfdt
create mode 100644 MdePkg/Library/BaseFdtLib/limits.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h
create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h
create mode 100644 MdePkg/Library/BaseFdtLib/string.h
--
2.39.1.windows.1
There's already a copy of libfdt plus "FdtLib" at
https://github.com/tianocore/edk2/tree/master/EmbeddedPkg/Library/FdtLib.
Please figure out what to do with it.
It's an old copy and has been accidentally uncrustify'd so it's
probably a good idea to at least ditch that specific copy for a git
submodule. I have discussed this overlap with Leif. After this patch series is added, the EmbeddedPkg maintainers can convert that package to use this lib and delete the duplicate sources.
Also, NAK to Yet Another libc Implementation (and not a particularly
good one at that).
Please provide constructive feedback on what is not good about this specific libc Implementation so that appropriate code updates could be made for this patch series. This is following the same pattern as other libs that are consuming a submodule that requires some amount of libc support. Getting down to one libc wrapper would be great. Do you want to provide a proposed implementation? That could be handled as a separate task.
--
Pedro
|
|
|
On Thu, Mar 30, 2023 at 11:59 PM Kinney, Michael D <michael.d.kinney@...> wrote:
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Thursday, March 30, 2023 2:50 PM
To: devel@edk2.groups.io; Lin, Benny <benny.lin@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Gao, Liming <gaoliming@...>; Liu, Zhiguang
<zhiguang.liu@...>; Sean Brogan <sean.brogan@...>; Michael Kubacki <mikuback@...>
Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
On Thu, Mar 30, 2023 at 6:13 PM Benny Lin <benny.lin@...> wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392
Add FDT support in EDK2 by submodule 3rd party libfdt
(https://github.com/devicetree-org/pylibfdt/tree/main/libfdt)
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Sean Brogan <sean.brogan@...>
Cc: Michael Kubacki <mikuback@...>
Signed-off-by: Benny Lin <benny.lin@...>
Benny Lin (2):
MdePkg: Support FDT library.
.pytool: Support FDT library.
.gitmodules | 3 +
.pytool/CISettings.py | 2 +
MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 +
MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++
MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++
MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++
MdePkg/Library/BaseFdtLib/libfdt | 1 +
MdePkg/Library/BaseFdtLib/limits.h | 10 +
MdePkg/Library/BaseFdtLib/stdbool.h | 10 +
MdePkg/Library/BaseFdtLib/stddef.h | 10 +
MdePkg/Library/BaseFdtLib/stdint.h | 10 +
MdePkg/Library/BaseFdtLib/stdlib.h | 10 +
MdePkg/Library/BaseFdtLib/string.h | 10 +
MdePkg/MdePkg.ci.yaml | 17 +-
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 2 +
ReadMe.rst | 1 +
19 files changed, 988 insertions(+), 2 deletions(-)
create mode 100644 MdePkg/Include/Library/FdtLib.h
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.inf
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni
create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
create mode 160000 MdePkg/Library/BaseFdtLib/libfdt
create mode 100644 MdePkg/Library/BaseFdtLib/limits.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h
create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h
create mode 100644 MdePkg/Library/BaseFdtLib/string.h
--
2.39.1.windows.1
There's already a copy of libfdt plus "FdtLib" at
https://github.com/tianocore/edk2/tree/master/EmbeddedPkg/Library/FdtLib.
Please figure out what to do with it.
It's an old copy and has been accidentally uncrustify'd so it's
probably a good idea to at least ditch that specific copy for a git
submodule. I have discussed this overlap with Leif. After this patch series is
added, the EmbeddedPkg maintainers can convert that package to use
this lib and delete the duplicate sources.
Ok, SGTM.
Also, NAK to Yet Another libc Implementation (and not a particularly
good one at that).
Please provide constructive feedback on what is not good about this specific libc
Implementation so that appropriate code updates could be made for this patch series.
Done. This is following the same pattern as other libs that are consuming a submodule
that requires some amount of libc support.
Getting down to one libc wrapper would be great. Do you want to provide a proposed
implementation? That could be handled as a separate task. I would like it if we could stop contributing to that problem. We very much *know* there is a problem with both libc fragments and compiler intrinsic fragments all over edk2. A proper, correct C standard library is not trivial to implement (hence the multiple problems I did find from a quick read of the patch). We also have a whole libc implementation in edk2-libc that seems to be permanently gathering dust until Intel touches it for Python purposes from time to time. So between crypto, libfdt, etc, could we try to unify things here a bit? Thanks, Pedro
|
|
|
toggle quoted message
Show quoted text
-----Original Message-----
From: Pedro Falcato <pedro.falcato@...>
Sent: Thursday, March 30, 2023 4:27 PM
To: Kinney, Michael D <michael.d.kinney@...>
Cc: devel@edk2.groups.io; Lin, Benny <benny.lin@...>; Gao, Liming <gaoliming@...>; Liu, Zhiguang
<zhiguang.liu@...>; Sean Brogan <sean.brogan@...>; Michael Kubacki <mikuback@...>; Leif
Lindholm <llindhol@...>
Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
On Thu, Mar 30, 2023 at 11:59 PM Kinney, Michael D
<michael.d.kinney@...> wrote:
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Thursday, March 30, 2023 2:50 PM
To: devel@edk2.groups.io; Lin, Benny <benny.lin@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Gao, Liming <gaoliming@...>; Liu, Zhiguang
<zhiguang.liu@...>; Sean Brogan <sean.brogan@...>; Michael Kubacki <mikuback@...>
Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
On Thu, Mar 30, 2023 at 6:13 PM Benny Lin <benny.lin@...> wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392
Add FDT support in EDK2 by submodule 3rd party libfdt
(https://github.com/devicetree-org/pylibfdt/tree/main/libfdt)
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Sean Brogan <sean.brogan@...>
Cc: Michael Kubacki <mikuback@...>
Signed-off-by: Benny Lin <benny.lin@...>
Benny Lin (2):
MdePkg: Support FDT library.
.pytool: Support FDT library.
.gitmodules | 3 +
.pytool/CISettings.py | 2 +
MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 +
MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++
MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++
MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++
MdePkg/Library/BaseFdtLib/libfdt | 1 +
MdePkg/Library/BaseFdtLib/limits.h | 10 +
MdePkg/Library/BaseFdtLib/stdbool.h | 10 +
MdePkg/Library/BaseFdtLib/stddef.h | 10 +
MdePkg/Library/BaseFdtLib/stdint.h | 10 +
MdePkg/Library/BaseFdtLib/stdlib.h | 10 +
MdePkg/Library/BaseFdtLib/string.h | 10 +
MdePkg/MdePkg.ci.yaml | 17 +-
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 2 +
ReadMe.rst | 1 +
19 files changed, 988 insertions(+), 2 deletions(-)
create mode 100644 MdePkg/Include/Library/FdtLib.h
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.inf
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni
create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
create mode 160000 MdePkg/Library/BaseFdtLib/libfdt
create mode 100644 MdePkg/Library/BaseFdtLib/limits.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h
create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h
create mode 100644 MdePkg/Library/BaseFdtLib/string.h
--
2.39.1.windows.1
There's already a copy of libfdt plus "FdtLib" at
https://github.com/tianocore/edk2/tree/master/EmbeddedPkg/Library/FdtLib.
Please figure out what to do with it.
It's an old copy and has been accidentally uncrustify'd so it's
probably a good idea to at least ditch that specific copy for a git
submodule. I have discussed this overlap with Leif. After this patch series is
added, the EmbeddedPkg maintainers can convert that package to use
this lib and delete the duplicate sources.
Ok, SGTM.
Also, NAK to Yet Another libc Implementation (and not a particularly
good one at that).
Please provide constructive feedback on what is not good about this specific libc
Implementation so that appropriate code updates could be made for this patch series.
Done.
This is following the same pattern as other libs that are consuming a submodule
that requires some amount of libc support.
Getting down to one libc wrapper would be great. Do you want to provide a proposed
implementation? That could be handled as a separate task. I would like it if we could stop contributing to that problem. We very
much *know* there is a problem with both libc fragments and compiler
intrinsic fragments all over edk2.
A proper, correct C standard library is not trivial to implement
(hence the multiple problems I did find from a quick read of the
patch). Appreciate the feedback. Agree that any libc API that is implemented in a wrapper should conform to the standard. We also have a whole libc implementation in edk2-libc that seems to be
permanently gathering dust until Intel touches it for Python purposes
from time to time. So between crypto, libfdt, etc, could we try to
unify things here a bit? edk2-libc to too large for FW components and it is out of date. Would you like to volunteer to lead a design and implementation that is right-sized for FW modules and could be the one wrapper that works for all current use cases and could be extended in the future as needed to support additional use cases? Don’t need all of libc. Just enough to support the APIs used by the submodules used so far.
Thanks,
Pedro
|
|
|
On Fri, Mar 31, 2023 at 12:32 AM Kinney, Michael D <michael.d.kinney@...> wrote: Appreciate the feedback. Agree that any libc API that is implemented in a
wrapper should conform to the standard.
We also have a whole libc implementation in edk2-libc that seems to be
permanently gathering dust until Intel touches it for Python purposes
from time to time. So between crypto, libfdt, etc, could we try to
unify things here a bit? edk2-libc to too large for FW components and it is out of date.
Would you like to volunteer to lead a design and implementation that
is right-sized for FW modules and could be the one wrapper that works
for all current use cases and could be extended in the future as
needed to support additional use cases? Don’t need all of libc. Just
enough to support the APIs used by the submodules used so far.
Mike, I wrote up a quick RFC patch for a bunch of libc bits that you needed in this case (BaseFdtLib and libfdt). It's very much a WIP and only supports GCC/clang. MSVC needs some support when it comes to limits.h (because of LP64 vs Windows's LLP64), but nothing too hard certainly. See https://edk2.groups.io/g/devel/topic/rfc_patch_1_1_mdepkg_add_a/97965830?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,97965830,previd%3D1680229851681438282,nextid%3D1680190220621190228&previd=1680229851681438282&nextid=1680190220621190228 Comments welcome. -- Pedro
|
|
|
Hi, Getting down to one libc wrapper would be great. Do you want to provide a proposed
implementation? That could be handled as a separate task. I would like it if we could stop contributing to that problem. We very
much *know* there is a problem with both libc fragments and compiler
intrinsic fragments all over edk2.
I'd suggest to start with what we already have in the tree. Which is (not fully sure the list is actually complete): - CryptoPkg/Library/Include/ carrying the bits needed to make openssl compile, and - CryptoPkg/Library/IntrinsicLib (again, for openssl, mostly IA32, some X64) and, - ArmPkg/Library/CompilerIntrinsicsLib (mostly ARM, some AARCH64). Can we move that over to MdePkg so everyone can easily share it instead of adding more copies of the same to the tree? I have an old branch gathering dust doing that for the intrinsics, I can try rebasing it to latest master next week ... take care, Gerd
|
|
|
On Thu, Mar 30, 2023 at 22:50:18 +0100, Pedro Falcato wrote: On Thu, Mar 30, 2023 at 6:13 PM Benny Lin <benny.lin@...> wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392
Add FDT support in EDK2 by submodule 3rd party libfdt
(https://github.com/devicetree-org/pylibfdt/tree/main/libfdt)
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Sean Brogan <sean.brogan@...>
Cc: Michael Kubacki <mikuback@...>
Signed-off-by: Benny Lin <benny.lin@...>
Benny Lin (2):
MdePkg: Support FDT library.
.pytool: Support FDT library.
.gitmodules | 3 +
.pytool/CISettings.py | 2 +
MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 +
MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++
MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++
MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++
MdePkg/Library/BaseFdtLib/libfdt | 1 +
MdePkg/Library/BaseFdtLib/limits.h | 10 +
MdePkg/Library/BaseFdtLib/stdbool.h | 10 +
MdePkg/Library/BaseFdtLib/stddef.h | 10 +
MdePkg/Library/BaseFdtLib/stdint.h | 10 +
MdePkg/Library/BaseFdtLib/stdlib.h | 10 +
MdePkg/Library/BaseFdtLib/string.h | 10 +
MdePkg/MdePkg.ci.yaml | 17 +-
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 2 +
ReadMe.rst | 1 +
19 files changed, 988 insertions(+), 2 deletions(-)
create mode 100644 MdePkg/Include/Library/FdtLib.h
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.inf
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni
create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
create mode 160000 MdePkg/Library/BaseFdtLib/libfdt
create mode 100644 MdePkg/Library/BaseFdtLib/limits.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h
create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h
create mode 100644 MdePkg/Library/BaseFdtLib/string.h
--
2.39.1.windows.1
There's already a copy of libfdt plus "FdtLib" at
https://github.com/tianocore/edk2/tree/master/EmbeddedPkg/Library/FdtLib.
Please figure out what to do with it. Like much of EmbeddedPkg, it's a halfway-house minimal effort legacy thing. As soon as the actively maintained platforms have moved away from it, I will delete it. It's an old copy and has been accidentally uncrustify'd so it's
probably a good idea to at least ditch that specific copy for a git
submodule. Yes, but a migration path for existing users is preferable to breaking the world. / Leif
|
|
|
On Fri, Mar 31, 2023 at 00:26:37 +0100, Pedro Falcato wrote: We also have a whole libc implementation in edk2-libc that seems to be
permanently gathering dust until Intel touches it for Python purposes
from time to time. So between crypto, libfdt, etc, could we try to
unify things here a bit? Personally, I'd quite like to nuke edk2-libc. The only effect I'm seeing from it is that it keeps misleading people into believing it's sensible to expect full POSIX compliance under UEFI, then embed those expectations into their organisational workflows. Meanwhile, it receives (next to) no security fixes. / Leif
|
|
|
On Fri, Mar 31, 2023 at 12:39 PM Gerd Hoffmann <kraxel@...> wrote:
Hi,
Getting down to one libc wrapper would be great. Do you want to provide a proposed
implementation? That could be handled as a separate task. I would like it if we could stop contributing to that problem. We very
much *know* there is a problem with both libc fragments and compiler
intrinsic fragments all over edk2.
I'd suggest to start with what we already have in the tree. Which is
(not fully sure the list is actually complete):
- CryptoPkg/Library/Include/ carrying the bits needed to make openssl
compile, and
- CryptoPkg/Library/IntrinsicLib (again, for openssl, mostly IA32, some
X64) and,
- ArmPkg/Library/CompilerIntrinsicsLib (mostly ARM, some AARCH64).
Can we move that over to MdePkg so everyone can easily share it instead
of adding more copies of the same to the tree?
Yes please. The problem with starting with what's in the tree is that it's very messy and sometimes of super dubious quality. For instance, on CryptoPkg there's the same lack of rigour in the headers and CrtWrapper.c that I would rather avoid, as to make this a relatively proper thing (did you know if OpenSSL strcpy's over 4KiB it silently fails?). I have an old branch gathering dust doing that for the intrinsics, I can
try rebasing it to latest master next week ... Yes please! I did think about bringing in compiler-rt from LLVM for high quality relatively self-contained intrinsics a good while ago. There's the open question that GCC and clang require intrinsics and a very small set of libc string functions to properly generate code. Could BaseTools implicitly link this in? If we do it correctly, there's no reason why the binaries would grow beyond what it requires from the intrinsics set. And I actually did see problems with lacking memcpy when trying out tree-wide GCC12/clang -ftrivial-auto-var-init... Anyway, if you want help with this or want to sync up efforts, do ping me off-list :) -- Pedro
|
|
|
Andrei Warkentin
How does this relate to the existing EmbeddedPkg/Library/FdtLib code? Is there a specific plan to move away from this in existing components? I did look in the BZ ( https://bugzilla.tianocore.org/show_bug.cgi?id=4392) but this doesn't seem to acknowledge that there is existing Fdt support in EmbeddedPkg. A
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Benny
Lin
Sent: Thursday, March 30, 2023 11:52 AM
To: devel@edk2.groups.io
Cc: Lin, Benny <benny.lin@...>; Kinney, Michael D
<michael.d.kinney@...>; Gao, Liming <gaoliming@...>;
Liu, Zhiguang <zhiguang.liu@...>; Sean Brogan
<sean.brogan@...>; Michael Kubacki
<mikuback@...>
Subject: [edk2-devel] [PATCH 0/2] Support FDT library.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392
Add FDT support in EDK2 by submodule 3rd party libfdt
(https://github.com/devicetree-org/pylibfdt/tree/main/libfdt)
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Sean Brogan <sean.brogan@...>
Cc: Michael Kubacki <mikuback@...>
Signed-off-by: Benny Lin <benny.lin@...>
Benny Lin (2):
MdePkg: Support FDT library.
.pytool: Support FDT library.
.gitmodules | 3 +
.pytool/CISettings.py | 2 +
MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++
MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 +
MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++
MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++
MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++
MdePkg/Library/BaseFdtLib/libfdt | 1 +
MdePkg/Library/BaseFdtLib/limits.h | 10 +
MdePkg/Library/BaseFdtLib/stdbool.h | 10 +
MdePkg/Library/BaseFdtLib/stddef.h | 10 +
MdePkg/Library/BaseFdtLib/stdint.h | 10 +
MdePkg/Library/BaseFdtLib/stdlib.h | 10 +
MdePkg/Library/BaseFdtLib/string.h | 10 +
MdePkg/MdePkg.ci.yaml | 17 +-
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 2 +
ReadMe.rst | 1 +
19 files changed, 988 insertions(+), 2 deletions(-) create mode 100644
MdePkg/Include/Library/FdtLib.h create mode 100644
MdePkg/Library/BaseFdtLib/BaseFdtLib.inf
create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni
create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h
create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
create mode 160000 MdePkg/Library/BaseFdtLib/libfdt create mode 100644
MdePkg/Library/BaseFdtLib/limits.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h
create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h
create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h
create mode 100644 MdePkg/Library/BaseFdtLib/string.h
--
2.39.1.windows.1
|
|
|
Thanks for the nice feedback Pedro, Gerd and Andrei! Yeah it seems like a valid concern here as Mik mentioned on edk2-libc, and it seems to fits edk2 long term interest on this. Can we file this as an issue in Bugzilla for tracking or something? Since this will take some time to work on this as it involves a bigger discussion, personally I think we could get this FDT patch in first meanwhile, and also remove the FDT from Embedded Pkg as next step, per discussion with Leif? What do you think?
How does this relate to the existing EmbeddedPkg/Library/FdtLib code? Is there a specific plan to move away from this in existing components?
I did look in the BZ (https://bugzilla.tianocore.org/show_bug.cgi?id=4392) but this doesn't seem to acknowledge that there is existing Fdt support in EmbeddedPkg.
A
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Benny
> Lin
> Sent: Thursday, March 30, 2023 11:52 AM
> To: devel@edk2.groups.io
> Cc: Lin, Benny <benny.lin@...>; Kinney, Michael D
> <michael.d.kinney@...>; Gao, Liming <gaoliming@...>;
> Liu, Zhiguang <zhiguang.liu@...>; Sean Brogan
> <sean.brogan@...>; Michael Kubacki
> <mikuback@...>
> Subject: [edk2-devel] [PATCH 0/2] Support FDT library.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392
> Add FDT support in EDK2 by submodule 3rd party libfdt
> (https://github.com/devicetree-org/pylibfdt/tree/main/libfdt)
>
> Cc: Michael D Kinney <michael.d.kinney@...>
> Cc: Liming Gao <gaoliming@...>
> Cc: Zhiguang Liu <zhiguang.liu@...>
> Cc: Sean Brogan <sean.brogan@...>
> Cc: Michael Kubacki <mikuback@...>
> Signed-off-by: Benny Lin <benny.lin@...>
>
> Benny Lin (2):
> MdePkg: Support FDT library.
> .pytool: Support FDT library.
>
> .gitmodules | 3 +
> .pytool/CISettings.py | 2 +
> MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++
> MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++
> MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 +
> MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++
> MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++
> MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++
> MdePkg/Library/BaseFdtLib/libfdt | 1 +
> MdePkg/Library/BaseFdtLib/limits.h | 10 +
> MdePkg/Library/BaseFdtLib/stdbool.h | 10 +
> MdePkg/Library/BaseFdtLib/stddef.h | 10 +
> MdePkg/Library/BaseFdtLib/stdint.h | 10 +
> MdePkg/Library/BaseFdtLib/stdlib.h | 10 +
> MdePkg/Library/BaseFdtLib/string.h | 10 +
> MdePkg/MdePkg.ci.yaml | 17 +-
> MdePkg/MdePkg.dec | 4 +
> MdePkg/MdePkg.dsc | 2 +
> ReadMe.rst | 1 +
> 19 files changed, 988 insertions(+), 2 deletions(-) create mode 100644
> MdePkg/Include/Library/FdtLib.h create mode 100644
> MdePkg/Library/BaseFdtLib/BaseFdtLib.inf
> create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni
> create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c
> create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h
> create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
> create mode 160000 MdePkg/Library/BaseFdtLib/libfdt create mode 100644
> MdePkg/Library/BaseFdtLib/limits.h
> create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h
> create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h
> create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h
> create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h
> create mode 100644 MdePkg/Library/BaseFdtLib/string.h
>
> --
> 2.39.1.windows.1
>
>
>
>
>
|
|
|
On Thu, Apr 6, 2023 at 5:34 PM Sheng Lean Tan <sheng.tan@...> wrote:
Thanks for the nice feedback Pedro, Gerd and Andrei! Yeah it seems like a valid concern here as Mik mentioned on edk2-libc, and it seems to fits edk2 long term interest on this.
Can we file this as an issue in Bugzilla for tracking or something? Since this will take some time to work on this as it involves a bigger discussion, personally I think we could get this FDT patch in first meanwhile, and also remove the FDT from Embedded Pkg as next step, per discussion with Leif?
What do you think?
I'm all for not merging this without a proper solution in that regard (I even presented a quick RFC solution which wasn't tested by anyone involved in this patch, yet). But if there really is an urgent need for this lib, I'm O-K with merging this given that all my concerns are addressed (minus libc duplication). -- Pedro
|
|
|
Andrei Warkentin
I think in general it would be nice to understand the long term picture of a change, esp. since there is already FDT support in EDK2 in various forms (with libraries and drivers depending on the existing FdtLib). So it would really of confusing to see another FDT library in MdePkg, without a clear reasoning for the work (this isn't reflected in the BZ) and a clear action plan to end up with just one FDT library in MdePkg in some identified time frame.
I do think FDT lib *does* belong in MdePkg, but it seems the shortest path to get there is to simply move the existing EmbeddedPkg one (and update all users). Subsequent cleanup can be incremental. And regardless, every existing FdtLib user ought to be updated to use the new one, so there need to be more patches (we're not just throwing the code over the wall, right?)
A
toggle quoted message
Show quoted text
-----Original Message-----
From: Pedro Falcato <pedro.falcato@...>
Sent: Friday, April 7, 2023 8:24 AM
To: devel@edk2.groups.io; Tan, Lean Sheng <sheng.tan@...>
Cc: Warkentin, Andrei <andrei.warkentin@...>; Lin, Benny
<benny.lin@...>; Kinney, Michael D <michael.d.kinney@...>;
Gao, Liming <gaoliming@...>; Liu, Zhiguang
<zhiguang.liu@...>; Sean Brogan <sean.brogan@...>;
Michael Kubacki <mikuback@...>
Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
On Thu, Apr 6, 2023 at 5:34 PM Sheng Lean Tan
<sheng.tan@...> wrote:
Thanks for the nice feedback Pedro, Gerd and Andrei! Yeah it seems like a
valid concern here as Mik mentioned on edk2-libc, and it seems to fits edk2
long term interest on this.
Can we file this as an issue in Bugzilla for tracking or something? Since this will take some time to work on this as it involves a bigger discussion,
personally I think we could get this FDT patch in first meanwhile, and also
remove the FDT from Embedded Pkg as next step, per discussion with Leif?
What do you think? I'm all for not merging this without a proper solution in that regard (I even
presented a quick RFC solution which wasn't tested by anyone involved in
this patch, yet).
But if there really is an urgent need for this lib, I'm O-K with merging this
given that all my concerns are addressed (minus libc duplication).
--
Pedro
|
|
|
The main advantage of the new lib is that it depends on a submodule for the FDT
content so we can easily move to new releases for bug fixes or features.
The FDT content in the EmbeddedPkg is a snap-shot of the code from a long time
ago. We have been discouraging that approach and trying to move to released
content from a well support submodule if one is available.
Once this new better supported version is accepted, we can then incrementally
remove the duplicate content with the existing consumers moving from use of
EmbeddedPkg version to the MdePkg version and finally the removal of the
duplicate content in the EmbeddedPkg.
Mike
toggle quoted message
Show quoted text
-----Original Message-----
From: Warkentin, Andrei <andrei.warkentin@...>
Sent: Friday, April 7, 2023 3:36 PM
To: Pedro Falcato <pedro.falcato@...>; devel@edk2.groups.io; Tan, Lean Sheng <sheng.tan@...>
Cc: Lin, Benny <benny.lin@...>; Kinney, Michael D <michael.d.kinney@...>; Gao, Liming <gaoliming@...>;
Liu, Zhiguang <zhiguang.liu@...>; Sean Brogan <sean.brogan@...>; Michael Kubacki <mikuback@...>
Subject: RE: [edk2-devel] [PATCH 0/2] Support FDT library.
I think in general it would be nice to understand the long term picture of a change, esp. since there is already FDT support in
EDK2 in various forms (with libraries and drivers depending on the existing FdtLib). So it would really of confusing to see
another FDT library in MdePkg, without a clear reasoning for the work (this isn't reflected in the BZ) and a clear action plan
to end up with just one FDT library in MdePkg in some identified time frame.
I do think FDT lib *does* belong in MdePkg, but it seems the shortest path to get there is to simply move the existing
EmbeddedPkg one (and update all users). Subsequent cleanup can be incremental. And regardless, every existing FdtLib user ought
to be updated to use the new one, so there need to be more patches (we're not just throwing the code over the wall, right?)
A
-----Original Message-----
From: Pedro Falcato <pedro.falcato@...>
Sent: Friday, April 7, 2023 8:24 AM
To: devel@edk2.groups.io; Tan, Lean Sheng <sheng.tan@...>
Cc: Warkentin, Andrei <andrei.warkentin@...>; Lin, Benny
<benny.lin@...>; Kinney, Michael D <michael.d.kinney@...>;
Gao, Liming <gaoliming@...>; Liu, Zhiguang
<zhiguang.liu@...>; Sean Brogan <sean.brogan@...>;
Michael Kubacki <mikuback@...>
Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
On Thu, Apr 6, 2023 at 5:34 PM Sheng Lean Tan
<sheng.tan@...> wrote:
Thanks for the nice feedback Pedro, Gerd and Andrei! Yeah it seems like a
valid concern here as Mik mentioned on edk2-libc, and it seems to fits edk2
long term interest on this.
Can we file this as an issue in Bugzilla for tracking or something? Since this will take some time to work on this as it involves a bigger discussion,
personally I think we could get this FDT patch in first meanwhile, and also
remove the FDT from Embedded Pkg as next step, per discussion with Leif?
What do you think? I'm all for not merging this without a proper solution in that regard (I even
presented a quick RFC solution which wasn't tested by anyone involved in
this patch, yet).
But if there really is an urgent need for this lib, I'm O-K with merging this
given that all my concerns are addressed (minus libc duplication).
--
Pedro
|
|
|
Thanks Mike for the proposal layout! It sounds good to me :)
Hi Pedro, I went through the email chain again, basically these are 2 of your main concerns (correct me if I'm wrong): 1. a good idea to at least ditch that specific copy (current FDT in Embedded Pkg) for a git submodule. 2. Rework to remove/reduce libc Implementation as there is a problem with both libc fragments and compiler intrinsic fragments all over edk2. Should unify standards between crypto, libfdt, etc, could we try here
I guess Mike has provided a plan to answer your first question, and the 2nd question would require a broader discussion with a few key owners.
So it seems like we could get the current patchset from Benny Lin in for now? Any minor clean up needed for the current patch?
The main advantage of the new lib is that it depends on a submodule for the FDT
content so we can easily move to new releases for bug fixes or features.
The FDT content in the EmbeddedPkg is a snap-shot of the code from a long time
ago. We have been discouraging that approach and trying to move to released
content from a well support submodule if one is available.
Once this new better supported version is accepted, we can then incrementally
remove the duplicate content with the existing consumers moving from use of
EmbeddedPkg version to the MdePkg version and finally the removal of the
duplicate content in the EmbeddedPkg.
Mike
> -----Original Message-----
> From: Warkentin, Andrei <andrei.warkentin@...>
> Sent: Friday, April 7, 2023 3:36 PM
> To: Pedro Falcato <pedro.falcato@...>; devel@edk2.groups.io; Tan, Lean Sheng <sheng.tan@...>
> Cc: Lin, Benny <benny.lin@...>; Kinney, Michael D <michael.d.kinney@...>; Gao, Liming <gaoliming@...>;
> Liu, Zhiguang <zhiguang.liu@...>; Sean Brogan <sean.brogan@...>; Michael Kubacki <mikuback@...>
> Subject: RE: [edk2-devel] [PATCH 0/2] Support FDT library.
>
> I think in general it would be nice to understand the long term picture of a change, esp. since there is already FDT support in
> EDK2 in various forms (with libraries and drivers depending on the existing FdtLib). So it would really of confusing to see
> another FDT library in MdePkg, without a clear reasoning for the work (this isn't reflected in the BZ) and a clear action plan
> to end up with just one FDT library in MdePkg in some identified time frame.
>
> I do think FDT lib *does* belong in MdePkg, but it seems the shortest path to get there is to simply move the existing
> EmbeddedPkg one (and update all users). Subsequent cleanup can be incremental. And regardless, every existing FdtLib user ought
> to be updated to use the new one, so there need to be more patches (we're not just throwing the code over the wall, right?)
>
> A
>
> > -----Original Message-----
> > From: Pedro Falcato <pedro.falcato@...>
> > Sent: Friday, April 7, 2023 8:24 AM
> > To: devel@edk2.groups.io; Tan, Lean Sheng <sheng.tan@...>
> > Cc: Warkentin, Andrei <andrei.warkentin@...>; Lin, Benny
> > <benny.lin@...>; Kinney, Michael D <michael.d.kinney@...>;
> > Gao, Liming <gaoliming@...>; Liu, Zhiguang
> > <zhiguang.liu@...>; Sean Brogan <sean.brogan@...>;
> > Michael Kubacki <mikuback@...>
> > Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
> >
> > On Thu, Apr 6, 2023 at 5:34 PM Sheng Lean Tan
> > <sheng.tan@...> wrote:
> > >
> > > Thanks for the nice feedback Pedro, Gerd and Andrei! Yeah it seems like a
> > valid concern here as Mik mentioned on edk2-libc, and it seems to fits edk2
> > long term interest on this.
> > > Can we file this as an issue in Bugzilla for tracking or something? Since this
> > will take some time to work on this as it involves a bigger discussion,
> > personally I think we could get this FDT patch in first meanwhile, and also
> > remove the FDT from Embedded Pkg as next step, per discussion with Leif?
> > > What do you think?
> >
> > I'm all for not merging this without a proper solution in that regard (I even
> > presented a quick RFC solution which wasn't tested by anyone involved in
> > this patch, yet).
> >
> > But if there really is an urgent need for this lib, I'm O-K with merging this
> > given that all my concerns are addressed (minus libc duplication).
> >
> > --
> > Pedro
|
|
|
On Tue, Apr 11, 2023 at 2:20 PM Lean Sheng Tan <sheng.tan@...> wrote:
Thanks Mike for the proposal layout!
It sounds good to me :)
Hi Pedro,
I went through the email chain again, basically these are 2 of your main concerns (correct me if I'm wrong):
1. a good idea to at least ditch that specific copy (current FDT in Embedded Pkg) for a git submodule.
2. Rework to remove/reduce libc Implementation as there is a problem with both libc fragments and compiler intrinsic fragments all over edk2. Should unify standards between crypto, libfdt, etc, could we try here
I guess Mike has provided a plan to answer your first question, and the 2nd question would require a broader discussion with a few key owners.
So it seems like we could get the current patchset from Benny Lin in for now? Any minor clean up needed for the current patch?
No. 3. Lots of questions and comments on the actual patch set regarding the quality of the libc implementation. Which should be fixed, regardless of centralizing a libc implementation. Also questions on the FdtLib itself (why are we wrapping pure libfdt functions with FluffyIdentifierNames and SCARY_TYPEDEFS?). I also sent out an RFC for a central libc for GCC/clang based toolchains, which should cover the libc usage of libfdt. Asked for testing, got ignored. So a NAK from me, in its current state. -- Pedro
|
|
|
toggle quoted message
Show quoted text
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Tuesday, April 11, 2023 9:07 AM
To: Tan, Lean Sheng <sheng.tan@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Warkentin, Andrei <andrei.warkentin@...>; devel@edk2.groups.io;
Lin, Benny <benny.lin@...>; Gao, Liming <gaoliming@...>; Liu, Zhiguang <zhiguang.liu@...>; Sean
Brogan <sean.brogan@...>; Michael Kubacki <mikuback@...>
Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
On Tue, Apr 11, 2023 at 2:20 PM Lean Sheng Tan <sheng.tan@...> wrote:
Thanks Mike for the proposal layout!
It sounds good to me :)
Hi Pedro,
I went through the email chain again, basically these are 2 of your main concerns (correct me if I'm wrong):
1. a good idea to at least ditch that specific copy (current FDT in Embedded Pkg) for a git submodule.
2. Rework to remove/reduce libc Implementation as there is a problem with both libc fragments and compiler intrinsic
fragments all over edk2. Should unify standards between crypto, libfdt, etc, could we try here
I guess Mike has provided a plan to answer your first question, and the 2nd question would require a broader discussion
with a few key owners.
So it seems like we could get the current patchset from Benny Lin in for now? Any minor clean up needed for the current
patch?
No.
3. Lots of questions and comments on the actual patch set regarding
the quality of the libc implementation. Which should be fixed,
regardless of centralizing a libc implementation.
Also questions on the FdtLib itself (why are we wrapping pure
libfdt functions with FluffyIdentifierNames and SCARY_TYPEDEFS?). This is done to accommodate potential future changes to the submodule APIs and types. The wrapper lib is a common practice in EDK II use of submodules. The calling code can use the APIs/types defined in EDK II lib class. If there are changes to the submodule APIs/types, we only have to update one location. Can also provide flexibility to move to a different submodule or implementation if there is a better option in the future. Look at CryptoPkg CryptoLibs as an example. We have an implementation that layers on top of openssl. We also have experiments looking at mbedtls. No changes to calling code that uses CrytoLibs. I also sent out an RFC for a central libc for GCC/clang based
toolchains, which should cover the libc usage of libfdt. Asked for
testing, got ignored. I provided feedback on your work on a central libc and asked if you are able to test the libc uses currently checked into edk2. Are you able to help with that testing?
So a NAK from me, in its current state.
--
Pedro
|
|
|
