[coreboot-gerrit] Change in coreboot[master]: inteltool: Add dumping of full PCR ports

[Youness Alaoui (Code Review)]

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.


-- 
To view, visit https://review.coreboot.org/19593
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iede4ac601355e2be377bc986d62d20098980ec35
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Youness Alaoui <snifikino at gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes