Re: [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws

[Alexander Graf <agraf@suse.de>]

On 05.09.14 09:23, Christian Borntraeger wrote:
> On 05/09/14 00:29, Alexander Graf wrote:
>>
>>
>> On 04.09.14 17:32, Jens Freimann wrote:
>>> From: Cornelia Huck <cornelia.huck@de.ibm.com>
>>>
>>> Add support for format-0 ccws in channel programs. As a format-1 ccw
>>> contains the same information as format-0 ccws, only supporting larger
>>> addresses, simply convert every ccw to format-1 as we walk the chain.
>>>
>>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
>>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>>> ---
>>>  hw/s390x/css.c        | 30 +++++++++++++++++++++---------
>>>  hw/s390x/css.h        |  1 +
>>>  target-s390x/ioinst.h | 10 ++++++++++
>>>  3 files changed, 32 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>>> index 49c2aaf..34637cb 100644
>>> --- a/hw/s390x/css.c
>>> +++ b/hw/s390x/css.c
>>> @@ -243,17 +243,25 @@ static void copy_sense_id_to_guest(SenseId *dest, SenseId *src)
>>>      }
>>>  }
>>>  
>>> -static CCW1 copy_ccw_from_guest(hwaddr addr)
>>> +static CCW1 copy_ccw_from_guest(hwaddr addr, bool fmt1)
>>>  {
>>> -    CCW1 tmp;
>>> +    CCW0 tmp0;
>>> +    CCW1 tmp1;
>>>      CCW1 ret;
>>>  
>>> -    cpu_physical_memory_read(addr, &tmp, sizeof(tmp));
>>> -    ret.cmd_code = tmp.cmd_code;
>>> -    ret.flags = tmp.flags;
>>> -    ret.count = be16_to_cpu(tmp.count);
>>> -    ret.cda = be32_to_cpu(tmp.cda);
>>> -
>>> +    if (fmt1) {
>>> +        cpu_physical_memory_read(addr, &tmp1, sizeof(tmp1));
>>> +        ret.cmd_code = tmp1.cmd_code;
>>> +        ret.flags = tmp1.flags;
>>> +        ret.count = be16_to_cpu(tmp1.count);
>>> +        ret.cda = be32_to_cpu(tmp1.cda);
>>> +    } else {
>>> +        cpu_physical_memory_read(addr, &tmp0, sizeof(tmp0));
>>> +        ret.cmd_code = tmp0.cmd_code;
>>> +        ret.flags = tmp0.flags;
>>> +        ret.count = be16_to_cpu(tmp0.count);
>>> +        ret.cda = be16_to_cpu(tmp0.cda1) | (tmp0.cda0 << 16);
>>> +    }
>>>      return ret;
>>>  }
>>>  
>>> @@ -268,7 +276,8 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr)
>>>          return -EIO;
>>>      }
>>>  
>>> -    ccw = copy_ccw_from_guest(ccw_addr);
>>> +    /* Translate everything to format-1 ccws - the information is the same. */
>>> +    ccw = copy_ccw_from_guest(ccw_addr, sch->ccw_fmt_1);
>>>  
>>>      /* Check for invalid command codes. */
>>>      if ((ccw.cmd_code & 0x0f) == 0) {
>>> @@ -386,6 +395,7 @@ static void sch_handle_start_func(SubchDev *sch, ORB *orb)
>>>              s->ctrl |= (SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND);
>>>              return;
>>>          }
>>> +        sch->ccw_fmt_1 = !!(orb->ctrl0 & ORB_CTRL0_MASK_FMT);
>>>      } else {
>>>          s->ctrl &= ~(SCSW_ACTL_SUSP | SCSW_ACTL_RESUME_PEND);
>>>      }
>>> @@ -1347,6 +1357,7 @@ void subch_device_save(SubchDev *s, QEMUFile *f)
>>>          qemu_put_byte(f, s->id.ciw[i].command);
>>>          qemu_put_be16(f, s->id.ciw[i].count);
>>>      }
>>> +    qemu_put_byte(f, s->ccw_fmt_1);
>>
>> This changes the migration stream format. Please increase the version
>> number for the device that gets migrated, so that we have the chance to
>> catch it.
>>
>> Though - does migration work at all yet? :)
> 
> Not yet.
> Myself and Jens are currently testing Davids latest patches regarding CPU states. (just chasing the final (tm) bug). 
> After that I can push the initial cpu migration patch set.
> So maybe we can leave this as is? Whatever you suggest.

Well, if migration doesn't work at all yet I think it's ok to consider
all of the state in flux.

However, please make sure that once you have migration work for the
first time, that any change like this has to also increase the version
number of the device migration stream.


Alex