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

Sergii Dmytruk posted 22 patches 7 months, 1 week ago
There is a newer version of this series
[PATCH v2 02/22] include/xen/slr-table.h: Secure Launch Resource Table definitions
Posted by Sergii Dmytruk 7 months, 1 week ago
The file provides constants, structures and several helper functions for
parsing SLRT.

Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
---
 xen/include/xen/slr-table.h | 268 ++++++++++++++++++++++++++++++++++++
 1 file changed, 268 insertions(+)
 create mode 100644 xen/include/xen/slr-table.h

diff --git a/xen/include/xen/slr-table.h b/xen/include/xen/slr-table.h
new file mode 100644
index 0000000000..6f0018bceb
--- /dev/null
+++ b/xen/include/xen/slr-table.h
@@ -0,0 +1,268 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ *  Copyright (c) 2025 Apertus Solutions, LLC
+ *  Copyright (c) 2025 Oracle and/or its affiliates.
+ *  Copyright (c) 2025 3mdeb Sp. z o.o
+ *
+ *  Secure Launch Resource Table definitions
+ */
+
+#ifndef XEN__SLR_TABLE_H
+#define XEN__SLR_TABLE_H
+
+#include <xen/types.h>
+
+#define UEFI_SLR_TABLE_GUID \
+    { 0x877a9b2aU, 0x0385, 0x45d1, { 0xa0, 0x34, 0x9d, 0xac, 0x9c, 0x9e, 0x56, 0x5f } }
+
+/* SLR table header values */
+#define SLR_TABLE_MAGIC         0x4452544d
+#define SLR_TABLE_REVISION      1
+
+/* Current revisions for the policy and UEFI config */
+#define SLR_POLICY_REVISION         1
+#define SLR_UEFI_CONFIG_REVISION    1
+
+/* SLR defined architectures */
+#define SLR_INTEL_TXT   1
+#define SLR_AMD_SKINIT  2
+
+/* SLR defined bootloaders */
+#define SLR_BOOTLOADER_INVALID  0
+#define SLR_BOOTLOADER_GRUB     1
+
+/* Log formats */
+#define SLR_DRTM_TPM12_LOG      1
+#define SLR_DRTM_TPM20_LOG      2
+
+/* DRTM Policy Entry Flags */
+#define SLR_POLICY_FLAG_MEASURED    0x1
+#define SLR_POLICY_IMPLICIT_SIZE    0x2
+
+/* Array Lengths */
+#define TPM_EVENT_INFO_LENGTH       32
+#define TXT_VARIABLE_MTRRS_LENGTH   32
+
+/* Tags */
+#define SLR_ENTRY_INVALID       0x0000
+#define SLR_ENTRY_DL_INFO       0x0001
+#define SLR_ENTRY_LOG_INFO      0x0002
+#define SLR_ENTRY_DRTM_POLICY   0x0003
+#define SLR_ENTRY_INTEL_INFO    0x0004
+#define SLR_ENTRY_AMD_INFO      0x0005
+#define SLR_ENTRY_ARM_INFO      0x0006
+#define SLR_ENTRY_UEFI_INFO     0x0007
+#define SLR_ENTRY_UEFI_CONFIG   0x0008
+#define SLR_ENTRY_END           0xffff
+
+/* Entity Types */
+#define SLR_ET_UNSPECIFIED        0x0000
+#define SLR_ET_SLRT               0x0001
+#define SLR_ET_BOOT_PARAMS        0x0002
+#define SLR_ET_SETUP_DATA         0x0003
+#define SLR_ET_CMDLINE            0x0004
+#define SLR_ET_UEFI_MEMMAP        0x0005
+#define SLR_ET_RAMDISK            0x0006
+#define SLR_ET_MULTIBOOT2_INFO    0x0007
+#define SLR_ET_MULTIBOOT2_MODULE  0x0008
+#define SLR_ET_TXT_OS2MLE         0x0010
+#define SLR_ET_UNUSED             0xffff
+
+/*
+ * 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;
+
+/*
+ * Common SLRT Table Header
+ */
+struct slr_entry_hdr
+{
+    uint32_t tag;
+    uint32_t size;
+} __packed;
+
+/*
+ * Boot loader context
+ */
+struct slr_bl_context
+{
+    uint16_t bootloader;
+    uint16_t reserved[3];
+    uint64_t context;
+} __packed;
+
+/*
+ * Prototype of a function pointed to by slr_entry_dl_info::dl_handler.
+ */
+typedef void (*dl_handler_func)(struct slr_bl_context *bl_context);
+
+/*
+ * DRTM Dynamic Launch Configuration
+ */
+struct slr_entry_dl_info
+{
+    struct slr_entry_hdr hdr;
+    uint64_t dce_size;
+    uint64_t dce_base;
+    uint64_t dlme_size;
+    uint64_t dlme_base;
+    uint64_t dlme_entry;
+    struct slr_bl_context bl_context;
+    uint64_t dl_handler;
+} __packed;
+
+/*
+ * TPM Log Information
+ */
+struct slr_entry_log_info
+{
+    struct slr_entry_hdr hdr;
+    uint16_t format;
+    uint16_t reserved;
+    uint32_t size;
+    uint64_t addr;
+} __packed;
+
+/*
+ * DRTM Measurement Entry
+ */
+struct slr_policy_entry
+{
+    uint16_t pcr;
+    uint16_t entity_type;
+    uint16_t flags;
+    uint16_t reserved;
+    uint64_t size;
+    uint64_t entity;
+    char evt_info[TPM_EVENT_INFO_LENGTH];
+} __packed;
+
+/*
+ * DRTM Measurement Policy
+ */
+struct slr_entry_policy
+{
+    struct slr_entry_hdr hdr;
+    uint16_t reserved[2];
+    uint16_t revision;
+    uint16_t nr_entries;
+    struct slr_policy_entry policy_entries[];
+} __packed;
+
+/*
+ * Secure Launch defined MTRR saving structures
+ */
+struct slr_txt_mtrr_pair
+{
+    uint64_t mtrr_physbase;
+    uint64_t mtrr_physmask;
+} __packed;
+
+struct slr_txt_mtrr_state
+{
+    uint64_t default_mem_type;
+    uint64_t mtrr_vcnt;
+    struct slr_txt_mtrr_pair mtrr_pair[TXT_VARIABLE_MTRRS_LENGTH];
+} __packed;
+
+/*
+ * Intel TXT Info table
+ */
+struct slr_entry_intel_info
+{
+    struct slr_entry_hdr hdr;
+    uint64_t boot_params_base;
+    uint64_t txt_heap;
+    uint64_t saved_misc_enable_msr;
+    struct slr_txt_mtrr_state saved_bsp_mtrrs;
+} __packed;
+
+/*
+ * AMD SKINIT Info table
+ */
+struct slr_entry_amd_info
+{
+    struct slr_entry_hdr hdr;
+    uint64_t next;
+    uint32_t type;
+    uint32_t len;
+    uint64_t slrt_size;
+    uint64_t slrt_base;
+    uint64_t boot_params_base;
+    uint16_t psp_version;
+    uint16_t reserved[3];
+} __packed;
+
+/*
+ * UEFI config measurement entry
+ */
+struct slr_uefi_cfg_entry
+{
+    uint16_t pcr;
+    uint16_t reserved;
+    uint32_t size;
+    uint64_t cfg; /* address or value */
+    char evt_info[TPM_EVENT_INFO_LENGTH];
+} __packed;
+
+struct slr_entry_uefi_config
+{
+    struct slr_entry_hdr hdr;
+    uint16_t reserved[2];
+    uint16_t revision;
+    uint16_t nr_entries;
+    struct slr_uefi_cfg_entry uefi_cfg_entries[];
+} __packed;
+
+static inline void *
+slr_end_of_entries(struct slr_table *table)
+{
+    return (uint8_t *)table + table->size;
+}
+
+static inline struct slr_entry_hdr *
+slr_next_entry(struct slr_table *table, struct slr_entry_hdr *curr)
+{
+    struct slr_entry_hdr *next = (struct slr_entry_hdr *)
+                                 ((uint8_t *)curr + curr->size);
+
+    if ( (void *)next >= slr_end_of_entries(table) )
+        return NULL;
+    if ( next->tag == SLR_ENTRY_END )
+        return NULL;
+
+    return next;
+}
+
+static inline struct slr_entry_hdr *
+slr_next_entry_by_tag (struct slr_table *table,
+                       struct slr_entry_hdr *entry,
+                       uint16_t tag)
+{
+    if ( !entry ) /* Start from the beginning */
+        entry = (struct slr_entry_hdr *)((uint8_t *)table + sizeof(*table));
+
+    for ( ; ; )
+    {
+        if ( entry->tag == tag )
+            return entry;
+
+        entry = slr_next_entry(table, entry);
+        if ( !entry )
+            return NULL;
+    }
+
+    return NULL;
+}
+
+#endif /* XEN__SLR_TABLE_H */
-- 
2.49.0
Re: [PATCH v2 02/22] include/xen/slr-table.h: Secure Launch Resource Table definitions
Posted by Jan Beulich 7 months ago
On 13.05.2025 19:05, Sergii Dmytruk wrote:
> The file provides constants, structures and several helper functions for
> parsing SLRT.
> 
> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
> ---
>  xen/include/xen/slr-table.h | 268 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 268 insertions(+)
>  create mode 100644 xen/include/xen/slr-table.h
> 
> --- /dev/null
> +++ b/xen/include/xen/slr-table.h
> @@ -0,0 +1,268 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

