diff mbox

[PULL,09/20] s390x/ipl: Remove redundant usage of gr7

Message ID 1463496377-9729-10-git-send-email-cornelia.huck@de.ibm.com
State New
Headers show

Commit Message

Cornelia Huck May 17, 2016, 2:46 p.m. UTC 
From: Alexander Yarygin <yarygin@linux.vnet.ibm.com>

We don't need to pass device address for pc-bios using gr7 anymore as
the pcbios completely relies on diag308 now, so we can remove it from
qemu. devno, ssid and cssid are migrated but the value was never reused,
so we can safely ignore these fields and migrate 0.

Signed-off-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/ipl.c | 33 ++++++---------------------------
 1 file changed, 6 insertions(+), 27 deletions(-)

Comments

Paolo Bonzini May 18, 2016, 10:01 a.m. UTC | #1 
On 17/05/2016 16:46, Cornelia Huck wrote:
> From: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
> 
> We don't need to pass device address for pc-bios using gr7 anymore as
> the pcbios completely relies on diag308 now, so we can remove it from
> qemu. devno, ssid and cssid are migrated but the value was never reused,
> so we can safely ignore these fields and migrate 0.
> 
> Signed-off-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

This probably breaks reboot after migration from 2.6.

If that's fine, all the better. :)

Thanks,

Paolo

> ---
>  hw/s390x/ipl.c | 33 ++++++---------------------------
>  1 file changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 2da0f1e..5786b5e 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -208,50 +208,27 @@ static Property s390_ipl_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -/*
> - * In addition to updating the iplstate, this function returns:
> - * - 0 if system was ipled with external kernel
> - * - -1 if no valid boot device was found
> - * - ccw id of the boot device otherwise
> - */
> -static uint64_t s390_update_iplstate(S390IPLState *ipl)
> +static bool s390_gen_initial_iplb(S390IPLState *ipl)
>  {
>      DeviceState *dev_st;
>  
> -    if (ipl->iplb_valid) {
> -        ipl->cssid = 0;
> -        ipl->ssid = 0;
> -        ipl->devno = ipl->iplb.devno;
> -        goto out;
> -    }
> -
> -    if (ipl->kernel) {
> -        return 0;
> -    }
> -
>      dev_st = get_boot_device(0);
>      if (dev_st) {
>          VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast(
>              OBJECT(qdev_get_parent_bus(dev_st)->parent),
>                  TYPE_VIRTIO_CCW_DEVICE);
>          if (ccw_dev) {
> -            ipl->cssid = ccw_dev->sch->cssid;
> -            ipl->ssid = ccw_dev->sch->ssid;
> -            ipl->devno = ccw_dev->sch->devno;
>              ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
>              ipl->iplb.blk0_len =
>                  cpu_to_be32(S390_IPLB_MIN_CCW_LEN - S390_IPLB_HEADER_LEN);
>              ipl->iplb.pbt = S390_IPL_TYPE_CCW;
>              ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
>              ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
> -            ipl->iplb_valid = true;
> -            goto out;
> +            return true;
>          }
>      }
>  
> -    return -1;
> -out:
> -    return (uint32_t) (ipl->cssid << 24 | ipl->ssid << 16 | ipl->devno);
> +    return false;
>  }
>  
>  void s390_ipl_update_diag308(IplParameterBlock *iplb)
> @@ -289,7 +266,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
>  
>      if (!ipl->kernel || ipl->iplb_valid) {
>          cpu->env.psw.addr = ipl->bios_start_addr;
> -        cpu->env.regs[7] = s390_update_iplstate(ipl);
> +        if (!ipl->iplb_valid) {
> +            ipl->iplb_valid = s390_gen_initial_iplb(ipl);
> +        }
>      }
>  }
>  
>
Christian Borntraeger May 18, 2016, 10:09 a.m. UTC | #2 
On 05/18/2016 12:01 PM, Paolo Bonzini wrote:
> 
> 
> On 17/05/2016 16:46, Cornelia Huck wrote:
>> From: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
>>
>> We don't need to pass device address for pc-bios using gr7 anymore as
>> the pcbios completely relies on diag308 now, so we can remove it from
>> qemu. devno, ssid and cssid are migrated but the value was never reused,
>> so we can safely ignore these fields and migrate 0.
>>
>> Signed-off-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> 
> This probably breaks reboot after migration from 2.6.
> 
> If that's fine, all the better. :)

Our BIOS is in RAM and usually overwritten by the guest.
On reboot this is reloaded from whatever BIOS is on the target,
so migration should still work.
I have verified that migration between 2.4 and 2.6  plus these
patches forth and back still allows to reboot.

In other words the bios and the code in ipl.c have to be updated
in lockstep, then everything is fine.



