Hi Luca,
Spent some time doing more testing on this patch since initially I had only tested it on
basic programs. After "copyinmsg: allow for the last message element to have msgt_number = 0.
"
it can boot a basic system up to the bash shell. I haven't seen any other issues so far.
Hi,
Il 17/05/23 05:03, Flavio Cruz ha scritto:
> * Make full use of the 8 bytes available in mach_msg_type_t by moving
> into the unused 4 bytes. This allows us to use 32bits for
> mach_msg_type_number_t whether we use the longform or not.
> * Make mach_msg_type_long_t exactly the same as mach_msg_type_t. I'm not
> changing any of the code but keeping the same interface using macros.
> Updating MiG is strongly encouraged since it will generate better code
> to handle this new format.
>
> After this change, any compatibility with compiled binaries for Hurd x86_64
> will break since the message format is different. However, the new
> schema simplifies the overall ABI, without having "holes" and also
> avoids the need to have a 16 byte mach_msg_type_long_t.
>
> Tested with simple programs on pure 64 bit and 64/32 bit combinations >
> ---
> Not too thrilled about the use of the macros for msgtl_name/size/number
> but happy to hear about other alternatives. Potentially we could
> introduce accessors and update all the calls to use them.
>
> Note that the follow up MiG patch is necessary to make this work.
>
> include/mach/message.h | 37 +++++++++++++--
> ipc/ipc_kmsg.c | 4 ++
> x86_64/copy_user.c | 100 ++++++++++++++++++++++++++++++++---------
> 3 files changed, 115 insertions(+), 26 deletions(-)
>
> diff --git a/include/mach/message.h b/include/mach/message.h
> index 0eab9d41..d1783715 100644
> --- a/include/mach/message.h
> +++ b/include/mach/message.h
> @@ -222,6 +222,30 @@ typedef unsigned int mach_msg_type_size_t;
> typedef natural_t mach_msg_type_number_t;
>
> typedef struct {
> +#ifdef __x86_64__
> + /*
> + * For 64 bits, this struct is 8 bytes long so we
> + * can pack the same amount of information as mach_msg_type_long_t.
> + * Note that for 64 bit userland, msgt_size only needs to be 8 bits long
> + * but for kernel compatibility with 32 bit userland we allow it to be
> + * 16 bits long.
> + *
> + * Effectively, we don't need mach_msg_type_long_t but we are keeping it
> + * for a while to make the code similar between 32 and 64 bits.
> + *
> + * We also keep the msgt_longform bit around simply because it makes it
> + * very easy to convert messages from a 32 bit userland into a 64 bit
> + * kernel. Otherwise, we would have to replicate some of the MiG logic
> + * internally in the kernel.
> + */
> + unsigned int msgt_inline : 1,
> + msgt_longform : 1,
> + msgt_deallocate : 1,
> + msgt_name : 8,
> + msgt_size : 16,
> + msgt_unused : 5;
> + mach_msg_type_number_t msgt_number;
> +#else
> unsigned int msgt_name : 8,
> msgt_size : 8,
> msgt_number : 12,
> @@ -229,18 +253,23 @@ typedef struct {
> msgt_longform : 1,
> msgt_deallocate : 1,
> msgt_unused : 1;
> -#ifdef __x86_64__
> - /* TODO: We want to eventually use this in favor of mach_msg_type_long_t
> - * as it makes the mach_msg protocol require only mach_msg_type_t. */
> - mach_msg_type_number_t unused_msgtl_number;
> #endif
> } __attribute__ ((aligned (__alignof__ (uintptr_t)))) mach_msg_type_t;
Maybe the user/kernel variants of mach_msg_type_t and
mach_msg_type_long_t should also be moved here from copy_user.c, just as
the header struct (or all the user variants to a kernel-specific header)
Yes, that makes sense, but maybe we can do it in a separate patch?
> +/* On x86_64 this is equivalent to mach_msg_type_t. */
> typedef struct {
> mach_msg_type_t msgtl_header;
> +#ifdef __x86_64__
> +/* Since we don't have these separate fields, we remap them
> + * into the fields from mach_msg_type_t. */
> +#define msgtl_name msgtl_header.msgt_name
> +#define msgtl_size msgtl_header.msgt_size
> +#define msgtl_number msgtl_header.msgt_number
> +#else
> unsigned short msgtl_name;
> unsigned short msgtl_size;
> natural_t msgtl_number;
> +#endif
> } __attribute__ ((aligned (__alignof__ (uintptr_t)))) mach_msg_type_long_t;
one alternative to the #defines could be something similar to
typedef union {
mach_msg_type_t msgtl_header;
struct {
unsigned int msgt_unused1 : 3,
msgt_name : 8,
msgt_size : 16,
msgt_unused2 : 5;
mach_msg_type_number_t msgt_number;
};
} __attribute__ ((aligned (__alignof__ (uintptr_t)))) mach_msg_type_long_t;
This would replicate the definition of the bitfields, but IMHO it would
be more readable than another level of macro. I guess we can ensure both
definitions are consistent with some static asserts.
Thanks, I feel this is better than using macros. Sent version 2.
Luca