In article <cam+xf6aotvw2b8uupf5_p3zqepscuabhmybxvm2xyqklg0m...@mail.gmail.com>,
NONAKA Kimihiro <nona...@gmail.com> wrote:
>Hi,
>
>2014-09-15 3:07 GMT+09:00 Christos Zoulas <chris...@astron.com>:
>
>> 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
>
