| Message ID | 20210820155918.7518-18-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support | expand |
On 8/20/21 10:58 AM, Brijesh Singh wrote: > +2.4 SNP_SET_EXT_CONFIG > +---------------------- > +:Technology: sev-snp > +:Type: hypervisor ioctl cmd > +:Parameters (in): struct sev_data_snp_ext_config > +:Returns (out): 0 on success, -negative on error > + > +The SNP_SET_EXT_CONFIG is used to set the system-wide configuration such as > +reported TCB version in the attestation report. The command is similar to > +SNP_CONFIG command defined in the SEV-SNP spec. The main difference is the > +command also accepts an additional certificate blob defined in the GHCB > +specification. > + > +If the certs_address is zero, then previous certificate blob will deleted. > +For more information on the certificate blob layout, see the GHCB spec > +(extended guest request message). Hi Brijesh, Just to be clear, is the documentation you're referring to regarding the layout of the certificate blob specified on page 47 of the GHCB spec? More specifically, is it the `struct cert_table` on that page? https://developer.amd.com/wp-content/resources/56421.pdf If so, where is the VCEK certificate layout documented? Connor > +/** > + * struct sev_data_snp_ext_config - system wide configuration value for SNP. > + * > + * @config_address: address of the struct sev_user_data_snp_config or 0 when > + * reported_tcb does not need to be updated. > + * @certs_address: address of extended guest request certificate chain or > + * 0 when previous certificate should be removed on SNP_SET_EXT_CONFIG. > + * @certs_len: length of the certs > + */ > +struct sev_user_data_ext_snp_config { > + __u64 config_address; /* In */ > + __u64 certs_address; /* In */ > + __u32 certs_len; /* In */ > +};
On 9/1/21 4:02 PM, Connor Kuehl wrote: > On 8/20/21 10:58 AM, Brijesh Singh wrote: >> +2.4 SNP_SET_EXT_CONFIG >> +---------------------- >> +:Technology: sev-snp >> +:Type: hypervisor ioctl cmd >> +:Parameters (in): struct sev_data_snp_ext_config >> +:Returns (out): 0 on success, -negative on error >> + >> +The SNP_SET_EXT_CONFIG is used to set the system-wide configuration such as >> +reported TCB version in the attestation report. The command is similar to >> +SNP_CONFIG command defined in the SEV-SNP spec. The main difference is the >> +command also accepts an additional certificate blob defined in the GHCB >> +specification. >> + >> +If the certs_address is zero, then previous certificate blob will deleted. >> +For more information on the certificate blob layout, see the GHCB spec >> +(extended guest request message). > Hi Brijesh, > > Just to be clear, is the documentation you're referring to regarding the > layout of the certificate blob specified on page 47 of the GHCB spec? > More specifically, is it the `struct cert_table` on that page? Yes that is correct. > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fwp-content%2Fresources%2F56421.pdf&data=04%7C01%7Cbrijesh.singh%40amd.com%7C62df2fe1cb384de88ed708d96d8bda20%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637661270135555480%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=V4S8atM%2BTlZ%2BiIlddRjpTNIx4yecGEoETuFVjeNWWNQ%3D&reserved=0 > > If so, where is the VCEK certificate layout documented? You can get the VCEK from the KDS using the chip id. The certificate is standard X.509. thanks > > Connor > >> +/** >> + * struct sev_data_snp_ext_config - system wide configuration value for SNP. >> + * >> + * @config_address: address of the struct sev_user_data_snp_config or 0 when >> + * reported_tcb does not need to be updated. >> + * @certs_address: address of extended guest request certificate chain or >> + * 0 when previous certificate should be removed on SNP_SET_EXT_CONFIG. >> + * @certs_len: length of the certs >> + */ >> +struct sev_user_data_ext_snp_config { >> + __u64 config_address; /* In */ >> + __u64 certs_address; /* In */ >> + __u32 certs_len; /* In */ >> +};
` On Fri, Aug 20, 2021 at 9:00 AM Brijesh Singh <brijesh.singh@amd.com> wrote: > > The SEV-SNP firmware provides the SNP_CONFIG command used to set the > system-wide configuration value for SNP guests. The information includes > the TCB version string to be reported in guest attestation reports. > > Version 2 of the GHCB specification adds an NAE (SNP extended guest > request) that a guest can use to query the reports that include additional > certificates. > > In both cases, userspace provided additional data is included in the > attestation reports. The userspace will use the SNP_SET_EXT_CONFIG > command to give the certificate blob and the reported TCB version string > at once. Note that the specification defines certificate blob with a > specific GUID format; the userspace is responsible for building the > proper certificate blob. The ioctl treats it an opaque blob. > > While it is not defined in the spec, but let's add SNP_GET_EXT_CONFIG > command that can be used to obtain the data programmed through the > SNP_SET_EXT_CONFIG. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > Documentation/virt/coco/sevguest.rst | 28 +++++++ > drivers/crypto/ccp/sev-dev.c | 115 +++++++++++++++++++++++++++ > drivers/crypto/ccp/sev-dev.h | 3 + > include/uapi/linux/psp-sev.h | 17 ++++ > 4 files changed, 163 insertions(+) > > diff --git a/Documentation/virt/coco/sevguest.rst b/Documentation/virt/coco/sevguest.rst > index 7c51da010039..64a1b5167b33 100644 > --- a/Documentation/virt/coco/sevguest.rst > +++ b/Documentation/virt/coco/sevguest.rst > @@ -134,3 +134,31 @@ See GHCB specification for further detail on how to parse the certificate blob. > The SNP_PLATFORM_STATUS command is used to query the SNP platform status. The > status includes API major, minor version and more. See the SEV-SNP > specification for further details. > + > +2.4 SNP_SET_EXT_CONFIG > +---------------------- > +:Technology: sev-snp > +:Type: hypervisor ioctl cmd > +:Parameters (in): struct sev_data_snp_ext_config > +:Returns (out): 0 on success, -negative on error > + > +The SNP_SET_EXT_CONFIG is used to set the system-wide configuration such as > +reported TCB version in the attestation report. The command is similar to > +SNP_CONFIG command defined in the SEV-SNP spec. The main difference is the > +command also accepts an additional certificate blob defined in the GHCB > +specification. > + > +If the certs_address is zero, then previous certificate blob will deleted. > +For more information on the certificate blob layout, see the GHCB spec > +(extended guest request message). > + > + > +2.4 SNP_GET_EXT_CONFIG > +---------------------- > +:Technology: sev-snp > +:Type: hypervisor ioctl cmd > +:Parameters (in): struct sev_data_snp_ext_config > +:Returns (out): 0 on success, -negative on error > + > +The SNP_SET_EXT_CONFIG is used to query the system-wide configuration set > +through the SNP_SET_EXT_CONFIG. > diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c > index 16c6df5d412c..9ba194acbe85 100644 > --- a/drivers/crypto/ccp/sev-dev.c > +++ b/drivers/crypto/ccp/sev-dev.c > @@ -1132,6 +1132,10 @@ static int __sev_snp_shutdown_locked(int *error) > if (!sev->snp_inited) > return 0; > > + /* Free the memory used for caching the certificate data */ > + kfree(sev->snp_certs_data); > + sev->snp_certs_data = NULL; > + > /* SHUTDOWN requires the DF_FLUSH */ > wbinvd_on_all_cpus(); > __sev_do_cmd_locked(SEV_CMD_SNP_DF_FLUSH, NULL, NULL); > @@ -1436,6 +1440,111 @@ static int sev_ioctl_snp_platform_status(struct sev_issue_cmd *argp) > return ret; > } > > +static int sev_ioctl_snp_get_config(struct sev_issue_cmd *argp) > +{ > + struct sev_device *sev = psp_master->sev_data; > + struct sev_user_data_ext_snp_config input; > + int ret; > + > + if (!sev->snp_inited || !argp->data) > + return -EINVAL; > + > + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) > + return -EFAULT; > + > + /* Copy the TCB version programmed through the SET_CONFIG to userspace */ > + if (input.config_address) { > + if (copy_to_user((void * __user)input.config_address, > + &sev->snp_config, sizeof(struct sev_user_data_snp_config))) > + return -EFAULT; > + } > + > + /* Copy the extended certs programmed through the SNP_SET_CONFIG */ > + if (input.certs_address && sev->snp_certs_data) { > + if (input.certs_len < sev->snp_certs_len) { > + /* Return the certs length to userspace */ > + input.certs_len = sev->snp_certs_len; This API to retrieve the length of the certs seems pretty odd. We only return the length if the input.certs_address is non-NULL. But if we know the length how did we allocate an address to write to `input.certs_address`? > + > + ret = -ENOSR; > + goto e_done; > + } > + > + if (copy_to_user((void * __user)input.certs_address, > + sev->snp_certs_data, sev->snp_certs_len)) > + return -EFAULT; > + } > + > + ret = 0; > + > +e_done: > + if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) > + ret = -EFAULT; > + > + return ret; > +} > + > +static int sev_ioctl_snp_set_config(struct sev_issue_cmd *argp, bool writable) > +{ > + struct sev_device *sev = psp_master->sev_data; > + struct sev_user_data_ext_snp_config input; > + struct sev_user_data_snp_config config; > + void *certs = NULL; > + int ret = 0; > + > + if (!sev->snp_inited || !argp->data) > + return -EINVAL; > + > + if (!writable) > + return -EPERM; > + > + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) > + return -EFAULT; > + > + /* Copy the certs from userspace */ > + if (input.certs_address) { > + if (!input.certs_len || !IS_ALIGNED(input.certs_len, PAGE_SIZE)) > + return -EINVAL; > + > + certs = psp_copy_user_blob(input.certs_address, input.certs_len); Is `psp_copy_user_blob()` implemented in this patch series? When I searched through the patches, I only found an implementation that always returns an error. But maybe I missed the implementation? Also, out of curiosity, any reason we cannot use copy_from_user here? > + if (IS_ERR(certs)) > + return PTR_ERR(certs); > + } > + > + /* Issue the PSP command to update the TCB version using the SNP_CONFIG. */ > + if (input.config_address) { > + if (copy_from_user(&config, > + (void __user *)input.config_address, sizeof(config))) { > + ret = -EFAULT; > + goto e_free; > + } > + > + ret = __sev_do_cmd_locked(SEV_CMD_SNP_CONFIG, &config, &argp->error); > + if (ret) > + goto e_free; > + > + memcpy(&sev->snp_config, &config, sizeof(config)); > + } > + > + /* > + * If the new certs are passed then cache it else free the old certs. > + */ > + if (certs) { > + kfree(sev->snp_certs_data); > + sev->snp_certs_data = certs; > + sev->snp_certs_len = input.certs_len; > + } else { > + kfree(sev->snp_certs_data); > + sev->snp_certs_data = NULL; > + sev->snp_certs_len = 0; > + } > + > + return 0; > + > +e_free: > + kfree(certs); > + return ret; > +} > + > static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) > { > void __user *argp = (void __user *)arg; > @@ -1490,6 +1599,12 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) > case SNP_PLATFORM_STATUS: > ret = sev_ioctl_snp_platform_status(&input); > break; > + case SNP_SET_EXT_CONFIG: > + ret = sev_ioctl_snp_set_config(&input, writable); > + break; > + case SNP_GET_EXT_CONFIG: > + ret = sev_ioctl_snp_get_config(&input); > + break; What is the intended use of `SNP_GET_EXT_CONFIG`. Yes, I get that it returns the "EXT config" previously set via `SNP_SET_EXT_CONFIG`. But presumably the caller can keep track of what it's previously passed to `SNP_SET_EXT_CONFIG`. Does it really need to call into the kernel to get these certs? > default: > ret = -EINVAL; > goto out; > diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h > index fe5d7a3ebace..d2fe1706311a 100644 > --- a/drivers/crypto/ccp/sev-dev.h > +++ b/drivers/crypto/ccp/sev-dev.h > @@ -66,6 +66,9 @@ struct sev_device { > > bool snp_inited; > struct snp_host_map snp_host_map[MAX_SNP_HOST_MAP_BUFS]; > + void *snp_certs_data; > + u32 snp_certs_len; > + struct sev_user_data_snp_config snp_config; > }; > > int sev_dev_init(struct psp_device *psp); > diff --git a/include/uapi/linux/psp-sev.h b/include/uapi/linux/psp-sev.h > index ffd60e8b0a31..60e7a8d1a18e 100644 > --- a/include/uapi/linux/psp-sev.h > +++ b/include/uapi/linux/psp-sev.h > @@ -29,6 +29,8 @@ enum { > SEV_GET_ID, /* This command is deprecated, use SEV_GET_ID2 */ > SEV_GET_ID2, > SNP_PLATFORM_STATUS, > + SNP_SET_EXT_CONFIG, > + SNP_GET_EXT_CONFIG, > > SEV_MAX, > }; > @@ -190,6 +192,21 @@ struct sev_user_data_snp_config { > __u8 rsvd[52]; > } __packed; > > +/** > + * struct sev_data_snp_ext_config - system wide configuration value for SNP. > + * > + * @config_address: address of the struct sev_user_data_snp_config or 0 when > + * reported_tcb does not need to be updated. > + * @certs_address: address of extended guest request certificate chain or > + * 0 when previous certificate should be removed on SNP_SET_EXT_CONFIG. > + * @certs_len: length of the certs > + */ > +struct sev_user_data_ext_snp_config { > + __u64 config_address; /* In */ > + __u64 certs_address; /* In */ > + __u32 certs_len; /* In */ > +}; > + > /** > * struct sev_issue_cmd - SEV ioctl parameters > * > -- > 2.17.1 >
On 9/9/21 10:27 PM, Marc Orr wrote: > ` > > On Fri, Aug 20, 2021 at 9:00 AM Brijesh Singh <brijesh.singh@amd.com> wrote: >> The SEV-SNP firmware provides the SNP_CONFIG command used to set the >> system-wide configuration value for SNP guests. The information includes >> the TCB version string to be reported in guest attestation reports. >> >> Version 2 of the GHCB specification adds an NAE (SNP extended guest >> request) that a guest can use to query the reports that include additional >> certificates. >> >> In both cases, userspace provided additional data is included in the >> attestation reports. The userspace will use the SNP_SET_EXT_CONFIG >> command to give the certificate blob and the reported TCB version string >> at once. Note that the specification defines certificate blob with a >> specific GUID format; the userspace is responsible for building the >> proper certificate blob. The ioctl treats it an opaque blob. >> >> While it is not defined in the spec, but let's add SNP_GET_EXT_CONFIG >> command that can be used to obtain the data programmed through the >> SNP_SET_EXT_CONFIG. >> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> Documentation/virt/coco/sevguest.rst | 28 +++++++ >> drivers/crypto/ccp/sev-dev.c | 115 +++++++++++++++++++++++++++ >> drivers/crypto/ccp/sev-dev.h | 3 + >> include/uapi/linux/psp-sev.h | 17 ++++ >> 4 files changed, 163 insertions(+) >> >> diff --git a/Documentation/virt/coco/sevguest.rst b/Documentation/virt/coco/sevguest.rst >> index 7c51da010039..64a1b5167b33 100644 >> --- a/Documentation/virt/coco/sevguest.rst >> +++ b/Documentation/virt/coco/sevguest.rst >> @@ -134,3 +134,31 @@ See GHCB specification for further detail on how to parse the certificate blob. >> The SNP_PLATFORM_STATUS command is used to query the SNP platform status. The >> status includes API major, minor version and more. See the SEV-SNP >> specification for further details. >> + >> +2.4 SNP_SET_EXT_CONFIG >> +---------------------- >> +:Technology: sev-snp >> +:Type: hypervisor ioctl cmd >> +:Parameters (in): struct sev_data_snp_ext_config >> +:Returns (out): 0 on success, -negative on error >> + >> +The SNP_SET_EXT_CONFIG is used to set the system-wide configuration such as >> +reported TCB version in the attestation report. The command is similar to >> +SNP_CONFIG command defined in the SEV-SNP spec. The main difference is the >> +command also accepts an additional certificate blob defined in the GHCB >> +specification. >> + >> +If the certs_address is zero, then previous certificate blob will deleted. >> +For more information on the certificate blob layout, see the GHCB spec >> +(extended guest request message). >> + >> + >> +2.4 SNP_GET_EXT_CONFIG >> +---------------------- >> +:Technology: sev-snp >> +:Type: hypervisor ioctl cmd >> +:Parameters (in): struct sev_data_snp_ext_config >> +:Returns (out): 0 on success, -negative on error >> + >> +The SNP_SET_EXT_CONFIG is used to query the system-wide configuration set >> +through the SNP_SET_EXT_CONFIG. >> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c >> index 16c6df5d412c..9ba194acbe85 100644 >> --- a/drivers/crypto/ccp/sev-dev.c >> +++ b/drivers/crypto/ccp/sev-dev.c >> @@ -1132,6 +1132,10 @@ static int __sev_snp_shutdown_locked(int *error) >> if (!sev->snp_inited) >> return 0; >> >> + /* Free the memory used for caching the certificate data */ >> + kfree(sev->snp_certs_data); >> + sev->snp_certs_data = NULL; >> + >> /* SHUTDOWN requires the DF_FLUSH */ >> wbinvd_on_all_cpus(); >> __sev_do_cmd_locked(SEV_CMD_SNP_DF_FLUSH, NULL, NULL); >> @@ -1436,6 +1440,111 @@ static int sev_ioctl_snp_platform_status(struct sev_issue_cmd *argp) >> return ret; >> } >> >> +static int sev_ioctl_snp_get_config(struct sev_issue_cmd *argp) >> +{ >> + struct sev_device *sev = psp_master->sev_data; >> + struct sev_user_data_ext_snp_config input; >> + int ret; >> + >> + if (!sev->snp_inited || !argp->data) >> + return -EINVAL; >> + >> + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) >> + return -EFAULT; >> + >> + /* Copy the TCB version programmed through the SET_CONFIG to userspace */ >> + if (input.config_address) { >> + if (copy_to_user((void * __user)input.config_address, >> + &sev->snp_config, sizeof(struct sev_user_data_snp_config))) >> + return -EFAULT; >> + } >> + >> + /* Copy the extended certs programmed through the SNP_SET_CONFIG */ >> + if (input.certs_address && sev->snp_certs_data) { >> + if (input.certs_len < sev->snp_certs_len) { >> + /* Return the certs length to userspace */ >> + input.certs_len = sev->snp_certs_len; > This API to retrieve the length of the certs seems pretty odd. We only > return the length if the input.certs_address is non-NULL. But if we > know the length how did we allocate an address to write to > `input.certs_address`? Ah good point, I should provide an option to query the length when input.cert_address == 0. This will make it much cleaner that there are two approaches to get the length. >> + >> + ret = -ENOSR; >> + goto e_done; >> + } >> + >> + if (copy_to_user((void * __user)input.certs_address, >> + sev->snp_certs_data, sev->snp_certs_len)) >> + return -EFAULT; >> + } >> + >> + ret = 0; >> + >> +e_done: >> + if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) >> + ret = -EFAULT; >> + >> + return ret; >> +} >> + >> +static int sev_ioctl_snp_set_config(struct sev_issue_cmd *argp, bool writable) >> +{ >> + struct sev_device *sev = psp_master->sev_data; >> + struct sev_user_data_ext_snp_config input; >> + struct sev_user_data_snp_config config; >> + void *certs = NULL; >> + int ret = 0; >> + >> + if (!sev->snp_inited || !argp->data) >> + return -EINVAL; >> + >> + if (!writable) >> + return -EPERM; >> + >> + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) >> + return -EFAULT; >> + >> + /* Copy the certs from userspace */ >> + if (input.certs_address) { >> + if (!input.certs_len || !IS_ALIGNED(input.certs_len, PAGE_SIZE)) >> + return -EINVAL; >> + >> + certs = psp_copy_user_blob(input.certs_address, input.certs_len); > Is `psp_copy_user_blob()` implemented in this patch series? When I > searched through the patches, I only found an implementation that > always returns an error. But maybe I missed the implementation? > > Also, out of curiosity, any reason we cannot use copy_from_user here? psp_copy_user_blob() is a wrapper around memdup_user() -- which does kmalloc() + copy_to_user(). The wrapper performs some additional checks such as the blob size etc, we limit the blob size to 16K to avoid copying a random large data from userspace. >> + if (IS_ERR(certs)) >> + return PTR_ERR(certs); >> + } >> + >> + /* Issue the PSP command to update the TCB version using the SNP_CONFIG. */ >> + if (input.config_address) { >> + if (copy_from_user(&config, >> + (void __user *)input.config_address, sizeof(config))) { >> + ret = -EFAULT; >> + goto e_free; >> + } >> + >> + ret = __sev_do_cmd_locked(SEV_CMD_SNP_CONFIG, &config, &argp->error); >> + if (ret) >> + goto e_free; >> + >> + memcpy(&sev->snp_config, &config, sizeof(config)); >> + } >> + >> + /* >> + * If the new certs are passed then cache it else free the old certs. >> + */ >> + if (certs) { >> + kfree(sev->snp_certs_data); >> + sev->snp_certs_data = certs; >> + sev->snp_certs_len = input.certs_len; >> + } else { >> + kfree(sev->snp_certs_data); >> + sev->snp_certs_data = NULL; >> + sev->snp_certs_len = 0; >> + } >> + >> + return 0; >> + >> +e_free: >> + kfree(certs); >> + return ret; >> +} >> + >> static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) >> { >> void __user *argp = (void __user *)arg; >> @@ -1490,6 +1599,12 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) >> case SNP_PLATFORM_STATUS: >> ret = sev_ioctl_snp_platform_status(&input); >> break; >> + case SNP_SET_EXT_CONFIG: >> + ret = sev_ioctl_snp_set_config(&input, writable); >> + break; >> + case SNP_GET_EXT_CONFIG: >> + ret = sev_ioctl_snp_get_config(&input); >> + break; > What is the intended use of `SNP_GET_EXT_CONFIG`. Yes, I get that it > returns the "EXT config" previously set via `SNP_SET_EXT_CONFIG`. But > presumably the caller can keep track of what it's previously passed to > `SNP_SET_EXT_CONFIG`. Does it really need to call into the kernel to > get these certs? This is mainly to help in the cases where the provisioning tools may not keep track of the programmed TCB version and would prefer to read the TCB version before updating.
diff --git a/Documentation/virt/coco/sevguest.rst b/Documentation/virt/coco/sevguest.rst index 7c51da010039..64a1b5167b33 100644 --- a/Documentation/virt/coco/sevguest.rst +++ b/Documentation/virt/coco/sevguest.rst @@ -134,3 +134,31 @@ See GHCB specification for further detail on how to parse the certificate blob. The SNP_PLATFORM_STATUS command is used to query the SNP platform status. The status includes API major, minor version and more. See the SEV-SNP specification for further details. + +2.4 SNP_SET_EXT_CONFIG +---------------------- +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Parameters (in): struct sev_data_snp_ext_config +:Returns (out): 0 on success, -negative on error + +The SNP_SET_EXT_CONFIG is used to set the system-wide configuration such as +reported TCB version in the attestation report. The command is similar to +SNP_CONFIG command defined in the SEV-SNP spec. The main difference is the +command also accepts an additional certificate blob defined in the GHCB +specification. + +If the certs_address is zero, then previous certificate blob will deleted. +For more information on the certificate blob layout, see the GHCB spec +(extended guest request message). + + +2.4 SNP_GET_EXT_CONFIG +---------------------- +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Parameters (in): struct sev_data_snp_ext_config +:Returns (out): 0 on success, -negative on error + +The SNP_SET_EXT_CONFIG is used to query the system-wide configuration set +through the SNP_SET_EXT_CONFIG. diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index 16c6df5d412c..9ba194acbe85 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -1132,6 +1132,10 @@ static int __sev_snp_shutdown_locked(int *error) if (!sev->snp_inited) return 0; + /* Free the memory used for caching the certificate data */ + kfree(sev->snp_certs_data); + sev->snp_certs_data = NULL; + /* SHUTDOWN requires the DF_FLUSH */ wbinvd_on_all_cpus(); __sev_do_cmd_locked(SEV_CMD_SNP_DF_FLUSH, NULL, NULL); @@ -1436,6 +1440,111 @@ static int sev_ioctl_snp_platform_status(struct sev_issue_cmd *argp) return ret; } +static int sev_ioctl_snp_get_config(struct sev_issue_cmd *argp) +{ + struct sev_device *sev = psp_master->sev_data; + struct sev_user_data_ext_snp_config input; + int ret; + + if (!sev->snp_inited || !argp->data) + return -EINVAL; + + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) + return -EFAULT; + + /* Copy the TCB version programmed through the SET_CONFIG to userspace */ + if (input.config_address) { + if (copy_to_user((void * __user)input.config_address, + &sev->snp_config, sizeof(struct sev_user_data_snp_config))) + return -EFAULT; + } + + /* Copy the extended certs programmed through the SNP_SET_CONFIG */ + if (input.certs_address && sev->snp_certs_data) { + if (input.certs_len < sev->snp_certs_len) { + /* Return the certs length to userspace */ + input.certs_len = sev->snp_certs_len; + + ret = -ENOSR; + goto e_done; + } + + if (copy_to_user((void * __user)input.certs_address, + sev->snp_certs_data, sev->snp_certs_len)) + return -EFAULT; + } + + ret = 0; + +e_done: + if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) + ret = -EFAULT; + + return ret; +} + +static int sev_ioctl_snp_set_config(struct sev_issue_cmd *argp, bool writable) +{ + struct sev_device *sev = psp_master->sev_data; + struct sev_user_data_ext_snp_config input; + struct sev_user_data_snp_config config; + void *certs = NULL; + int ret = 0; + + if (!sev->snp_inited || !argp->data) + return -EINVAL; + + if (!writable) + return -EPERM; + + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) + return -EFAULT; + + /* Copy the certs from userspace */ + if (input.certs_address) { + if (!input.certs_len || !IS_ALIGNED(input.certs_len, PAGE_SIZE)) + return -EINVAL; + + certs = psp_copy_user_blob(input.certs_address, input.certs_len); + if (IS_ERR(certs)) + return PTR_ERR(certs); + } + + /* Issue the PSP command to update the TCB version using the SNP_CONFIG. */ + if (input.config_address) { + if (copy_from_user(&config, + (void __user *)input.config_address, sizeof(config))) { + ret = -EFAULT; + goto e_free; + } + + ret = __sev_do_cmd_locked(SEV_CMD_SNP_CONFIG, &config, &argp->error); + if (ret) + goto e_free; + + memcpy(&sev->snp_config, &config, sizeof(config)); + } + + /* + * If the new certs are passed then cache it else free the old certs. + */ + if (certs) { + kfree(sev->snp_certs_data); + sev->snp_certs_data = certs; + sev->snp_certs_len = input.certs_len; + } else { + kfree(sev->snp_certs_data); + sev->snp_certs_data = NULL; + sev->snp_certs_len = 0; + } + + return 0; + +e_free: + kfree(certs); + return ret; +} + static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) { void __user *argp = (void __user *)arg; @@ -1490,6 +1599,12 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) case SNP_PLATFORM_STATUS: ret = sev_ioctl_snp_platform_status(&input); break; + case SNP_SET_EXT_CONFIG: + ret = sev_ioctl_snp_set_config(&input, writable); + break; + case SNP_GET_EXT_CONFIG: + ret = sev_ioctl_snp_get_config(&input); + break; default: ret = -EINVAL; goto out; diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h index fe5d7a3ebace..d2fe1706311a 100644 --- a/drivers/crypto/ccp/sev-dev.h +++ b/drivers/crypto/ccp/sev-dev.h @@ -66,6 +66,9 @@ struct sev_device { bool snp_inited; struct snp_host_map snp_host_map[MAX_SNP_HOST_MAP_BUFS]; + void *snp_certs_data; + u32 snp_certs_len; + struct sev_user_data_snp_config snp_config; }; int sev_dev_init(struct psp_device *psp); diff --git a/include/uapi/linux/psp-sev.h b/include/uapi/linux/psp-sev.h index ffd60e8b0a31..60e7a8d1a18e 100644 --- a/include/uapi/linux/psp-sev.h +++ b/include/uapi/linux/psp-sev.h @@ -29,6 +29,8 @@ enum { SEV_GET_ID, /* This command is deprecated, use SEV_GET_ID2 */ SEV_GET_ID2, SNP_PLATFORM_STATUS, + SNP_SET_EXT_CONFIG, + SNP_GET_EXT_CONFIG, SEV_MAX, }; @@ -190,6 +192,21 @@ struct sev_user_data_snp_config { __u8 rsvd[52]; } __packed; +/** + * struct sev_data_snp_ext_config - system wide configuration value for SNP. + * + * @config_address: address of the struct sev_user_data_snp_config or 0 when + * reported_tcb does not need to be updated. + * @certs_address: address of extended guest request certificate chain or + * 0 when previous certificate should be removed on SNP_SET_EXT_CONFIG. + * @certs_len: length of the certs + */ +struct sev_user_data_ext_snp_config { + __u64 config_address; /* In */ + __u64 certs_address; /* In */ + __u32 certs_len; /* In */ +}; + /** * struct sev_issue_cmd - SEV ioctl parameters *
The SEV-SNP firmware provides the SNP_CONFIG command used to set the system-wide configuration value for SNP guests. The information includes the TCB version string to be reported in guest attestation reports. Version 2 of the GHCB specification adds an NAE (SNP extended guest request) that a guest can use to query the reports that include additional certificates. In both cases, userspace provided additional data is included in the attestation reports. The userspace will use the SNP_SET_EXT_CONFIG command to give the certificate blob and the reported TCB version string at once. Note that the specification defines certificate blob with a specific GUID format; the userspace is responsible for building the proper certificate blob. The ioctl treats it an opaque blob. While it is not defined in the spec, but let's add SNP_GET_EXT_CONFIG command that can be used to obtain the data programmed through the SNP_SET_EXT_CONFIG. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- Documentation/virt/coco/sevguest.rst | 28 +++++++ drivers/crypto/ccp/sev-dev.c | 115 +++++++++++++++++++++++++++ drivers/crypto/ccp/sev-dev.h | 3 + include/uapi/linux/psp-sev.h | 17 ++++ 4 files changed, 163 insertions(+)