[coreboot-gerrit] Change in coreboot[master]: arch/x86: Fix S3 resume without stage cache

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37683 )

Change subject: arch/x86: Fix S3 resume without stage cache
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37683/7/src/arch/x86/postcar_loader... 
File src/arch/x86/postcar_loader.c:

https://review.coreboot.org/c/coreboot/+/37683/7/src/arch/x86/postcar_loader... 
PS7, Line 240: 	if (prog_entry(&prog) == NULL)
Am I correct that this logic is assuming prog_entry(&prog) == NULL under these conditions?

1. Non-resume path
2. Resume with NO_STAGE_CACHE

Can we explicitly handle those conditions in the if above and here? I feel like it's hard to follow the various combinations we're handling. I think the compiler can figure out the duplicate code paths and make it more readable for humans.

if (romstange_handoff_is_resume()) {
  if (CONFIG(NO_STAGE_CACHE)) {
    load_postcar_cbfs();
  } else {
    stage_cache_load_stage()
    finalize_load()
    if (prog_entry(&prog) == NULL)
      postcar_cache_invalid();
  }
} else {
  load_postcar_cbfs();
}

That seems more straight forward in handling the conditions. I know it's more verbose, but it's easy to follow.