Re: [PATCH v2 02/22] include/xen/slr-table.h: Secure Launch Resource Table definitions

[Sergii Dmytruk <sergii.dmytruk@xxxxxxxxx>]

On Mon, Jun 02, 2025 at 09:31:11AM +0200, Jan Beulich wrote:
> >>> +#define UEFI_SLR_TABLE_GUID \
> >>> +    { 0x877a9b2aU, 0x0385, 0x45d1, { 0xa0, 0x34, 0x9d, 0xac, 0x9c, 0x9e, 
> >>> 0x56, 0x5f } }
> >>
> >> I'm not sure this is a good place to put UEFI GUIDs. Considering e.g ...
> >
> > It's here because the GUID is related more to SLRT than to EFI.  I can
> > move it if there is a more fitting place for table GUIDs.
>
> It'll (at least somewhat) depend on where it's going to be used. A common 
> problem
> when definitions / declarations are introduced without any use.

It's only used in xen/common/efi/boot.c (patch #20), so looks like it
should actually be defined there like the rest of GUIDs.

> >>> +/*
> >>> + * Primary SLR Table Header
> >>> + */
> >>> +struct slr_table
> >>> +{
> >>> +    uint32_t magic;
> >>> +    uint16_t revision;
> >>> +    uint16_t architecture;
> >>> +    uint32_t size;
> >>> +    uint32_t max_size;
> >>> +    /* entries[] */
> >>> +} __packed;
> >>
> >> If x86-specific, the question on the need for some of the __packed arises
> >> again.
> >
> > The table is used to communicate data from pre-DRTM world to DRTM-world
> > and is produced and consumed by unrelated software components that don't
> > necessarily pad structures the same way by default.
>
> How do other environments matter when this header is solely used by Xen?

Xen uses this header to read data prepared for it.  Packing is an easy
way to ensure the data will be parsed consistently regardless of the
architecture or software component which prepared it (i.e., a way to
enforce proper "API").

Regards