Re: [Qemu-devel] [PULL 09/20] s390x/ipl: Remove redundant usage of gr7

[Christian Borntraeger <borntraeger@de.ibm.com>]

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);
>> +        }
>>      }
>>  }
>>  
>>
>