From: ross.philipson@oracle.com
To: Kim Phillips <kim.phillips@amd.com>,
linux-kernel@vger.kernel.org, x86@kernel.org,
linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org,
linux-crypto@vger.kernel.org, kexec@lists.infradead.org,
linux-efi@vger.kernel.org
Cc: dpsmith@apertussolutions.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, hpa@zytor.com,
dave.hansen@linux.intel.com, ardb@kernel.org,
mjg59@srcf.ucam.org, James.Bottomley@hansenpartnership.com,
peterhuewe@gmx.de, jarkko@kernel.org, jgg@ziepe.ca,
luto@amacapital.net, nivedita@alum.mit.edu,
herbert@gondor.apana.org.au, davem@davemloft.net,
kanth.ghatraju@oracle.com, trenchboot-devel@googlegroups.com
Subject: Re: [PATCH v8 04/15] x86: Secure Launch Resource Table header file
Date: Mon, 1 Apr 2024 11:25:25 -0700 [thread overview]
Message-ID: <057e658c-ace1-4568-a680-139f724ecabe@oracle.com> (raw)
In-Reply-To: <8d543a15-af62-4403-b2e0-3b395edfe9e4@amd.com>
On 3/29/24 3:38 PM, 'Kim Phillips' via trenchboot-devel wrote:
> Hi Ross,
>
> On 2/14/24 4:18 PM, 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 <ross.philipson@oracle.com>
>> ---
>> 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 000000000000..42020988233a
>> --- /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;
>
> Do these need to have their endianness specified with, e.g., __le32?
The working assumption was this would be handled by the way the pre and
post launch code was built for a given platform.
>
>> + /* entries[] */
>
> Instead of the above line, a legit 'entries' can be enabled using:
>
> DECLARE_FLEX_ARRAY(struct slr_entry_hdr, entries);
I just declared these without the macro. See below...
>
>> +} __packed;
>
> You'd have to move this above struct slr_table which would need it:
>
>> +/*
>> + * 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[] */
... for example
struct slr_policy_entry 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;
>
> Shouldn't these three structs be added as part of their
> separate per-vendor enablement patches?
They got dropped for now. They will be introduced as they are needed.
For AMD a platform specific structure would probably hold the address of
the SKL which in AMD terms is the Secure Loader Block.
>
>> +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));
>
> Back to the 'entries', the above line can now be made more readable:
>
> entry = table->entries;
>
> That's just one example, this flex array simplification can be made
> in other structs in this series, too.
This one may have escaped me. I can take a look or if you want to submit
a PR, the working v9 branch is here:
https://github.com/TrenchBoot/linux/tree/linux-sl-6.7
We have a format that we use for commit messages to make rebases easier.
It was documented somewhere but I can't find it right now. It should be
obvious looking at the existing commit though.
Thank you,
Ross
>
> Cheers,
>
> Kim
>
>> +
>> + 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 */
>
next prev parent reply other threads:[~2024-04-01 18:26 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-14 22:18 [PATCH v8 00/15] x86: Trenchboot secure dynamic launch Linux kernel support Ross Philipson
2024-02-14 22:18 ` [PATCH v8 01/15] x86/boot: Place kernel_info at a fixed offset Ross Philipson
2024-02-15 7:56 ` Ard Biesheuvel
2024-02-15 10:56 ` Daniel Kiper
2024-03-21 13:45 ` Daniel P. Smith
2024-03-22 14:18 ` H. Peter Anvin
2024-03-23 1:33 ` Daniel P. Smith
2024-02-14 22:18 ` [PATCH v8 02/15] Documentation/x86: Secure Launch kernel documentation Ross Philipson
2024-02-14 22:18 ` [PATCH v8 03/15] x86: Secure Launch Kconfig Ross Philipson
2024-02-15 7:59 ` Ard Biesheuvel
2024-02-15 22:20 ` ross.philipson
2024-02-14 22:18 ` [PATCH v8 04/15] x86: Secure Launch Resource Table header file Ross Philipson
2024-02-15 8:08 ` Ard Biesheuvel
2024-02-22 2:03 ` Andrew Cooper
2024-02-22 2:10 ` ross.philipson
2024-02-22 17:49 ` ross.philipson
2024-03-29 22:38 ` Kim Phillips
2024-03-29 22:38 ` Kim Phillips
2024-03-29 22:38 ` Kim Phillips
2024-04-01 18:25 ` ross.philipson [this message]
2024-02-14 22:18 ` [PATCH v8 05/15] x86: Secure Launch main " Ross Philipson
2024-02-14 22:18 ` [PATCH v8 06/15] x86: Add early SHA support for Secure Launch early measurements Ross Philipson
2024-02-15 8:17 ` Ard Biesheuvel
2024-02-22 3:04 ` Andrew Cooper
2024-02-22 9:34 ` Ard Biesheuvel
2024-02-22 12:30 ` Andrew Cooper
2024-02-23 9:27 ` Ard Biesheuvel
2024-02-23 16:42 ` Andrew Cooper
2024-02-23 17:54 ` Eric Biggers
2024-02-23 18:20 ` Andrew Cooper
2024-02-23 18:30 ` Eric Biggers
2024-04-03 16:32 ` Andy Lutomirski
2024-04-03 23:56 ` Eric Biggers
2024-04-04 4:55 ` ross.philipson
2024-04-04 14:55 ` Jarkko Sakkinen
2024-02-14 22:18 ` [PATCH v8 07/15] x86: Secure Launch kernel early boot stub Ross Philipson
2024-02-15 8:29 ` Ard Biesheuvel
2024-02-15 22:26 ` ross.philipson
2024-02-14 22:18 ` [PATCH v8 08/15] x86: Secure Launch kernel late " Ross Philipson
2024-02-14 22:18 ` [PATCH v8 09/15] x86: Secure Launch SMP bringup support Ross Philipson
2024-02-14 22:18 ` [PATCH v8 10/15] kexec: Secure Launch kexec SEXIT support Ross Philipson
2024-02-14 22:18 ` [PATCH v8 11/15] reboot: Secure Launch SEXIT support on reboot paths Ross Philipson
2024-02-14 22:18 ` [PATCH v8 12/15] tpm: Add ability to set the preferred locality the TPM chip uses Ross Philipson
2024-02-14 22:18 ` [PATCH v8 13/15] tpm: Add sysfs interface to allow setting and querying the preferred locality Ross Philipson
2024-02-14 22:18 ` [PATCH v8 14/15] x86: Secure Launch late initcall platform module Ross Philipson
2024-02-15 8:40 ` Ard Biesheuvel
2024-02-22 13:57 ` Daniel P. Smith
2024-02-23 9:36 ` Ard Biesheuvel
2024-03-21 14:11 ` Daniel P. Smith
2024-02-16 1:53 ` kernel test robot
2024-02-17 7:53 ` kernel test robot
2024-02-14 22:18 ` [PATCH v8 15/15] x86: EFI stub DRTM launch support for Secure Launch Ross Philipson
2024-02-15 9:01 ` Ard Biesheuvel
2024-02-21 20:17 ` ross.philipson
2024-02-21 20:37 ` H. Peter Anvin
2024-02-21 23:24 ` Ard Biesheuvel
2024-02-17 7:31 ` kernel test robot
2024-02-17 20:06 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=057e658c-ace1-4568-a680-139f724ecabe@oracle.com \
--to=ross.philipson@oracle.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=ardb@kernel.org \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=davem@davemloft.net \
--cc=dpsmith@apertussolutions.com \
--cc=herbert@gondor.apana.org.au \
--cc=hpa@zytor.com \
--cc=jarkko@kernel.org \
--cc=jgg@ziepe.ca \
--cc=kanth.ghatraju@oracle.com \
--cc=kexec@lists.infradead.org \
--cc=kim.phillips@amd.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mingo@redhat.com \
--cc=mjg59@srcf.ucam.org \
--cc=nivedita@alum.mit.edu \
--cc=peterhuewe@gmx.de \
--cc=tglx@linutronix.de \
--cc=trenchboot-devel@googlegroups.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).