[oi-dev] Broadcom wireless drivers for OpenIndiana

[Jean-Pierre]

Hi Jim,

Jim Klimov wrote:
> I am having a problem with compilation of the driver. I've traced it to
> the new layout_check() routine which finds discrepancies between actual
> and expected structure sizes, and fails loading of the driver (better be
> more verbose about that, too - took me a while to track that "Can't load
> module: exec format error" error while trying various compiler/linker
> options).
>
> I'm reading MSDN now, and it seems that the expected offsets are wrong
> in the routine, but need someone to confitm :)
>
> Points in question:
>
> * Microsoft definition of the *IO_STACK_LOCATION* structure, i.e.
>
> http://msdn.microsoft.com/en-us/library/windows/hardware/ff550659(v=vs.85).aspx
>
> typedef struct _IO_STACK_LOCATION {
> UCHAR MajorFunction;
> UCHAR MinorFunction;
> UCHAR Flags;
> UCHAR Control;
> union {
> ...
>
> struct {
> ULONG Length;
> ULONG POINTER_ALIGNMENT Key;
> LARGE_INTEGER ByteOffset;
> } Read;
> ...
>
> ULONG seems to be defined as a 32-bit word, though I can't be certain now.

Yes ULONG is an unsigned 32-bit pointer.

>
> * ndis driver definition in include/ntoskrnl_var.h (~line 835):
>
> /* See
> http://msdn.microsoft.com/en-us/library/windows/hardware/ff550659(v=vs.85).aspx
> */
> struct io_stack_location {
> uint8_t isl_major;
> uint8_t isl_minor;
> uint8_t isl_flags;
> uint8_t isl_ctl;
> ...
>
> union {
> struct {
> uint32_t isl_len;
> /* "POINTER_ALIGNMENT" according to msdn */
> uint32_t *isl_key;
> uint64_t isl_byteoff;
> } isl_read;
> ...
>
> * validation in if_ndis.c (~line 480):
>
> || (offsetof(struct io_stack_location,
> isl_parameters.isl_read.isl_len) != 8)
> || (offsetof(struct io_stack_location,
> isl_parameters.isl_read.isl_byteoff) != 24)
> || (offsetof(struct io_stack_location,
> isl_parameters.isl_ioctl.isl_obuflen) != 8)
> || (offsetof(struct io_stack_location,
> isl_parameters.isl_ioctl.isl_ibuflen) != 16)
> || (offsetof(struct io_stack_location,
> isl_parameters.isl_ioctl.isl_iocode) != 24)
> || (offsetof(struct io_stack_location,
> isl_devobj) != 40)
> || (offsetof(struct io_stack_location,
> &nbs p;isl_completionfunc) != 56));
>
> I can't get where the offset 8 for first component in unions comes into
> play? Before it are only four 1-byte variables, and the offsetof also
> returns 4...

Yes, it took me also some time to figure it out.

Tke key element is the attribute "POINTER_ALIGNMENT" on
the msdn description you copied above. On a 64-bit CPU
this is supposed to mean alignment to a multiple of 8
bytes. The attribute is defined for the field isl_key
(and others), but this recursively implies the outer
structures and union to also be aligned, otherwise the
inner alignment would not hold. Compilers are expected
to care for it.

I spent much time on this issue, and I added the checks
so that other users would not get stuck on it.

The ndis emulator is bridging between the Windows world
and the Unix one which have different rules for function
calls and structure layout, so you have to make sure
the compiler takes both into account.

Depending on how your compiler behaves, you may have to
insert a dummy field to force the expected alignment.
I have already had to insert such fillers in the structure,
but this one is just a consequence of usual alignment
rules.

>
> The header does define padding for amd64 cases, but for later-occurring
> variables.
>
> So I guess it is a question for Jean-Pierre: where did you derive the
> numbers in layout_check from? Are they really correct?

I have had to check them in the binary image of the driver,
so I am pretty sure about them.

Jean-Pierre