On 7/15/26 4:37 PM, Collin Walling wrote:
On 7/7/26 19:40, Zhuoying Cai wrote:/* should not return */ write_reset_psw(entry->compdat.load_psw); + + if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT) { + update_cert_list(&cert_list); + update_iirb(&comp_list, &cert_list); + free(tmp_cert_buf); + } +Sorry if I missed this from the previous rounds of review, but why was this moved outside of `zipl_run_secure()`? It seems out of place here. If there is justification for it, I'd suggest at least moving this chunk a few lines up to before the `write_reset_psw` call to keep the "IPL jump" code together. The comment above is meant for the following chunk: ``` /* should not return */ write_reset_psw(entry->compdat.load_psw); jump_to_IPL_code(0); return -1; ``` Otherwise the comment is a bit misleading. I suppose one could argue the comment should actually be placed abouve `jump_to_IPL_code`, but let's not bother with that change.
This was something I had asked for, but I don't recall if it was in a review comment or internal discussion, but a brief summary for the sake of posterity...
Moving the call to update_iirb() was necessitated by a different change, which was to use a specific memory address range for storing the cert list. The fundamental issue is that we wanted to store the cert list at a calculated address to ensure it is not clobbered before the kernel reads it. My idea was to reclaim the space previously used for chained IPLBs. The chained IPLBs and the cert list are never needed at the same time, so that isn't an issue; however, the impact of reclaiming the space is that we destroy our IPLB chain, meaning, once we write the cert list, if the IPL subsequently fails, we no longer have the fallback IPLB to try the next device in boot order. To deal with this we delayed updating the cert list and IIRB until we are committed to boot from the selected device, that is to say, once we have progressed through the IPL far enough that there is no longer any possibility of falling back to a different device. As such, we wait until we are ready to actually hand over control to the kernel before overwriting any of the IPL data.
There is some flexibility with exactly where the calls can be placed, but the idea is that there should not be any paths that return for retry on error after we update the cert list. By placing the update calls directly before the jump we know the only two outcomes remaining are to either successfully hand over control and never return, or for the jump itself to fail, which would be a catastrophic error and necessitate that the IPL is aborted entirely anyway. There is no condition such that we may update the cert list or IIRB and then realize we actually should try booting from a different device instead.
Regards, Jared Rossi
