Re: [PATCH v12 18/35] hw/s390x/ipl: Rework s390_ipl_map_iplb_chain for certificate storage

[Jared Rossi]

On 7/2/26 10:56 AM, Eric Farman wrote:
On Wed, 2026-07-01 at 16:49 -0400, Zhuoying Cai wrote:
Rework s390_ipl_map_iplb_chain to always allocate maximum memory for
the IPLB chain, regardless of the number of boot devices. This space
is also used to store certificates during secure boot, providing a
safe location for certificates until the kernel reads them during boot.

Rename next_iplb to ipl_data to better reflect its multiple purposes:
storing both IPLB chains and certificate data.

Signed-off-by: Zhuoying Cai <zy...@linux.ibm.com>
---
  hw/s390x/ipl.c              | 13 ++++++++++---
  include/hw/s390x/ipl/qipl.h |  2 +-
  pc-bios/s390-ccw/iplb.h     |  4 ++--
  3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 9cb41511e8..7734b6edc7 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -425,10 +425,9 @@ static S390PCIBusDevice *s390_get_pci_device(DeviceState 
*dev_st, int *devtype)
      return pbdev;
  }
  
-static uint64_t s390_ipl_map_iplb_chain(IplParameterBlock *iplb_chain)
+static uint64_t s390_ipl_map_iplb_chain(IplParameterBlock *iplb_chain, 
uint16_t count)
  {
      S390IPLState *ipl = get_ipl_device();
-    uint16_t count = be16_to_cpu(ipl->qipl.chain_len);
      uint64_t len = sizeof(IplParameterBlock) * count;
      uint64_t chain_addr = find_iplb_chain_addr(ipl->bios_start_addr, count);
  
@@ -629,8 +628,16 @@ static bool s390_init_all_iplbs(S390IPLState *ipl)
              dev_st = get_boot_device(i);
              s390_build_iplb(dev_st, &iplb_chain[i - 1]);
          }
+    }
  
-        ipl->qipl.next_iplb = cpu_to_be64(s390_ipl_map_iplb_chain(iplb_chain));
+    /*
+     * Allocate maximum space for IPLB chain and/or certificate storage.
+     * Once a valid boot device is found, this space will be used to store
+     * certificates if secure boot is enabled.
+     */
+    if (iplb_num > 1 || s390_has_certificate()) {
+        ipl->qipl.ipl_data = cpu_to_be64(s390_ipl_map_iplb_chain(iplb_chain,
+                                                                 MAX_BOOT_DEVS 
- 1));
I think iplb_chain needs to be explicitly initialized to zero now that you're 
allocating a larger
space. load_next_iplb() is still honoring the length of the count when walking 
the IPLBs, but the
above does a memcpy of the whole array.


+1.  Eric is correct.  I missed this in my review.  We copy all 7 IPLB 
chain indices, but build_iplb() only populates the indices it actually 
needs and the array isn't automatically cleared when it is first 
declared, so it could potentially contain garbage at the end.

Thanks,
Jared Rossi