[coreboot-gerrit] [M] Change in coreboot[main]: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver

[Felix Held (Code Review)]

Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya.

Felix Held has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85634?usp=email )

Change subject: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver
......................................................................

Patch Set 19:

(2 comments)

File src/vendorcode/amd/opensil/genoa_poc/memmap.c:

https://review.coreboot.org/c/coreboot/+/85634/comment/379f72ad_3f8657f5?usp... :
PS16, Line 37: 	for (i = 0; i < ARRAY_SIZE(types); i++)
             : 		if (enum_type == types[i].type)
             : 			break;
             : 	if (i == ARRAY_SIZE(types))
             : 		return "Unknown type";
             : 	return types[i].string;
> Agreed, it's much easier to read this way. I added this change to the new patchset. […]

i'm fine with also doing that in the same patch

File src/vendorcode/amd/opensil/genoa_poc/memmap.c:

https://review.coreboot.org/c/coreboot/+/85634/comment/311a8f8c_d00a1443?usp... :
PS19, Line 16: 
i'd add a check to make sure that the size of the type field is what we expect:

_Static_assert(sizeof(uint32_t) == sizeof(((MEMORY_HOLE_DESCRIPTOR){0}).Type), "Unexpected size of MEMORY_HOLE_TYPES in the MEMORY_HOLE_DESCRIPTOR struct which doesn't match the code in drivers/amd/opensil/memmap.c");

if that doesn't work the next best thing would be

_Static_assert(sizeof(uint32_t) == sizeof(MEMORY_HOLE_TYPES), "Unexpected size of MEMORY_HOLE_TYPES which doesn't match the code in drivers/amd/opensil/memmap.c");

This check needs to be in this file, since only this file has the definitions of MEMORY_HOLE_DESCRIPTOR and MEMORY_HOLE_TYPES