[S] Change in coreboot[master]: soc/amd/common/block/cpu: Add missing cbfs_unmap()

[Tim Van Patten (Code Review)]

Attention is currently required from: Konrad Adamczyk, Jason Glenesk, Raul Rangel, Jakub Czapiga, Matt DeVillier, Paul Menzel, Grzegorz Bernacki, Arthur Heymans, Fred Reitberger, Karthik Ramasubramanian, Felix Held.

Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74777 )

Change subject: soc/amd/common/block/cpu: Add missing cbfs_unmap()
......................................................................

Patch Set 3:

(2 comments)

File src/soc/amd/common/block/cpu/update_microcode.c:

https://review.coreboot.org/c/coreboot/+/74777/comment/99ede524_92b71690 
PS3, Line 102: 			goto end;
I don't think we need a `goto` here, and is hurting readability more than helping simplify the code. I prefer `cbfs_unmap()` with `return` instead, especially since it's branching into the middle of a conditional block.

https://review.coreboot.org/c/coreboot/+/74777/comment/57bb888c_8aa3c208 
PS3, Line 113: 		cbfs_unmap((void *)ucode);
While the `>=` was added to make it more likely that this is called eventually, it also means it's possible for multiple CPUs to execute `cbfs_unmap()` at the same time, corrupting the CBFS cache data structures.

There are a few things here:
1. This should be reverted back to `==`, so it's performed exactly once.
2. We should crash if we see `> get_cpu_count()`, since that indicates we've hit a (likely) fatal error. 
3. The access to `ucode` needs to be within an semaphore that's incremented by every reader, so it's only freed once all current readers have completed.
* The `== get_cpu_count()` will need to block until all readers are done, since it's the only one that can safely call `cbfs_unmap()`.
* Otherwise, if the `> get_cpu_count()` case occurs, it's possible for CPUs to be loading microcode while the cache is being freed (and the memory potentially reused, meaning the microcode binary being loaded isn't actually microcode, but "random" data).
* If we ignore the `> get_cpu_count()` case, then we can skip this step, since the atomic counter should be enough to prevent freeing the `ucode` memory until everyone is done with it.

I'm not sure how possible `> get_cpu_count()` is, though. Ideally, we could catch that when releasing the other CPUs (before getting here?), but I don't know the details well enough.