[Qemu-devel] [PATCH 11/14] hw/vfio/ccw: avoid taking address members in packed structs

[Daniel P . Berrangé]

The GCC 9 compiler complains about many places in s390 code
that take the address of members of the 'struct SCHIB' which
is marked packed:

hw/vfio/ccw.c: In function ‘vfio_ccw_io_notifier_handler’:
hw/vfio/ccw.c:133:15: warning: taking address of packed member of ‘struct 
SCHIB’ may result in an unaligned pointer value \
[-Waddress-of-packed-member]
  133 |     SCSW *s = &sch->curr_status.scsw;
      |               ^~~~~~~~~~~~~~~~~~~~~~
hw/vfio/ccw.c:134:15: warning: taking address of packed member of ‘struct 
SCHIB’ may result in an unaligned pointer value \
[-Waddress-of-packed-member]
  134 |     PMCW *p = &sch->curr_status.pmcw;
      |               ^~~~~~~~~~~~~~~~~~~~~~

...snip many more...

Almost all of these are just done for convenience to avoid
typing out long variable/field names when referencing struct
members. We can get most of this convenience by taking the
address of the 'struct SCHIB' instead, avoiding triggering
the compiler warnings.

In a couple of places we copy via a local variable which is
a technique already applied elsewhere in s390 code for this
problem.

Signed-off-by: Daniel P. Berrangé <address@hidden>
---
 hw/vfio/ccw.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 9246729a75..c44d13cc50 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -130,8 +130,8 @@ static void vfio_ccw_io_notifier_handler(void *opaque)
     S390CCWDevice *cdev = S390_CCW_DEVICE(vcdev);
     CcwDevice *ccw_dev = CCW_DEVICE(cdev);
     SubchDev *sch = ccw_dev->sch;
-    SCSW *s = &sch->curr_status.scsw;
-    PMCW *p = &sch->curr_status.pmcw;
+    SCHIB *schib = &sch->curr_status;
+    SCSW s;
     IRB irb;
     int size;
 
@@ -145,33 +145,33 @@ static void vfio_ccw_io_notifier_handler(void *opaque)
         switch (errno) {
         case ENODEV:
             /* Generate a deferred cc 3 condition. */
-            s->flags |= SCSW_FLAGS_MASK_CC;
-            s->ctrl &= ~SCSW_CTRL_MASK_STCTL;
-            s->ctrl |= (SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND);
+            schib->scsw.flags |= SCSW_FLAGS_MASK_CC;
+            schib->scsw.ctrl &= ~SCSW_CTRL_MASK_STCTL;
+            schib->scsw.ctrl |= (SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND);
             goto read_err;
         case EFAULT:
             /* Memory problem, generate channel data check. */
-            s->ctrl &= ~SCSW_ACTL_START_PEND;
-            s->cstat = SCSW_CSTAT_DATA_CHECK;
-            s->ctrl &= ~SCSW_CTRL_MASK_STCTL;
-            s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY |
+            schib->scsw.ctrl &= ~SCSW_ACTL_START_PEND;
+            schib->scsw.cstat = SCSW_CSTAT_DATA_CHECK;
+            schib->scsw.ctrl &= ~SCSW_CTRL_MASK_STCTL;
+            schib->scsw.ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY |
                        SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND;
             goto read_err;
         default:
             /* Error, generate channel program check. */
-            s->ctrl &= ~SCSW_ACTL_START_PEND;
-            s->cstat = SCSW_CSTAT_PROG_CHECK;
-            s->ctrl &= ~SCSW_CTRL_MASK_STCTL;
-            s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY |
+            schib->scsw.ctrl &= ~SCSW_ACTL_START_PEND;
+            schib->scsw.cstat = SCSW_CSTAT_PROG_CHECK;
+            schib->scsw.ctrl &= ~SCSW_CTRL_MASK_STCTL;
+            schib->scsw.ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY |
                        SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND;
             goto read_err;
         }
     } else if (size != vcdev->io_region_size) {
         /* Information transfer error, generate channel-control check. */
-        s->ctrl &= ~SCSW_ACTL_START_PEND;
-        s->cstat = SCSW_CSTAT_CHN_CTRL_CHK;
-        s->ctrl &= ~SCSW_CTRL_MASK_STCTL;
-        s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY |
+        schib->scsw.ctrl &= ~SCSW_ACTL_START_PEND;
+        schib->scsw.cstat = SCSW_CSTAT_CHN_CTRL_CHK;
+        schib->scsw.ctrl &= ~SCSW_CTRL_MASK_STCTL;
+        schib->scsw.ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY |
                    SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND;
         goto read_err;
     }
@@ -179,11 +179,13 @@ static void vfio_ccw_io_notifier_handler(void *opaque)
     memcpy(&irb, region->irb_area, sizeof(IRB));
 
     /* Update control block via irb. */
-    copy_scsw_to_guest(s, &irb.scsw);
+    s = schib->scsw;
+    copy_scsw_to_guest(&s, &irb.scsw);
+    schib->scsw = s;
 
     /* If a uint check is pending, copy sense data. */
-    if ((s->dstat & SCSW_DSTAT_UNIT_CHECK) &&
-        (p->chars & PMCW_CHARS_MASK_CSENSE)) {
+    if ((schib->scsw.dstat & SCSW_DSTAT_UNIT_CHECK) &&
+        (schib->pmcw.chars & PMCW_CHARS_MASK_CSENSE)) {
         memcpy(sch->sense_data, irb.ecw, sizeof(irb.ecw));
     }
 
-- 
2.20.1