Re: [PATCH] coreboot: Remove GRUB_PACKED from grub_linuxbios_mem_region struct

[Joonas Kylmälä]

Hi,

I continued discussion about this patch on #coreboot IRC channel. As was
brought up on that discussion there is still a theoretical possibility
that the user compiled coreboot and grub with different compiler
settings for alignment. In those probably rare cases (as I understood
from the #coreboot discussion) the grub code would fail. As far as I
know there is currently nothing we can do in grub code about that case
of having different compiler settings. One solution proposed to the
coreboot table ABI was to use some other data structure in coreboot
instead of structs for conveying this information. There was more
details but it was beyond my understanding so please join the IRC
channel or be in contact some other way with the coreboot developers if
you are interested in making the coreboot table ABI more reliable.

So just to be clear: I would still like the patch to be applied to grub
since there is no fix yet in coreboot and no plans to my knowledge for
changing the coreboot table ABI. Also I'm working on the
aarch64+coreboot port of grub so for that we will need this patch. I
will slowly start sending those other patches too as I progress with the
port.

Joonas

Joonas Kylmälä:
> GRUB_PACKED allowed only using this struct with architectures
> that use 4 byte or less aligned memory pointers. Coreboot doesn't use
> the packed attribute when writing this to the coreboot table meaning
> it is safe to remove GRUB_PACKED here.
> 
> This change makes it now possible to support 64 bit architectures that
> expect 8 byte aligned pointers.
> 
> Signed-off-by: Joonas Kylmälä <address@hidden>
> ---
>  include/grub/coreboot/lbio.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/grub/coreboot/lbio.h b/include/grub/coreboot/lbio.h
> index 5076d36c7..3a640fcae 100644
> --- a/include/grub/coreboot/lbio.h
> +++ b/include/grub/coreboot/lbio.h
> @@ -97,7 +97,7 @@ struct grub_linuxbios_mem_region
>    grub_uint64_t size;
>  #define GRUB_MACHINE_MEMORY_AVAILABLE                1
>    grub_uint32_t type;
> -} GRUB_PACKED;
> +};
>  typedef struct grub_linuxbios_mem_region *mem_region_t;
>  
>  grub_err_t
>