Attention is currently required from: Anastasia Klimchuk, Peter Marheine.
Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84840?usp=email )
Change subject: cli_client: Add rpmc command support ......................................................................
Patch Set 9:
(4 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84840/comment/64bc5425_ed573d5f?usp... : PS8, Line 1272: any_rpmc_op
Would it ever be needed to set multiple commands at the same time? And what would that mean? […]
Before your question the `cli_classic_validate_singleop(&operation_specified);` before any of the options prevented multiple rpmc operations.
But thanks to this question I realized that using multiple commands in one call is actually very usefull. Updating the hmac key is something that needs to be done at least after every chip reset. So calling --update-hmac-key and then immediatly after something like --get-counter is probably something a lot of users want to do. The commands don't get into the way of eachother so running multiple commands at after each other is possible. I have adjusted the order slightly to have --get-rpmc-status run last.
https://review.coreboot.org/c/flashrom/+/84840/comment/cf31a42e_332e4a78?usp... : PS8, Line 1289: (options.enable_wp && options.disable_wp)
Would it make sense to add few checks similar to this, what do you think? […]
See also #1272.
The else if is now gone so every commands gets run. I have also added a rpmc_root_key_file to the cli_opts struct so the commands don't get into the way of -w, -r etc..
Doing something like writing something to flash and then incrementing the counter is probably how the counters are meant to be used. So beeing able to run other commands together with them is mostly usefull. I can't imagine any other scenerio where running a rpmc command next to some other command is destructive, so in my opinion there are no other checks necessary.
File doc/classic_cli_manpage.rst:
https://review.coreboot.org/c/flashrom/+/84840/comment/47cc9cc7_9a3dc029?usp... : PS8, Line 334: If your chip is detected correctly but the feature is not enabled, try using : ``-c "SFDP-capable chip"`` for automatic feature detection.
I want to think how to maybe rephrase it to be more clear. […]
I have added a quote to the error message, this should make it better to understand.
Running multiple commands together would currently either fail at the rpmc command or force the read and write to use SFDP-capable chip (which works perfectly in my testing). If the commands are run in seperate calls to flashrom (which read and write have to be anyways, since they mutually exclusive) everything works as normal, you just have to specify the chip manually for the rpmc command.
I also had another idea for detection we could always poll the SFDP-capable chip and then silently use it, instead of the user specified one, for the rpmc commands. But that is something I would try in a different commit.
File doc/release_notes/devel.rst:
https://review.coreboot.org/c/flashrom/+/84840/comment/ee5453a0_0d1068e6?usp... : PS8, Line 35: Adding support for RPMC commands as specified by JESD260 to the cli_client. Main
Let's give this paragraph its own header, it's a big deal: […]
Done