ec: Build should analyze shared_mem_acquire() calls |
||||
Issue descriptionThe size of shared memory is determined by the amount of free RAM left after static allocations. The size of allocations is typically fixed/known at compile time. We should be able to determine at compile time if any of the calls to shared_mem_acquire() will always fail. If so, we should fail the build. Incentive: I just broke my cr50 system by increasing a task stack size large enough that the firmware update TPM commands can't allocate a page-size buffer in shmem. Of course, I didn't notice until I tried to update from the broken firmware and discovered I couldn't.
,
Apr 9 2018
I have a very basic idea how to do this, at least for non-cr50 boards (cr50 uses malloc, so it'd be more difficult to do this analysis correctly). One possible implementation here: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1002713/1, comments welcome.
,
Apr 11 2018
This bug reminds me https://b.corp.google.com/issues/35582029, and I forget to rebase the change from glados to master: https://chromium-review.googlesource.com/#/c/chromiumos/platform/ec/+/408216
,
Apr 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/3a9b89116fd0b3e358706f9c5822d4ee944978b6 commit 3a9b89116fd0b3e358706f9c5822d4ee944978b6 Author: Nicolas Boichat <drinkcat@chromium.org> Date: Wed Apr 11 02:13:28 2018 nautilus: Shrink accelerometer FIFO to 512 entries It appears that the shared memory buffer on Nautilus starts at 200c7720 D __shared_mem_buf That's 29.78 kb into the RAM. Software sync needs 1kb, so we should be fine, expect that the last 2kb of RAM are supposed to be reserved for the "booter" (NPCX_BTRAM_SIZE). We shrink the accelerometer FIFO to 512 entries, freeing up 4kb of RAM, and increase the UART TX buffer to 4kb, to make use of 3kb of that freed up space: 200c7320 D __shared_mem_buf That's 28.78 kb into the RAM. BRANCH=poppy BUG= chromium:739771 TEST=make BOARD=nautilus -j, check that shared_mem_buf offset is < 29 kb. Change-Id: I361a439b847d31d3415f2ee66229bd32f8816e2d Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1002712 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Reviewed-by: Philip Chen <philipchen@chromium.org> Reviewed-by: Gwendal Grignou <gwendal@google.com> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/3a9b89116fd0b3e358706f9c5822d4ee944978b6/board/nautilus/board.h
,
Apr 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/c9bc9f1d2b7b623b78ebd4babfd2cce7a5c0b79f commit c9bc9f1d2b7b623b78ebd4babfd2cce7a5c0b79f Author: Nicolas Boichat <drinkcat@chromium.org> Date: Wed Apr 11 03:32:19 2018 nautilus: Shrink accelerometer FIFO to 512 entries It appears that the shared memory buffer on Nautilus starts at 200c7720 D __shared_mem_buf That's 29.78 kb into the RAM. Software sync needs 1kb, so we should be fine, expect that the last 2kb of RAM are supposed to be reserved for the "booter" (NPCX_BTRAM_SIZE). We shrink the accelerometer FIFO to 512 entries, freeing up 4kb of RAM, and increase the UART TX buffer to 4kb, to make use of 3kb of that freed up space: 200c7320 D __shared_mem_buf That's 28.78 kb into the RAM. BRANCH=poppy BUG= chromium:739771 TEST=make BOARD=nautilus -j, check that shared_mem_buf offset is < 29 kb. Change-Id: I361a439b847d31d3415f2ee66229bd32f8816e2d Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1002712 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Reviewed-by: Philip Chen <philipchen@chromium.org> Reviewed-by: Gwendal Grignou <gwendal@google.com> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> (cherry picked from commit 3a9b89116fd0b3e358706f9c5822d4ee944978b6) Reviewed-on: https://chromium-review.googlesource.com/1006315 Commit-Queue: Philip Chen <philipchen@chromium.org> Tested-by: Philip Chen <philipchen@chromium.org> [modify] https://crrev.com/c9bc9f1d2b7b623b78ebd4babfd2cce7a5c0b79f/board/nautilus/board.h
,
May 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d commit 6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d Author: Nicolas Boichat <drinkcat@chromium.org> Date: Tue May 08 03:45:54 2018 shared_mem: Assert that shared memory size is large enough We add a configuration option to set the minimum shared memory size (CONFIG_SHAREDMEM_MINIMUM_SIZE), so that the link will fail if there is not enough IRAM left. Also, we add 2 macros around shared_mem_acquire, that check, at build time, that the shared memory size is sufficient for the allocation: - SHARED_MEM_ACQUIRE_CHECK should be used instead of shared_mem_acquire, when size is known in advance. - SHARED_MEM_CHECK_SIZE should be used when only a maximum size is known. This does not account for "jump tags" that boards often add on jump from RO to RW. Luckily, RW usually does not do verification, and does not need as much shared memory. BRANCH=none BUG= chromium:739771 TEST=make buildall -j, no error Change-Id: Ic4c72938affe65fe8f8bc17ee5111c1798fc536f Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1002713 Reviewed-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-by: Randall Spangler <rspangler@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/common/vboot_hash.c [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/core/cortex-m/ec.lds.S [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/include/shared_mem.h [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/core/cortex-m0/ec.lds.S [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/common/keyboard_8042.c [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/common/vboot/common.c [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/common/host_command.c [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/include/config.h [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/common/nvmem_vars.c [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/common/rwsig.c [modify] https://crrev.com/6e8cbe40eea777aeaeffe0fa0781a0a5d1763a4d/core/nds32/ec.lds.S
,
May 8 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by rspangler@chromium.org
, Jul 6 2017