| Message ID | 20220128171804.569796-41-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand |
On Fri, Jan 28, 2022 at 10:19 AM Brijesh Singh <brijesh.singh@amd.com> wrote: > > Version 2 of GHCB specification provides Non Automatic Exit (NAE) that can > be used by the SEV-SNP guest to communicate with the PSP without risk from > a malicious hypervisor who wishes to read, alter, drop or replay the > messages sent. > > SNP_LAUNCH_UPDATE can insert two special pages into the guest’s memory: > the secrets page and the CPUID page. The PSP firmware populate the contents > of the secrets page. The secrets page contains encryption keys used by the > guest to interact with the firmware. Because the secrets page is encrypted > with the guest’s memory encryption key, the hypervisor cannot read the > keys. See SEV-SNP firmware spec for further details on the secrets page > format. > > Create a platform device that the SEV-SNP guest driver can bind to get the > platform resources such as encryption key and message id to use to > communicate with the PSP. The SEV-SNP guest driver provides a userspace > interface to get the attestation report, key derivation, extended > attestation report etc. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/include/asm/sev.h | 4 +++ > arch/x86/kernel/sev.c | 61 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 65 insertions(+) > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h > index 9830ee1d6ef0..ca977493eb72 100644 > --- a/arch/x86/include/asm/sev.h > +++ b/arch/x86/include/asm/sev.h > @@ -95,6 +95,10 @@ struct snp_req_data { > unsigned int data_npages; > }; > > +struct snp_guest_platform_data { > + u64 secrets_gpa; > +}; > + > #ifdef CONFIG_AMD_MEM_ENCRYPT > extern struct static_key_false sev_es_enable_key; > extern void __sev_es_ist_enter(struct pt_regs *regs); > diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c > index 1d3ac83226fc..1e56ab00d1f4 100644 > --- a/arch/x86/kernel/sev.c > +++ b/arch/x86/kernel/sev.c > @@ -19,6 +19,9 @@ > #include <linux/kernel.h> > #include <linux/mm.h> > #include <linux/cpumask.h> > +#include <linux/efi.h> > +#include <linux/platform_device.h> > +#include <linux/io.h> > > #include <asm/cpu_entry_area.h> > #include <asm/stacktrace.h> > @@ -34,6 +37,7 @@ > #include <asm/cpu.h> > #include <asm/apic.h> > #include <asm/cpuid.h> > +#include <asm/setup.h> > > #define DR7_RESET_VALUE 0x400 > > @@ -2177,3 +2181,60 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned > return ret; > } > EXPORT_SYMBOL_GPL(snp_issue_guest_request); > + > +static struct platform_device guest_req_device = { > + .name = "snp-guest", > + .id = -1, > +}; > + > +static u64 get_secrets_page(void) > +{ > + u64 pa_data = boot_params.cc_blob_address; > + struct cc_blob_sev_info info; > + void *map; > + > + /* > + * The CC blob contains the address of the secrets page, check if the > + * blob is present. > + */ > + if (!pa_data) > + return 0; > + > + map = early_memremap(pa_data, sizeof(info)); > + memcpy(&info, map, sizeof(info)); > + early_memunmap(map, sizeof(info)); > + > + /* smoke-test the secrets page passed */ > + if (!info.secrets_phys || info.secrets_len != PAGE_SIZE) > + return 0; This seems like an error condition worth noting. If no cc_blob_address is passed it makes sense not to log but what if the address passed fails this smoke test, why not log? > + > + return info.secrets_phys; > +} > + > +static int __init init_snp_platform_device(void) > +{ > + struct snp_guest_platform_data data; > + u64 gpa; > + > + if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) > + return -ENODEV; > + > + gpa = get_secrets_page(); > + if (!gpa) > + return -ENODEV; > + > + data.secrets_gpa = gpa; > + if (platform_device_add_data(&guest_req_device, &data, sizeof(data))) > + goto e_fail; > + > + if (platform_device_register(&guest_req_device)) > + goto e_fail; > + > + pr_info("SNP guest platform device initialized.\n"); > + return 0; > + > +e_fail: > + pr_err("Failed to initialize SNP guest device\n"); > + return -ENODEV; > +} > +device_initcall(init_snp_platform_device); > -- > 2.25.1 >
On 2/1/22 2:21 PM, Peter Gonda wrote: >> + /* smoke-test the secrets page passed */ >> + if (!info.secrets_phys || info.secrets_len != PAGE_SIZE) >> + return 0; > > This seems like an error condition worth noting. If no cc_blob_address > is passed it makes sense not to log but what if the address passed > fails this smoke test, why not log? If the smoke test fails, there will not be the /dev/sev-guest device, and userspace should log it accordingly. Having said so, I am not against logging if it helps. thanks
On Fri, Jan 28, 2022 at 11:18:01AM -0600, Brijesh Singh wrote: > Version 2 of GHCB specification provides Non Automatic Exit (NAE) that can ^ ^ ^ the a event type > be used by the SEV-SNP guest to communicate with the PSP without risk from > a malicious hypervisor who wishes to read, alter, drop or replay the > messages sent. > > SNP_LAUNCH_UPDATE can insert two special pages into the guest’s memory: > the secrets page and the CPUID page. The PSP firmware populate the contents "populates" > of the secrets page. The secrets page contains encryption keys used by the > guest to interact with the firmware. Because the secrets page is encrypted > with the guest’s memory encryption key, the hypervisor cannot read the > keys. See SEV-SNP firmware spec for further details on the secrets page > format. > > Create a platform device that the SEV-SNP guest driver can bind to get the > platform resources such as encryption key and message id to use to > communicate with the PSP. The SEV-SNP guest driver provides a userspace > interface to get the attestation report, key derivation, extended > attestation report etc. ... > +static int __init init_snp_platform_device(void) snp_init_platform_device() > +{ > + struct snp_guest_platform_data data; > + u64 gpa; > + > + if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) > + return -ENODEV; > + > + gpa = get_secrets_page(); > + if (!gpa) > + return -ENODEV; > + > + data.secrets_gpa = gpa; > + if (platform_device_add_data(&guest_req_device, &data, sizeof(data))) > + goto e_fail; > + > + if (platform_device_register(&guest_req_device)) > + goto e_fail; > + > + pr_info("SNP guest platform device initialized.\n"); > + return 0; > + > +e_fail: > + pr_err("Failed to initialize SNP guest device\n"); > + return -ENODEV; So when someone tries to debug why the platform device doesn't register properly, this error message is ambiguous and two of the error paths don't even issue one. Either issue a different error message before you return each time or remove it completely and let someone who really needs it, add it. I'd vote for former...
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index 9830ee1d6ef0..ca977493eb72 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -95,6 +95,10 @@ struct snp_req_data { unsigned int data_npages; }; +struct snp_guest_platform_data { + u64 secrets_gpa; +}; + #ifdef CONFIG_AMD_MEM_ENCRYPT extern struct static_key_false sev_es_enable_key; extern void __sev_es_ist_enter(struct pt_regs *regs); diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c index 1d3ac83226fc..1e56ab00d1f4 100644 --- a/arch/x86/kernel/sev.c +++ b/arch/x86/kernel/sev.c @@ -19,6 +19,9 @@ #include <linux/kernel.h> #include <linux/mm.h> #include <linux/cpumask.h> +#include <linux/efi.h> +#include <linux/platform_device.h> +#include <linux/io.h> #include <asm/cpu_entry_area.h> #include <asm/stacktrace.h> @@ -34,6 +37,7 @@ #include <asm/cpu.h> #include <asm/apic.h> #include <asm/cpuid.h> +#include <asm/setup.h> #define DR7_RESET_VALUE 0x400 @@ -2177,3 +2181,60 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned return ret; } EXPORT_SYMBOL_GPL(snp_issue_guest_request); + +static struct platform_device guest_req_device = { + .name = "snp-guest", + .id = -1, +}; + +static u64 get_secrets_page(void) +{ + u64 pa_data = boot_params.cc_blob_address; + struct cc_blob_sev_info info; + void *map; + + /* + * The CC blob contains the address of the secrets page, check if the + * blob is present. + */ + if (!pa_data) + return 0; + + map = early_memremap(pa_data, sizeof(info)); + memcpy(&info, map, sizeof(info)); + early_memunmap(map, sizeof(info)); + + /* smoke-test the secrets page passed */ + if (!info.secrets_phys || info.secrets_len != PAGE_SIZE) + return 0; + + return info.secrets_phys; +} + +static int __init init_snp_platform_device(void) +{ + struct snp_guest_platform_data data; + u64 gpa; + + if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) + return -ENODEV; + + gpa = get_secrets_page(); + if (!gpa) + return -ENODEV; + + data.secrets_gpa = gpa; + if (platform_device_add_data(&guest_req_device, &data, sizeof(data))) + goto e_fail; + + if (platform_device_register(&guest_req_device)) + goto e_fail; + + pr_info("SNP guest platform device initialized.\n"); + return 0; + +e_fail: + pr_err("Failed to initialize SNP guest device\n"); + return -ENODEV; +} +device_initcall(init_snp_platform_device);
Version 2 of GHCB specification provides Non Automatic Exit (NAE) that can be used by the SEV-SNP guest to communicate with the PSP without risk from a malicious hypervisor who wishes to read, alter, drop or replay the messages sent. SNP_LAUNCH_UPDATE can insert two special pages into the guest’s memory: the secrets page and the CPUID page. The PSP firmware populate the contents of the secrets page. The secrets page contains encryption keys used by the guest to interact with the firmware. Because the secrets page is encrypted with the guest’s memory encryption key, the hypervisor cannot read the keys. See SEV-SNP firmware spec for further details on the secrets page format. Create a platform device that the SEV-SNP guest driver can bind to get the platform resources such as encryption key and message id to use to communicate with the PSP. The SEV-SNP guest driver provides a userspace interface to get the attestation report, key derivation, extended attestation report etc. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/include/asm/sev.h | 4 +++ arch/x86/kernel/sev.c | 61 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+)