Re: CVS commit: src/sys/sys

[christos%astron.com@localhost (Christos Zoulas)]

In article 
<CAM+xf6AoTVw2B8UUpf5_P3zQePSCuabhmYbXVm2XyQKLg0MUZw%mail.gmail.com@localhost>,
NONAKA Kimihiro  <nonakap%gmail.com@localhost> wrote:
>Hi,
>
>2014-09-15 3:07 GMT+09:00 Christos Zoulas <christos%astron.com@localhost>:
>
>> Perhaps it is better to memcpy the data to some aligned struct rather
>> than referencing it directly and making it packed (which is not portable).
>
>I wrote the following patch. ok?

Yes, I think it is better. I'd move the XXX: comment before the sb_dd
declaration... Eventually we could merge the byte swapping routines I guess
between the kernel and userland. What do others think?

Thanks,

christos

>
>Index: ./sys/sys/bootblock.h
>===================================================================
>RCS file: /cvsroot/src/sys/sys/bootblock.h,v
>retrieving revision 1.57
>diff -u -r1.57 bootblock.h
>--- ./sys/sys/bootblock.h    14 Sep 2014 17:39:06 -0000    1.57
>+++ ./sys/sys/bootblock.h    15 Sep 2014 02:50:00 -0000
>@@ -815,7 +815,7 @@
>     uint32_t    descBlock;    /* first block of driver */
>     uint16_t    descSize;    /* driver size in blocks */
>     uint16_t    descType;    /* system type */
>-} __packed;
>+};
>
> /*
>  *    system types; Apple reserves 0-15
>@@ -834,6 +834,10 @@
>     uint32_t    sbData;        /* (used internally by ROM) */
>     uint16_t    sbDrvrCount;    /* number of driver descriptors */
>     struct apple_drvr_descriptor sb_dd[APPLE_DRVR_MAP_MAX_DESCRIPTORS];
>+                    /* XXX: Don't access directly.
>+                     * Data abort occurs by an access to
>+                     * unaligned address on arm.
>+                     */
>     uint16_t    pad[3];
> } __packed;
>
>Index: ./sys/dev/dkwedge/dkwedge_apple.c
>===================================================================
>RCS file: /cvsroot/src/sys/dev/dkwedge/dkwedge_apple.c,v
>retrieving revision 1.1
>diff -u -r1.1 dkwedge_apple.c
>--- ./sys/dev/dkwedge/dkwedge_apple.c    7 Apr 2012 05:36:10 -0000    1.1
>+++ ./sys/dev/dkwedge/dkwedge_apple.c    15 Sep 2014 02:50:00 -0000
>@@ -82,8 +82,12 @@
>     if (ap->sbDrvrCount >= APPLE_DRVR_MAP_MAX_DESCRIPTORS)
>         ap->sbDrvrCount = APPLE_DRVR_MAP_MAX_DESCRIPTORS;
>
>-    for (i = 0; i < ap->sbDrvrCount; i++)
>-        swap_apple_drvr_descriptor(&ap->sb_dd[i]);
>+    for (i = 0; i < ap->sbDrvrCount; i++) {
>+        struct apple_drvr_descriptor sdd;
>+        memcpy(&sdd, &ap->sb_dd[i], sizeof(ap->sb_dd[0]));
>+        swap_apple_drvr_descriptor(&sdd);
>+        memcpy(&ap->sb_dd[i], &sdd, sizeof(ap->sb_dd[0]));
>+    }
> }
>
> static void
>Index: ./sbin/apmlabel/apmlabel.c
>===================================================================
>RCS file: /cvsroot/src/sbin/apmlabel/apmlabel.c,v
>retrieving revision 1.3
>diff -u -r1.3 apmlabel.c
>--- ./sbin/apmlabel/apmlabel.c    19 Oct 2013 01:09:58 -0000    1.3
>+++ ./sbin/apmlabel/apmlabel.c    15 Sep 2014 02:50:00 -0000
>@@ -249,9 +249,12 @@
>     BE32TOH(drvr->sbData);
>     BE16TOH(drvr->sbDrvrCount);
>     for (i=0; i<APPLE_DRVR_MAP_MAX_DESCRIPTORS; i++) {
>-        BE32TOH(drvr->sb_dd[i].descBlock);
>-        BE16TOH(drvr->sb_dd[i].descSize);
>-        BE16TOH(drvr->sb_dd[i].descType);
>+        struct apple_drvr_descriptor sdd;
>+        memcpy(&sdd, &drvr->sb_dd[i], sizeof(drvr->sb_dd[0]));
>+        BE32TOH(sdd.descBlock);
>+        BE16TOH(sdd.descSize);
>+        BE16TOH(sdd.descType);
>+        memcpy(&drvr->sb_dd[i], &sdd, sizeof(drvr->sb_dd[0]));
>     }
>
>     return drvr;
>
>
>Regards,
>-- 
>NONAKA Kimihiro
>