Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36208 )
Change subject: security/vboot: Add vboot callbacks to support EC software sync ......................................................................
Patch Set 2:
(9 comments)
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... File src/security/vboot/sync_ec.c:
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 3: * : * Copyright 2019 Google LLC : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
There's a cleanup effort going on that will also replace the text here with a simple SPDX header. […]
I talked with Martin and he said it's ok to just use this w/o the 'Copyright' line for now.
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 41: EC_LPC_HOST_PACKET_SIZE
Yes, on x86 the stack is in CAR. […]
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 50: vboot_sync_ec
Not yet.
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 52: static uint8_t temp_workbuf[VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE];
See below.
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 64: memcpy(ctx.workbuf, (void *)p, wd->buffer_size);
See bug for timeline.
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 68: vbnv_init(ctx.nvdata);
Let's move schedule discussions to the bug (b/112198832).
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 331: vb2_error_t VbExEcVbootDone(int in_recovery)
Yes, but the intention there was to run that check as soon as the EC is in RW. […]
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 336: /* Unsupported */
I don't think the goal here should be to permanently leave vestigial parts of EC software sync runni […]
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 398: vb2_error_t VbExEcGetExpectedImage(int devidx, enum VbSelectFirmware_t select,
I'll look into that.
Done