lkml.org 
[lkml]   [2023]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH RFC v8 27/56] crypto: ccp: Add the SNP_{SET,GET}_EXT_CONFIG command
    From
    On 2/23/23 00:38, Zhi Wang wrote:
    > On Wed, 22 Feb 2023 16:43:54 -0600
    > "Kalra, Ashish" <ashish.kalra@amd.com> wrote:
    >
    >> On 2/22/2023 6:32 AM, Zhi Wang wrote:
    >>> On Mon, 20 Feb 2023 12:38:18 -0600
    >>> Michael Roth <michael.roth@amd.com> wrote:
    >>>
    >>>> From: Brijesh Singh <brijesh.singh@amd.com>
    >>>>
    >>>> 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>
    >>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
    >>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
    >>>> ---
    >>>> Documentation/virt/coco/sev-guest.rst | 27 ++++++
    >>>> drivers/crypto/ccp/sev-dev.c | 123 ++++++++++++++++++++++++++
    >>>> drivers/crypto/ccp/sev-dev.h | 4 +
    >>>> include/uapi/linux/psp-sev.h | 17 ++++
    >>>> 4 files changed, 171 insertions(+)
    >>>>
    >>>> diff --git a/Documentation/virt/coco/sev-guest.rst b/Documentation/virt/coco/sev-guest.rst
    >>>> index 11ea67c944df..6cad4226c348 100644
    >>>> --- a/Documentation/virt/coco/sev-guest.rst
    >>>> +++ b/Documentation/virt/coco/sev-guest.rst
    >>>> @@ -145,6 +145,33 @@ 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.5 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 the previous certificate blob will deleted.
    >>>> +For more information on the certificate blob layout, see the GHCB spec
    >>>> +(extended guest request message).
    >>>> +
    >>>> +2.6 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_GET_EXT_CONFIG is used to query the system-wide configuration set
    >>>> +through the SNP_SET_EXT_CONFIG.
    >>>> +
    >>>> 3. SEV-SNP CPUID Enforcement
    >>>> ============================
    >>>>
    >>>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
    >>>> index 65e13a562f3b..b56b00ca2cd4 100644
    >>>> --- a/drivers/crypto/ccp/sev-dev.c
    >>>> +++ b/drivers/crypto/ccp/sev-dev.c
    >>>> @@ -1481,6 +1481,10 @@ static int __sev_snp_shutdown_locked(int *error)
    >>>> data.length = sizeof(data);
    >>>> data.iommu_snp_shutdown = 1;
    >>>>
    >>>> + /* Free the memory used for caching the certificate data */
    >>>> + kfree(sev->snp_certs_data);
    >>>> + sev->snp_certs_data = NULL;
    >>>> +
    >>>> wbinvd_on_all_cpus();
    >>>>
    >>>> retry:
    >>>> @@ -1793,6 +1797,118 @@ 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_initialized || !argp->data)
    >>>> + return -EINVAL;
    >>>> +
    >>>> + memset(&input, 0, sizeof(input));
    >>>> +
    >>>> + 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;
    >>>> + }
    >>>> +
    >>>
    >>> What about if input.certs_len > sev->snp_certs_len? Is it possbile for the
    >>> userspace to know the length of data in the buffer? (I guess it might be able
    >>> to know the certs len through the blob data, but a comment here would be nice)
    >>>
    >>
    >> If userspace provides an input buffer/length smaller then snp_certs_len,
    >> then the above returns the "required" certs length back to userspace.
    >>
    >> And what is the issue if input.certs_len > sev->snp_certs_len, the
    >> buffer returned back to userspace is sev->snp_certs_len as below.
    >>
    >
    > My point is: How can the userspace know the length of return data is shorter
    > than input.certs_len when input.certs_len > sev->snp_serts_len? as the length
    > is only returned when input.certs_len < sev->snp_certs_len.

    The returned data has a defined format that can be used to calculate the
    overall length.

    Thanks,
    Tom

    >
    >> Thanks,
    >> Ashish
    >>
    >>>> + 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_initialized || !argp->data)
    >>>> + return -EINVAL;
    >>>> +
    >>>> + if (!writable)
    >>>> + return -EPERM;
    >>>> +
    >>>> + memset(&input, 0, sizeof(input));
    >>>> +
    >>>> + 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) {
    >>>> + memset(&config, 0, sizeof(config));
    >>>> + 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.
    >>>> + */
    >>>> + mutex_lock(&sev->snp_certs_lock);
    >>>> + 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;
    >>>> + }
    >>>> + mutex_unlock(&sev->snp_certs_lock);
    >>>> +
    >>>> + 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;
    >>>> @@ -1847,6 +1963,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;
    >>>> @@ -1962,6 +2084,7 @@ int sev_dev_init(struct psp_device *psp)
    >>>> goto e_sev;
    >>>>
    >>>> sev->cmd_buf_backup = (uint8_t *)sev->cmd_buf + PAGE_SIZE;
    >>>> + mutex_init(&sev->snp_certs_lock);
    >>>>
    >>>> psp->sev_data = sev;
    >>>>
    >>>> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
    >>>> index 19d79f9d4212..41d5353d5bab 100644
    >>>> --- a/drivers/crypto/ccp/sev-dev.h
    >>>> +++ b/drivers/crypto/ccp/sev-dev.h
    >>>> @@ -66,6 +66,10 @@ struct sev_device {
    >>>>
    >>>> bool snp_initialized;
    >>>> struct snp_host_map snp_host_map[MAX_SNP_HOST_MAP_BUFS];
    >>>> + void *snp_certs_data;
    >>>> + u32 snp_certs_len;
    >>>> + struct mutex snp_certs_lock;
    >>>> + 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 5adfaea7df97..c20d37586d21 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,
    >>>> };
    >>>> @@ -192,6 +194,21 @@ struct sev_user_data_snp_config {
    >>>> __u8 rsvd1[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
    >>>> *
    >>>
    >

    \
     
     \ /
      Last update: 2023-03-27 00:33    [W:5.747 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site