Heinrich,
On Fri, Nov 05, 2021 at 06:35:08PM +0900, AKASHI Takahiro wrote:
On Fri, Nov 05, 2021 at 11:35:00AM +0900, AKASHI Takahiro wrote:
On Thu, Nov 04, 2021 at 08:02:40PM -0600, Simon Glass wrote:
Hi Takahiro,
On Thu, 4 Nov 2021 at 19:04, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
Hi, Simon,
On Thu, Nov 04, 2021 at 09:11:59AM -0600, Simon Glass wrote:
Hi Mark,
On Thu, 4 Nov 2021 at 08:31, Mark Kettenis mark.kettenis@xs4all.nl wrote:
> From: Simon Glass sjg@chromium.org > Date: Wed, 3 Nov 2021 20:51:25 -0600 > > Hi Mark, > > On Tue, 2 Nov 2021 at 09:13, Mark Kettenis mark.kettenis@xs4all.nl wrote: > > > > > From: Simon Glass sjg@chromium.org > > > Date: Tue, 2 Nov 2021 08:56:50 -0600 > > > > > > Hi Takahiro, > > > > > > > > - can we just build the tool always? > > > > > > > > This is one of my questions. > > > > Why do you want to do so while there are bunch of tools that are > > > > not always built. > > > > > > Because I think all tools should be built always. It is fine if that > > > happens due to CONFIG options but we should try to avoid making it > > > complicated. > > > > Well, unless this patchset fixes things, we can't, because > > mkeficapsule doesn't build on OpenBSD. I tried looking into it, but I > > can't figure out how this is even supposed to compile as a host tool: > > > > > > In file included from tools/mkeficapsule.c:8: > > In file included from include/malloc.h:369: > > include/linux/string.h:15:24: error: conflicting types for 'strspn' > > extern __kernel_size_t strspn(const char *,const char *); > > ^ > > /usr/include/string.h:88:9: note: previous declaration is here > > size_t strspn(const char *, const char *); > > My guess is that linux/string.h should not be included, or perhaps > __kernel_size_t should be defined to size_t. > > I doubt it would take an age to figure out, with a bit of fiddling.
Well, I think the problem is quite fundamental. Indeed I agree that linux/string.h shouldn't be included. It gets pulled in because the tools include <malloc.h>. Modern software really shouldn't include that header anymore, and we removed it in OpenBSD some time ago. But even with that fixed, things break since the same header gets pulled in from <efi.h>.
Redefining __kernel_size_t doesn't provide a way out:
tools/mkeficapsule.c:23:16: error: typedef redefinition with different types ('size_t' (aka 'unsigned long') vs 'unsigned int') typedef size_t __kernel_size_t; ^ ./arch/arm/include/asm/posix_types.h:37:23: note: previous definition is here typedef unsigned int __kernel_size_t; ^
This is on an amd64 host, so "unsigned int" clearly is the wrong type for size_t.
The fundamental problem seems to be that <efi.h> isn't safe to include in a "host" tool because it includes "target" headers that accidentally resolve to "system" headers on Linux systems.
Maybe Takahiro or Heinrich have an idea how to fix that? But in the meantime it would be good if building this tool would remain optional.
Yes let's ask them to fix that as I agree this sounds wrong. We have several efi headers so perhaps just need to have the right stuff in each.
As far as I know, you initially introduced efi.h and efi_api.h. What is your intent to have the two?
I think that efi_api.h contains definitions and interfaces defined in UEFI specification for building EFI application/modules, hence I believe that it should be target-independent. Right?
But it *includes* efi.h which also contains some definitions defined in UEFI specification, while efi.h is only for U-Boot as UEFI application.
I suspect that is the root cause.
Yes I think you are right.
Or should we thoroughly use linux headers like "efi/efi.h" in this tool?
Well either way, we need host builds to not include U-Boot headers.
Yeah, but there are still lots of host tools which include U-Boot headers. In addition, I'm not quite sure whether *generic* efi headers, like efi/efi.h, are available across different host OSs.
I looked through linux's efi headers under /usr/include/efi, but they don't provide enough set of definitions to make mkeficapsule buildable. Particularly, capsule-related structure definitions are missing.
So modifying U-Boot headers and removing target-dependent coding would be more practical. (I don't know yet whether it is feasible or not.)
What's your thought here?
Or even adding host-tools-local headers would be more optimal.
I prefer this approach, though.
-Takahiro Akashi
-Takahiro Akashi
-Takahiro Akashi
- Simon
-Takahiro Akashi
It is OK to have it optional with a CONFIG, but it should be enabled by default, otherwise no one will know it is there.
Can we get the OpenBSD environment into CI or is that just too hard?
Regards, Simon