md driver compat32: fix structure padding for arm, powerpc

[• karels]

Summary

Because the 32-bit md_ioctl structure contains 64-bit members, arm
and powerpc add padding to a multiple of 8. i386 doesn't do this.
The md_ioctl32 definition was correct for amd64/i386 without padding,
but wrong for arm64 and powerpc64. Make packed conditional on
amd64, and test for the expected size on non-amd64. Note that
mdconfig is used in the ATF test suite. Note, I verified the
structure size for powerpc, but was unable to test.

[jrtc27]

sys/dev/md/md.c

150

Hm, this shouldn't be packed, md_ioctl isn't. Dropping packed would fix this instead.

[• karels]

sys/dev/md/md.c

150

That would break i386 compatibility on amd64.

[jrtc27]

sys/dev/md/md.c

150

Even on amd64 it shouldn't be packed, just packed(4). Though ideally we'd have an __aligned(4) freebsd32_uint64_t type (and an off_t variant) for that to avoid the #ifdefs everywhere they're needed.

[• karels]

sys/dev/md/md.c

150

I'm not sure what you are proposing for this fix. I could switch to ifdef'ing packed/packed(4) here; I see freebsd32_misc.c does that in a few places. A general solution seems like a much bigger change that we need now. But these structures are constructed by hand in any case.

[jrtc27]

sys/dev/md/md.c

150

Yes, #ifdef __amd64__ __packed(4) #endif

[• karels]

Change from explicit padding to using packed only on amd64.

[• karels]

sys/dev/md/md.c

150

I see no provision for __packed(4); the compiler chokes on the construct. I used the existing packed syntax with ifdefs.

[jrtc27]

sys/dev/md/md.c

150

Hm, interesting, you can't give packed an integer, but you can for the MS-style pragma variant accepted by both GCC and Clang, which OpenZFS uses for its 32-bit ioctl compat:

#ifdef __amd64__
#pragma pack(push, 4)
#endif
struct md_ioctl32 {
        ...
};
#ifdef __amd64__
#pragma pack(pop)
#endif

But making it more pedantically correct for amd64 is a separate issue that doesn't need to be done here, just ifdef'ing what's already done for amd64 seems fine.

[jrtc27]

LGTM provided commit message is altered to reflect the updated diff

[• karels]

In D41339#941327, @jrtc27 wrote:

LGTM provided commit message is altered to reflect the updated diff

The commit message has already been updated, on the review as well as in git.

[jrtc27]

In D41339#941331, @karels wrote:

In D41339#941327, @jrtc27 wrote:

LGTM provided commit message is altered to reflect the updated diff

The commit message has already been updated, on the review as well as in git.

So it has; I should have checked it again when the page refreshed after leaving my first comment