2023-05-04 14:52:08

by Ross Philipson

[permalink] [raw]
Subject: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

Introduce the Secure Launch Resource Table which forms the formal
interface between the pre and post launch code.

Signed-off-by: Ross Philipson <[email protected]>
---
include/linux/slr_table.h | 270 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 270 insertions(+)
create mode 100644 include/linux/slr_table.h

diff --git a/include/linux/slr_table.h b/include/linux/slr_table.h
new file mode 100644
index 0000000..d4b76e5
--- /dev/null
+++ b/include/linux/slr_table.h
@@ -0,0 +1,270 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Secure Launch Resource Table
+ *
+ * Copyright (c) 2023, Oracle and/or its affiliates.
+ */
+
+#ifndef _LINUX_SLR_TABLE_H
+#define _LINUX_SLR_TABLE_H
+
+/* Put this in efi.h if it becomes a standard */
+#define SLR_TABLE_GUID EFI_GUID(0x877a9b2a, 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_ENTRY_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_TXT_OS2MLE 0x0010
+#define SLR_ET_UNUSED 0xffff
+
+#ifndef __ASSEMBLY__
+
+/*
+ * Primary SLR Table Header
+ */
+struct slr_table {
+ u32 magic;
+ u16 revision;
+ u16 architecture;
+ u32 size;
+ u32 max_size;
+ /* entries[] */
+} __packed;
+
+/*
+ * Common SLRT Table Header
+ */
+struct slr_entry_hdr {
+ u16 tag;
+ u16 size;
+} __packed;
+
+/*
+ * Boot loader context
+ */
+struct slr_bl_context {
+ u16 bootloader;
+ u16 reserved;
+ u64 context;
+} __packed;
+
+/*
+ * DRTM Dynamic Launch Configuration
+ */
+struct slr_entry_dl_info {
+ struct slr_entry_hdr hdr;
+ struct slr_bl_context bl_context;
+ u64 dl_handler;
+ u64 dce_base;
+ u32 dce_size;
+ u64 dlme_entry;
+} __packed;
+
+/*
+ * TPM Log Information
+ */
+struct slr_entry_log_info {
+ struct slr_entry_hdr hdr;
+ u16 format;
+ u16 reserved;
+ u64 addr;
+ u32 size;
+} __packed;
+
+/*
+ * DRTM Measurement Policy
+ */
+struct slr_entry_policy {
+ struct slr_entry_hdr hdr;
+ u16 revision;
+ u16 nr_entries;
+ /* policy_entries[] */
+} __packed;
+
+/*
+ * DRTM Measurement Entry
+ */
+struct slr_policy_entry {
+ u16 pcr;
+ u16 entity_type;
+ u16 flags;
+ u16 reserved;
+ u64 entity;
+ u64 size;
+ char evt_info[TPM_EVENT_INFO_LENGTH];
+} __packed;
+
+/*
+ * Secure Launch defined MTRR saving structures
+ */
+struct slr_txt_mtrr_pair {
+ u64 mtrr_physbase;
+ u64 mtrr_physmask;
+} __packed;
+
+struct slr_txt_mtrr_state {
+ u64 default_mem_type;
+ u64 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;
+ u64 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;
+} __packed;
+
+/*
+ * ARM DRTM Info table
+ */
+struct slr_entry_arm_info {
+ struct slr_entry_hdr hdr;
+} __packed;
+
+struct slr_entry_uefi_config {
+ struct slr_entry_hdr hdr;
+ u16 revision;
+ u16 nr_entries;
+ /* uefi_cfg_entries[] */
+} __packed;
+
+struct slr_uefi_cfg_entry {
+ u16 pcr;
+ u16 reserved;
+ u64 cfg; /* address or value */
+ u32 size;
+ char evt_info[TPM_EVENT_INFO_LENGTH];
+} __packed;
+
+static inline void *slr_end_of_entrys(struct slr_table *table)
+{
+ return (((void *)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 *)
+ ((u8 *)curr + curr->size);
+
+ if ((void *)next >= slr_end_of_entrys(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,
+ u16 tag)
+{
+ if (!entry) /* Start from the beginning */
+ entry = (struct slr_entry_hdr *)(((u8 *)table) + sizeof(*table));
+
+ for ( ; ; ) {
+ if (entry->tag == tag)
+ return entry;
+
+ entry = slr_next_entry(table, entry);
+ if (!entry)
+ return NULL;
+ }
+
+ return NULL;
+}
+
+static inline int
+slr_add_entry(struct slr_table *table,
+ struct slr_entry_hdr *entry)
+{
+ struct slr_entry_hdr *end;
+
+ if ((table->size + entry->size) > table->max_size)
+ return -1;
+
+ memcpy((u8 *)table + table->size - sizeof(*end), entry, entry->size);
+ table->size += entry->size;
+
+ end = (struct slr_entry_hdr *)((u8 *)table + table->size - sizeof(*end));
+ end->tag = SLR_ENTRY_END;
+ end->size = sizeof(*end);
+
+ return 0;
+}
+
+static inline void
+slr_init_table(struct slr_table *slrt, u16 architecture, u32 max_size)
+{
+ struct slr_entry_hdr *end;
+
+ slrt->magic = SLR_TABLE_MAGIC;
+ slrt->revision = SLR_TABLE_REVISION;
+ slrt->architecture = architecture;
+ slrt->size = sizeof(*slrt) + sizeof(*end);
+ slrt->max_size = max_size;
+ end = (struct slr_entry_hdr *)((u8 *)slrt + sizeof(*slrt));
+ end->tag = SLR_ENTRY_END;
+ end->size = sizeof(*end);
+}
+
+#endif /* !__ASSEMBLY */
+
+#endif /* _LINUX_SLR_TABLE_H */
--
1.8.3.1


2023-05-05 16:26:28

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On Thu, May 04, 2023 at 02:50:13PM +0000, Ross Philipson wrote:
> Introduce the Secure Launch Resource Table which forms the formal
> interface between the pre and post launch code.
>
> Signed-off-by: Ross Philipson <[email protected]>
> ---
> include/linux/slr_table.h | 270 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 270 insertions(+)
> create mode 100644 include/linux/slr_table.h
>
> diff --git a/include/linux/slr_table.h b/include/linux/slr_table.h

...

> +static inline void *slr_end_of_entrys(struct slr_table *table)
> +{
> + return (((void *)table) + table->size);
> +}
> +
> +static inline struct slr_entry_hdr *
> +slr_next_entry(struct slr_table *table,
> + struct slr_entry_hdr *curr)

nit: The indentation of the line above doesn't align with the '(' on
the line before it.

2023-05-05 17:35:52

by Ross Philipson

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On 5/5/23 12:22, Simon Horman wrote:
> On Thu, May 04, 2023 at 02:50:13PM +0000, Ross Philipson wrote:
>> Introduce the Secure Launch Resource Table which forms the formal
>> interface between the pre and post launch code.
>>
>> Signed-off-by: Ross Philipson <[email protected]>
>> ---
>> include/linux/slr_table.h | 270 ++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 270 insertions(+)
>> create mode 100644 include/linux/slr_table.h
>>
>> diff --git a/include/linux/slr_table.h b/include/linux/slr_table.h
>
> ...
>
>> +static inline void *slr_end_of_entrys(struct slr_table *table)
>> +{
>> + return (((void *)table) + table->size);
>> +}
>> +
>> +static inline struct slr_entry_hdr *
>> +slr_next_entry(struct slr_table *table,
>> + struct slr_entry_hdr *curr)
>
> nit: The indentation of the line above doesn't align with the '(' on
> the line before it.

Yea I see there is another one above that is not aligned too. We will
fix those.

Thanks
Ross

2023-05-12 11:11:35

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On Thu, May 04, 2023 at 02:50:13PM +0000, Ross Philipson wrote:

> +#define SLR_TABLE_MAGIC 0x4452544d

From convention I'd expect this to be 0x534c5254, but not really an
issue.

> +/* SLR defined bootloaders */
> +#define SLR_BOOTLOADER_INVALID 0
> +#define SLR_BOOTLOADER_GRUB 1

Oof. Having the kernel know about bootloaders has not worked out super
well for us in the past. If someone writes a new bootloader, are they
unable to Secure Launch any existing kernels? The pragmatic thing for
them to do would be to just pretend they're grub, which kind of defeats
the point of having this definition...

> +} __packed;

Random nit - why are they all packed? Are there circumstances where two
pieces of code with different assumptions about alignment will be
looking at a single instance of a table? It doesn't seem likely we're
going to be doing DRTM in a 32-bit firmware environment while launching
a 64-bit kernel?

2023-05-15 21:02:06

by Daniel P. Smith

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On 5/10/23 19:04, Jarkko Sakkinen wrote:
> On Thu May 4, 2023 at 5:50 PM EEST, Ross Philipson wrote:
>> Introduce the Secure Launch Resource Table which forms the formal
>> interface between the pre and post launch code.
>>
>> Signed-off-by: Ross Philipson <[email protected]>
>> ---
>> include/linux/slr_table.h | 270 ++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 270 insertions(+)
>> create mode 100644 include/linux/slr_table.h
>>
>> diff --git a/include/linux/slr_table.h b/include/linux/slr_table.h
>> new file mode 100644
>> index 0000000..d4b76e5
>> --- /dev/null
>> +++ b/include/linux/slr_table.h
>> @@ -0,0 +1,270 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Secure Launch Resource Table
>> + *
>> + * Copyright (c) 2023, Oracle and/or its affiliates.
>> + */
>> +
>> +#ifndef _LINUX_SLR_TABLE_H
>> +#define _LINUX_SLR_TABLE_H
>> +
>> +/* Put this in efi.h if it becomes a standard */
>> +#define SLR_TABLE_GUID EFI_GUID(0x877a9b2a, 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_ENTRY_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
>
> "Enums are preferred when defining several related constants."
>
> See:
>
> https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl

These values are only used for a u16 field in a packed structure.
Correct me if I am mistaken, but if an enum is used this will result in
type/casting handling to/from the u16 which would negate some of the
main benefits of using an enum.

v/r,
dps


2023-05-15 21:25:41

by Daniel P. Smith

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On 5/12/23 06:55, Matthew Garrett wrote:
> On Thu, May 04, 2023 at 02:50:13PM +0000, Ross Philipson wrote:
>
>> +#define SLR_TABLE_MAGIC 0x4452544d
>
> From convention I'd expect this to be 0x534c5254, but not really an
> issue.

Apologies, but which convention?

>> +/* SLR defined bootloaders */
>> +#define SLR_BOOTLOADER_INVALID 0
>> +#define SLR_BOOTLOADER_GRUB 1
>
> Oof. Having the kernel know about bootloaders has not worked out super
> well for us in the past. If someone writes a new bootloader, are they
> unable to Secure Launch any existing kernels? The pragmatic thing for
> them to do would be to just pretend they're grub, which kind of defeats
> the point of having this definition...

Actually, this is not for making the kernel know about bootloaders. This
is dealing with the challenge created when the preamble was split for
efi-stub, and similar use cases, where what sets up the preamble, ie.
the bootloader, is separate from what invokes the dynamic launch, ie.
the DLE handler. The reality is that even in the simplest implementation
of the DLE handler, a remnant of GRUB for call back from efi-stub, there
is information that is needed to cross the gap.

>> +} __packed;
>
> Random nit - why are they all packed? Are there circumstances where two
> pieces of code with different assumptions about alignment will be
> looking at a single instance of a table? It doesn't seem likely we're
> going to be doing DRTM in a 32-bit firmware environment while launching
> a 64-bit kernel?

We wrote the TrenchBoot Secure Launch general spec [1] with as much
forethought as possible for the target environments. Specifically, the
desire is to have a common approach for x86 (Intel and AMD), Arm, and
perhaps down the road the POWER arch. In particular, I do not believe
there is anything in the Arm DRTM beta spec that prohibits a mixed 32/64
bit environment. In the end it is better to for the spec to be safe for
those environments then having to make changes to the spec later down
the road.

[1] https://trenchboot.org/specifications/Secure_Launch/

v/r,
dps

2023-05-15 21:26:21

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On Mon, May 15, 2023 at 05:15:15PM -0400, Daniel P. Smith wrote:
> On 5/12/23 06:55, Matthew Garrett wrote:
> > On Thu, May 04, 2023 at 02:50:13PM +0000, Ross Philipson wrote:
> >
> > > +#define SLR_TABLE_MAGIC 0x4452544d
> >
> > From convention I'd expect this to be 0x534c5254, but not really an
> > issue.
>
> Apologies, but which convention?

Tables in ACPI and UEFI tend to have magic that corresponds to their
name, so a table called SLRT would tend to have magic that matches the
ASCII values for that. In this case the SLRT has DRTM as its magic,
which is a touch unexpected.

> > Oof. Having the kernel know about bootloaders has not worked out super
> > well for us in the past. If someone writes a new bootloader, are they
> > unable to Secure Launch any existing kernels? The pragmatic thing for
> > them to do would be to just pretend they're grub, which kind of defeats
> > the point of having this definition...
>
> Actually, this is not for making the kernel know about bootloaders. This is
> dealing with the challenge created when the preamble was split for efi-stub,
> and similar use cases, where what sets up the preamble, ie. the bootloader,
> is separate from what invokes the dynamic launch, ie. the DLE handler. The
> reality is that even in the simplest implementation of the DLE handler, a
> remnant of GRUB for call back from efi-stub, there is information that is
> needed to cross the gap.

What if I don't use grub, but use something that behaves equivalently?
Which value should be used here?

> We wrote the TrenchBoot Secure Launch general spec [1] with as much
> forethought as possible for the target environments. Specifically, the
> desire is to have a common approach for x86 (Intel and AMD), Arm, and
> perhaps down the road the POWER arch. In particular, I do not believe there
> is anything in the Arm DRTM beta spec that prohibits a mixed 32/64 bit
> environment. In the end it is better to for the spec to be safe for those
> environments then having to make changes to the spec later down the road.

Ok.

2023-05-16 00:52:04

by Daniel P. Smith

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On 5/15/23 17:22, Matthew Garrett wrote:
> On Mon, May 15, 2023 at 05:15:15PM -0400, Daniel P. Smith wrote:
>> On 5/12/23 06:55, Matthew Garrett wrote:
>>> On Thu, May 04, 2023 at 02:50:13PM +0000, Ross Philipson wrote:
>>>
>>>> +#define SLR_TABLE_MAGIC 0x4452544d
>>>
>>> From convention I'd expect this to be 0x534c5254, but not really an
>>> issue.
>>
>> Apologies, but which convention?
>
> Tables in ACPI and UEFI tend to have magic that corresponds to their
> name, so a table called SLRT would tend to have magic that matches the
> ASCII values for that. In this case the SLRT has DRTM as its magic,
> which is a touch unexpected.

While the SLRT is meant for UEFI and non-UEFI environments, DRTM is
definitely a hold over when we started this and you are correct,
probably not the best choice. In fact, I agree that SLRT is a far better
magic. We will update the spec and fix it in the series.

>>> Oof. Having the kernel know about bootloaders has not worked out super
>>> well for us in the past. If someone writes a new bootloader, are they
>>> unable to Secure Launch any existing kernels? The pragmatic thing for
>>> them to do would be to just pretend they're grub, which kind of defeats
>>> the point of having this definition...
>>
>> Actually, this is not for making the kernel know about bootloaders. This is
>> dealing with the challenge created when the preamble was split for efi-stub,
>> and similar use cases, where what sets up the preamble, ie. the bootloader,
>> is separate from what invokes the dynamic launch, ie. the DLE handler. The
>> reality is that even in the simplest implementation of the DLE handler, a
>> remnant of GRUB for call back from efi-stub, there is information that is
>> needed to cross the gap.
>
> What if I don't use grub, but use something that behaves equivalently?
> Which value should be used here?

Generally we would request that the bootloader submit a request to
register for a value to be reserved in the spec. That aside, the intent
here is to allow for the possibility for the DLE handler to be
independent from the bootloader, but this does not have to be this way.
If a non-open entity decides to produce their own implementation, they
can freely use a unallocated value at their own risk that it could be
allocated to another bootloader in the future. Though in this scenario
it likely would not matter as the non-open DLE handler would only be
present when the non-open bootloader was present.

>> We wrote the TrenchBoot Secure Launch general spec [1] with as much
>> forethought as possible for the target environments. Specifically, the
>> desire is to have a common approach for x86 (Intel and AMD), Arm, and
>> perhaps down the road the POWER arch. In particular, I do not believe there
>> is anything in the Arm DRTM beta spec that prohibits a mixed 32/64 bit
>> environment. In the end it is better to for the spec to be safe for those
>> environments then having to make changes to the spec later down the road.
>
> Ok.

Thank you for the review!

v/r,
dps

2023-05-16 01:55:08

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On Mon, May 15, 2023 at 08:41:00PM -0400, Daniel P. Smith wrote:
> On 5/15/23 17:22, Matthew Garrett wrote:
> > What if I don't use grub, but use something that behaves equivalently?
> > Which value should be used here?
>
> Generally we would request that the bootloader submit a request to register
> for a value to be reserved in the spec. That aside, the intent here is to
> allow for the possibility for the DLE handler to be independent from the
> bootloader, but this does not have to be this way. If a non-open entity
> decides to produce their own implementation, they can freely use a
> unallocated value at their own risk that it could be allocated to another
> bootloader in the future. Though in this scenario it likely would not matter
> as the non-open DLE handler would only be present when the non-open
> bootloader was present.

Is the expectation that the DLE will always be shipped with the
bootloader? I think I'm not entirely clear on what's consuming this and
why.


2023-06-16 20:07:27

by Daniel P. Smith

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On 5/15/23 21:43, Matthew Garrett wrote:
> On Mon, May 15, 2023 at 08:41:00PM -0400, Daniel P. Smith wrote:
>> On 5/15/23 17:22, Matthew Garrett wrote:
>>> What if I don't use grub, but use something that behaves equivalently?
>>> Which value should be used here?
>>
>> Generally we would request that the bootloader submit a request to register
>> for a value to be reserved in the spec. That aside, the intent here is to
>> allow for the possibility for the DLE handler to be independent from the
>> bootloader, but this does not have to be this way. If a non-open entity
>> decides to produce their own implementation, they can freely use a
>> unallocated value at their own risk that it could be allocated to another
>> bootloader in the future. Though in this scenario it likely would not matter
>> as the non-open DLE handler would only be present when the non-open
>> bootloader was present.
>
> Is the expectation that the DLE will always be shipped with the
> bootloader? I think I'm not entirely clear on what's consuming this and
> why.
>

No, in fact, an early idea proposed by a pair of us in the TrenchBoot
community was to have it live either as a Runtime Service that was
loaded by a UEFI app or in the coreboot UEFI payload.

2023-06-16 20:21:34

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On Fri, Jun 16, 2023 at 04:01:09PM -0400, Daniel P. Smith wrote:
> On 5/15/23 21:43, Matthew Garrett wrote:
> > On Mon, May 15, 2023 at 08:41:00PM -0400, Daniel P. Smith wrote:
> > > On 5/15/23 17:22, Matthew Garrett wrote:
> > > > What if I don't use grub, but use something that behaves equivalently?
> > > > Which value should be used here?
> > >
> > > Generally we would request that the bootloader submit a request to register
> > > for a value to be reserved in the spec. That aside, the intent here is to
> > > allow for the possibility for the DLE handler to be independent from the
> > > bootloader, but this does not have to be this way. If a non-open entity
> > > decides to produce their own implementation, they can freely use a
> > > unallocated value at their own risk that it could be allocated to another
> > > bootloader in the future. Though in this scenario it likely would not matter
> > > as the non-open DLE handler would only be present when the non-open
> > > bootloader was present.
> >
> > Is the expectation that the DLE will always be shipped with the
> > bootloader? I think I'm not entirely clear on what's consuming this and
> > why.
> >
>
> No, in fact, an early idea proposed by a pair of us in the TrenchBoot
> community was to have it live either as a Runtime Service that was loaded by
> a UEFI app or in the coreboot UEFI payload.

Ok, then I think I'm still confused. If I want to write a new bootloader
but make use of the existing DLE, what contract am I establishing and
what value should I be putting in here?

2023-07-07 19:42:54

by Daniel P. Smith

[permalink] [raw]
Subject: Re: [PATCH v6 04/14] x86: Secure Launch Resource Table header file

On 6/16/23 16:15, Matthew Garrett wrote:
> On Fri, Jun 16, 2023 at 04:01:09PM -0400, Daniel P. Smith wrote:
>> On 5/15/23 21:43, Matthew Garrett wrote:
>>> On Mon, May 15, 2023 at 08:41:00PM -0400, Daniel P. Smith wrote:
>>>> On 5/15/23 17:22, Matthew Garrett wrote:
>>>>> What if I don't use grub, but use something that behaves equivalently?
>>>>> Which value should be used here?
>>>>
>>>> Generally we would request that the bootloader submit a request to register
>>>> for a value to be reserved in the spec. That aside, the intent here is to
>>>> allow for the possibility for the DLE handler to be independent from the
>>>> bootloader, but this does not have to be this way. If a non-open entity
>>>> decides to produce their own implementation, they can freely use a
>>>> unallocated value at their own risk that it could be allocated to another
>>>> bootloader in the future. Though in this scenario it likely would not matter
>>>> as the non-open DLE handler would only be present when the non-open
>>>> bootloader was present.
>>>
>>> Is the expectation that the DLE will always be shipped with the
>>> bootloader? I think I'm not entirely clear on what's consuming this and
>>> why.
>>>
>>
>> No, in fact, an early idea proposed by a pair of us in the TrenchBoot
>> community was to have it live either as a Runtime Service that was loaded by
>> a UEFI app or in the coreboot UEFI payload.
>
> Ok, then I think I'm still confused. If I want to write a new bootloader
> but make use of the existing DLE, what contract am I establishing and
> what value should I be putting in here?

Apologies on the delayed response, vacation and what not.

I believe I know where the confusion is coming from, let me see if I can
explain better by why that field came about. The motivation for the SLRT
came out of our agreement to use a callback mechanism to support
entering efi-stub and then going back to a dynamic launch aware hook to
complete the initiation of the dynamic launch. The SLRT was devised as a
platform and kernel agnostic means to handle the launch. As such, there
was a desire to use that interface, and the underlying DLE code, whether
GRUB was launching the kernel via the UEFI interface or the traditional
interface. Skipping the details, but it boils down to the fact that in
the non-UEFI case, functionality from core GRUB was needed. As a result,
to provide maximum flexibility for other bootloaders, and to make it
easier on us, we add the ability to pass a context object across the
interface. Thus allowing GRUB's DLE handler to have a single entry that
could be called externally by efi-stub or directly from GRUB proper.

IOW, the bootloader context is a means to provide a bootloader with them
means to implement a private interface between the bootloader proper and
a DLE handler that it installed into memory should its implementation
require it.

There is an underlying question within your question, and that is of
reuse. In this case, we wrote GRUB's DLE handler was written
specifically to be used by GRUB. It was written to provide a stable and
demonstrable implementation of the SL interface. In the future it may
get refactored or a common standalone implementation, e.g., the
previously mentioned UEFI runtime service, may arise that would be
reusable by multiple bootloaders.

I hope this helped explain the purpose and use of this area of the
table. Please let me know if it did not.

V/r,
DPS