GPL-2.0-only is, I think, the one to use for new code.

> +/*
> + *  Copyright (c) 2025 Apertus Solutions, LLC
> + *  Copyright (c) 2025 Oracle and/or its affiliates.
> + *  Copyright (c) 2025 3mdeb Sp. z o.o

I'm curious: Considering the (just) 2 S-o-b, where's the 3rd copyright
line coming from?

> + *  Secure Launch Resource Table definitions
> + */
> +
> +#ifndef XEN__SLR_TABLE_H
> +#define XEN__SLR_TABLE_H
> +
> +#include <xen/types.h>

Looks like xen/stdint.h would suffice?

> +#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 ...

> +/* SLR table header values */
> +#define SLR_TABLE_MAGIC         0x4452544d
> +#define SLR_TABLE_REVISION      1
> +
> +/* Current revisions for the policy and UEFI config */
> +#define SLR_POLICY_REVISION         1
> +#define SLR_UEFI_CONFIG_REVISION    1

... this, is the whole concept perhaps bound to UEFI? In which casethe
whole header may want to move to the efi/ subdir?

> +/* SLR defined architectures */
> +#define SLR_INTEL_TXT   1
> +#define SLR_AMD_SKINIT  2

These are both x86, yet the header is put in the common include dir?

> +/* SLR defined bootloaders */
> +#define SLR_BOOTLOADER_INVALID  0
> +#define SLR_BOOTLOADER_GRUB     1
> +
> +/* Log formats */
> +#define SLR_DRTM_TPM12_LOG      1
> +#define SLR_DRTM_TPM20_LOG      2
> +
> +/* DRTM Policy Entry Flags */
> +#define SLR_POLICY_FLAG_MEASURED    0x1
> +#define SLR_POLICY_IMPLICIT_SIZE    0x2
> +
> +/* Array Lengths */
> +#define TPM_EVENT_INFO_LENGTH       32
> +#define TXT_VARIABLE_MTRRS_LENGTH   32
> +
> +/* Tags */
> +#define SLR_ENTRY_INVALID       0x0000
> +#define SLR_ENTRY_DL_INFO       0x0001
> +#define SLR_ENTRY_LOG_INFO      0x0002
> +#define SLR_ENTRY_DRTM_POLICY   0x0003
> +#define SLR_ENTRY_INTEL_INFO    0x0004
> +#define SLR_ENTRY_AMD_INFO      0x0005
> +#define SLR_ENTRY_ARM_INFO      0x0006
> +#define SLR_ENTRY_UEFI_INFO     0x0007
> +#define SLR_ENTRY_UEFI_CONFIG   0x0008
> +#define SLR_ENTRY_END           0xffff
> +
> +/* Entity Types */
> +#define SLR_ET_UNSPECIFIED        0x0000
> +#define SLR_ET_SLRT               0x0001
> +#define SLR_ET_BOOT_PARAMS        0x0002
> +#define SLR_ET_SETUP_DATA         0x0003
> +#define SLR_ET_CMDLINE            0x0004
> +#define SLR_ET_UEFI_MEMMAP        0x0005
> +#define SLR_ET_RAMDISK            0x0006
> +#define SLR_ET_MULTIBOOT2_INFO    0x0007
> +#define SLR_ET_MULTIBOOT2_MODULE  0x0008
> +#define SLR_ET_TXT_OS2MLE         0x0010
> +#define SLR_ET_UNUSED             0xffff
> +
> +/*
> + * 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.

> +/*
> + * Common SLRT Table Header
> + */
> +struct slr_entry_hdr
> +{
> +    uint32_t tag;
> +    uint32_t size;
> +} __packed;
> +
> +/*
> + * Boot loader context
> + */
> +struct slr_bl_context
> +{
> +    uint16_t bootloader;
> +    uint16_t reserved[3];
> +    uint64_t context;
> +} __packed;
> +
> +/*
> + * Prototype of a function pointed to by slr_entry_dl_info::dl_handler.
> + */
> +typedef void (*dl_handler_func)(struct slr_bl_context *bl_context);

