[RFC,v8,27/56] crypto: ccp: Add the SNP_{SET,GET}_EXT_CONFIG command

  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(+)
  

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 (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
>   *
  

On 2/22/23 06:32, 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/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;

We should be consistent with the other SEV ioctls that return required 
lengths and return -EIO here instead -ENOSR.

Thanks,
Tom

>> +			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 (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;
>> +}
  

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.

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
>>    *
>
  

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.

> 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
> >>    *
> >
  

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
>>>>     *
>>>
>