Re: [PATCH mig] Make MIG work for pure 64 bit kernel and userland.

[Luca]

Il 12/02/23 07:15, Flavio Cruz ha scritto:
> Made changes to MIG so that generated stubs can be compiled when using
> a 64 bit userland. The most important change here is to ensure
> that data is always 8-byte aligned. Not doing that will lead to
> undefined behavior since on 64 bits many structures can contain 64 bit
> scalars which require 8-byte alignment. Even if x86 is usually fine with
> unaligned accesses, compilers are taking advantage of this UD which may
> break us in the future
> (http://pzemtsov.github.io/2016/11/06/bug-story-alignment-on-x86.html)
>
> The downside is that messages get significantly larger since we have
> to pad the initial mach_msg_type_t so that the data item can be 8-byte
> aligned. In the future, I want to make MIG layout the data in a way
> that non-complex data can be packed together (essentially, how XNU messages
> work) and to force userland to create larger structures so that we don't
> have to do any structure resizing while in the kernel. For now, this unblocks
> future work on 64 bits and is quite simple to understand.
>
> For the 64 bit / 32 bit configuration, we can pass --enable-user32 when
> targeting x86_64, and MIG will generate stubs as before. I am unsure
> whether there exists undefined behavior in that particular configuration
> since most of the structures used by the kernel do not seem to use 8
> byte scalars so potentially we should be OK there.

As mentioned in the article, using memcpy() to a correctly aligned 
pointer seems to avoid the possible alignment issues. Since in the 
kernel we need to use copyin/copyout anyway, wouldn't this be enough?

As for the mig stubs, if I understand correctly the issue stems from the 
fact that they use a struct to create the message fields in memory, so 
the binary format of a message needs to have the same alignment 
requirements. Would using memcpy here avoid the alignment issue?
I think it would even simplify the stub code, e.g. for array types, and 
I'm not sure it would decrease performance.

> As a follow up, we will need to change gnumach to iterate over these
> messages correctly (to follow the 8-byte alignment rules).

I see that currently even the 64-bit version is compiled with 
-mno-3dnow -mno-mmx -mno-sse -mno-sse2, do you mean to also remove these 
flags?


> diff --git a/utils.h b/utils.h
> index 4be4f4c..477ea74 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -39,6 +39,8 @@
>     ({ __typeof__ (a) _a = (a); \
>         __typeof__ (b) _b = (b); \
>       _a > _b ? _b : _a; })
> +#define ALIGN(x, alignment) \
> +    ((x) + (alignment)-1) & ~((alignment)-1)

Maybe this could be replaced by the mach_msg_align() macro from 
mach/message.h. As far as I understand, they have similar purpose, and 
it may already include the USER32 setting, in the future.


Luca