qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup
@ 2014-09-04 15:32 Jens Freimann
  2014-09-04 15:32 ` [Qemu-devel] [PATCH 1/3] s390x: remove duplicate defines in SCLP code Jens Freimann
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jens Freimann @ 2014-09-04 15:32 UTC (permalink / raw)
  To: Christian Borntraeger, Alexander Graf, Cornelia Huck
  Cc: Jens Freimann, qemu-devel

Cornelia, Christian, Alex,

here are two css patches and a small sclp cleanup. 

Patch 1 remove duplicate defines in SCLP code 
Patch 2 adds support for format-0 ccws
Patch 3 a css bugfix adding a limit of 255 to ccws chains without data transfer 

regards
Jens


Cornelia Huck (2):
  s390x/css: support format-0 ccws
  s390x/css: catch ccw sequence errors

Jens Freimann (1):
  s390x: remove duplicate defines in SCLP code

 hw/s390x/css.c          | 40 +++++++++++++++++++++++++++++++---------
 hw/s390x/css.h          |  2 ++
 include/hw/s390x/sclp.h |  2 --
 target-s390x/ioinst.h   | 10 ++++++++++
 4 files changed, 43 insertions(+), 11 deletions(-)

-- 
1.8.5.5

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/3] s390x: remove duplicate defines in SCLP code
  2014-09-04 15:32 [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup Jens Freimann
@ 2014-09-04 15:32 ` Jens Freimann
  2014-09-04 15:32 ` [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws Jens Freimann
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Jens Freimann @ 2014-09-04 15:32 UTC (permalink / raw)
  To: Christian Borntraeger, Alexander Graf, Cornelia Huck
  Cc: Jens Freimann, qemu-devel

Let's get rid of these duplicate defines.

Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 include/hw/s390x/sclp.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 5c43574..ec07a11 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -28,8 +28,6 @@
 #define SCLP_UNASSIGN_STORAGE                   0x000C0001
 #define SCLP_CMD_READ_EVENT_DATA                0x00770005
 #define SCLP_CMD_WRITE_EVENT_DATA               0x00760005
-#define SCLP_CMD_READ_EVENT_DATA                0x00770005
-#define SCLP_CMD_WRITE_EVENT_DATA               0x00760005
 #define SCLP_CMD_WRITE_EVENT_MASK               0x00780005
 
 /* SCLP Memory hotplug codes */
-- 
1.8.5.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws
  2014-09-04 15:32 [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup Jens Freimann
  2014-09-04 15:32 ` [Qemu-devel] [PATCH 1/3] s390x: remove duplicate defines in SCLP code Jens Freimann
@ 2014-09-04 15:32 ` Jens Freimann
  2014-09-04 22:29   ` Alexander Graf
  2014-09-04 15:32 ` [Qemu-devel] [PATCH 3/3] s390x/css: catch ccw sequence errors Jens Freimann
  2014-09-04 17:58 ` [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup Christian Borntraeger
  3 siblings, 1 reply; 8+ messages in thread
From: Jens Freimann @ 2014-09-04 15:32 UTC (permalink / raw)
  To: Christian Borntraeger, Alexander Graf, Cornelia Huck; +Cc: qemu-devel

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);
     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;
-- 
1.8.5.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 3/3] s390x/css: catch ccw sequence errors
  2014-09-04 15:32 [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup Jens Freimann
  2014-09-04 15:32 ` [Qemu-devel] [PATCH 1/3] s390x: remove duplicate defines in SCLP code Jens Freimann
  2014-09-04 15:32 ` [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws Jens Freimann
@ 2014-09-04 15:32 ` Jens Freimann
  2014-09-04 17:58 ` [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup Christian Borntraeger
  3 siblings, 0 replies; 8+ messages in thread
From: Jens Freimann @ 2014-09-04 15:32 UTC (permalink / raw)
  To: Christian Borntraeger, Alexander Graf, Cornelia Huck; +Cc: qemu-devel

From: Cornelia Huck <cornelia.huck@de.ibm.com>

We must not allow chains of more than 255 ccws without data transfer.

Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/css.c | 10 ++++++++++
 hw/s390x/css.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 34637cb..b67c039 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -294,6 +294,13 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr)
 
     check_len = !((ccw.flags & CCW_FLAG_SLI) && !(ccw.flags & CCW_FLAG_DC));
 
+    if (!ccw.cda) {
+        if (sch->ccw_no_data_cnt == 255) {
+            return -EINVAL;
+        }
+        sch->ccw_no_data_cnt++;
+    }
+
     /* Look at the command. */
     switch (ccw.cmd_code) {
     case CCW_CMD_NOOP:
@@ -396,6 +403,7 @@ static void sch_handle_start_func(SubchDev *sch, ORB *orb)
             return;
         }
         sch->ccw_fmt_1 = !!(orb->ctrl0 & ORB_CTRL0_MASK_FMT);
+        sch->ccw_no_data_cnt = 0;
     } else {
         s->ctrl &= ~(SCSW_ACTL_SUSP | SCSW_ACTL_RESUME_PEND);
     }
@@ -1358,6 +1366,7 @@ void subch_device_save(SubchDev *s, QEMUFile *f)
         qemu_put_be16(f, s->id.ciw[i].count);
     }
     qemu_put_byte(f, s->ccw_fmt_1);
