[M] Change in coreboot[master]: soc/intel/cmn/cse: Refactor CSE RW FW Version implementation

[Julius Werner (Code Review)]

Attention is currently required from: Subrata Banik.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77174?usp=email )

The change is no longer submittable: All-Comments-Resolved is unsatisfied now.

Change subject: soc/intel/cmn/cse: Refactor CSE RW FW Version implementation
......................................................................

Patch Set 2:

(4 comments)

Patchset:

PS2: 
I still think the name VSD doesn't really fit and you should call it something CSE-related, e.g. "CSE info" or "CSE data" or whatever. But I can compromise on that if you insist.

File src/soc/intel/common/block/cse/cse_lite.c:

https://review.coreboot.org/c/coreboot/+/77174/comment/626570eb_3da36cab :
PS2, Line 230: 	/* Ensure the sync only execute at once from ENV_ROMSTAGE */
You should be able to just use a CBMEM_CREATION_HOOK (instead of READY), then this should only run once in romstage.

https://review.coreboot.org/c/coreboot/+/77174/comment/9f3cfe6c_60edbb7a :
PS2, Line 235: 	vsd_info = cbmem_add(CBMEM_ID_FW_VSD_INFO, sizeof(*vsd_info));
nit: Might be worth to factor out the cbmem_add and memcpy into another helper function since you have it twice. Should also check the result of cbmem_add.

https://review.coreboot.org/c/coreboot/+/77174/comment/85abe326_e7a1a37d :
PS2, Line 239: CBMEM_READY_HOOK_EARLY(preram_vsd_sync_cache_to_cbmem);
The EARLY hooks are only for when you care about the position for this in memory being stable between boots (such as for the CBMEM console). That shouldn't be necessary here, so just use a normal hook. (In fact, since this doesn't get added in recovery mode it is important that it is not EARLY, otherwise it could cause other EARLY hook spaces like the console to move around in recovery mode.)