Re: [qemu-s390x] [PATCH v5 05/12] s390-ccw: move auxiliary IPL data to separate location

[Christian Borntraeger]

On 02/05/2018 09:57 PM, Collin L. Walling wrote:
> The s390-ccw firmware needs some information in support of the
> boot process which is not available on the native machine.
> Examples are the netboot firmware load address and now the
> boot menu parameters.
> 
> While storing that data in unused fields of the IPL parameter block
> works, that approach could create problems if the parameter block
> definition should change in the future. Because then a guest could
> overwrite these fields using the set IPLB diagnose.
> 
> In fact the data in question is of more global nature and not really
> tied to an IPL device, so separating it is rather logical.
> 
> This commit introduces a new structure to hold firmware relevant
> IPL parameters set by QEMU. The data is stored at location 204 (dec)
> and can contain up to 7 32-bit words. This area is available to
> programming in the z/Architecture Principles of Operation and
> can thus safely be used by the firmware until the IPL has completed.

This content is set by QEMU/the ccw bios on every IPL, so we do not
have to change anything for migration. Correct?

> 
> Signed-off-by: Viktor Mihajlovski <address@hidden>
> ---
>  hw/s390x/ipl.c          | 18 +++++++++++++++++-
>  hw/s390x/ipl.h          | 11 +++++++++--
>  pc-bios/s390-ccw/iplb.h | 12 ++++++++++--
>  pc-bios/s390-ccw/main.c |  6 +++++-
>  4 files changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 0d06fc1..3e3c3b8 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -399,6 +399,20 @@ void s390_reipl_request(void)
>      qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>  }
> 
> +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, 1);
> +    if (!addr || len < 204 + sizeof(QemuIplParameters)) {
> +        error_report("Cannot set QEMU IPL parameters");
> +    }
> +    memcpy(addr + 204, &ipl->iplb.qipl, sizeof(QemuIplParameters));
> +    cpu_physical_memory_unmap(addr, len, 1, len);
> +}
> +
>  void s390_ipl_prepare_cpu(S390CPU *cpu)
>  {
>      S390IPLState *ipl = get_ipl_device();
> @@ -418,8 +432,10 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
>              error_report_err(err);
>              vm_stop(RUN_STATE_INTERNAL_ERROR);
>          }
> -        ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr);
> +        ipl->iplb.qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
>      }
> +    s390_ipl_prepare_qipl(cpu);
> +
>  }
> 
>  static void s390_ipl_reset(DeviceState *dev)
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 8a705e0..68dcaf8 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -16,8 +16,7 @@
>  #include "cpu.h"
> 
>  struct IplBlockCcw {
> -    uint64_t netboot_start_addr;
> -    uint8_t  reserved0[77];
> +    uint8_t  reserved0[85];
>      uint8_t  ssid;
>      uint16_t devno;
>      uint8_t  vm_flags;
> @@ -59,6 +58,13 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi;
> 
>  #define DIAG308_FLAGS_LP_VALID 0x80
> 
> +struct QemuIplParameters {
> +    uint8_t  reserved1[4];
> +    uint64_t netboot_start_addr;
> +    uint8_t  reserved2[16];
> +} QEMU_PACKED;
> +typedef struct QemuIplParameters QemuIplParameters;
> +
>  union IplParameterBlock {
>      struct {
>          uint32_t len;
> @@ -74,6 +80,7 @@ union IplParameterBlock {
>              IplBlockFcp fcp;
>              IplBlockQemuScsi scsi;
>          };
> +        QemuIplParameters qipl;
>      } QEMU_PACKED;
>      struct {
>          uint8_t  reserved1[110];
> diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
> index 890aed9..12f6e40 100644
> --- a/pc-bios/s390-ccw/iplb.h
> +++ b/pc-bios/s390-ccw/iplb.h
> @@ -13,8 +13,7 @@
>  #define IPLB_H
> 
>  struct IplBlockCcw {
> -    uint64_t netboot_start_addr;
> -    uint8_t  reserved0[77];
> +    uint8_t  reserved0[85];
>      uint8_t  ssid;
>      uint16_t devno;
>      uint8_t  vm_flags;
> @@ -73,6 +72,15 @@ typedef struct IplParameterBlock IplParameterBlock;
> 
>  extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> 
> +struct QemuIplParameters {
> +    uint8_t  reserved1[4];
> +    uint64_t netboot_start_addr;
> +    uint8_t  reserved2[16];
> +} __attribute__ ((packed));
> +typedef struct QemuIplParameters QemuIplParameters;
> +
> +extern QemuIplParameters qipl;
> +
>  #define S390_IPL_TYPE_FCP 0x00
>  #define S390_IPL_TYPE_CCW 0x02
>  #define S390_IPL_TYPE_QEMU_SCSI 0xff
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index e857ce4..825a1a3 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -16,6 +16,7 @@ char stack[PAGE_SIZE * 8] 
> __attribute__((__aligned__(PAGE_SIZE)));
>  static SubChannelId blk_schid = { .one = 1 };
>  IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
>  static char loadparm[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
> +QemuIplParameters qipl;
> 
>  /*
>   * Priniciples of Operations (SA22-7832-09) chapter 17 requires that
> @@ -81,6 +82,7 @@ static void virtio_setup(void)
>      uint16_t dev_no;
>      char ldp[] = "LOADPARM=[________]\n";
>      VDev *vdev = virtio_get_device();
> +    QemuIplParameters *early_qipl = (QemuIplParameters *)204;
> 
>      /*
>       * We unconditionally enable mss support. In every sane configuration,
> @@ -93,6 +95,8 @@ static void virtio_setup(void)
>      memcpy(ldp + 10, loadparm, 8);
>      sclp_print(ldp);
> 
> +    memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
> +
>      if (store_iplb(&iplb)) {
>          switch (iplb.pbt) {
>          case S390_IPL_TYPE_CCW:
> @@ -127,7 +131,7 @@ static void virtio_setup(void)
> 
>      if (virtio_get_device_type() == VIRTIO_ID_NET) {
>          sclp_print("Network boot device detected\n");
> -        vdev->netboot_start_addr = iplb.ccw.netboot_start_addr;
> +        vdev->netboot_start_addr = qipl.netboot_start_addr;
>      } else {
>          virtio_blk_setup_device(blk_schid);
>