> 
> Thanks,
> 
> Paolo
> 
>> ---
>>  hw/s390x/ipl.c | 33 ++++++---------------------------
>>  1 file changed, 6 insertions(+), 27 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 2da0f1e..5786b5e 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -208,50 +208,27 @@ static Property s390_ipl_properties[] = {
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> -/*
>> - * In addition to updating the iplstate, this function returns:
>> - * - 0 if system was ipled with external kernel
>> - * - -1 if no valid boot device was found
>> - * - ccw id of the boot device otherwise
>> - */
>> -static uint64_t s390_update_iplstate(S390IPLState *ipl)
>> +static bool s390_gen_initial_iplb(S390IPLState *ipl)
>>  {
>>      DeviceState *dev_st;
>>  
>> -    if (ipl->iplb_valid) {
>> -        ipl->cssid = 0;
>> -        ipl->ssid = 0;
>> -        ipl->devno = ipl->iplb.devno;
>> -        goto out;
>> -    }
>> -
>> -    if (ipl->kernel) {
>> -        return 0;
>> -    }
>> -
>>      dev_st = get_boot_device(0);
>>      if (dev_st) {
>>          VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast(
>>              OBJECT(qdev_get_parent_bus(dev_st)->parent),
>>                  TYPE_VIRTIO_CCW_DEVICE);
>>          if (ccw_dev) {
>> -            ipl->cssid = ccw_dev->sch->cssid;
>> -            ipl->ssid = ccw_dev->sch->ssid;
>> -            ipl->devno = ccw_dev->sch->devno;
>>              ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
>>              ipl->iplb.blk0_len =
>>                  cpu_to_be32(S390_IPLB_MIN_CCW_LEN - S390_IPLB_HEADER_LEN);
>>              ipl->iplb.pbt = S390_IPL_TYPE_CCW;
>>              ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
>>              ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
>> -            ipl->iplb_valid = true;
>> -            goto out;
>> +            return true;
>>          }
>>      }
>>  
>> -    return -1;
>> -out:
>> -    return (uint32_t) (ipl->cssid << 24 | ipl->ssid << 16 | ipl->devno);
>> +    return false;
>>  }
>>  
>>  void s390_ipl_update_diag308(IplParameterBlock *iplb)
>> @@ -289,7 +266,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
>>  
>>      if (!ipl->kernel || ipl->iplb_valid) {
>>          cpu->env.psw.addr = ipl->bios_start_addr;
>> -        cpu->env.regs[7] = s390_update_iplstate(ipl);
>> +        if (!ipl->iplb_valid) {
>> +            ipl->iplb_valid = s390_gen_initial_iplb(ipl);
>> +        }
>>      }
>>  }
>>  
>>
>
Paolo Bonzini May 18, 2016, 10:37 a.m. UTC | #3 
On 18/05/2016 12:09, Christian Borntraeger wrote:
>> > This probably breaks reboot after migration from 2.6.
>> > 
>> > If that's fine, all the better. :)
> Our BIOS is in RAM and usually overwritten by the guest.
> On reboot this is reloaded from whatever BIOS is on the target,
> so migration should still work.

Indeed if the BIOS is not in its own RAMBlock then it's fine.

Thanks!

Paolo

> I have verified that migration between 2.4 and 2.6  plus these
> patches forth and back still allows to reboot.
> 
> In other words the bios and the code in ipl.c have to be updated
> in lockstep, then everything is fine.
> 
> 
>
diff mbox

Patch

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 2da0f1e..5786b5e 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -208,50 +208,27 @@  static Property s390_ipl_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-/*
- * In addition to updating the iplstate, this function returns:
- * - 0 if system was ipled with external kernel
- * - -1 if no valid boot device was found
- * - ccw id of the boot device otherwise
- */
-static uint64_t s390_update_iplstate(S390IPLState *ipl)
+static bool s390_gen_initial_iplb(S390IPLState *ipl)
 {
     DeviceState *dev_st;
 
-    if (ipl->iplb_valid) {
-        ipl->cssid = 0;
-        ipl->ssid = 0;
-        ipl->devno = ipl->iplb.devno;
-        goto out;
-    }
-
-    if (ipl->kernel) {
-        return 0;
-    }
-
     dev_st = get_boot_device(0);
     if (dev_st) {
         VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast(
             OBJECT(qdev_get_parent_bus(dev_st)->parent),
                 TYPE_VIRTIO_CCW_DEVICE);
         if (ccw_dev) {
-            ipl->cssid = ccw_dev->sch->cssid;
-            ipl->ssid = ccw_dev->sch->ssid;
-            ipl->devno = ccw_dev->sch->devno;
             ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
             ipl->iplb.blk0_len =
                 cpu_to_be32(S390_IPLB_MIN_CCW_LEN - S390_IPLB_HEADER_LEN);
             ipl->iplb.pbt = S390_IPL_TYPE_CCW;
             ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
             ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
-            ipl->iplb_valid = true;
-            goto out;
+            return true;
         }
     }
 
-    return -1;
-out:
-    return (uint32_t) (ipl->cssid << 24 | ipl->ssid << 16 | ipl->devno);
+    return false;
 }
 
 void s390_ipl_update_diag308(IplParameterBlock *iplb)
@@ -289,7 +266,9 @@  void s390_ipl_prepare_cpu(S390CPU *cpu)
 
     if (!ipl->kernel || ipl->iplb_valid) {
         cpu->env.psw.addr = ipl->bios_start_addr;
-        cpu->env.regs[7] = s390_update_iplstate(ipl);
+        if (!ipl->iplb_valid) {
+            ipl->iplb_valid = s390_gen_initial_iplb(ipl);
+        }
     }
 }