Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/19593 )
Change subject: inteltool: Add dumping of full PCR ports ......................................................................
Patch Set 1:
(3 comments)
I didn't finish reviewing this but I thought better to post it now than wait until after the weekend.
https://review.coreboot.org/#/c/19593/1/util/inteltool/Makefile File util/inteltool/Makefile:
Line 25: CFLAGS ?= -O2 -g -Wall -W -I$(top)/src/commonlib/include -Wextra -pedantic Any specific reason you're adding these flags ? And shouldn't they be added on their own commit ?
https://review.coreboot.org/#/c/19593/1/util/inteltool/pcr.c File util/inteltool/pcr.c:
Line 114: printf("P2SB Control = 0x%08"PRIx32"\n", pci_read_long(p2sb, 0xe0)); Sould this even be in the pcr_init ? I think it belongs in its own print_pcr_endpoints_info function.
Line 132: endpoint_mask & ~(1 << j)); This looks like it enables an endpoint to see if it's locked or not. But it doesn't restore it back to how it was. Also, the 'endpoint_mask == pci_read_long' check will fail for the other 31 endpoints as soon as the first endpoint is not locked.