2021-06-02 14:08:40

by Brijesh Singh

[permalink] [raw]
Subject: [PATCH Part1 RFC v3 20/22] x86/boot: Add Confidential Computing address to setup_header

While launching the encrypted guests, the hypervisor may need to provide
some additional information that will used during the guest boot. In the
case of AMD SEV-SNP the information includes the address of the secrets
and CPUID pages. The secrets page contains information such as a VM to
PSP communication key and CPUID page contain PSP filtered CPUID values.

When booting under the EFI based BIOS, the EFI configuration table
contains an entry for the confidential computing blob. In order to support
booting encrypted guests on non EFI VM, the hypervisor to pass these
additional information to the kernel with different method.

For this purpose expand the struct setup_header to hold the physical
address of the confidential computing blob location. Being zero means it
isn't passed.

Signed-off-by: Brijesh Singh <[email protected]>
---
Documentation/x86/boot.rst | 27 +++++++++++++++++++++++++++
arch/x86/boot/header.S | 7 ++++++-
arch/x86/include/uapi/asm/bootparam.h | 1 +
3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index fc844913dece..9b32805617bb 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -75,6 +75,8 @@ Protocol 2.14 BURNT BY INCORRECT COMMIT
DO NOT USE!!! ASSUME SAME AS 2.13.

Protocol 2.15 (Kernel 5.5) Added the kernel_info and kernel_info.setup_type_max.
+
+Protocol 2.16 (Kernel 5.14) Added the confidential computing blob address
============= ============================================================

.. note::
@@ -226,6 +228,7 @@ Offset/Size Proto Name Meaning
0260/4 2.10+ init_size Linear memory required during initialization
0264/4 2.11+ handover_offset Offset of handover entry point
0268/4 2.15+ kernel_info_offset Offset of the kernel_info
+026C/4 2.16+ cc_blob_address Physical address of the confidential computing blob
=========== ======== ===================== ============================================

.. note::
@@ -1026,6 +1029,30 @@ Offset/size: 0x000c/4

This field contains maximal allowed type for setup_data and setup_indirect structs.

+============ ==================
+Field name: cc_blob_address
+Type: write (optional)
+Offset/size: 0x26C/4
+Protocol: 2.16+
+============ ==================
+
+ This field can be set by the boot loader to tell the kernel the physical address
+ of the confidential computing blob info.
+
+ A value of 0 indicates that either the blob is not provided or use the EFI configuration
+ table to retrieve the blob location.
+
+ In the case of AMD SEV the blob look like this::
+
+ struct cc_blob_sev_info {
+ u32 magic; /* 0x45444d41 (AMDE) */
+ u16 version;
+ u16 reserved;
+ u64 secrets_phys; /* pointer to secrets page */
+ u32 secrets_len;
+ u64 cpuid_phys; /* pointer to cpuid page */
+ u32 cpuid_len;
+ }

The Image Checksum
==================
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 6dbd7e9f74c9..b4a014a18f91 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -303,7 +303,7 @@ _start:
# Part 2 of the header, from the old setup.S

.ascii "HdrS" # header signature
- .word 0x020f # header version number (>= 0x0105)
+ .word 0x0210 # header version number (>= 0x0105)
# or else old loadlin-1.5 will fail)
.globl realmode_swtch
realmode_swtch: .word 0, 0 # default_switch, SETUPSEG
@@ -577,6 +577,11 @@ pref_address: .quad LOAD_PHYSICAL_ADDR # preferred load addr
init_size: .long INIT_SIZE # kernel initialization size
handover_offset: .long 0 # Filled in by build.c
kernel_info_offset: .long 0 # Filled in by build.c
+cc_blob_address: .long 0 # (Header version 0x210 or later)
+ # If nonzero, a 32-bit pointer to
+ # the confidential computing blob.
+ # The blob will contain the information
+ # used while booting the encrypted VM.

# End of setup header #####################################################

diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index b25d3f82c2f3..210e1a0fb4ce 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -102,6 +102,7 @@ struct setup_header {
__u32 init_size;
__u32 handover_offset;
__u32 kernel_info_offset;
+ __u32 cc_blob_address;
} __attribute__((packed));

struct sys_desc_table {
--
2.17.1


2021-06-23 10:23:58

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH Part1 RFC v3 20/22] x86/boot: Add Confidential Computing address to setup_header

