The larger focus of the TrenchBoot project (https://github.com/TrenchBoot) is to
enhance the boot security and integrity in a unified manner. The first area of
focus has been on the Trusted Computing Group's Dynamic Launch for establishing
a hardware Root of Trust for Measurement, also know as DRTM (Dynamic Root of
Trust for Measurement). The project has been and continues to work on providing
a unified means to Dynamic Launch that is a cross-platform (Intel and AMD) and
cross-architecture (x86 and Arm), with our recent involvment in the upcoming
Arm DRTM specification. The order of introducing DRTM to the Linux kernel
follows the maturity of DRTM in the architectures. Intel's Trusted eXecution
Technology (TXT) is present today and only requires a preamble loader, e.g. a
boot loader, and an OS kernel that is TXT-aware. AMD DRTM implementation has
been present since the introduction of AMD-V but requires an additional
component that is AMD specific and referred to in the specification as the
Secure Loader, which the TrenchBoot project has an active prototype in
development. Finally Arm's implementation is in specification development stage
and the project is looking to support it when it becomes available.

This patchset provides detailed documentation of DRTM, the approach used for
adding the capbility, and relevant API/ABI documentation. In addition to the
documentation the patch set introduces Intel TXT support as the first platform
for Linux Secure Launch.

A quick note on terminology. The larger open source project itself is called
TrenchBoot, which is hosted on Github (links below). The kernel feature enabling
the use of Dynamic Launch technology is referred to as "Secure Launch" within
the kernel code. As such the prefixes sl_/SL_ or slaunch/SLAUNCH will be seen
in the code. The stub code discussed above is referred to as the SL stub.

Links:

The TrenchBoot project including documentation:

https://trenchboot.org

The TrenchBoot project on Github:

https://github.com/trenchboot

Intel TXT is documented in its own specification and in the SDM Instruction Set volume:

https://www.intel.com/content/dam/www/public/us/en/documents/guides/intel-txt-software-development-guide.pdf
https://software.intel.com/en-us/articles/intel-sdm

AMD SKINIT is documented in the System Programming manual:

https://www.amd.com/system/files/TechDocs/24593.pdf

The TrenchBoot project provides a quick start guide to help get a system
up and running with Secure Launch for Linux:

https://github.com/TrenchBoot/documentation/blob/master/QUICKSTART.md

Patch set based on commit:

torvalds/master/77f587896757708780a7e8792efe62939f25a5ab

Thanks
Ross Philipson and Daniel P. Smith

Changes in v2:

 - Modified 32b entry code to prevent causing relocations in the compressed
   kernel.
 - Dropped patches for compressed kernel TPM PCR extender.
 - Modified event log code to insert log delimiter events and not rely
   on TPM access.
 - Stop extending PCRs in the early Secure Launch stub code.
 - Removed Kconfig options for hash algorithms and use the algorithms the
   ACM used.
 - Match Secure Launch measurement algorithm use to those reported in the
   TPM 2.0 event log.
 - Read the TPM events out of the TPM and extend them into the PCRs using
   the mainline TPM driver. This is done in the late initcall module.
 - Allow use of alternate PCR 19 and 20 for post ACM measurements.
 - Add Kconfig constraints needed by Secure Launch (disable KASLR
   and add x2apic dependency).
 - Fix testing of SL_FLAGS when determining if Secure Launch is active
   and the architecture is TXT.
 - Use SYM_DATA_START_LOCAL macros in early entry point code.
 - Security audit changes:
   - Validate buffers passed to MLE do not overlap the MLE and are
     properly laid out.
   - Validate buffers and memory regions used by the MLE are
     protected by IOMMU PMRs.
 - Force IOMMU to not use passthrough mode during a Secure Launch.
 - Prevent KASLR use during a Secure Launch.

Changes in v3:

 - Introduce x86 documentation patch to provide background, overview
   and configuration/ABI information for the Secure Launch kernel
   feature.
 - Remove the IOMMU patch with special cases for disabling IOMMU
   passthrough. Configuring the IOMMU is now a documentation matter
   in the previously mentioned new patch.
 - Remove special case KASLR disabling code. Configuring KASLR is now
   a documentation matter in the previously mentioned new patch.
 - Fix incorrect panic on TXT public register read.
 - Properly handle and measure setup_indirect bootparams in the early
   launch code.
 - Use correct compressed kernel image base address when testing buffers
   in the early launch stub code. This bug was introduced by the changes
   to avoid relocation in the compressed kernel.
 - Use CPUID feature bits instead of CPUID vendor strings to determine
   if SMX mode is supported and the system is Intel.
 - Remove early NMI re-enable on the BSP. This can be safely done later
   on the BSP after an IDT is setup.

Changes in v4:
 - Expand the cover letter to provide more context to the order that DRTM
   support will be added.
 - Removed debug tracing in TPM request locality funciton and fixed
   local variable declarations.
 - Fixed missing break in default case in slmodule.c.
 - Reworded commit messages in patches 1 and 2 per suggestions.

Changes in v5:
 - Comprehensive documentation rewrite.
 - Use boot param loadflags to communicate Secure Launch status to
   kernel proper.
 - Fix incorrect check of X86_FEATURE_BIT_SMX bit.
 - Rename the alternate details and authorities PCR support.
 - Refactor the securityfs directory and file setup in slmodule.c.
 - Misc. cleanup from internal code reviews.
 - Use reverse fir tree format for variables.

Changes in v6:
 - Support for the new Secure Launch Resourse Table that standardizes
   the information passed and forms the ABI between the pre and post
   launch code.
 - Support for booting Linux through the EFI stub entry point and
   then being able to do a Secure Launch once EFI stub is done and EBS
   is called.
 - Updates to the documentation to reflect the previous two items listed.

Changes in v7:
 - Switch to using MONITOR/MWAIT instead of NMIs to park the APs for
   later bringup by the SMP code.
 - Use static inline dummy functions instead of macros when the Secure
   Launch feature is disabled.
 - Move early SHA1 code to lib/crypto and pull it in from there.
 - Numerous formatting fixes from comments on LKML.
 - Remove efi-stub/DL stub patch temporarily for redesign/rework.

Changes in v8:
 - Reintroduce efi-stub Linux kernel booting through the dynamic launch
   stub (DL stub).
 - Add new approach to setting localities > 0 through kernel and sysfs
   interfaces in the TPM mainline driver.
 - General code cleanup from v7 post comments.

Changes in v9:
 - Updated DL stub support for recent changes to EFI stub in the kernel.
 - Added patches to fix locality changing support in the TPM driver
   (these patches originally were posted as a separate set).
 - Enhanced Secure Launch TPM locality 2 setting in the TPM driver.
 - Added locality setting support through sysfs for user land to access.
 - Split up SHA1 and SHA256 changes into separate patches and updated
   the commit messages to be more clear (per request from upstream
   review).
 - Fix Clang compile issues detected by kernel test robot.
 - Modifications to the Secure Launch Resource Table ABI:
   . Use flex arrays in table structures.
   . Update and move fields in tables to make everything 8b aligned.
   . Add 2 new DLME fields and a txt_heap address field.
   . Remove platform specific tables that are not defined yet (AMD/ARM).
 - Update Kconfig dependencies for Secure Launch with SHA1/SHA256/TPM.
 - Remove push/pop of rsi since boot params is now stored in r15.
 - Update outdated kernel documentation.
 - Misc. comment fixes for type-os and mispellings.

Changes in v10:
 - Removed patch #1 from previous set that forced the kernel_info
   section at a fixed offset.
 - Add changes from Ard Biesheuvel to use the link step to generate the
   proper relative offsets for the MLE header in the kernel_info
   section.
 - Fix sizes and alignment slightly in the SLR table. Add comments to
   the SLR header to indicate it is defined by the TrenchBoot project.
 - Remove incorrect extra pop instruction noted in the head_64.S
   changes.
 - Use the prefix tpm/tpm2 to distinguish between TPM versions as is
   done in the rest of the TPM related code.
 - Rework the TPM locality setting/reporting changes to use "default"
   locality as opposed to "preferred". Remove uneeded extra locality
   function in the TPM interface (call the chip function directly).
 - Adopt comment/documentation changes to code and commit message per
   requests from the community.
 - Use u64 for the boot params physical address to avoid truncating
   pointers during casts.
 - Split adding of new MSR registers into its own patch.
 - Attempt to further address justification for using SHA-1 algorithm.
   Pick up some code suggestions for the SHA-1 patch.
 - Introduct slaunch_is_txt_launch() function per Jarkko Sakkinen's
   suggestion.
 - Implement minor changes to the EFI stub code per suggestions.

Changes in v11:
 - Add section to user documents about SHA-1 usage with Secure
   Launch as requested.
 - General cleanup and grammar fixes to the Linux user documentation.
 - Fix use of CONFIG_SECURE_LAUNCH in the EFI stub code to prevent
   32b build failures.
 - Cleanup of early event log handling code.

Daniel P. Smith (7):
  Documentation/x86: Secure Launch kernel documentation
  x86: Add early SHA-1 support for Secure Launch early measurements
  x86: Add early SHA-256 support for Secure Launch early measurements
  tpm: Protect against locality counter underflow
  tpm: Ensure tpm is in known state at startup
  tpm: Make locality requests return consistent values
  x86: Secure Launch late initcall platform module

Ross Philipson (13):
  x86: Secure Launch Kconfig
  x86: Secure Launch Resource Table header file
  x86: Secure Launch main header file
  x86/msr: Add variable MTRR base/mask and x2apic ID registers
  x86/boot: Place TXT MLE header in the kernel_info section
  x86: Secure Launch kernel early boot stub
  x86: Secure Launch kernel late boot stub
  x86: Secure Launch SMP bringup support
  kexec: Secure Launch kexec SEXIT support
  x86/reboot: Secure Launch SEXIT support on reboot paths
  tpm: Add ability to set the default locality the TPM chip uses
  tpm: Add sysfs interface to allow setting and querying the default
    locality
  x86/efi: EFI stub DRTM launch support for Secure Launch

 Documentation/arch/x86/boot.rst               |  21 +
 Documentation/security/index.rst              |   1 +
 .../security/launch-integrity/index.rst       |  11 +
 .../security/launch-integrity/principles.rst  | 317 ++++++++
 .../secure_launch_details.rst                 | 588 ++++++++++++++
 .../secure_launch_overview.rst                | 252 ++++++
 arch/x86/Kconfig                              |  11 +
 arch/x86/boot/compressed/Makefile             |   3 +
 arch/x86/boot/compressed/head_64.S            |  29 +
 arch/x86/boot/compressed/kernel_info.S        |  50 +-
 arch/x86/boot/compressed/sha1.c               |   6 +
 arch/x86/boot/compressed/sha256.c             |   6 +
 arch/x86/boot/compressed/sl_main.c            | 584 ++++++++++++++
 arch/x86/boot/compressed/sl_stub.S            | 726 ++++++++++++++++++
 arch/x86/boot/compressed/vmlinux.lds.S        |   7 +
 arch/x86/include/asm/msr-index.h              |   5 +
 arch/x86/include/asm/realmode.h               |   3 +
 arch/x86/include/uapi/asm/bootparam.h         |   1 +
 arch/x86/kernel/Makefile                      |   2 +
 arch/x86/kernel/asm-offsets.c                 |  20 +
 arch/x86/kernel/reboot.c                      |  10 +
 arch/x86/kernel/setup.c                       |   3 +
 arch/x86/kernel/slaunch.c                     | 596 ++++++++++++++
 arch/x86/kernel/slmodule.c                    | 508 ++++++++++++
 arch/x86/kernel/smpboot.c                     |  43 +-
 arch/x86/realmode/init.c                      |   3 +
 arch/x86/realmode/rm/header.S                 |   3 +
 arch/x86/realmode/rm/trampoline_64.S          |  32 +
 drivers/char/tpm/tpm-chip.c                   |  24 +-
 drivers/char/tpm/tpm-sysfs.c                  |  30 +
 drivers/char/tpm/tpm_tis_core.c               |  27 +-
 drivers/firmware/efi/libstub/efistub.h        |   8 +
 drivers/firmware/efi/libstub/x86-stub.c       |  99 +++
 drivers/iommu/intel/dmar.c                    |   4 +
 include/crypto/sha1.h                         |   1 +
 include/linux/slaunch.h                       | 548 +++++++++++++
 include/linux/slr_table.h                     | 276 +++++++
 include/linux/tpm.h                           |  10 +
 kernel/kexec_core.c                           |   4 +
 lib/crypto/sha1.c                             |  81 ++
 40 files changed, 4940 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/security/launch-integrity/index.rst
 create mode 100644 Documentation/security/launch-integrity/principles.rst
 create mode 100644 Documentation/security/launch-integrity/secure_launch_details.rst
 create mode 100644 Documentation/security/launch-integrity/secure_launch_overview.rst
 create mode 100644 arch/x86/boot/compressed/sha1.c
 create mode 100644 arch/x86/boot/compressed/sha256.c
 create mode 100644 arch/x86/boot/compressed/sl_main.c
 create mode 100644 arch/x86/boot/compressed/sl_stub.S
 create mode 100644 arch/x86/kernel/slaunch.c
 create mode 100644 arch/x86/kernel/slmodule.c
 create mode 100644 include/linux/slaunch.h
 create mode 100644 include/linux/slr_table.h

-- 
2.39.3

On Fri Sep 13, 2024 at 11:04 PM EEST, Ross Philipson wrote:
> The larger focus of the TrenchBoot project (https://github.com/TrenchBoot) is to
> enhance the boot security and integrity in a unified manner. The first area of
> focus has been on the Trusted Computing Group's Dynamic Launch for establishing
> a hardware Root of Trust for Measurement, also know as DRTM (Dynamic Root of
> Trust for Measurement). The project has been and continues to work on providing
> a unified means to Dynamic Launch that is a cross-platform (Intel and AMD) and
> cross-architecture (x86 and Arm), with our recent involvment in the upcoming
> Arm DRTM specification. The order of introducing DRTM to the Linux kernel
> follows the maturity of DRTM in the architectures. Intel's Trusted eXecution
> Technology (TXT) is present today and only requires a preamble loader, e.g. a
> boot loader, and an OS kernel that is TXT-aware. AMD DRTM implementation has
> been present since the introduction of AMD-V but requires an additional
> component that is AMD specific and referred to in the specification as the
> Secure Loader, which the TrenchBoot project has an active prototype in
> development. Finally Arm's implementation is in specification development stage
> and the project is looking to support it when it becomes available.
>
> This patchset provides detailed documentation of DRTM, the approach used for
> adding the capbility, and relevant API/ABI documentation. In addition to the
> documentation the patch set introduces Intel TXT support as the first platform
> for Linux Secure Launch.
>
> A quick note on terminology. The larger open source project itself is called
> TrenchBoot, which is hosted on Github (links below). The kernel feature enabling
> the use of Dynamic Launch technology is referred to as "Secure Launch" within
> the kernel code. As such the prefixes sl_/SL_ or slaunch/SLAUNCH will be seen
> in the code. The stub code discussed above is referred to as the SL stub.

1. I don't see any tags in most of the patches so don't get the rush. This
   includes also patches for x86. Why I would care to review TPM patches
   when there is over a dozen unreviewed and untested patches before it?
2. TPM patches have been in circulation in and out of the patch set
   for some time now with little or no improvement.

Why the sudden buzz? I have not heard much about this since last early
summer.  Have to spend some time recalling what this is about anyway. I
cannot trust that my tags make any sense before more reviewed/tested-by
tags before the TPM patches.

BR, Jarkko

On Fri, Nov 01 2024 at 12:28, Jarkko Sakkinen wrote:
> On Fri Sep 13, 2024 at 11:04 PM EEST, Ross Philipson wrote:
>> A quick note on terminology. The larger open source project itself is called
>> TrenchBoot, which is hosted on Github (links below). The kernel feature enabling
>> the use of Dynamic Launch technology is referred to as "Secure Launch" within
>> the kernel code. As such the prefixes sl_/SL_ or slaunch/SLAUNCH will be seen
>> in the code. The stub code discussed above is referred to as the SL stub.
>
> 1. I don't see any tags in most of the patches so don't get the rush. This
>    includes also patches for x86. Why I would care to review TPM patches
>    when there is over a dozen unreviewed and untested patches before it?
> 2. TPM patches have been in circulation in and out of the patch set
>    for some time now with little or no improvement.
>
> Why the sudden buzz? I have not heard much about this since last early
> summer.  Have to spend some time recalling what this is about anyway. I
> cannot trust that my tags make any sense before more reviewed/tested-by
> tags before the TPM patches.

If I intend to merge the patches then I surely have looked at them
deeply. I don't have to send a reviewed-by just to apply them
afterwards.

There was enough motion on these patches and this posting is in your
inbox for 6 weeks now without any reaction from you.

The TPM changes are very much independent from the x86 specific ones, so
why do you want x86 review tags in order to look at the ones which are
specific to your subsystem especially as some of them seem to address
real short comings there independent of trenchboot.

Thanks,

        tglx

On Fri Nov 1, 2024 at 10:34 PM EET, Thomas Gleixner wrote:
> On Fri, Nov 01 2024 at 12:28, Jarkko Sakkinen wrote:
> > On Fri Sep 13, 2024 at 11:04 PM EEST, Ross Philipson wrote:
> >> A quick note on terminology. The larger open source project itself is called
> >> TrenchBoot, which is hosted on Github (links below). The kernel feature enabling
> >> the use of Dynamic Launch technology is referred to as "Secure Launch" within
> >> the kernel code. As such the prefixes sl_/SL_ or slaunch/SLAUNCH will be seen
> >> in the code. The stub code discussed above is referred to as the SL stub.
> >
> > 1. I don't see any tags in most of the patches so don't get the rush. This
> >    includes also patches for x86. Why I would care to review TPM patches
> >    when there is over a dozen unreviewed and untested patches before it?
> > 2. TPM patches have been in circulation in and out of the patch set
> >    for some time now with little or no improvement.
> >
> > Why the sudden buzz? I have not heard much about this since last early
> > summer.  Have to spend some time recalling what this is about anyway. I
> > cannot trust that my tags make any sense before more reviewed/tested-by
> > tags before the TPM patches.
>
> If I intend to merge the patches then I surely have looked at them
> deeply. I don't have to send a reviewed-by just to apply them
> afterwards.
>
> There was enough motion on these patches and this posting is in your
> inbox for 6 weeks now without any reaction from you.
>
> The TPM changes are very much independent from the x86 specific ones, so
> why do you want x86 review tags in order to look at the ones which are
> specific to your subsystem especially as some of them seem to address
> real short comings there independent of trenchboot.

I think we can sort them out independently as long as we find a
conclusion how to address locality change.

>
> Thanks,
>
>         tglx

BR, Jarkko

On Fri Nov 1, 2024 at 11:13 PM EET, Jarkko Sakkinen wrote:
> On Fri Nov 1, 2024 at 10:34 PM EET, Thomas Gleixner wrote:
> > On Fri, Nov 01 2024 at 12:28, Jarkko Sakkinen wrote:
> > > On Fri Sep 13, 2024 at 11:04 PM EEST, Ross Philipson wrote:
> > >> A quick note on terminology. The larger open source project itself is called
> > >> TrenchBoot, which is hosted on Github (links below). The kernel feature enabling
> > >> the use of Dynamic Launch technology is referred to as "Secure Launch" within
> > >> the kernel code. As such the prefixes sl_/SL_ or slaunch/SLAUNCH will be seen
> > >> in the code. The stub code discussed above is referred to as the SL stub.
> > >
> > > 1. I don't see any tags in most of the patches so don't get the rush. This
> > >    includes also patches for x86. Why I would care to review TPM patches
> > >    when there is over a dozen unreviewed and untested patches before it?
> > > 2. TPM patches have been in circulation in and out of the patch set
> > >    for some time now with little or no improvement.
> > >
> > > Why the sudden buzz? I have not heard much about this since last early
> > > summer.  Have to spend some time recalling what this is about anyway. I
> > > cannot trust that my tags make any sense before more reviewed/tested-by
> > > tags before the TPM patches.
> >
> > If I intend to merge the patches then I surely have looked at them
> > deeply. I don't have to send a reviewed-by just to apply them
> > afterwards.
> >
> > There was enough motion on these patches and this posting is in your
> > inbox for 6 weeks now without any reaction from you.
> >
> > The TPM changes are very much independent from the x86 specific ones, so
> > why do you want x86 review tags in order to look at the ones which are
> > specific to your subsystem especially as some of them seem to address
> > real short comings there independent of trenchboot.
>
> I think we can sort them out independently as long as we find a
> conclusion how to address locality change.

And to be fair: there was no reaction from anyone. It is mostly x86
patch set, meaning that I was waiting for some reaction first from that
side.  And I did respond to that when it came.

IMHO: let's get a solution for that one problem and then it should be
fine as far as I'm concerned.

BR, Jarkko

On Fri, Nov 01 2024 at 23:19, Jarkko Sakkinen wrote:
> On Fri Nov 1, 2024 at 11:13 PM EET, Jarkko Sakkinen wrote:
>> I think we can sort them out independently as long as we find a
>> conclusion how to address locality change.
>
> And to be fair: there was no reaction from anyone. It is mostly x86
> patch set, meaning that I was waiting for some reaction first from that
> side.  And I did respond to that when it came.

The x86 side is mostly self contained, so the damage there is minimal,
but the TPM parts are changing the generic operations and the x86 parts
depend on them.

So let's not create a chicken and egg problem and solve the TPM parts,
which at my cursory glance are partially legitimate fixes, independent
of the actual trenchboot x86 functionality.

Thanks,

        tglx

On Sat Nov 2, 2024 at 12:04 AM EET, Thomas Gleixner wrote:
> On Fri, Nov 01 2024 at 23:19, Jarkko Sakkinen wrote:
> > On Fri Nov 1, 2024 at 11:13 PM EET, Jarkko Sakkinen wrote:
> >> I think we can sort them out independently as long as we find a
> >> conclusion how to address locality change.
> >
> > And to be fair: there was no reaction from anyone. It is mostly x86
> > patch set, meaning that I was waiting for some reaction first from that
> > side.  And I did respond to that when it came.
>
> The x86 side is mostly self contained, so the damage there is minimal,
> but the TPM parts are changing the generic operations and the x86 parts
> depend on them.
>
> So let's not create a chicken and egg problem and solve the TPM parts,
> which at my cursory glance are partially legitimate fixes, independent
> of the actual trenchboot x86 functionality.

Yeah, I'm already writing a (draft/RFC) patch to demonstrate my
proposal that I sent so all good.

>
> Thanks,
>
>         tglx

BR, Jarkko

On Fri, Sep 13 2024 at 13:04, Ross Philipson wrote:
> The larger focus of the TrenchBoot project (https://github.com/TrenchBoot) is to
> enhance the boot security and integrity in a unified manner. The first area of
> focus has been on the Trusted Computing Group's Dynamic Launch for establishing
> a hardware Root of Trust for Measurement, also know as DRTM (Dynamic Root of
> Trust for Measurement). The project has been and continues to work on providing
> a unified means to Dynamic Launch that is a cross-platform (Intel and AMD) and
> cross-architecture (x86 and Arm), with our recent involvment in the upcoming
> Arm DRTM specification. The order of introducing DRTM to the Linux kernel
> follows the maturity of DRTM in the architectures. Intel's Trusted eXecution
> Technology (TXT) is present today and only requires a preamble loader, e.g. a
> boot loader, and an OS kernel that is TXT-aware. AMD DRTM implementation has
> been present since the introduction of AMD-V but requires an additional
> component that is AMD specific and referred to in the specification as the
> Secure Loader, which the TrenchBoot project has an active prototype in
> development. Finally Arm's implementation is in specification development stage
> and the project is looking to support it when it becomes available.
>
> This patchset provides detailed documentation of DRTM, the approach used for
> adding the capbility, and relevant API/ABI documentation. In addition to the
> documentation the patch set introduces Intel TXT support as the first platform
> for Linux Secure Launch.

So this looks pretty reasonable to me by now and I'm inclined to take it
through the tip x86 tree, but that needs reviewed/acked-by's from the
crypto and TPM folks. EFI has been reviewed already.

Can we make progress on this please?

Thanks,

        tglx

On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote:
> On Fri, Sep 13 2024 at 13:04, Ross Philipson wrote:
> > The larger focus of the TrenchBoot project (https://github.com/TrenchBoot) is to
> > enhance the boot security and integrity in a unified manner. The first area of
> > focus has been on the Trusted Computing Group's Dynamic Launch for establishing
> > a hardware Root of Trust for Measurement, also know as DRTM (Dynamic Root of
> > Trust for Measurement). The project has been and continues to work on providing
> > a unified means to Dynamic Launch that is a cross-platform (Intel and AMD) and
> > cross-architecture (x86 and Arm), with our recent involvment in the upcoming
> > Arm DRTM specification. The order of introducing DRTM to the Linux kernel
> > follows the maturity of DRTM in the architectures. Intel's Trusted eXecution
> > Technology (TXT) is present today and only requires a preamble loader, e.g. a
> > boot loader, and an OS kernel that is TXT-aware. AMD DRTM implementation has
> > been present since the introduction of AMD-V but requires an additional
> > component that is AMD specific and referred to in the specification as the
> > Secure Loader, which the TrenchBoot project has an active prototype in
> > development. Finally Arm's implementation is in specification development stage
> > and the project is looking to support it when it becomes available.
> >
> > This patchset provides detailed documentation of DRTM, the approach used for
> > adding the capbility, and relevant API/ABI documentation. In addition to the
> > documentation the patch set introduces Intel TXT support as the first platform
> > for Linux Secure Launch.
>
> So this looks pretty reasonable to me by now and I'm inclined to take it
> through the tip x86 tree, but that needs reviewed/acked-by's from the
> crypto and TPM folks. EFI has been reviewed already.
>
> Can we make progress on this please?

So TPM patches do have bunch of glitches:

- 15/20: I don't get this. There is nothing to report unless tree
  is falling. The reported-by tag literally meaningless. Maybe this
  is something that makes sense with this feature. Explain from that
  angle.
- 16/20: Is this actually a bug fix? If it is should be before 15/20.
- 17/20: the commit message could do a better job explaining how the
  locality can vary. I'm not sure how this will be used by rest of
  the patch set.
- 18/20: I'm not confident we want to give privilege to set locality
  to the user space. The commit message neither makes a case of this.
  Has this been tested to together with bus encryption (just checking)?

>
> Thanks,
>
>         tglx

BR, Jarkko

On Fri, Nov 01 2024 at 00:37, Jarkko Sakkinen wrote:
> On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote:
>> So this looks pretty reasonable to me by now and I'm inclined to take it
>> through the tip x86 tree, but that needs reviewed/acked-by's from the
>> crypto and TPM folks. EFI has been reviewed already.
>>
>> Can we make progress on this please?
>
> So TPM patches do have bunch of glitches:
>
> - 15/20: I don't get this. There is nothing to report unless tree
>   is falling. The reported-by tag literally meaningless. Maybe this
>   is something that makes sense with this feature. Explain from that
>   angle.
> - 16/20: Is this actually a bug fix? If it is should be before 15/20.
> - 17/20: the commit message could do a better job explaining how the
>   locality can vary. I'm not sure how this will be used by rest of
>   the patch set.
> - 18/20: I'm not confident we want to give privilege to set locality
>   to the user space. The commit message neither makes a case of this.
>   Has this been tested to together with bus encryption (just checking)?

Can you please explicitely voice your detailed technical concerns in
replies to the actual patches?

Thanks,

        tglx

On Fri Nov 1, 2024 at 1:08 AM EET, Thomas Gleixner wrote:
> On Fri, Nov 01 2024 at 00:37, Jarkko Sakkinen wrote:
> > On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote:
> >> So this looks pretty reasonable to me by now and I'm inclined to take it
> >> through the tip x86 tree, but that needs reviewed/acked-by's from the
> >> crypto and TPM folks. EFI has been reviewed already.
> >>
> >> Can we make progress on this please?
> >
> > So TPM patches do have bunch of glitches:
> >
> > - 15/20: I don't get this. There is nothing to report unless tree
> >   is falling. The reported-by tag literally meaningless. Maybe this
> >   is something that makes sense with this feature. Explain from that
> >   angle.
> > - 16/20: Is this actually a bug fix? If it is should be before 15/20.
> > - 17/20: the commit message could do a better job explaining how the
> >   locality can vary. I'm not sure how this will be used by rest of
> >   the patch set.
> > - 18/20: I'm not confident we want to give privilege to set locality
> >   to the user space. The commit message neither makes a case of this.
> >   Has this been tested to together with bus encryption (just checking)?
>
> Can you please explicitely voice your detailed technical concerns in
> replies to the actual patches?

Yes, I did that.

>
> Thanks,
>
>         tglx

BR, Jarkko

On Fri Nov 1, 2024 at 1:08 AM EET, Thomas Gleixner wrote:
> On Fri, Nov 01 2024 at 00:37, Jarkko Sakkinen wrote:
> > On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote:
> >> So this looks pretty reasonable to me by now and I'm inclined to take it
> >> through the tip x86 tree, but that needs reviewed/acked-by's from the
> >> crypto and TPM folks. EFI has been reviewed already.
> >>
> >> Can we make progress on this please?
> >
> > So TPM patches do have bunch of glitches:
> >
> > - 15/20: I don't get this. There is nothing to report unless tree
> >   is falling. The reported-by tag literally meaningless. Maybe this
> >   is something that makes sense with this feature. Explain from that
> >   angle.
> > - 16/20: Is this actually a bug fix? If it is should be before 15/20.
> > - 17/20: the commit message could do a better job explaining how the
> >   locality can vary. I'm not sure how this will be used by rest of
> >   the patch set.
> > - 18/20: I'm not confident we want to give privilege to set locality
> >   to the user space. The commit message neither makes a case of this.
> >   Has this been tested to together with bus encryption (just checking)?
>
> Can you please explicitely voice your detailed technical concerns in
> replies to the actual patches?

- 15/20 looks like a rigged patch. I don't really know why it is done
  so it is hard to either suggest how "resolve it".
- 16/20 probably makes sense but if it is a bug fix or part of it is,
  the bug fix should have relevant fixes etc tags so that it can be
  picked up to stable kernels.
- 17-18/20: I'd speak about this as the "one whole" i.e. here the
  privilege to be able change locality during run-time is really
  concerning. Could the locality be figured out for the kernel
  command-line instead? The sysfs attribute can exist as read-only.

So yeah, the way I see it 15-16 are the more trivial issue to sort
out (probably) but with 17-18 we have an actual architectural concern
for kernel overall.

> Thanks,
>
>         tglx

BR, Jarkko

On Fri Nov 1, 2024 at 2:33 AM EET, Jarkko Sakkinen wrote:
> On Fri Nov 1, 2024 at 1:08 AM EET, Thomas Gleixner wrote:
> > On Fri, Nov 01 2024 at 00:37, Jarkko Sakkinen wrote:
> > > On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote:
> > >> So this looks pretty reasonable to me by now and I'm inclined to take it
> > >> through the tip x86 tree, but that needs reviewed/acked-by's from the
> > >> crypto and TPM folks. EFI has been reviewed already.
> > >>
> > >> Can we make progress on this please?
> > >
> > > So TPM patches do have bunch of glitches:
> > >
> > > - 15/20: I don't get this. There is nothing to report unless tree
> > >   is falling. The reported-by tag literally meaningless. Maybe this
> > >   is something that makes sense with this feature. Explain from that
> > >   angle.
> > > - 16/20: Is this actually a bug fix? If it is should be before 15/20.
> > > - 17/20: the commit message could do a better job explaining how the
> > >   locality can vary. I'm not sure how this will be used by rest of
> > >   the patch set.
> > > - 18/20: I'm not confident we want to give privilege to set locality
> > >   to the user space. The commit message neither makes a case of this.
> > >   Has this been tested to together with bus encryption (just checking)?
> >
> > Can you please explicitely voice your detailed technical concerns in
> > replies to the actual patches?
>
> - 15/20 looks like a rigged patch. I don't really know why it is done
>   so it is hard to either suggest how "resolve it".
> - 16/20 probably makes sense but if it is a bug fix or part of it is,
>   the bug fix should have relevant fixes etc tags so that it can be
>   picked up to stable kernels.
> - 17-18/20: I'd speak about this as the "one whole" i.e. here the
>   privilege to be able change locality during run-time is really
>   concerning. Could the locality be figured out for the kernel
>   command-line instead? The sysfs attribute can exist as read-only.
>
> So yeah, the way I see it 15-16 are the more trivial issue to sort
> out (probably) but with 17-18 we have an actual architectural concern
> for kernel overall.

Further:

15/20: I can accept this without reported-by tag (or changed as
suggested-by). It does not harm.
16/20: I'll re-review this with time. I'll try to get this done
latest next week.

So let's put focus only on 17 and 18. Can this problem be sorted out
by kernel command-line parameter? In the case of locality we want to
keep regular "chain of trust" i.e. boot-loader makes the decision,
*even* in the case of DRTM. I would call this almost as constraint
that would be wise to set.

BR, Jarkko

On Fri, 1 Nov 2024 at 01:40, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Fri Nov 1, 2024 at 2:33 AM EET, Jarkko Sakkinen wrote:
> > On Fri Nov 1, 2024 at 1:08 AM EET, Thomas Gleixner wrote:
> > > On Fri, Nov 01 2024 at 00:37, Jarkko Sakkinen wrote:
> > > > On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote:
> > > >> So this looks pretty reasonable to me by now and I'm inclined to take it
> > > >> through the tip x86 tree, but that needs reviewed/acked-by's from the
> > > >> crypto and TPM folks. EFI has been reviewed already.
> > > >>
> > > >> Can we make progress on this please?
> > > >
> > > > So TPM patches do have bunch of glitches:
> > > >
> > > > - 15/20: I don't get this. There is nothing to report unless tree
> > > >   is falling. The reported-by tag literally meaningless. Maybe this
> > > >   is something that makes sense with this feature. Explain from that
> > > >   angle.
> > > > - 16/20: Is this actually a bug fix? If it is should be before 15/20.
> > > > - 17/20: the commit message could do a better job explaining how the
> > > >   locality can vary. I'm not sure how this will be used by rest of
> > > >   the patch set.
> > > > - 18/20: I'm not confident we want to give privilege to set locality
> > > >   to the user space. The commit message neither makes a case of this.
> > > >   Has this been tested to together with bus encryption (just checking)?
> > >
> > > Can you please explicitely voice your detailed technical concerns in
> > > replies to the actual patches?
> >
> > - 15/20 looks like a rigged patch. I don't really know why it is done
> >   so it is hard to either suggest how "resolve it".
> > - 16/20 probably makes sense but if it is a bug fix or part of it is,
> >   the bug fix should have relevant fixes etc tags so that it can be
> >   picked up to stable kernels.
> > - 17-18/20: I'd speak about this as the "one whole" i.e. here the
> >   privilege to be able change locality during run-time is really
> >   concerning. Could the locality be figured out for the kernel
> >   command-line instead? The sysfs attribute can exist as read-only.
> >
> > So yeah, the way I see it 15-16 are the more trivial issue to sort
> > out (probably) but with 17-18 we have an actual architectural concern
> > for kernel overall.
>
> Further:
>
> 15/20: I can accept this without reported-by tag (or changed as
> suggested-by). It does not harm.
> 16/20: I'll re-review this with time. I'll try to get this done
> latest next week.
>
> So let's put focus only on 17 and 18. Can this problem be sorted out
> by kernel command-line parameter? In the case of locality we want to
> keep regular "chain of trust" i.e. boot-loader makes the decision,
> *even* in the case of DRTM. I would call this almost as constraint
> that would be wise to set.
>

Please don't add a kernel command line parameter for this - the code
running in the decompressor will be the one setting it and there are
better ways to pass information between these components (and the
slaunch stack is already doing that in any case)

Also, let's have this discussion in the appropriate place, i.e., on
the thread for each respective patch.

On Fri Nov 1, 2024 at 10:50 AM EET, Ard Biesheuvel wrote:
> On Fri, 1 Nov 2024 at 01:40, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Fri Nov 1, 2024 at 2:33 AM EET, Jarkko Sakkinen wrote:
> > > On Fri Nov 1, 2024 at 1:08 AM EET, Thomas Gleixner wrote:
> > > > On Fri, Nov 01 2024 at 00:37, Jarkko Sakkinen wrote:
> > > > > On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote:
> > > > >> So this looks pretty reasonable to me by now and I'm inclined to take it
> > > > >> through the tip x86 tree, but that needs reviewed/acked-by's from the
> > > > >> crypto and TPM folks. EFI has been reviewed already.
> > > > >>
> > > > >> Can we make progress on this please?
> > > > >
> > > > > So TPM patches do have bunch of glitches:
> > > > >
> > > > > - 15/20: I don't get this. There is nothing to report unless tree
> > > > >   is falling. The reported-by tag literally meaningless. Maybe this
> > > > >   is something that makes sense with this feature. Explain from that
> > > > >   angle.
> > > > > - 16/20: Is this actually a bug fix? If it is should be before 15/20.
> > > > > - 17/20: the commit message could do a better job explaining how the
> > > > >   locality can vary. I'm not sure how this will be used by rest of
> > > > >   the patch set.
> > > > > - 18/20: I'm not confident we want to give privilege to set locality
> > > > >   to the user space. The commit message neither makes a case of this.
> > > > >   Has this been tested to together with bus encryption (just checking)?
> > > >
> > > > Can you please explicitely voice your detailed technical concerns in
> > > > replies to the actual patches?
> > >
> > > - 15/20 looks like a rigged patch. I don't really know why it is done
> > >   so it is hard to either suggest how "resolve it".
> > > - 16/20 probably makes sense but if it is a bug fix or part of it is,
> > >   the bug fix should have relevant fixes etc tags so that it can be
> > >   picked up to stable kernels.
> > > - 17-18/20: I'd speak about this as the "one whole" i.e. here the
> > >   privilege to be able change locality during run-time is really
> > >   concerning. Could the locality be figured out for the kernel
> > >   command-line instead? The sysfs attribute can exist as read-only.
> > >
> > > So yeah, the way I see it 15-16 are the more trivial issue to sort
> > > out (probably) but with 17-18 we have an actual architectural concern
> > > for kernel overall.
> >
> > Further:
> >
> > 15/20: I can accept this without reported-by tag (or changed as
> > suggested-by). It does not harm.
> > 16/20: I'll re-review this with time. I'll try to get this done
> > latest next week.
> >
> > So let's put focus only on 17 and 18. Can this problem be sorted out
> > by kernel command-line parameter? In the case of locality we want to
> > keep regular "chain of trust" i.e. boot-loader makes the decision,
> > *even* in the case of DRTM. I would call this almost as constraint
> > that would be wise to set.
> >
>
> Please don't add a kernel command line parameter for this - the code
> running in the decompressor will be the one setting it and there are
> better ways to pass information between these components (and the
> slaunch stack is already doing that in any case)

Not sure if I follow this (I don't know what "decompressor" is).

> Also, let's have this discussion in the appropriate place, i.e., on
> the thread for each respective patch.

Sure, I just did not have a lot of time to download the patch.

BR, Jarkko

On Fri Nov 1, 2024 at 11:18 AM EET, Jarkko Sakkinen wrote:
> On Fri Nov 1, 2024 at 10:50 AM EET, Ard Biesheuvel wrote:
> > On Fri, 1 Nov 2024 at 01:40, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > >
> > > On Fri Nov 1, 2024 at 2:33 AM EET, Jarkko Sakkinen wrote:
> > > > On Fri Nov 1, 2024 at 1:08 AM EET, Thomas Gleixner wrote:
> > > > > On Fri, Nov 01 2024 at 00:37, Jarkko Sakkinen wrote:
> > > > > > On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote:
> > > > > >> So this looks pretty reasonable to me by now and I'm inclined to take it
> > > > > >> through the tip x86 tree, but that needs reviewed/acked-by's from the
> > > > > >> crypto and TPM folks. EFI has been reviewed already.
> > > > > >>
> > > > > >> Can we make progress on this please?
> > > > > >
> > > > > > So TPM patches do have bunch of glitches:
> > > > > >
> > > > > > - 15/20: I don't get this. There is nothing to report unless tree
> > > > > >   is falling. The reported-by tag literally meaningless. Maybe this
> > > > > >   is something that makes sense with this feature. Explain from that
> > > > > >   angle.
> > > > > > - 16/20: Is this actually a bug fix? If it is should be before 15/20.
> > > > > > - 17/20: the commit message could do a better job explaining how the
> > > > > >   locality can vary. I'm not sure how this will be used by rest of
> > > > > >   the patch set.
> > > > > > - 18/20: I'm not confident we want to give privilege to set locality
> > > > > >   to the user space. The commit message neither makes a case of this.
> > > > > >   Has this been tested to together with bus encryption (just checking)?
> > > > >
> > > > > Can you please explicitely voice your detailed technical concerns in
> > > > > replies to the actual patches?
> > > >
> > > > - 15/20 looks like a rigged patch. I don't really know why it is done
> > > >   so it is hard to either suggest how "resolve it".
> > > > - 16/20 probably makes sense but if it is a bug fix or part of it is,
> > > >   the bug fix should have relevant fixes etc tags so that it can be
> > > >   picked up to stable kernels.
> > > > - 17-18/20: I'd speak about this as the "one whole" i.e. here the
> > > >   privilege to be able change locality during run-time is really
> > > >   concerning. Could the locality be figured out for the kernel
> > > >   command-line instead? The sysfs attribute can exist as read-only.
> > > >
> > > > So yeah, the way I see it 15-16 are the more trivial issue to sort
> > > > out (probably) but with 17-18 we have an actual architectural concern
> > > > for kernel overall.
> > >
> > > Further:
> > >
> > > 15/20: I can accept this without reported-by tag (or changed as
> > > suggested-by). It does not harm.
> > > 16/20: I'll re-review this with time. I'll try to get this done
> > > latest next week.
> > >
> > > So let's put focus only on 17 and 18. Can this problem be sorted out
> > > by kernel command-line parameter? In the case of locality we want to
> > > keep regular "chain of trust" i.e. boot-loader makes the decision,
> > > *even* in the case of DRTM. I would call this almost as constraint
> > > that would be wise to set.
> > >
> >
> > Please don't add a kernel command line parameter for this - the code
> > running in the decompressor will be the one setting it and there are
> > better ways to pass information between these components (and the
> > slaunch stack is already doing that in any case)
>
> Not sure if I follow this (I don't know what "decompressor" is).

Right you refer to the process running GETSEC[SENTER], sorry, in the page with
this detail.

BR, Jarkko