I believe we can ignore Jenkins' warnings on memlayout.h?

Currently we are getting some errors like this on compile:
In file included from src/arch/x86/memlayout.ld:53:
src/arch/x86/car.ld:32:1: error: macro "ALIGN" requires 2 arguments, but only 1 given
  VBOOT2_WORK(., 12K)
 ^~~~~~~~~~~~

These will be fixed once this CL is committed and upstreamed (downstreamed?):
https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1480751

View Change

7 comments:

• File src/include/memlayout.h:

  • Patch Set #2, Line 162: asserted minimum

    This is probably not relevant any more.

    Done

  • Patch Set #2, Line 164: REGION(vboot2_work, addr, CONFIG_VBOOT_WORKING_DATA_SIZE, 16)

    I'd prefer if we stick to the model that memlayout sections always contain address and size, even if […]

    Yeah, I was on the fence about this. I have switched it back to including a size argument. Hopefully that should be more in line with the other TTB area you mentioned.

    (But I've also changed all the 16K instances to 12K. Thus there are a few 4K gaps now. Is that important enough to mention in the ldscript?)

• File src/security/vboot/Kconfig:

  • Patch Set #2, Line 95: VBOOT_WORKING_DATA_SIZE

    This shouldn't be a Kconfig anyway, because it's not intended to change. […]

    I came up with a slightly different strategy to avoid having to directly include a "vboot constants" file:
    https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1480751
    Since we prefer that all vboot use goes through vb2_api.h, I think this is a better approach.
    (Of course now we have to wait for cros vboot commit, import upstream to coreboot, uprev vboot repo, yadda yadda.)

• File src/security/vboot/Makefile.inc:

  • Patch Set #2, Line 25: # When VBOOT_STARTS_IN_ROMSTAGE is selected, DRAM is already up by the time

    Can we push this change as a separate CL just to keep things clearer?

    Moved to https://review.coreboot.org/c/coreboot/+/31541

  • Patch Set #2, Line 28: verstage-generic-ccopts += -D__PRE_RAM__

    Huh, funny, I thought we had this already... […]

    Ack

• File src/security/vboot/vboot_loader.c:

  • Patch Set #1, Line 37: ==

    If we expect this CONFIG_VBOOT_WORKING_DATA_SIZE to always be same as VB2_WORKBUF_RECOMMENDED_SIZE, […]

    Done

• File src/soc/cavium/cn81xx/include/soc/memlayout.ld:

  • Patch Set #1, Line 37: )

    If we end up increasing the global config value, then the macros for REGION already take care of com […]

    As per later comment by jwerner, we decide to include the number here after all.

To view, visit change 31474. To unsubscribe, or for help writing mail filters, visit settings.