| Message ID | 20220128171804.569796-30-brijesh.singh@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand |
On Fri, Jan 28, 2022 at 11:17:50AM -0600, Brijesh Singh wrote: > +/* > + * AMD SEV Confidential computing blob structure. The structure is > + * defined in OVMF UEFI firmware header: > + * https://github.com/tianocore/edk2/blob/master/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h So looking at that typedef struct CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION there: typedef struct { UINT32 Header; UINT16 Version; UINT16 Reserved1; UINT64 SecretsPhysicalAddress; UINT32 SecretsSize; UINT64 CpuidPhysicalAddress; UINT32 CpuidLSize; } CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION; > + */ > +#define CC_BLOB_SEV_HDR_MAGIC 0x45444d41 > +struct cc_blob_sev_info { > + u32 magic; That's called "Header" there. > + u16 version; > + u16 reserved; > + u64 secrets_phys; > + u32 secrets_len; > + u32 rsvd1; You've added that member for padding but the fw blob one doesn't have it. But if we get a blob from the firmware and the structure layout differs, how is this supposed to even work? > + u64 cpuid_phys; > + u32 cpuid_len; > + u32 rsvd2; That one too. Or are you going to change the blob layout in ovmf too, to match?
On 2/4/22 10:21 AM, Borislav Petkov wrote: > On Fri, Jan 28, 2022 at 11:17:50AM -0600, Brijesh Singh wrote: >> +/* >> + * AMD SEV Confidential computing blob structure. The structure is >> + * defined in OVMF UEFI firmware header: >> + * https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fblob%2Fmaster%2FOvmfPkg%2FInclude%2FGuid%2FConfidentialComputingSevSnpBlob.h&data=04%7C01%7Cbrijesh.singh%40amd.com%7C334b7454d7a541f3497d08d9e7fa796c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637795885258984697%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=XNPvA7re7WHpAgzeuOC%2Buu0ql18P6KOIbP5ZriFsxEY%3D&reserved=0 > So looking at that typedef struct CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION there: > > typedef struct { > UINT32 Header; > UINT16 Version; > UINT16 Reserved1; > UINT64 SecretsPhysicalAddress; > UINT32 SecretsSize; > UINT64 CpuidPhysicalAddress; > UINT32 CpuidLSize; > } CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION; > >> + */ >> +#define CC_BLOB_SEV_HDR_MAGIC 0x45444d41 >> +struct cc_blob_sev_info { >> + u32 magic; > That's called "Header" there. I will rename it to keep it consistent with OVMF header. >> + u16 version; >> + u16 reserved; >> + u64 secrets_phys; >> + u32 secrets_len; >> + u32 rsvd1; > You've added that member for padding but the fw blob one doesn't have > it. > > But if we get a blob from the firmware and the structure layout differs, > how is this supposed to even work? > >> + u64 cpuid_phys; >> + u32 cpuid_len; >> + u32 rsvd2; > That one too. > > Or are you going to change the blob layout in ovmf too, to match? Yes, that's the plan. I have tested my OVMF with v9 and everything looks good at the runtime. Will send OVMF patch to match the blob layout.
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index a3203b2caaca..1a7e21bb6eea 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -42,6 +42,24 @@ struct es_em_ctxt { struct es_fault_info fi; }; +/* + * AMD SEV Confidential computing blob structure. The structure is + * defined in OVMF UEFI firmware header: + * https://github.com/tianocore/edk2/blob/master/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h + */ +#define CC_BLOB_SEV_HDR_MAGIC 0x45444d41 +struct cc_blob_sev_info { + u32 magic; + u16 version; + u16 reserved; + u64 secrets_phys; + u32 secrets_len; + u32 rsvd1; + u64 cpuid_phys; + u32 cpuid_len; + u32 rsvd2; +}; + void do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code); static inline u64 lower_bits(u64 val, unsigned int bits) diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index b25d3f82c2f3..1ac5acca72ce 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -10,6 +10,7 @@ #define SETUP_EFI 4 #define SETUP_APPLE_PROPERTIES 5 #define SETUP_JAILHOUSE 6 +#define SETUP_CC_BLOB 7 #define SETUP_INDIRECT (1<<31) diff --git a/include/linux/efi.h b/include/linux/efi.h index ccd4d3f91c98..984aa688997a 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -390,6 +390,7 @@ void efi_native_runtime_setup(void); #define EFI_CERT_SHA256_GUID EFI_GUID(0xc1c41626, 0x504c, 0x4092, 0xac, 0xa9, 0x41, 0xf9, 0x36, 0x93, 0x43, 0x28) #define EFI_CERT_X509_GUID EFI_GUID(0xa5c059a1, 0x94e4, 0x4aa7, 0x87, 0xb5, 0xab, 0x15, 0x5c, 0x2b, 0xf0, 0x72) #define EFI_CERT_X509_SHA256_GUID EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, 0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed) +#define EFI_CC_BLOB_GUID EFI_GUID(0x067b1f5f, 0xcf26, 0x44c5, 0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42) /* * This GUID is used to pass to the kernel proper the struct screen_info
While launching the encrypted guests, the hypervisor may need to provide some additional information during the guest boot. When booting under the EFI based BIOS, the EFI configuration table contains an entry for the confidential computing blob that contains the required information. To support booting encrypted guests on non-EFI VM, the hypervisor needs to pass this additional information to the kernel with a different method. For this purpose, introduce SETUP_CC_BLOB type in setup_data to hold the physical address of the confidential computing blob location. The boot loader or hypervisor may choose to use this method instead of EFI configuration table. The CC blob location scanning should give preference to setup_data data over the EFI configuration table. In AMD SEV-SNP, the CC blob contains the address of the secrets and CPUID pages. The secrets page includes information such as a VM to PSP communication key and CPUID page contains PSP filtered CPUID values. Define the AMD SEV confidential computing blob structure. While at it, define the EFI GUID for the confidential computing blob. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/include/asm/sev.h | 18 ++++++++++++++++++ arch/x86/include/uapi/asm/bootparam.h | 1 + include/linux/efi.h | 1 + 3 files changed, 20 insertions(+)