Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40520 )
Change subject: [RFC]drivers/smmstore: Implement SMMSTORE version 2 ......................................................................
Patch Set 3:
(4 comments)
Rewrote everything to use a fixed communication buffer. The payload must query a coreboot table to get the communication buffer address and use that for all read/write operations.
https://review.coreboot.org/c/coreboot/+/40520/1/src/drivers/smmstore/smi.c File src/drivers/smmstore/smi.c:
https://review.coreboot.org/c/coreboot/+/40520/1/src/drivers/smmstore/smi.c@... PS1, Line 63: case SMMSTORE_CMD_INFO: {
Given that less SMM code is typically better/desirable, would you want to have a Kconfig option for […]
Added a Kconfig as the V1 API allows to write to arbitrary locations in DRAM, while V2 only allows to write to a CBMEM buffer.
https://review.coreboot.org/c/coreboot/+/40520/1/src/drivers/smmstore/store.... File src/drivers/smmstore/store.c:
https://review.coreboot.org/c/coreboot/+/40520/1/src/drivers/smmstore/store.... PS1, Line 270: static struct smmstore_params_info info_params;
Good point, I'll rework that
Done
https://review.coreboot.org/c/coreboot/+/40520/1/src/drivers/smmstore/store.... PS1, Line 272: smmstore_update_info
You probably want to check if the base of the region is 64KiB aligned? Maybe the erase ops of region […]
Done
https://review.coreboot.org/c/coreboot/+/40520/1/src/drivers/smmstore/store.... PS1, Line 304: if (offset >= info_params.block_size || block_id >= info_params.num_blocks) { : printk(BIOS_WARNING, "access out of region requested\n");
Maybe some more fine grained error messages could be useful when debugging things?
Done