On Tue, Jun 22, 2021 at 11:30:43PM -0500, Michael Roth wrote:
> Quoting Borislav Petkov (2021-06-18 10:05:28)
> > On Fri, Jun 18, 2021 at 08:57:12AM -0500, Brijesh Singh wrote:
> > > Don't have any strong reason to keep it separate, I can define a new
> > > type and use the setup_data to pass this information.
> >
> > setup_data is exactly for use cases like that - pass a bunch of data
> > to the kernel. So there's no need for a separate thing. Also see that
> > kernel_info thing which got added recently for read_only data.
>
> Hi Boris,
>
> There's one side-effect to this change WRT the CPUID page (which I think
> we're hoping to include in RFC v4).
>
> With CPUID page we need to access it very early in boot, for both
> boot/compressed kernel, and the uncompressed kernel. At first this was
> implemented by moving the early EFI table parsing code from
> arch/x86/kernel/boot/compressed/acpi.c into a little library to handle early
> EFI table parsing to fetch the Confidential Computing blob to get the CPUID
> page address.
>
> This was a bit messy since we needed to share that library between
> boot/compressed and uncompressed, and at that early stage things like
> fixup_pointer() are needed in some places, else even basic things like
> accessing EFI64_LOADER_SIGNATURE and various EFI helper functions could crash
> in uncompressed otherwise, so the library code needed to be fixed up
> accordingly.
>
> To simplify things we ended up simply keeping the early EFI table parsing in
> boot/compressed, and then having boot/compressed initialize
> setup_data.cc_blob_address so that the uncompressed kernel could access it
> from there (acpi does something similar with rdsp address).

Yes, except the rsdp address is not vendor-specific but an x86 ACPI
thing, so pretty much omnipresent.

Also, acpi_rsdp_addr is part of boot_params and that struct is full
of padding holes and obsolete members so reusing a u32 there is a lot
"easier" than changing the setup_header. So can you put that address in
boot_params instead?

> Now that we're moving it to setup_data, this becomes a bit more awkward,
> since we need to reserve memory in boot/compressed to store the setup_data
> entry, then add it to the linked list to pass along to uncompressed kernel.
> In turn that also means we need to add an identity mapping for this in
> ident_map_64.c, so I'm not sure that's the best approach.
>
> So just trying to pin what the best approach is:
>
> a) move cc_blob to setup_data, and do the above-described to pass
> cc_blob_address from boot/compressed to uncompressed to avoid early
> EFI parsing in uncompressed
> b) move cc_blob to setup_data, and do the EFI table parsing in both
> boot/compressed. leave setup_data allocation/init for BIOS/bootloader
> c) keep storing cc_blob_address in setup_header.cc_blob_address
> d) something else?

Leaving in the whole text for newly CCed TDX folks in case they're going
to need something like that.

And if so, then both vendors should even share the field definition.

Dave, Sathya, you can find the whole subthread here:

https://lkml.kernel.org/r/[email protected]

in case you need background info on the topic at hand.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-06-25 17:03:45

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH Part1 RFC v3 20/22] x86/boot: Add Confidential Computing address to setup_header

On Fri, Jun 25, 2021 at 10:24:01AM -0500, Brijesh Singh wrote:
> In the case of EFI, the CC blob structure is dynamically allocated
> and passed through the EFI configuration table. The grub will not
> know what value to pass in the cmdline unless we improve it to read
> the EFI configuration table and rebuild the cmdline.

Or simply parse the EFI table.

To repeat my question: why do you need the CC blob in the boot kernel?

Then, how does it work then in the !EFI case?

The script glue that starts the lightweight container goes and
"prepares" that blob and passes it to guest kernel? In which case
setup_data should do the job, methinks.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-06-28 13:45:40

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH Part1 RFC v3 20/22] x86/boot: Add Confidential Computing address to setup_header

On Fri, Jun 25, 2021 at 01:14:17PM -0500, Michael Roth wrote:
> So non-EFI case would rely purely on the setup_data entry for both (though
> we could still use boot_params->cc_blob_address to avoid the need to scan
> setup_data list in proper kernel as well, but scanning it early on doesn't
> have the same issues as with EFI config table so it's not really
> necessary there).

Yeah, sure, we can simply always use boot_params->cc_blob_address just
like acpi_rsdp_addr and always put the CC blob address there.

> I opted to give setup_data precedence over EFI, since if a bootloader goes
> the extra mile of packaging up a setup_data argument instead of just leaving
> it to firmware/EFI config table, it might be out of some extra need. For
> example, if we do have a shared definition for both SEV and TDX, maybe the
> bootloader needs to synthesize multiple EFI table entries, and a unified
> setup_data will be easier for the kernel to consume than replicating that same
> work, and maybe over time the fallback can be deprecated. And containers will
> more than likely prefer setup_data approach, which might drive future changes
> that aren't in lockstep with EFI definitions as well.

Yah, that makes perfect sense. And you/Brijesh should put the gist of
that in a comment over the code so that people are aware. The less we
rely on firmware, the better.

> Brijesh can correct me if I'm wrong, but I believe that's the intent, and the
> setup_data approach definitely seems workable for that aspect.

Oki doki, I think we're all on the same page then. :-)

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette