diff mbox series

[16/18] s390x: Rebuild IPLB for SCSI device directly from DIAG308

Message ID 20240927005117.1679506-17-jrossi@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: Add Full Boot Order Support | expand

Commit Message

Jared Rossi Sept. 27, 2024, 12:51 a.m. UTC 
From: Jared Rossi <jrossi@linux.ibm.com>

Because virtio-scsi type devices use a non-architected IPLB pbt code they cannot
be set and stored normally. Instead, the IPLB must be rebuilt during re-ipl.

As s390x does not natively support multiple boot devices, the devno field is
used to store the position in the boot order for the device.

Handling the rebuild as part of DIAG308 removes the need to check the devices
for invalid IPLBs later in the IPL.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>

---
 hw/s390x/ipl.h              |  11 ++--
 include/hw/s390x/ipl/qipl.h |   3 +-
 hw/s390x/ipl.c              | 104 ++++++++++--------------------------
 pc-bios/s390-ccw/jump2ipl.c |  11 ++--
 target/s390x/diag.c         |   9 +++-
 5 files changed, 54 insertions(+), 84 deletions(-)

Comments

Thomas Huth Sept. 30, 2024, 12:15 p.m. UTC | #1 
On 27/09/2024 02.51, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Because virtio-scsi type devices use a non-architected IPLB pbt code they cannot
> be set and stored normally. Instead, the IPLB must be rebuilt during re-ipl.
> 
> As s390x does not natively support multiple boot devices, the devno field is
> used to store the position in the boot order for the device.
> 
> Handling the rebuild as part of DIAG308 removes the need to check the devices
> for invalid IPLBs later in the IPL.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> 
> ---
...
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index ba66847b9c..86c995b580 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -448,7 +448,6 @@ void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp)
>   
>   static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
>   {
> -    S390IPLState *ipl = get_ipl_device();
>       CcwDevice *ccw_dev = NULL;
>       SCSIDevice *sd;
>       int devtype;
> @@ -481,9 +480,6 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
>               iplb->ccw.ssid = ccw_dev->sch->ssid & 3;
>               break;
>           case CCW_DEVTYPE_VIRTIO_NET:
> -            /* The S390IPLState netboot is true if ANY IPLB may use netboot */
> -            ipl->netboot = true;
> -            /* Fall through to CCW_DEVTYPE_VIRTIO case */
>           case CCW_DEVTYPE_VIRTIO:
>               iplb->len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
>               iplb->blk0_len =
> @@ -508,6 +504,16 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
>       return false;
>   }
>   
> +
> +void s390_rebuild_iplb(uint16_t dev_index, IplParameterBlock *iplb) {

Please put the curly brace on a new line for function declarations.

> +    S390IPLState *ipl = get_ipl_device();
> +    uint16_t index;
> +    index = ipl->rebuilt_iplb ? ipl->iplb_index : dev_index;
> +
> +    ipl->rebuilt_iplb = s390_build_iplb(get_boot_device(index), iplb);
> +    ipl->iplb_index = index;
> +}
...
> @@ -661,35 +629,33 @@ IplParameterBlock *s390_ipl_get_iplb(void)
>       return &ipl->iplb;
>   }
>   
> -void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
> +static void s390_ipl_prepare_qipl(S390CPU *cpu)
>   {
>       S390IPLState *ipl = get_ipl_device();
> +    uint8_t *addr;
> +    uint64_t len = 4096;
> +
> +    addr = cpu_physical_memory_map(cpu->env.psa, &len, true);
> +    if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
> +        error_report("Cannot set QEMU IPL parameters");
> +        return;
> +    }
> +
> +    memcpy(addr + QIPL_ADDRESS, &ipl->qipl, sizeof(QemuIplParameters));
> +    cpu_physical_memory_unmap(addr, len, 1, len);
> +}

Why did you move the s390_ipl_prepare_qipl() function around? It does not 
seem to get moved in the new code below, so the movement does not seem to be 
required?

> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
> +{
> +    S390IPLState *ipl = get_ipl_device();
>       if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
>           /* use CPU 0 for full resets */
>           ipl->reset_cpu_index = 0;
>       } else {
>           ipl->reset_cpu_index = cs->cpu_index;
>       }
> -    ipl->reset_type = reset_type;
>   
> -    if (reset_type == S390_RESET_REIPL &&
> -        ipl->iplb_valid &&
> -        !ipl->netboot &&
> -        ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
> -        is_virtio_scsi_device(&ipl->iplb)) {
> -        CcwDevice *ccw_dev = s390_get_ccw_device(get_boot_device(0), NULL);
> -
> -        if (ccw_dev &&
> -            cpu_to_be16(ccw_dev->sch->devno) == ipl->iplb.ccw.devno &&
> -            (ccw_dev->sch->ssid & 3) == ipl->iplb.ccw.ssid) {
> -            /*
> -             * this is the original boot device's SCSI
> -             * so restore IPL parameter info from it
> -             */
> -            ipl->iplb_valid = s390_build_iplb(get_boot_device(0), &ipl->iplb);
> -        }
> -    }
> +    ipl->reset_type = reset_type;
>       if (reset_type == S390_RESET_MODIFIED_CLEAR ||
>           reset_type == S390_RESET_LOAD_NORMAL ||
>           reset_type == S390_RESET_PV) {
> @@ -725,20 +691,6 @@ void s390_ipl_clear_reset_request(void)
>       ipl->reset_cpu_index = 0;
>   }
>   
> -static void s390_ipl_prepare_qipl(S390CPU *cpu)
> -{
> -    S390IPLState *ipl = get_ipl_device();
> -    uint8_t *addr;
> -    uint64_t len = 4096;
> -
> -    addr = cpu_physical_memory_map(cpu->env.psa, &len, true);
> -    if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
> -        error_report("Cannot set QEMU IPL parameters");
> -        return;
> -    }
> -    memcpy(addr + QIPL_ADDRESS, &ipl->qipl, sizeof(QemuIplParameters));
> -    cpu_physical_memory_unmap(addr, len, 1, len);
> -}
>

  Thomas
Jared Rossi Sept. 30, 2024, 1:46 p.m. UTC | #2 
On 9/30/24 8:15 AM, Thomas Huth wrote:
> On 27/09/2024 02.51, jrossi@linux.ibm.com wrote:
>> From: Jared Rossi <jrossi@linux.ibm.com> ... 
>> @@ -661,35 +629,33 @@ IplParameterBlock *s390_ipl_get_iplb(void)
>>       return &ipl->iplb;
>>   }
>>   -void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
>> +static void s390_ipl_prepare_qipl(S390CPU *cpu)
>>   {
>>       S390IPLState *ipl = get_ipl_device();
>> +    uint8_t *addr;
>> +    uint64_t len = 4096;
>> +
>> +    addr = cpu_physical_memory_map(cpu->env.psa, &len, true);
>> +    if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
>> +        error_report("Cannot set QEMU IPL parameters");
>> +        return;
>> +    }
>> +
>> +    memcpy(addr + QIPL_ADDRESS, &ipl->qipl, sizeof(QemuIplParameters));
>> +    cpu_physical_memory_unmap(addr, len, 1, len);
>> +}
>
> Why did you move the s390_ipl_prepare_qipl() function around? It does 
> not seem to get moved in the new code below, so the movement does not 
> seem to be required?
>
>
>  Thomas
>

You are correct, it is not necessary to move it.  I moved it in preparation
for a change I ultimately ended up reverting, but I forgot to revert moving
the function itself.  I’ll fix it.

Jared Rossi
diff mbox series

Patch

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index e4af8e3782..d1ebc384d8 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -24,6 +24,7 @@ 
 
 void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp);
 void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp);
+void s390_rebuild_iplb(uint16_t index, IplParameterBlock *iplb);
 void s390_ipl_update_diag308(IplParameterBlock *iplb);
 int s390_ipl_prepare_pv_header(Error **errp);
 int s390_ipl_pv_unpack(void);
@@ -65,7 +66,8 @@  struct S390IPLState {
     bool enforce_bios;
     bool iplb_valid;
     bool iplb_valid_pv;
-    bool netboot;
+    bool rebuilt_iplb;
+    uint16_t iplb_index;
     /* reset related properties don't have to be migrated or reset */
     enum s390_reset reset_type;
     int reset_cpu_index;
@@ -172,11 +174,14 @@  static inline bool iplb_valid_pv(IplParameterBlock *iplb)
 
 static inline bool iplb_valid(IplParameterBlock *iplb)
 {
+    uint32_t len = be32_to_cpu(iplb->len);
+
     switch (iplb->pbt) {
     case S390_IPL_TYPE_FCP:
-        return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_FCP_LEN;
+        return len >= S390_IPLB_MIN_FCP_LEN;
     case S390_IPL_TYPE_CCW:
-        return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN;
+        return (len >= S390_IPLB_MIN_CCW_LEN);
+    case S390_IPL_TYPE_QEMU_SCSI:
     default:
         return false;
     }
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 15342ac5cd..0f518adb62 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -28,7 +28,8 @@ 
  */
 struct QemuIplParameters {
     uint8_t  qipl_flags;
-    uint8_t  reserved1[3];
+    uint8_t  index;
+    uint8_t  reserved1[2];
     uint64_t reserved2;
     uint32_t boot_menu_timeout;
     uint8_t  reserved3[2];
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index ba66847b9c..86c995b580 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -448,7 +448,6 @@  void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp)
 
 static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
 {
-    S390IPLState *ipl = get_ipl_device();
     CcwDevice *ccw_dev = NULL;
     SCSIDevice *sd;
     int devtype;
@@ -481,9 +480,6 @@  static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
             iplb->ccw.ssid = ccw_dev->sch->ssid & 3;
             break;
         case CCW_DEVTYPE_VIRTIO_NET:
-            /* The S390IPLState netboot is true if ANY IPLB may use netboot */
-            ipl->netboot = true;
-            /* Fall through to CCW_DEVTYPE_VIRTIO case */
         case CCW_DEVTYPE_VIRTIO:
             iplb->len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
             iplb->blk0_len =
@@ -508,6 +504,16 @@  static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
     return false;
 }
 
+
+void s390_rebuild_iplb(uint16_t dev_index, IplParameterBlock *iplb) {
+    S390IPLState *ipl = get_ipl_device();
+    uint16_t index;
+    index = ipl->rebuilt_iplb ? ipl->iplb_index : dev_index;
+
+    ipl->rebuilt_iplb = s390_build_iplb(get_boot_device(index), iplb);
+    ipl->iplb_index = index;
+}
+
 static bool s390_init_all_iplbs(S390IPLState *ipl)
 {
     int iplb_num = 0;
@@ -560,44 +566,6 @@  static bool s390_init_all_iplbs(S390IPLState *ipl)
     return iplb_num;
 }
 
-static bool is_virtio_ccw_device_of_type(IplParameterBlock *iplb,
-                                         int virtio_id)
-{
-    uint8_t cssid;
-    uint8_t ssid;
-    uint16_t devno;
-    uint16_t schid;
-    SubchDev *sch = NULL;
-
-    if (iplb->pbt != S390_IPL_TYPE_CCW) {
-        return false;
-    }
-
-    devno = be16_to_cpu(iplb->ccw.devno);
-    ssid = iplb->ccw.ssid & 3;
-
-    for (schid = 0; schid < MAX_SCHID; schid++) {
-        for (cssid = 0; cssid < MAX_CSSID; cssid++) {
-            sch = css_find_subch(1, cssid, ssid, schid);
-
-            if (sch && sch->devno == devno) {
-                return sch->id.cu_model == virtio_id;
-            }
-        }
-    }
-    return false;
-}
-
-static bool is_virtio_net_device(IplParameterBlock *iplb)
-{
-    return is_virtio_ccw_device_of_type(iplb, VIRTIO_ID_NET);
-}
-
-static bool is_virtio_scsi_device(IplParameterBlock *iplb)
-{
-    return is_virtio_ccw_device_of_type(iplb, VIRTIO_ID_SCSI);
-}
-
 static void update_machine_ipl_properties(IplParameterBlock *iplb)
 {
     Object *machine = qdev_get_machine();
@@ -637,7 +605,7 @@  void s390_ipl_update_diag308(IplParameterBlock *iplb)
         ipl->iplb = *iplb;
         ipl->iplb_valid = true;
     }
-    ipl->netboot = is_virtio_net_device(iplb);
+
     update_machine_ipl_properties(iplb);
 }
 
@@ -661,35 +629,33 @@  IplParameterBlock *s390_ipl_get_iplb(void)
     return &ipl->iplb;
 }
 
-void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
+static void s390_ipl_prepare_qipl(S390CPU *cpu)
 {
     S390IPLState *ipl = get_ipl_device();
+    uint8_t *addr;
+    uint64_t len = 4096;
+
+    addr = cpu_physical_memory_map(cpu->env.psa, &len, true);
+    if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
+        error_report("Cannot set QEMU IPL parameters");
+        return;
+    }
+
+    memcpy(addr + QIPL_ADDRESS, &ipl->qipl, sizeof(QemuIplParameters));
+    cpu_physical_memory_unmap(addr, len, 1, len);
+}
 
+void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
+{
+    S390IPLState *ipl = get_ipl_device();
     if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
         /* use CPU 0 for full resets */
         ipl->reset_cpu_index = 0;
     } else {
         ipl->reset_cpu_index = cs->cpu_index;
     }
-    ipl->reset_type = reset_type;
 
-    if (reset_type == S390_RESET_REIPL &&
-        ipl->iplb_valid &&
-        !ipl->netboot &&
-        ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
-        is_virtio_scsi_device(&ipl->iplb)) {
-        CcwDevice *ccw_dev = s390_get_ccw_device(get_boot_device(0), NULL);
-
-        if (ccw_dev &&
-            cpu_to_be16(ccw_dev->sch->devno) == ipl->iplb.ccw.devno &&
-            (ccw_dev->sch->ssid & 3) == ipl->iplb.ccw.ssid) {
-            /*
-             * this is the original boot device's SCSI
-             * so restore IPL parameter info from it
-             */
-            ipl->iplb_valid = s390_build_iplb(get_boot_device(0), &ipl->iplb);
-        }
-    }
+    ipl->reset_type = reset_type;
     if (reset_type == S390_RESET_MODIFIED_CLEAR ||
         reset_type == S390_RESET_LOAD_NORMAL ||
         reset_type == S390_RESET_PV) {
@@ -725,20 +691,6 @@  void s390_ipl_clear_reset_request(void)
     ipl->reset_cpu_index = 0;
 }
 
-static void s390_ipl_prepare_qipl(S390CPU *cpu)
-{
-    S390IPLState *ipl = get_ipl_device();
-    uint8_t *addr;
-    uint64_t len = 4096;
-
-    addr = cpu_physical_memory_map(cpu->env.psa, &len, true);
-    if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
-        error_report("Cannot set QEMU IPL parameters");
-        return;
-    }
-    memcpy(addr + QIPL_ADDRESS, &ipl->qipl, sizeof(QemuIplParameters));
-    cpu_physical_memory_unmap(addr, len, 1, len);
-}
 
 int s390_ipl_prepare_pv_header(Error **errp)
 {
diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
index d45aec6694..bcf0e12f02 100644
--- a/pc-bios/s390-ccw/jump2ipl.c
+++ b/pc-bios/s390-ccw/jump2ipl.c
@@ -39,10 +39,15 @@  int jump_to_IPL_code(uint64_t address)
     write_subsystem_identification();
     write_iplb_location();
 
-    /* prevent unknown IPL types in the guest */
+    /*
+     * The IPLB for QEMU SCSI type devices must be rebuilt during re-ipl. The
+     * iplb.devno is set to the boot position of the target SCSI device.
+     */
     if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {
-        iplb.pbt = S390_IPL_TYPE_CCW;
-        set_iplb(&iplb);
+        iplb.devno = qipl.index;
+        if (!set_iplb(&iplb)) {
+            panic("Failed to set IPLB");
+        }
     }
 
     /*
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 27ffd48576..a1fd54ddac 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -133,7 +133,14 @@  void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
 
         valid = subcode == DIAG308_PV_SET ? iplb_valid_pv(iplb) : iplb_valid(iplb);
         if (!valid) {
-            env->regs[r1 + 1] = DIAG_308_RC_INVALID;
+            if (subcode == DIAG308_SET && iplb->pbt == S390_IPL_TYPE_QEMU_SCSI) {
+                s390_rebuild_iplb(iplb->devno, iplb);
+                s390_ipl_update_diag308(iplb);
+                env->regs[r1 + 1] = DIAG_308_RC_OK;
+            } else {
+                env->regs[r1 + 1] = DIAG_308_RC_INVALID;
+            }
+
             goto out;
         }