On 28.03.2023 18:07, Andrew Cooper wrote: > On 28/03/2023 5:01 pm, Jan Beulich wrote: >> On 28.03.2023 17:12, Andrew Cooper wrote: >>> On 28/03/2023 3:19 pm, Jan Beulich wrote: >>>> On 27.03.2023 21:41, Andrew Cooper wrote: >>>>> Right now, the boot flow depends on the second pass over >>>>> bootstrap_map()/find_cpio_data() altering ucode_blob.data to use the >>>>> directmap >>>>> alias of the CPIO module, where previously it caches the early boostrap >>>>> mapping. >>>>> >>>>> If the scan is successful, it will be successful the second time too, but >>>>> there's no point repeating the work. Cache the module index, offset and >>>>> size >>>>> to short circuit things the second time around. >>>> If the scan failed, it will fail the 2nd time too. Maybe deal with >>>> this case as well, e.g. by clearing ucode_scan at the end of >>>> microcode_scan_module() when nothing was found? >>> See patch 5. It can only become true then because of how the callers >>> are arranged. >> Right, I've meanwhile seen you do it there. That's fine. Yet I think it >> could also be done earlier (and if I'm not mistaken also ahead of all >> of the rearrangements you do). > > Prior to patch 5, calls into scan are guarded with "if ( ucode_scan )" > as well as there being an "if ( !ucode_scan )" check. > > Clobbering ucode_scan prior to patch 5 breaks the second pass to cache > the ucode we loaded on the BSP.
How that? We didn't load anything if scan failed on the first pass, and clearing ucode_scan if the first pass failed would merely suppress a fruitless second pass. Of course we cannot clear ucode_scan in the case that the first pass succeeded, but my earlier suggestion was limited to the failure case. The "!ucode_scan" check also is dead code, btw (and hence doesn't matter here), since neither caller would invoke the function in that case in the first place. Jan
