WIP: cbmem: Fix endianness on BE systems #62
Conversation
Signed-off-by: Igor Bagnucki <[email protected]>
util/cbmem/cbmem.c
Outdated
| #ifdef __OpenBSD__ | ||
| #include <sys/param.h> | ||
| #include <sys/sysctl.h> | ||
| #endif | ||
|
|
||
| #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) | ||
| #define FIX_ENDIANNESS(a) (sizeof(a) == 1 ? (a) :\ |
There was a problem hiding this comment.
This is definitely not a proper way of doing it. We cannot assume that FW endianness will always be different than OS, such approach will break on all x86. Even on PPC64 we could possibly have BE OS, so we have to know when to swap bytes and when not. This needs some redesign of coreboot tables because as of now there is no indication about endianness saved in them.
There are some functions that can convert from BE to host, like ntohl() used in dt_update_cells(), they can be used instead of bswap.
There was a problem hiding this comment.
Agree, this cannot be finally implemented like this. I have created a macro to make it easier to fix later making necessary checks in just one place instead of many.
There was a problem hiding this comment.
This needs some redesign of coreboot tables because as of now there is no indication about endianness saved in them.
Looking at sys/arch Power seems to be the first non-LE architecture, so one way of doing it could be to always use LE for tables. Another way is to use a heuristic like PR already does: header_bytes and header_checksum in wrong endianness are easily detectable. In both cases compatibility is preserved.
"LBIO" could also be reversed if structure uses BE. And if it will always LE so far, size can be made 1 or 2 bytes and the rest would become flags to indicate endianness and something else in the future.
| debug("Mapping failed %zuB of physical memory at 0x%llx.\n", | ||
| mapping->virt_size, phys - mapping->offset); | ||
| debug("Mapping failed %zuB of physical memory at 0x%llx.\n Error: %s\n", | ||
| mapping->virt_size, phys - mapping->offset, strerror(errno)); |
There was a problem hiding this comment.
Have you checked if strerror(errno) is available on BSD systems?
There was a problem hiding this comment.
Should be available as it is documented.
Additionally this function is already used in cbmem here.
There was a problem hiding this comment.
Right, but at that point it is protected with #if defined(linux). We'll have to see if CI likes it or not. It is standard C and not POSIX only, so it should work.
Signed-off-by: Igor Bagnucki <[email protected]>
Signed-off-by: Igor Bagnucki <[email protected]>
Signed-off-by: Igor Bagnucki <[email protected]>
Signed-off-by: Igor Bagnucki <[email protected]>
Signed-off-by: Igor Bagnucki <[email protected]>
Signed-off-by: Igor Bagnucki <[email protected]>
|
@SergiiDmytruk Please confirm, but AFAIK the relevant part of this is already on https://github.com/Dasharo/coreboot/commits/raptor-cs_talos-2/release and this one can be closed. |
|
Correct, #197 replaced this PR. |
Signed-off-by: Igor Bagnucki [email protected]