+    qemu_put_byte(f, s->ccw_no_data_cnt);
     return;
 }
 
@@ -1414,6 +1423,7 @@ int subch_device_load(SubchDev *s, QEMUFile *f)
         s->id.ciw[i].count = qemu_get_be16(f);
     }
     s->ccw_fmt_1 = qemu_get_byte(f);
+    s->ccw_no_data_cnt = qemu_get_byte(f);
     return 0;
 }
 
diff --git a/hw/s390x/css.h b/hw/s390x/css.h
index 384a455..33104ac 100644
--- a/hw/s390x/css.h
+++ b/hw/s390x/css.h
@@ -78,6 +78,7 @@ struct SubchDev {
     bool last_cmd_valid;
     bool ccw_fmt_1;
     bool thinint_active;
+    uint8_t ccw_no_data_cnt;
     /* transport-provided data: */
     int (*ccw_cb) (SubchDev *, CCW1);
     SenseId id;
-- 
1.8.5.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup
  2014-09-04 15:32 [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup Jens Freimann
                   ` (2 preceding siblings ...)
  2014-09-04 15:32 ` [Qemu-devel] [PATCH 3/3] s390x/css: catch ccw sequence errors Jens Freimann
@ 2014-09-04 17:58 ` Christian Borntraeger
  3 siblings, 0 replies; 8+ messages in thread
From: Christian Borntraeger @ 2014-09-04 17:58 UTC (permalink / raw)
  To: Jens Freimann, Alexander Graf, Cornelia Huck; +Cc: qemu-devel

On 04/09/14 17:32, Jens Freimann wrote:
> Cornelia, Christian, Alex,
> 
> here are two css patches and a small sclp cleanup. 
> 
> Patch 1 remove duplicate defines in SCLP code 
> Patch 2 adds support for format-0 ccws
> Patch 3 a css bugfix adding a limit of 255 to ccws chains without data transfer 
> 
> regards
> Jens
> 
> 
> Cornelia Huck (2):
>   s390x/css: support format-0 ccws
>   s390x/css: catch ccw sequence errors
> 
> Jens Freimann (1):
>   s390x: remove duplicate defines in SCLP code
> 
>  hw/s390x/css.c          | 40 +++++++++++++++++++++++++++++++---------
>  hw/s390x/css.h          |  2 ++
>  include/hw/s390x/sclp.h |  2 --
>  target-s390x/ioinst.h   | 10 ++++++++++
>  4 files changed, 43 insertions(+), 11 deletions(-)
> 

Can you add your signed-off-by to Connies patches as well and resend? 

Christian

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws
  2014-09-04 15:32 ` [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws Jens Freimann
@ 2014-09-04 22:29   ` Alexander Graf
  2014-09-05  7:23     ` Christian Borntraeger
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Graf @ 2014-09-04 22:29 UTC (permalink / raw)
  To: Jens Freimann, Christian Borntraeger, Cornelia Huck; +Cc: qemu-devel



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;
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws
  2014-09-04 22:29   ` Alexander Graf
@ 2014-09-05  7:23     ` Christian Borntraeger
  2014-09-05  7:25       ` Alexander Graf
  0 siblings, 1 reply; 8+ messages in thread
From: Christian Borntraeger @ 2014-09-05  7:23 UTC (permalink / raw)
  To: Alexander Graf, Jens Freimann, Cornelia Huck; +Cc: qemu-devel

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;
>>
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws
  2014-09-05  7:23     ` Christian Borntraeger
@ 2014-09-05  7:25       ` Alexander Graf
  0 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2014-09-05  7:25 UTC (permalink / raw)
  To: Christian Borntraeger, Jens Freimann, Cornelia Huck; +Cc: qemu-devel



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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-09-05  7:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-04 15:32 [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup Jens Freimann
2014-09-04 15:32 ` [Qemu-devel] [PATCH 1/3] s390x: remove duplicate defines in SCLP code Jens Freimann
2014-09-04 15:32 ` [Qemu-devel] [PATCH 2/3] s390x/css: support format-0 ccws Jens Freimann
2014-09-04 22:29   ` Alexander Graf
2014-09-05  7:23     ` Christian Borntraeger
2014-09-05  7:25       ` Alexander Graf
2014-09-04 15:32 ` [Qemu-devel] [PATCH 3/3] s390x/css: catch ccw sequence errors Jens Freimann
2014-09-04 17:58 ` [Qemu-devel] [PATCH 0/3] s390x: css patches and small sclp cleanup Christian Borntraeger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).