aarch64-elf nm doesn't support '--no-weak'. Replace the 'nm --no-weak'
call with 'grep' with "[TDRCB]" pattern to collect the non-weak
symbols.
Change-Id: I19195034b31f39086946b7e5ee15317d6f5dd880
Signed-off-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/91566
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Define CB_BOOT_MODE_RTC_WAKE and LB_BOOT_MODE_RTC_WAKE in the
coreboot table headers. This allows the firmware to communicate
to the payload (such as depthcharge) that the device started
up due to a Real-Time Clock alarm.
Synchronize the change across:
- payloads/libpayload/include/coreboot_tables.h
- src/commonlib/include/commonlib/coreboot_tables.h
BUG=b:493760057
BRANCH=none
TEST=Build coreboot and libpayload. Verify that the new boot mode
is accessible in the payload.
Change-Id: I8f5e118e6965f29498ab5bb46e153bc6d24bc116
Signed-off-by: Subrata Banik <subratabanik@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/91764
Reviewed-by: Kapil Porwal <kapilporwal@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Add CONFIG_LP_DEFCONFIG_OVERRIDE_X64 flag to select default 64-bit
config file in payloads/libpayload/configs directory.
This is used in standalone environment. The existing libpayload
deconfig file is for boards with 32-bit format and deconfig_64
file is added for 64-bit without adding specific
board.[board name] file in libpayload.
BUG=none
TEST=Build with this new flag and check that the libpayload and
depthcharge.elf are built in 64-bit format.
Signed-off-by: Cliff Huang <cliff.huang@intel.com>
Change-Id: Iac07cf9e3c11e49955c69553407be76ef4f8c060
Reviewed-on: https://review.coreboot.org/c/coreboot/+/84107
Reviewed-by: Jérémy Compostella <jeremy.compostella@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kim, Wonkyu <wonkyu.kim@intel.com>
Move LZ4 magic number definitions to public header file so that it can
be used in the payload. Also rename LEGACY_MAGICNUMBER to
LZ4_LEGACY_MAGICNUMBER for clarity when using from the payload.
BUG=None
TEST=Build Hylia BIOS image and boot to OS/UI.
Change-Id: Ief180105ec3fa7abf1013d0c5408aa96edde681b
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/91152
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Currently, the system does not explicitly distinguish between a low
battery boot with a charger and one without. This is critical for
deciding whether to allow the boot to proceed or to protect the
battery.
This patch:
1. Re-introduces LB_BOOT_MODE_LOW_BATTERY to represent a critical
battery state without a charger present.
2. Refactors set_boot_mode() to accommodate off-mode charging and
evaluating battery health (low-batter w/ or w/o charger present)..
TEST=Verified on Bluey:
- Boot with charger + low battery enters LOW_BATTERY_CHARGING.
- Boot without charger + low battery enters LOW_BATTERY..
- Boot with normal battery enters NORMAL mode.
Change-Id: I2c9fa7eb61d1bbd6f9379c81577aee53ab6a0761
Signed-off-by: Subrata Banik <subratabanik@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90849
Reviewed-by: Kapil Porwal <kapilporwal@google.com>
Reviewed-by: Jayvik Desai <jayvik@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The current LB_BOOT_MODE_LOW_BATTERY actually implies a state where the
battery is below the critical threshold but a charger is attached,
allowing the system to boot into a charging-only or limited state.
Update the enum name to LB_BOOT_MODE_LOW_BATTERY_CHARGING across
coreboot tables and libpayload to better reflect this hardware state.
Changes:
- Rename boot mode enums in commonlib and libpayload.
- Update bluey mainboard logic to use the more descriptive name.
- Refactor is_low_power_boot() to is_low_power_boot_with_charger()
to improve code readability.
- Ensure the charger-present condition is explicitly checked in
romstage when setting the boot mode.
TEST=Verify bluey boots into off-mode charging and low-battery
charging modes correctly.
Change-Id: I2478c7519c781a8b5af78445899b7f9bf412cf42
Signed-off-by: Subrata Banik <subratabanik@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90845
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kapil Porwal <kapilporwal@google.com>
Reviewed-by: Jayvik Desai <jayvik@google.com>
Using this, the compiler is going to check if the printf formatting is
correct for our printk messages.
Since we already have the macro, might as well use it.
Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>
Change-Id: I247f24ed64c2be7fc411f5e2fdd38715698bc4e3
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90829
Reviewed-by: Julius Werner <jwerner@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Introduce the lb_panel_poweroff/cb_panel_poweroff structs to pass the
panel power-off commands from coreboot to payloads.
Also add mipi_panel_parse_commands() to libpayload libc, so that
payloads can utilize it to parse the power-off commands.
BUG=b:474187570
TEST=emerge-jedi coreboot libpayload
BRANCH=skywalker
Change-Id: I652178c8075a1f3aee356502e682ef9a4f3d1cf8
Signed-off-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90738
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yidi Lin <yidilin@google.com>
Reviewed-by: Chen-Tsung Hsieh <chentsung@google.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
CB_MEM_TAG is considered as an reserved memory. But we want to mange
this region in the payload and release this region when MTE is inactive.
Add CB_MEM_TAG memory range to usedmem_ranges to get a page table
mapping and mark it as used.
BUG=b:438666196,b:474306129
TEST=Check the tag region is wiped out in DEV mode.
Change-Id: I498407cbd44b1cc70ef769a63b8e40665ea67b28
Signed-off-by: Yidi Lin <yidilin@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90702
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Add bulk_timeout() callback to USB controllers that allows to issue bulk
transaction with configurable timeout. This allows to peek if there is
any incoming data from USB device without needing to wait 5 seconds if
there is no data.
'finalize' argument is omitted in bulk_timeout(), because any recent
controller doesn't use it in bulk() method anyway. For OHCI and UHCI,
which are only controllers using 'finalize', issuing bulk_timeout() with
USB_MAX_PROCESSING_TIME_US timeout is the same as issuing bulk() with
'finalize' set to zero.
Change-Id: I82dbe307b566e4fc6cca314924168f7ad677efe7
Signed-off-by: Tomasz Michalec <tmichalec@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90043
Reviewed-by: Julius Werner <jwerner@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
This patch adds config ARCH_RISCV_RV64 to support build of riscv64
payloads. New files under arch/riscv contain:
- Basic ldscript and payload entry point.
- Functions for riscv64 io and cache operations.
- Default timer code based on mtime delegation.
- Default cb_header_ptr passing with device tree to payload.
Change-Id: Ieb3d456d5edda87a3a4886ccfc17a7824c630427
Signed-off-by: Ziang Wang <wangziang.ok@bytedance.com>
Signed-off-by: Dong Wei <weidong.wd@bytedance.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/89646
Reviewed-by: Maximilian Brune <maximilian.brune@9elements.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Update the values of CB_MEM_TAG and LB_MEM_TAG from 7 to 17. This change
is necessary to avoid conflicts with the ACPI System Address Map
Interfaces specification.
Change-Id: I802cd724b8f330a9f814fb952ab824cfc23c0e67
Signed-off-by: Yidi Lin <yidilin@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90676
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Some boards require exposing more GPIOs to the payload via sysinfo.
Increase the maximum number of supported GPIO entries accordingly.
For example Padme will pass 9 GPIOs to payload.
BUG=b:461907110
TEST=Boot firmware and check GPIO counts in sysinfo.
BRANCH=None
Change-Id: Idb90896b82b56f65c3d46e53b36238717de0a6d1
Signed-off-by: Yang Wu <wuyang5@huaqin.corp-partner.google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90654
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-by: Yidi Lin <yidilin@google.com>
Add new memory type CB_MEM_TAG to coreboot_tables.h. This definition was
missing when CB:90470 was instroduced.
Change-Id: I76990706649bc1a4e45478760446dff40e871d77
Signed-off-by: Yidi Lin <yidilin@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90612
Reviewed-by: Chen-Tsung Hsieh <chentsung@google.com>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Commit de4148888c ("tests: Disable generation of lcov HTML") only
disabled the HTML generation for the coreboot tests, but not for
libpayload tests. So do it here as well.
Change-Id: I35458345c81de8b9936a17bb6fb5670b29a6d05e
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90608
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
That parameter was mistakenly added here while trying to silent lcov
complaints. Drop it again since it's not supported here.
Change-Id: I77bed4ebb7d3ef6daea39c812bacfcad7a72aee7
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90597
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Newer gcov/lcov versions shipped in CI container images throw a warning
and thus cause the CI to fail. It's unclear how to fix this warning at
the moment, but gcov isn't critical anyway. So disable this specific
warning for now, so that we can roll out new CI images.
Excluding file '/home/coreboot/node-root/workspace/test_coreboot/payloads/libpayload/tests/libcbfs/cbfs-lookup-test.c'
lcov: WARNING: (inconsistent) /home/coreboot/node-root/workspace/test_coreboot/payloads/libpayload/libcbfs/cbfs.c:79: unexecuted block on non-branch line with non-zero hit count. Use "geninfo --rc geninfo_unexecuted_blocks=1 to set count to zero.
(use "lcov --ignore-errors inconsistent,inconsistent ..." to suppress this warning)
Excluding file '/home/coreboot/node-root/workspace/test_coreboot/payloads/libpayload/tests/libcbfs/cbfs-lookup-test.c'
[snip]
Message summary:
1 warning message:
inconsistent: 1
genhtml: ERROR: (corrupt) unable to read trace file 'build/coverage/tests.info': genhtml: ERROR: (inconsistent) "/home/coreboot/node-root/workspace/test_coreboot/payloads/libpayload/libcbfs/cbfs.c":77: function 'cbfs_unmap' is not hit but line 79 is.
To skip consistency checks, see the 'check_data_consistency' section in man lcovrc(5).
(use "genhtml --ignore-errors inconsistent ..." to bypass this error)
(use "genhtml --ignore-errors corrupt ..." to bypass this error)
make[1]: *** [tests/Makefile.mk:277: coverage-report] Error 1
make: *** [util/testing/Makefile.mk:103: what-jenkins-does] Error 2
Change-Id: I2c52c53fbe856a8bca062f34c576fdfda3818f2b
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90554
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
GCC generates correct code for __builtin_bswapXX() on all architectures,
including ArmV4. It seems that whatever bug caused this to not work back
in commit 879ea7fce8 ("endian: Replace explicit byte swapping with
compiler builtin") has been fixed now. We can eliminate the swabXX()
functions and simplify the code.
All instances that had been calling these functions directly should have
been using real endianness conversions anyway.
Change-Id: I19713fd009aa5c0e01c4a42e0cf012364d6bed60
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90438
Reviewed-by: Maximilian Brune <maximilian.brune@9elements.com>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Frans Hendriks <fhendriks@eltan.com>
sysinfo contains both CBMEM_ID_MEMINFO and CBMEM_ID_MEM_CHIP.
This new function allows to easily retrieve the amount of memory the
system has based on the mentioned CBMEM entries structures.
BUG=b:450374306
TEST=Checked on Google/Brya/Omnigul (x86) - MEMINFO
TEST=Checked on Google/Rauru/Navi (ARM64) - MEM_CHIP
Change-Id: Iaf69c54dfb3d2eaffbccacee8bee3be425b78a3f
Signed-off-by: Jakub Czapiga <czapiga@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90430
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
We've accumulated a number of endianness-handling and related macros
that are duplicated between coreboot and libpayload. This patch reduces
duplication by merging them into a commonlib header. This has the added
side-benefit of bringing the coreboot implementation of beXXenc/dec()
functions to libpayload, which lead to better code generation by
avoiding https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92716.
Keep the htobell()-style functions in libpayload only since they're not
needed in coreboot and not preferred. Keep the cpu_to_beXX()-style
functions in coreboot only -- maybe we should deprecate those
eventually.
This patch is explicitly copying and relicensing some of the code I
originally added as GPLv2 in commit e8e92d60c4 ("endian.h: Add
be32dec/be32enc family of functions") to BSD-3.
Change-Id: I5eb83d44a98b3aa59bba65b8e22df668874d2668
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90308
Reviewed-by: Maximilian Brune <maximilian.brune@9elements.com>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Six years ago we had a problem with not getting SError exceptions in
coreboot, and we fixed it by setting the right SCR_EL3 bits to force
exception routing to EL3 (commit bb345abbfc ("arm64: Correctly unmask
asynchronous SError interrupts")). Turns out that we have the same
problem in the payload but we never fixed it there. EL2 exception
routing works differently, so in order to achieve the same effect here
we can either enable the HCR_EL2 AMO, FMO and IMO bits (respectively),
or we can just enable the TGE bit which traps everything. This patch
chooses the latter, and it also ensures that the PSTATE exception
masking (DAIF) bits are in the expected state (although they usually
already are).
This state will persist after handoff to the kernel or chained payload,
and will prevent transition into EL1 if not cleared first. This should
be fine since any code taking control in EL2 should be expected to
correctly reintialize HCR_EL2 before handing off into EL1 (the Linux
kernel has always reinitialized this very early after its entry point).
If any selfboot() payloads are broken after this change, the payload
should be fixed to reinitialize HCR_EL2 to 0 (or desired value).
Change-Id: I339eded5a5344b5753c94be82e4f1e52e00b39f4
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90127
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Yidi Lin <yidilin@google.com>
Reviewed-by: Chen-Tsung Hsieh <chentsung@google.com>
Extend the lb_framebuffer_flags struct to include one more bitfield
'has_dual_pipe' to indicate dual pipe support.
TEST=firmware display ok, in depthcharge with https://crrev.com/c/7129839
BRANCH=none
BUG=b:424782827
Change-Id: I082be80b4606090ed219820a407d80d9f429ea7e
Signed-off-by: Nancy Lin <nancy.lin@mediatek.corp-partner.google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/90038
Reviewed-by: Yidi Lin <yidilin@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Linux kernel images from upstream tree are compressed using legacy LZ4
format and not the modern LZ4 format. Hence support legacy LZ4
compression format to decompress and boot upstream Linux kernel images.
Also add unit test case to verify the currently supported LZ4
compression format as well as legacy LZ4 compression format.
References:
* https://github.com/lz4/lz4
BUG=None
TEST=make tests/liblz4/lz4-test
[==========] tests_liblz4_lz4-test(tests): Running 4 test(s).
[ RUN ] test_lz4
[ OK ] test_lz4
[ RUN ] test_lz4_partial_decompression
[ OK ] test_lz4_partial_decompression
[ RUN ] test_legacy_lz4
[ OK ] test_legacy_lz4
[ RUN ] test_legacy_lz4_partial_decompression
[ OK ] test_legacy_lz4_partial_decompression
[==========] tests_liblz4_lz4-test(tests): 4 test(s) run.
[ PASSED ] 4 test(s).
Change-Id: I7e3d407fc313e0937fd8d327840534de60d8c625
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/89854
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
In the arm64 exception handler in libpayload, we use the banked
exception stack pointer (SP_EL2, as opposed to the normal SP_EL0) not as
a normal stack pointer, but simply as a pointer to the exception_state
struct. This makes it easy to dump all registers into that struct on
context switch. We then immediately switch back to SP_EL0.
Yet, even though it is not really a stack for us, the aarch64
architecture still requires that SP_EL2 is 16 byte aligned at function
boundaries. If the exception_state struct is not thus aligned,
exceptions are broken. (I don't know why nobody ever hit this before,
but I hit it now while trying to pull in zstd code. I guess we just
don't have unaligned BSS entries that often and simply got lucky for a
while. 3 hours wasted on debugging. :( )
Change-Id: Id19184656fb9da68fe4bfdbc240c0c25b9d24cd6
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/89926
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
This patch fixes a number of rare edge cases in handling the precision
argument in printf. The existing printf implementation used 0 as the
sentinel value for "no precision provided", which makes sense for
integers (where 0 precision has the same effect as no precision, since
in both cases no extra zeroes will be added to the front). However, for
strings it can make an important difference, since callers may expect
that they can use `printf("%.*s", len, str)` to guarantee that `str`
doesn't get dereferenced when `len` is 0. Therefore, change the
implementation so that negative values are used to represent "no
precision provided", and 0 is a legitimate value.
print_string() also had the problem that it called strlen() on the
string before even evaluating the precision. That of course defeats the
purpose of the common "%.*s" pattern to access unterminated strings.
This patch fixes the problem.
Finally, this patch slightly modifies the behavior when printing a NULL
pointer as a string, to make sure width and precision values are still
taken into account in that case, and to change from `(NULL)` to `(null)`
to match the behavior in glibc.
Change-Id: I787c18e1d33006842cf758aeb87710f80f0e5a40
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/89837
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This reverts commit 4f13f72dbc ("libpayload: Define UCHAR_MAX/CHAR_MIN/CHAR_MAX").
Reason for revert: char can be either signed or unsigned. If it's
unsigned, then CHAR_MIN would be incorrectly defined as 128.
Change-Id: Id49ddfff2d91029dc191b8b64e8e3f325ad0a462
Signed-off-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/89469
Reviewed-by: Yidi Lin <yidilin@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This commit adds a new coreboot table, CB_TAG_BOOT_MODE, to pass
platform boot mode information to the payload.
The new table defines flags for low-battery mode and off-mode charging,
which are essential for a payload to properly initialize the charger
driver.
The cb_parse_boot_mode function is added to read this information, and
the sysinfo_t structure is updated to store the parsed boot mode data.
This ensures that the payload can accurately determine the system's
power state at boot and payload operations are also in sync with the
boot firmware.
The following scenarios were tested and verified:
Scenario 1: Low-battery, no charger
- coreboot detects the low-battery state and performs an immediate
shutdown after displaying the low-battery splash screen.
Scenario 2: Low-battery, charger attached
- coreboot detects the low-battery state but continues booting because
a charger is present. The payload receives the low-battery
information (using the same source as coreboot) and correctly
initiates the charging process.
Scenario 3: Off-mode charging
- The system boots directly from an S5 state due to a charger being
plugged in. coreboot detects the off-mode state, skips the firmware
splash screen, and hands off control to the payload, which then
starts charging.
Scenario 4: Normal boot
- The system boots without any low-battery or off-mode conditions.
coreboot and the payload both detect a normal boot (using the same
information), bypass charging initialization, and proceed to boot the
operating system.
TEST=Able to build and boot on google/quenbi device and verify the boot
mode flag is correctly passed.
Change-Id: Iec25c6fdfcdc5ea7c397d2430ac7b545e1e068f2
Signed-off-by: Subrata Banik <subratabanik@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/89015
Reviewed-by: Kapil Porwal <kapilporwal@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Our strsep() function is slightly incorrect in that it leaves the
`stringp` pointer pointing to the terminating NUL byte after parsing the
last token. The man page for official implementations says:
> In case no delimiter was found, the token is taken to be the entire
> string *stringp, and *stringp is made NULL.
This doesn't affect things in practice much because we also
(incorrectly) return NULL when called with `**stringp == '\0'`, meaning
the usual pattern of calling `strsep()` in a row without checking
results first still works when there are less tokens than expected,
since we terminate early from that case instead. But it does break the
edge cases where the caller wants to check if there were extra bytes
beyond the last token (`stringp == NULL`), and where we call `strsep()`
on a pointer pointing directly to a terminating NUL byte already
(supposed to return an empty string but our implementation actually
returns NULL). It doesn't look like these edge cases occur anywhere in
current libpayload or depthcharge code.
This patch fixes the issue and also adds a unit test to ensure it
remains correct in the future. (Also move the definition of the `errno`
variable from lib.c into string.c, because `perror()` in string.c is the
only function that actually needs that, and the crazy linker error you
get when only linking one but not the other into a test will waste you
half an hour to figure out.)
Change-Id: I610b5117710c110bcba4fac2a0bb6c13f4f8d046
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/88729
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This change addresses GCC-15 behavior where {0} union initializers only
clear the first member, leaving padding bits uninitialized. The new {}
initializer ensures full union clearing as required by C23.
Change-Id: I1d9b063d8bdd3d2f0b0f67e6c20eb484ff6a5cc5
Signed-off-by: Elyes Haouas <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/88732
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yidi Lin <yidilin@google.com>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
When returning from cbmem_console_snapshot() because of a corrupted
console structure, the memory that was just allocated was not being
freed as it "should be".
BUG=CID 1419477
Signed-off-by: Martin Roth <gaumless@gmail.com>
Change-Id: I85370fb49c9ef9a00cd2ea516fa80c9e152c9b48
Reviewed-on: https://review.coreboot.org/c/coreboot/+/88334
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Elyes Haouas <ehaouas@noos.fr>
This commit extends libpayload's understanding of SPI flash devices by
adding a flags field to both struct cb_spi_flash and struct
sysinfo_t.spi_flash.
The new CB_SPI_FLASH_FLAG_IN_4BYTE_ADDR_MODE flag will be populated
from the coreboot table's lb_spi_flash entry. This allows payloads to
reliably determine if the SPI flash is currently configured for 4-byte
addressing, enabling more robust flash operations without needing to
re-probe or re-enforce the mode.
Note: `erase_cmd` type was changed from uint32_t to uint8_t. This is
because only the lowest byte of the original uint32_t was ever used.
The change ensures proper sizing, maintains compatibility with older
coreboot tables, and makes the remaining space available for new fields.
BUG=b:417900125
TEST=Able to build google/bluey.
Change-Id: I101a50f899e82e9412024a049a9df59c5813313a
Signed-off-by: Subrata Banik <subratabanik@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/88182
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This reverts commit 4199351c1b which
originally reverted aedc177f00.
Reason for revert: CB:88063 fixed the bug that this patch exposed.
Change-Id: Ic7a798b4b9236b8c0c7ad8568562d11071ae96a9
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/88097
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
We use mmu_ranges to track the list of memory ranges and their types for
MMU initialization. We also keep track of used memory ranges in
usedmem_ranges, to avoid them from being re-allocated in
mmu_alloc_range().
The problem is, the CBMEM range (CB_MEM_TABLE) is added to mmu_ranges,
but is never marked as "used" in usedmem_ranges. This potentially causes
any allocation (for example the framebuffer) to overlap with CBMEM. This
issue is observed when DMA_DEFAULT_SIZE is reduced from 32MB to 1MB [1].
Prior to that change, because there isn't enough space above the
coreboot table (with the 4GB upper limit) to fit the 32MB requested
region, the DMA heap is always allocated *below* the coreboot table. And
because the coreboot table is usually the lowest within CBMEM, the DMA
heap region is allocated *below* the whole CBMEM, which happens the
avoid the issue.
Fix the bug by adding CB_MEM_TABLE ranges to usedmem_ranges. The ranges
in usedmem_ranges don't need to be combined because they are not for MMU
initialization (and there's only one CB_MEM_TABLE range).
[1] commit aedc177f00 ("libpayload: arm64: Reduce DMA allocator space to 1MB")
BUG=b:424107889
TEST=emerge-skywalker libpayload
BRANCH=none
Change-Id: Ie9ecafc17546e524253c60ab684ec10ff3495998
Signed-off-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/88063
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Bartłomiej Grzesik <bgrzesik@google.com>
Reviewed-by: Jakub "Kuba" Czapiga <czapiga@google.com>
This reverts commit aedc177f00.
Reason for revert: With this change depthchange clears parts of cbmem on Google/Corsola when display is cleared.
BUG=b:424107889
Change-Id: I6cc21693ddcaed59e41e333b773e0baeb29d3b40
Signed-off-by: Bartłomiej Grzesik <bgrzesik@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/88051
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
I got 'nm' usage error when i built coreinfo payload.
nm: unrecognized option '--no-weak'
it seems that this is occurred by using nm for host, not for coreboot.
So, I replace nm with $(NM)
Change-Id: I0a0a04b351c9131b1238e8cc7e63e396820494d9
Signed-off-by: NyeonWoo Kim <knw0507@naver.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87906
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Unit tests should always use a mock ndelay() defined within each test.
Therefore, select ARCH_HAS_NDELAY for ARCH_MOCK.
Change-Id: I2680e37034d4d13058b3e778d72e63fc6ed18313
Signed-off-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87855
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Weak symbols don't work as expected when you try to override them from
within the same static library. Static libraries are just archives of
individual objects, and when the linker tries to resolve a symbol
against it it simply uses the implementation from the first object that
has one, weak or not. It does not search through all remaining objects
to see if there's also a strong implementation.
We've had multiple cases in libpayload where builds were incorrectly
using the default implementation rather than an optimized arch-specific
implementation for years due to this issue. To prevent it from
recurring, this patch adds some postprocessing script to the Makefile
that checks for this situation and makes the build fail if it creeps in
again.
Change-Id: I9fcbc9b873901d126322b12954c349c08300369f
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87777
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
On arm64 device libpayload reserves 32MB of space for the DMA allocator.
DMA allocators are usually just used for small bounce buffers or DMA
descriptors for SPI, I2C or USB transfers, nothing that should get
anywhere near the size of megabytes.
Presumably the original number was just made arbitrarily large because
it didn't matter. But more recently we have had security applications
(guarding secrets that get received over SPI/I2C from firmware and must
not be visible to the OS after handoff) that made us want to erase the
entire DMA heap just to be sure no driver left a copy of any secret
lying around there. This means the size is no longer fully harmless
because erasing a larger heap takes more time.
Change the default to 1MB which should still be more than more than
enough for any real applications, but should bring the time required to
erase it back into negligible territory.
BUG=b:418942992
TEST=Booted Trogdor from USB.
Change-Id: Id56486203c512d7ff08909cac1a016adc44d8e68
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87780
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Weak symbols don't work as expected when you want to override them from
within the same static library. This patch changes the arch_ndelay()
function so that instead of having a weak generic implementation, the
choice between generic implementation and an arch-specific override is
explicitly made by Kconfig. Let's also drop the "arch_" prefix and just
call this ndelay().
Change-Id: Ie4fe2734e0683fa3537e2ebcabfe067e7499463a
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87776
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Reviewed-by: Jakub "Kuba" Czapiga <czapiga@google.com>
Our mechanism to override the default (pure C) memory function
implementations (memset, memcpy, memmove) with architecture-specific
optimized assembly versions doesn't actually work: it turns out that
weak functions don't work as you'd naively expect when you pack them
together with a strong definition from a different object into a static
library. When a linker tries to resolve a symbol from a static library,
it just picks the first one it finds, even if it is weak. It doesn't
evaluate all objects in the library to see if there are other strong
definitions.
To fix this, this patch gets rid of the weak symbols and uses Kconfigs
instead. It adds an optimized memmove() implementation for x86 because
that makes things easier (then all architectures either override all
three functions or none of them). Also remove memcmp() from the
functions that can be overridden for now because nobody ever needed that
anyway.
Change-Id: Iedf9898247f1999e56fde3233fad8b7cb36b1269
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87766
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The existing lanczos_weight() implementation naively follows the purely
mathematical definition for the `x == 0` special case. However, the
point of defining that special case is obviously to prevent division by
zero in the general case formula. Unfortunately we are still doing some
multiplications with `x` before we get to the division step, and our
fpmath library loses precision during multiplication. This can lead to
edge cases where `x` is not zero but `x_times_pi` later ends up being 0,
which causes the division to throw an exception after all. (I guess
we've just been lucky to not see this case in practice for now... it
requires the output pixel coordinate to be extremely close to but not
quite on the next input pixel coordinate, which may be rare in practice
with our scaling algorithms.)
This patch fixes the issue by implementing the special case later and
checking if `x_times_pi` is zero instead. Note that as long as we pass
this check, we can be confident that the division cannot fail even
though fpdiv() also truncates the divisor: this is because `x_times_pi`
was calculated from an fpmul() call with the constant fppi(), which has
34 significant bits. Even if x is the smallest possible non-zero value
after scaling for multiplication, the result `x_times_pi` must still
have 18 significant bits. That means it can be scaled down a further 16
bits for division without becoming zero.
Also add a simple unit test forcing exactly this condition to ensure the
code will not regress.
Change-Id: I2f212ee5df38252e97ec55aba3d2d25320c4b102
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87532
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-by: Jakub "Kuba" Czapiga <czapiga@google.com>
Disks larger than 2TB (technically disks with more than 2^32 blocks, but
for the common block size of 512 that comes out to 2TB) cannot represent
their full amount of blocks in the SCSI READ_CAPACITY(10) command used
by libpayload's USB mass storage driver. The entire driver isn't written
to support block addresses larger than 32 bits anyway.
The SCSI command has been designed in a clever way so that devices are
supposed to return the maximum value (0xffffffff) if the actual value
doesn't fit. However, our code adds one to the value (because it is
actually the address of the last block, but we want to know the number
of blocks). This makes it overflow back to 0 which is not great.
This patch caps the result before incrementing it so that the overflow
cannot occur, allowing us to at least access the first 2TB of super
large USB sticks.
Change-Id: Ic445923b7d588c4f523c4ed95a06291bc1969261
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87506
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Add an allocation of an empty buffer for the Android protected virtual
machine firmware within cbmem. The buffer will be filled by the payload
and the purpose is to just reserve the memory. cbmem is used to make
sure that the region won't overlap with other reserved regions
or device regions.
BUG=b:354045389
BUG=b:359340876
TEST=depthcharge receives the buffer through lib_sysinfo
BRANCH=main
Change-Id: I48efc033ac0f5fbfcf3a52fabf40be016cd4c6f7
Signed-off-by: Bartłomiej Grzesik <bgrzesik@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87107
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jakub "Kuba" Czapiga <czapiga@google.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
libpayload stdint.h only supports typedefs for datatypes of exact
bits. This makes libpayload less flexible to support libraries
that reference different data types.
Add fast data types in types.h.
BUG=b:386913035
Change-Id: Ie9197866ae9b6c27d3f26c11d8409ecb90321c74
Signed-off-by: Masa Nakura <nakura@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/86632
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-by: Hsuan-ting Chen <roccochen@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The x86 (AMD and Intel) spec defines it as Page-Map Level-4 Entry.
It is annoying when searching for the wrong abbreviation in the spec so
fix it everywhere it occurs.
source: Intel 64 spec April 2022 and AMD64 spec April 2024.
Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>
Change-Id: I730235beea69b3720f080bbade083c2eeed26587
Reviewed-on: https://review.coreboot.org/c/coreboot/+/86587
Reviewed-by: Jérémy Compostella <jeremy.compostella@intel.com>
Reviewed-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Benjamin Doron <benjamin.doron00@gmail.com>
Reviewed-by: Andy Ebrahiem <ahmet.ebrahiem@9elements.com>