It being an internal header, ...

> +/*
> + * DRTM Dynamic Launch Configuration
> + */
> +struct slr_entry_dl_info
> +{
> +    struct slr_entry_hdr hdr;
> +    uint64_t dce_size;
> +    uint64_t dce_base;
> +    uint64_t dlme_size;
> +    uint64_t dlme_base;
> +    uint64_t dlme_entry;
> +    struct slr_bl_context bl_context;
> +    uint64_t dl_handler;

... why can't this type be used here then? This would presumably avoid a
typecast later.

> +static inline void *
> +slr_end_of_entries(struct slr_table *table)
> +{
> +    return (uint8_t *)table + table->size;

Considering the function's return type, why not cast to void * (or perhaps
const void *, if the return type also can be such)?

> +}
> +
> +static inline struct slr_entry_hdr *
> +slr_next_entry(struct slr_table *table, struct slr_entry_hdr *curr)
> +{
> +    struct slr_entry_hdr *next = (struct slr_entry_hdr *)
> +                                 ((uint8_t *)curr + curr->size);
> +
> +    if ( (void *)next >= slr_end_of_entries(table) )
> +        return NULL;

Is this sufficient as a check? With it fulfilled, ...

> +    if ( next->tag == SLR_ENTRY_END )

... this member access may still be out of bounds. IOW the question is what
level of checking is really adequate here.

> +        return NULL;
> +
> +    return next;
> +}
> +
> +static inline struct slr_entry_hdr *
> +slr_next_entry_by_tag (struct slr_table *table,
> +                       struct slr_entry_hdr *entry,
> +                       uint16_t tag)
> +{
> +    if ( !entry ) /* Start from the beginning */
> +        entry = (struct slr_entry_hdr *)((uint8_t *)table + sizeof(*table));

Extending from the earlier comment - if the inner cast was to void * here,
the outer one could be dropped altogether.

Jan
Re: [PATCH v2 02/22] include/xen/slr-table.h: Secure Launch Resource Table definitions
Posted by Sergii Dmytruk 6 months, 4 weeks ago
On Wed, May 21, 2025 at 05:45:04PM +0200, Jan Beulich wrote:
> > +/* SPDX-License-Identifier: GPL-2.0 */
>
> GPL-2.0-only is, I think, the one to use for new code.

Right.

> > +/*
> > + *  Copyright (c) 2025 Apertus Solutions, LLC
> > + *  Copyright (c) 2025 Oracle and/or its affiliates.
> > + *  Copyright (c) 2025 3mdeb Sp. z o.o
>
> I'm curious: Considering the (just) 2 S-o-b, where's the 3rd copyright
> line coming from?

I'll add "Daniel P. Smith" (already in CC), not sure why his S-o-B
wasn't there.

> > +#include <xen/types.h>
>
> Looks like xen/stdint.h would suffice?

It would for types, but there is also use of `NULL`.

> > +#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.

> > +/* SLR table header values */
> > +#define SLR_TABLE_MAGIC         0x4452544d
> > +#define SLR_TABLE_REVISION      1
> > +
> > +/* Current revisions for the policy and UEFI config */
> > +#define SLR_POLICY_REVISION         1
> > +#define SLR_UEFI_CONFIG_REVISION    1
>
> ... this, is the whole concept perhaps bound to UEFI? In which casethe
> whole header may want to move to the efi/ subdir?

This isn't EFI-specific, legacy boot is supported.  Some types of
entries are there to provide EFI-specific information.

> > +/* SLR defined architectures */
> > +#define SLR_INTEL_TXT   1
> > +#define SLR_AMD_SKINIT  2
>
> These are both x86, yet the header is put in the common include dir?

It's x86-specific with the goal to add more architectures in the future.
I don't know, maybe the header should start as arch-specific and be
moved later, your call.

> > +/*
> > + * 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.

> > +/*
> > + * Prototype of a function pointed to by slr_entry_dl_info::dl_handler.
> > + */
> > +typedef void (*dl_handler_func)(struct slr_bl_context *bl_context);
>
> It being an internal header, ...
> > +    uint64_t dl_handler;
>
> ... why can't this type be used here then? This would presumably avoid a
> typecast later.

It's not an internal header in my understanding of the phrase, Xen
parses what a bootloader has passed to it.  In principle, pointers could
be 32-bit here.

> > +static inline void *
> > +slr_end_of_entries(struct slr_table *table)
> > +{
> > +    return (uint8_t *)table + table->size;
>
> Considering the function's return type, why not cast to void * (or perhaps
> const void *, if the return type also can be such)?

No particular reason other than that pointer arithmetic on
pointers-to-void typically causes build issues.  Can be changed for Xen.

> > +static inline struct slr_entry_hdr *
> > +slr_next_entry(struct slr_table *table, struct slr_entry_hdr *curr)
> > +{
> > +    struct slr_entry_hdr *next = (struct slr_entry_hdr *)
> > +                                 ((uint8_t *)curr + curr->size);
> > +
> > +    if ( (void *)next >= slr_end_of_entries(table) )
> > +        return NULL;
>
> Is this sufficient as a check? With it fulfilled, ...
>
> > +    if ( next->tag == SLR_ENTRY_END )
>
> ... this member access may still be out of bounds. IOW the question is what
> level of checking is really adequate here.

SLR_ENTRY_END should really end the table, but it won't hurt to check
for out of bounds.  Thanks, will correct the checks.

> > +static inline struct slr_entry_hdr *
> > +slr_next_entry_by_tag (struct slr_table *table,
> > +                       struct slr_entry_hdr *entry,
> > +                       uint16_t tag)
> > +{
> > +    if ( !entry ) /* Start from the beginning */
> > +        entry = (struct slr_entry_hdr *)((uint8_t *)table + sizeof(*table));
>
> Extending from the earlier comment - if the inner cast was to void * here,
> the outer one could be dropped altogether.
>
> Jan

Will update.

Regards
Re: [PATCH v2 02/22] include/xen/slr-table.h: Secure Launch Resource Table definitions
Posted by Jan Beulich 6 months, 2 weeks ago
On 24.05.2025 00:19, Sergii Dmytruk wrote:
> On Wed, May 21, 2025 at 05:45:04PM +0200, Jan Beulich wrote:
>>> +/*
>>> + *  Copyright (c) 2025 Apertus Solutions, LLC
>>> + *  Copyright (c) 2025 Oracle and/or its affiliates.
>>> + *  Copyright (c) 2025 3mdeb Sp. z o.o
>>
>> I'm curious: Considering the (just) 2 S-o-b, where's the 3rd copyright
>> line coming from?
> 
> I'll add "Daniel P. Smith" (already in CC), not sure why his S-o-B
> wasn't there.

Just to mention it: Be careful there; aiui you can't simply add someone else's
S-o-b without their agreement.

>>> +#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.

>>> +/*
>>> + * 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?

>>> +/*
>>> + * Prototype of a function pointed to by slr_entry_dl_info::dl_handler.
>>> + */
>>> +typedef void (*dl_handler_func)(struct slr_bl_context *bl_context);
>>
>> It being an internal header, ...
>>> +    uint64_t dl_handler;
>>
>> ... why can't this type be used here then? This would presumably avoid a
>> typecast later.
> 
> It's not an internal header in my understanding of the phrase, Xen
> parses what a bootloader has passed to it.  In principle, pointers could
> be 32-bit here.

"Internal" as opposed to "public", i.e. what's exposed to guests.

Jan
Re: [PATCH v2 02/22] include/xen/slr-table.h: Secure Launch Resource Table definitions
Posted by Sergii Dmytruk 6 months, 2 weeks ago
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
Re: [PATCH v2 02/22] include/xen/slr-table.h: Secure Launch Resource Table definitions
Posted by Andrew Cooper 7 months ago
On 21/05/2025 4:45 pm, Jan Beulich wrote:
> On 13.05.2025 19:05, Sergii Dmytruk wrote:
>> +/* SLR defined architectures */
>> +#define SLR_INTEL_TXT   1
>> +#define SLR_AMD_SKINIT  2
> These are both x86, yet the header is put in the common include dir?

ARM have a DRTM technology in progress, which will be 3 here in due course.

OpenPOWER and RISC-V are also investigating DRTM.

~Andrew