diff mbox

[2/3] s390x/css: support format-0 ccws

Message ID 1409844743-31009-3-git-send-email-jfrei@linux.vnet.ibm.com
State New
Headers show

Commit Message

Jens Freimann Sept. 4, 2014, 3:32 p.m. UTC
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(-)

Comments

Alexander Graf Sept. 4, 2014, 10:29 p.m. UTC | #1
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? :)


Alex

>      return;
>  }
>  
> @@ -1402,6 +1413,7 @@ int subch_device_load(SubchDev *s, QEMUFile *f)
>          s->id.ciw[i].command = qemu_get_byte(f);
>          s->id.ciw[i].count = qemu_get_be16(f);
>      }
> +    s->ccw_fmt_1 = qemu_get_byte(f);
>      return 0;
>  }
>  
> diff --git a/hw/s390x/css.h b/hw/s390x/css.h
> index c864ea7..384a455 100644
> --- a/hw/s390x/css.h
> +++ b/hw/s390x/css.h
> @@ -76,6 +76,7 @@ struct SubchDev {
>      hwaddr channel_prog;
>      CCW1 last_cmd;
>      bool last_cmd_valid;
> +    bool ccw_fmt_1;
>      bool thinint_active;
>      /* transport-provided data: */
>      int (*ccw_cb) (SubchDev *, CCW1);
> diff --git a/target-s390x/ioinst.h b/target-s390x/ioinst.h
> index 5bbc67d..29f6423 100644
> --- a/target-s390x/ioinst.h
> +++ b/target-s390x/ioinst.h
> @@ -156,6 +156,16 @@ typedef struct ORB {
>  #define ORB_CTRL1_MASK_ORBX 0x01
>  #define ORB_CTRL1_MASK_INVALID 0x3e
>  
> +/* channel command word (type 0) */
> +typedef struct CCW0 {
> +        uint8_t cmd_code;
> +        uint8_t cda0;
> +        uint16_t cda1;
> +        uint8_t flags;
> +        uint8_t reserved;
> +        uint16_t count;
> +} QEMU_PACKED CCW0;
> +
>  /* channel command word (type 1) */
>  typedef struct CCW1 {
>      uint8_t cmd_code;
>
Christian Borntraeger Sept. 5, 2014, 7:23 a.m. UTC | #2
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.


> 
> 
> Alex
> 
>>      return;
>>  }
>>  
>> @@ -1402,6 +1413,7 @@ int subch_device_load(SubchDev *s, QEMUFile *f)
>>          s->id.ciw[i].command = qemu_get_byte(f);
>>          s->id.ciw[i].count = qemu_get_be16(f);
>>      }
>> +    s->ccw_fmt_1 = qemu_get_byte(f);
>>      return 0;
>>  }
>>  
>> diff --git a/hw/s390x/css.h b/hw/s390x/css.h
>> index c864ea7..384a455 100644
>> --- a/hw/s390x/css.h
>> +++ b/hw/s390x/css.h
>> @@ -76,6 +76,7 @@ struct SubchDev {
>>      hwaddr channel_prog;
>>      CCW1 last_cmd;
>>      bool last_cmd_valid;
>> +    bool ccw_fmt_1;
>>      bool thinint_active;
>>      /* transport-provided data: */
>>      int (*ccw_cb) (SubchDev *, CCW1);
>> diff --git a/target-s390x/ioinst.h b/target-s390x/ioinst.h
>> index 5bbc67d..29f6423 100644
>> --- a/target-s390x/ioinst.h
>> +++ b/target-s390x/ioinst.h
>> @@ -156,6 +156,16 @@ typedef struct ORB {
>>  #define ORB_CTRL1_MASK_ORBX 0x01
>>  #define ORB_CTRL1_MASK_INVALID 0x3e
>>  
>> +/* channel command word (type 0) */
>> +typedef struct CCW0 {
>> +        uint8_t cmd_code;
>> +        uint8_t cda0;
>> +        uint16_t cda1;
>> +        uint8_t flags;
>> +        uint8_t reserved;
>> +        uint16_t count;
>> +} QEMU_PACKED CCW0;
>> +
>>  /* channel command word (type 1) */
>>  typedef struct CCW1 {
>>      uint8_t cmd_code;
>>
>
Alexander Graf Sept. 5, 2014, 7:25 a.m. UTC | #3
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
diff mbox

Patch

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);
     return;
 }
 
@@ -1402,6 +1413,7 @@  int subch_device_load(SubchDev *s, QEMUFile *f)
         s->id.ciw[i].command = qemu_get_byte(f);
         s->id.ciw[i].count = qemu_get_be16(f);
     }
+    s->ccw_fmt_1 = qemu_get_byte(f);
     return 0;
 }
 
diff --git a/hw/s390x/css.h b/hw/s390x/css.h
index c864ea7..384a455 100644
--- a/hw/s390x/css.h
+++ b/hw/s390x/css.h
@@ -76,6 +76,7 @@  struct SubchDev {
     hwaddr channel_prog;
     CCW1 last_cmd;
     bool last_cmd_valid;
+    bool ccw_fmt_1;
     bool thinint_active;
     /* transport-provided data: */
     int (*ccw_cb) (SubchDev *, CCW1);
diff --git a/target-s390x/ioinst.h b/target-s390x/ioinst.h
index 5bbc67d..29f6423 100644
--- a/target-s390x/ioinst.h
+++ b/target-s390x/ioinst.h
@@ -156,6 +156,16 @@  typedef struct ORB {
 #define ORB_CTRL1_MASK_ORBX 0x01
 #define ORB_CTRL1_MASK_INVALID 0x3e
 
+/* channel command word (type 0) */
+typedef struct CCW0 {
+        uint8_t cmd_code;
+        uint8_t cda0;
+        uint16_t cda1;
+        uint8_t flags;
+        uint8_t reserved;
+        uint16_t count;
+} QEMU_PACKED CCW0;
+
 /* channel command word (type 1) */
 typedef struct CCW1 {
     uint8_t cmd_code;