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

[Daniel P. Smith]

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 <ross.philipson@xxxxxxxxxx>
> > ---
> >   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