diff mbox series

[RFC,v8,52/56] ccp: Add support to decrypt the page

Message ID 20230220183847.59159-53-michael.roth@amd.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support | expand

Commit Message

Michael Roth Feb. 20, 2023, 6:38 p.m. UTC
From: Brijesh Singh <brijesh.singh@amd.com>

Add support to decrypt guest encrypted memory. These API interfaces can
be used for example to dump VMCBs on SNP guest exit.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
[mdr: minor commit fixups]
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 32 ++++++++++++++++++++++++++++++++
 include/linux/psp-sev.h      | 22 ++++++++++++++++++++--
 2 files changed, 52 insertions(+), 2 deletions(-)

Comments

Zhi Wang March 1, 2023, 9:20 p.m. UTC | #1
On Mon, 20 Feb 2023 12:38:43 -0600
Michael Roth <michael.roth@amd.com> wrote:

> From: Brijesh Singh <brijesh.singh@amd.com>
> 
> Add support to decrypt guest encrypted memory. These API interfaces can
> be used for example to dump VMCBs on SNP guest exit.
> 

What kinds of check will be applied from firmware when VMM decrypts this
page? I suppose there has to be kinda mechanism to prevent VMM to decrypt
any page in the guest. It would be nice to have some introduction about
it in the comments.

> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> [mdr: minor commit fixups]
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>  drivers/crypto/ccp/sev-dev.c | 32 ++++++++++++++++++++++++++++++++
>  include/linux/psp-sev.h      | 22 ++++++++++++++++++++--
>  2 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index e65563bc8298..bf5167b2acfc 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -2017,6 +2017,38 @@ int sev_guest_df_flush(int *error)
>  }
>  EXPORT_SYMBOL_GPL(sev_guest_df_flush);
>  
> +int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error)
> +{
> +	struct sev_data_snp_dbg data = {0};
> +	struct sev_device *sev;
> +	int ret;
> +
> +	if (!psp_master || !psp_master->sev_data)
> +		return -ENODEV;
> +
> +	sev = psp_master->sev_data;
> +
> +	if (!sev->snp_initialized)
> +		return -EINVAL;
> +
> +	data.gctx_paddr = sme_me_mask | (gctx_pfn << PAGE_SHIFT);
> +	data.src_addr = sme_me_mask | (src_pfn << PAGE_SHIFT);
> +	data.dst_addr = sme_me_mask | (dst_pfn << PAGE_SHIFT);
> +
> +	/* The destination page must be in the firmware state. */
> +	if (rmp_mark_pages_firmware(data.dst_addr, 1, false))
> +		return -EIO;
> +
> +	ret = sev_do_cmd(SEV_CMD_SNP_DBG_DECRYPT, &data, error);
> +
> +	/* Restore the page state */
> +	if (snp_reclaim_pages(data.dst_addr, 1, false))
> +		ret = -EIO;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(snp_guest_dbg_decrypt_page);
> +
>  int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
>  				unsigned long vaddr, unsigned long *npages, unsigned long *fw_err)
>  {
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 81bafc049eca..92116e2b74fd 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -710,7 +710,6 @@ struct sev_data_snp_dbg {
>  	u64 gctx_paddr;				/* In */
>  	u64 src_addr;				/* In */
>  	u64 dst_addr;				/* In */
> -	u32 len;				/* In */
>  } __packed;
>  
>  /**
> @@ -913,13 +912,27 @@ int sev_guest_decommission(struct sev_data_decommission *data, int *error);
>   * @error: SEV command return code
>   *
>   * Returns:
> + * 0 if the sev successfully processed the command
> + * -%ENODEV    if the sev device is not available
> + * -%ENOTSUPP  if the sev does not support SEV
> + * -%ETIMEDOUT if the sev command timed out
> + * -%EIO       if the sev returned a non-zero return code
> + */
> +int sev_do_cmd(int cmd, void *data, int *psp_ret);
> +
> +/**
> + * snp_guest_dbg_decrypt_page - perform SEV SNP_DBG_DECRYPT command
> + *
> + * @sev_ret: sev command return code
> + *
> + * Returns:
>   * 0 if the SEV successfully processed the command
>   * -%ENODEV    if the SEV device is not available
>   * -%ENOTSUPP  if the SEV does not support SEV
>   * -%ETIMEDOUT if the SEV command timed out
>   * -%EIO       if the SEV returned a non-zero return code
>   */
> -int sev_do_cmd(int cmd, void *data, int *psp_ret);
> +int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error);
>  
>  void *psp_copy_user_blob(u64 uaddr, u32 len);
>  void *snp_alloc_firmware_page(gfp_t mask);
> @@ -987,6 +1000,11 @@ static inline void *psp_copy_user_blob(u64 __user uaddr, u32 len) { return ERR_P
>  
>  void snp_mark_pages_offline(unsigned long pfn, unsigned int npages) {}
>  
> +static inline int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error)
> +{
> +	return -ENODEV;
> +}
> +
>  static inline void *snp_alloc_firmware_page(gfp_t mask)
>  {
>  	return NULL;
Dov Murik March 2, 2023, 5:59 a.m. UTC | #2
Hi Mike, Zhi,

On 01/03/2023 23:20, Zhi Wang wrote:
> On Mon, 20 Feb 2023 12:38:43 -0600
> Michael Roth <michael.roth@amd.com> wrote:
> 
>> From: Brijesh Singh <brijesh.singh@amd.com>
>>
>> Add support to decrypt guest encrypted memory. These API interfaces can
>> be used for example to dump VMCBs on SNP guest exit.
>>
> 
> What kinds of check will be applied from firmware when VMM decrypts this
> page? I suppose there has to be kinda mechanism to prevent VMM to decrypt
> any page in the guest. It would be nice to have some introduction about
> it in the comments.
> 

The SNP ABI spec says (section 8.27.2 SNP_DBG_DECRYPT):

  The firmware checks that the guest's policy allows debugging. If not,
  the firmware returns POLICY_FAILURE.

and in the Guest Policy (section 4.3):

  Bit 19 - DEBUG
  0: Debugging is disallowed.
  1: Debugging is allowed.

In the kernel, that firmware error code is defined as
SEV_RET_POLICY_FAILURE.


>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> [mdr: minor commit fixups]
>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>> ---
>>  drivers/crypto/ccp/sev-dev.c | 32 ++++++++++++++++++++++++++++++++
>>  include/linux/psp-sev.h      | 22 ++++++++++++++++++++--
>>  2 files changed, 52 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index e65563bc8298..bf5167b2acfc 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -2017,6 +2017,38 @@ int sev_guest_df_flush(int *error)
>>  }
>>  EXPORT_SYMBOL_GPL(sev_guest_df_flush);
>>  
>> +int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error)
>> +{
>> +	struct sev_data_snp_dbg data = {0};
>> +	struct sev_device *sev;
>> +	int ret;
>> +
>> +	if (!psp_master || !psp_master->sev_data)
>> +		return -ENODEV;
>> +
>> +	sev = psp_master->sev_data;
>> +
>> +	if (!sev->snp_initialized)
>> +		return -EINVAL;
>> +
>> +	data.gctx_paddr = sme_me_mask | (gctx_pfn << PAGE_SHIFT);
>> +	data.src_addr = sme_me_mask | (src_pfn << PAGE_SHIFT);
>> +	data.dst_addr = sme_me_mask | (dst_pfn << PAGE_SHIFT);

I guess this works, but I wonder why we need to turn on sme_me_mask on
teh dst_addr.  I thought that the firmware decrypts the guest page
(src_addr) to a plaintext page.  Couldn't find this requirement in the
SNP spec.


>> +
>> +	/* The destination page must be in the firmware state. */
>> +	if (rmp_mark_pages_firmware(data.dst_addr, 1, false))
>> +		return -EIO;
>> +
>> +	ret = sev_do_cmd(SEV_CMD_SNP_DBG_DECRYPT, &data, error);
>> +
>> +	/* Restore the page state */
>> +	if (snp_reclaim_pages(data.dst_addr, 1, false))
>> +		ret = -EIO;
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(snp_guest_dbg_decrypt_page);
>> +
>>  int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
>>  				unsigned long vaddr, unsigned long *npages, unsigned long *fw_err)
>>  {
>> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
>> index 81bafc049eca..92116e2b74fd 100644
>> --- a/include/linux/psp-sev.h
>> +++ b/include/linux/psp-sev.h
>> @@ -710,7 +710,6 @@ struct sev_data_snp_dbg {
>>  	u64 gctx_paddr;				/* In */
>>  	u64 src_addr;				/* In */
>>  	u64 dst_addr;				/* In */
>> -	u32 len;				/* In */
>>  } __packed;

The comment above this ^^^ struct still lists the 'len' field, and also
calls the first field 'handle' instead of 'gctx_paddr'.

Also - why is this change happening in this patch? Why was the incorrect
'len' field added in the first place in "[PATCH RFC v8 20/56]
crypto:ccp: Define the SEV-SNP commands" ? (the comment fixes should
probably go there too).



>>  
>>  /**
>> @@ -913,13 +912,27 @@ int sev_guest_decommission(struct sev_data_decommission *data, int *error);
>>   * @error: SEV command return code
>>   *
>>   * Returns:
>> + * 0 if the sev successfully processed the command
>> + * -%ENODEV    if the sev device is not available
>> + * -%ENOTSUPP  if the sev does not support SEV
>> + * -%ETIMEDOUT if the sev command timed out
>> + * -%EIO       if the sev returned a non-zero return code
>> + */

I think that if the word 'sev' would be 'SEV' in this comment, the diff
will be a bit less misleading (basically this patch should not introduce
changes to sev_do_cmd).

-Dov

>> +int sev_do_cmd(int cmd, void *data, int *psp_ret);
>> +
>> +/**
>> + * snp_guest_dbg_decrypt_page - perform SEV SNP_DBG_DECRYPT command
>> + *
>> + * @sev_ret: sev command return code
>> + *
>> + * Returns:
>>   * 0 if the SEV successfully processed the command
>>   * -%ENODEV    if the SEV device is not available
>>   * -%ENOTSUPP  if the SEV does not support SEV
>>   * -%ETIMEDOUT if the SEV command timed out
>>   * -%EIO       if the SEV returned a non-zero return code
>>   */
>> -int sev_do_cmd(int cmd, void *data, int *psp_ret);
>> +int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error);
>>  
>>  void *psp_copy_user_blob(u64 uaddr, u32 len);
>>  void *snp_alloc_firmware_page(gfp_t mask);
>> @@ -987,6 +1000,11 @@ static inline void *psp_copy_user_blob(u64 __user uaddr, u32 len) { return ERR_P
>>  
>>  void snp_mark_pages_offline(unsigned long pfn, unsigned int npages) {}
>>  
>> +static inline int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error)
>> +{
>> +	return -ENODEV;
>> +}
>> +
>>  static inline void *snp_alloc_firmware_page(gfp_t mask)
>>  {
>>  	return NULL;
>
Tom Lendacky March 2, 2023, 2:33 p.m. UTC | #3
On 3/1/23 23:59, Dov Murik wrote:
> Hi Mike, Zhi,
> 
> On 01/03/2023 23:20, Zhi Wang wrote:
>> On Mon, 20 Feb 2023 12:38:43 -0600
>> Michael Roth <michael.roth@amd.com> wrote:
>>
>>> From: Brijesh Singh <brijesh.singh@amd.com>
>>>
>>> Add support to decrypt guest encrypted memory. These API interfaces can
>>> be used for example to dump VMCBs on SNP guest exit.
>>>
>>
>> What kinds of check will be applied from firmware when VMM decrypts this
>> page? I suppose there has to be kinda mechanism to prevent VMM to decrypt
>> any page in the guest. It would be nice to have some introduction about
>> it in the comments.
>>
> 
> The SNP ABI spec says (section 8.27.2 SNP_DBG_DECRYPT):
> 
>    The firmware checks that the guest's policy allows debugging. If not,
>    the firmware returns POLICY_FAILURE.
> 
> and in the Guest Policy (section 4.3):
> 
>    Bit 19 - DEBUG
>    0: Debugging is disallowed.
>    1: Debugging is allowed.
> 
> In the kernel, that firmware error code is defined as
> SEV_RET_POLICY_FAILURE.
> 
> 
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>> [mdr: minor commit fixups]
>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>>> ---
>>>   drivers/crypto/ccp/sev-dev.c | 32 ++++++++++++++++++++++++++++++++
>>>   include/linux/psp-sev.h      | 22 ++++++++++++++++++++--
>>>   2 files changed, 52 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>>> index e65563bc8298..bf5167b2acfc 100644
>>> --- a/drivers/crypto/ccp/sev-dev.c
>>> +++ b/drivers/crypto/ccp/sev-dev.c
>>> @@ -2017,6 +2017,38 @@ int sev_guest_df_flush(int *error)
>>>   }
>>>   EXPORT_SYMBOL_GPL(sev_guest_df_flush);
>>>   
>>> +int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error)
>>> +{
>>> +	struct sev_data_snp_dbg data = {0};
>>> +	struct sev_device *sev;
>>> +	int ret;
>>> +
>>> +	if (!psp_master || !psp_master->sev_data)
>>> +		return -ENODEV;
>>> +
>>> +	sev = psp_master->sev_data;
>>> +
>>> +	if (!sev->snp_initialized)
>>> +		return -EINVAL;
>>> +
>>> +	data.gctx_paddr = sme_me_mask | (gctx_pfn << PAGE_SHIFT);
>>> +	data.src_addr = sme_me_mask | (src_pfn << PAGE_SHIFT);
>>> +	data.dst_addr = sme_me_mask | (dst_pfn << PAGE_SHIFT);
> 
> I guess this works, but I wonder why we need to turn on sme_me_mask on
> teh dst_addr.  I thought that the firmware decrypts the guest page
> (src_addr) to a plaintext page.  Couldn't find this requirement in the
> SNP spec.

This sme_me_mask tells the firmware how to access the host memory (similar 
to how DMA uses sme_me_mask when supplying addresses to devices under 
SME). This needs to match the pagetable mapping being used by the host, 
otherwise the contents will appears as ciphertext to the host if they are 
not in sync. Since the default pagetable mapping is encrypted, the 
sme_me_mask bit must be provided on the destination address. So it is not 
a spec requirement, but an SME implementation requirement.

Thanks,
Tom

> 
> 
>>> +
>>> +	/* The destination page must be in the firmware state. */
>>> +	if (rmp_mark_pages_firmware(data.dst_addr, 1, false))
>>> +		return -EIO;
>>> +
>>> +	ret = sev_do_cmd(SEV_CMD_SNP_DBG_DECRYPT, &data, error);
>>> +
>>> +	/* Restore the page state */
>>> +	if (snp_reclaim_pages(data.dst_addr, 1, false))
>>> +		ret = -EIO;
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(snp_guest_dbg_decrypt_page);
>>> +
>>>   int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
>>>   				unsigned long vaddr, unsigned long *npages, unsigned long *fw_err)
>>>   {
>>> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
>>> index 81bafc049eca..92116e2b74fd 100644
>>> --- a/include/linux/psp-sev.h
>>> +++ b/include/linux/psp-sev.h
>>> @@ -710,7 +710,6 @@ struct sev_data_snp_dbg {
>>>   	u64 gctx_paddr;				/* In */
>>>   	u64 src_addr;				/* In */
>>>   	u64 dst_addr;				/* In */
>>> -	u32 len;				/* In */
>>>   } __packed;
> 
> The comment above this ^^^ struct still lists the 'len' field, and also
> calls the first field 'handle' instead of 'gctx_paddr'.
> 
> Also - why is this change happening in this patch? Why was the incorrect
> 'len' field added in the first place in "[PATCH RFC v8 20/56]
> crypto:ccp: Define the SEV-SNP commands" ? (the comment fixes should
> probably go there too).
> 
> 
> 
>>>   
>>>   /**
>>> @@ -913,13 +912,27 @@ int sev_guest_decommission(struct sev_data_decommission *data, int *error);
>>>    * @error: SEV command return code
>>>    *
>>>    * Returns:
>>> + * 0 if the sev successfully processed the command
>>> + * -%ENODEV    if the sev device is not available
>>> + * -%ENOTSUPP  if the sev does not support SEV
>>> + * -%ETIMEDOUT if the sev command timed out
>>> + * -%EIO       if the sev returned a non-zero return code
>>> + */
> 
> I think that if the word 'sev' would be 'SEV' in this comment, the diff
> will be a bit less misleading (basically this patch should not introduce
> changes to sev_do_cmd).
> 
> -Dov
> 
>>> +int sev_do_cmd(int cmd, void *data, int *psp_ret);
>>> +
>>> +/**
>>> + * snp_guest_dbg_decrypt_page - perform SEV SNP_DBG_DECRYPT command
>>> + *
>>> + * @sev_ret: sev command return code
>>> + *
>>> + * Returns:
>>>    * 0 if the SEV successfully processed the command
>>>    * -%ENODEV    if the SEV device is not available
>>>    * -%ENOTSUPP  if the SEV does not support SEV
>>>    * -%ETIMEDOUT if the SEV command timed out
>>>    * -%EIO       if the SEV returned a non-zero return code
>>>    */
>>> -int sev_do_cmd(int cmd, void *data, int *psp_ret);
>>> +int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error);
>>>   
>>>   void *psp_copy_user_blob(u64 uaddr, u32 len);
>>>   void *snp_alloc_firmware_page(gfp_t mask);
>>> @@ -987,6 +1000,11 @@ static inline void *psp_copy_user_blob(u64 __user uaddr, u32 len) { return ERR_P
>>>   
>>>   void snp_mark_pages_offline(unsigned long pfn, unsigned int npages) {}
>>>   
>>> +static inline int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error)
>>> +{
>>> +	return -ENODEV;
>>> +}
>>> +
>>>   static inline void *snp_alloc_firmware_page(gfp_t mask)
>>>   {
>>>   	return NULL;
>>
Dov Murik March 2, 2023, 9:11 p.m. UTC | #4
On 02/03/2023 16:33, Tom Lendacky wrote:
> On 3/1/23 23:59, Dov Murik wrote:
>> Hi Mike, Zhi,
>>
>> On 01/03/2023 23:20, Zhi Wang wrote:
>>> On Mon, 20 Feb 2023 12:38:43 -0600
>>> Michael Roth <michael.roth@amd.com> wrote:
>>>
>>>> From: Brijesh Singh <brijesh.singh@amd.com>
>>>>
>>>> Add support to decrypt guest encrypted memory. These API interfaces can
>>>> be used for example to dump VMCBs on SNP guest exit.
>>>>
>>>
>>> What kinds of check will be applied from firmware when VMM decrypts this
>>> page? I suppose there has to be kinda mechanism to prevent VMM to
>>> decrypt
>>> any page in the guest. It would be nice to have some introduction about
>>> it in the comments.
>>>
>>
>> The SNP ABI spec says (section 8.27.2 SNP_DBG_DECRYPT):
>>
>>    The firmware checks that the guest's policy allows debugging. If not,
>>    the firmware returns POLICY_FAILURE.
>>
>> and in the Guest Policy (section 4.3):
>>
>>    Bit 19 - DEBUG
>>    0: Debugging is disallowed.
>>    1: Debugging is allowed.
>>
>> In the kernel, that firmware error code is defined as
>> SEV_RET_POLICY_FAILURE.
>>
>>
>>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>>> [mdr: minor commit fixups]
>>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>>>> ---
>>>>   drivers/crypto/ccp/sev-dev.c | 32 ++++++++++++++++++++++++++++++++
>>>>   include/linux/psp-sev.h      | 22 ++++++++++++++++++++--
>>>>   2 files changed, 52 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/crypto/ccp/sev-dev.c
>>>> b/drivers/crypto/ccp/sev-dev.c
>>>> index e65563bc8298..bf5167b2acfc 100644
>>>> --- a/drivers/crypto/ccp/sev-dev.c
>>>> +++ b/drivers/crypto/ccp/sev-dev.c
>>>> @@ -2017,6 +2017,38 @@ int sev_guest_df_flush(int *error)
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(sev_guest_df_flush);
>>>>   +int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64
>>>> dst_pfn, int *error)
>>>> +{
>>>> +    struct sev_data_snp_dbg data = {0};
>>>> +    struct sev_device *sev;
>>>> +    int ret;
>>>> +
>>>> +    if (!psp_master || !psp_master->sev_data)
>>>> +        return -ENODEV;
>>>> +
>>>> +    sev = psp_master->sev_data;
>>>> +
>>>> +    if (!sev->snp_initialized)
>>>> +        return -EINVAL;
>>>> +
>>>> +    data.gctx_paddr = sme_me_mask | (gctx_pfn << PAGE_SHIFT);
>>>> +    data.src_addr = sme_me_mask | (src_pfn << PAGE_SHIFT);
>>>> +    data.dst_addr = sme_me_mask | (dst_pfn << PAGE_SHIFT);
>>
>> I guess this works, but I wonder why we need to turn on sme_me_mask on
>> teh dst_addr.  I thought that the firmware decrypts the guest page
>> (src_addr) to a plaintext page.  Couldn't find this requirement in the
>> SNP spec.
> 
> This sme_me_mask tells the firmware how to access the host memory
> (similar to how DMA uses sme_me_mask when supplying addresses to devices
> under SME). This needs to match the pagetable mapping being used by the
> host, otherwise the contents will appears as ciphertext to the host if
> they are not in sync. Since the default pagetable mapping is encrypted,
> the sme_me_mask bit must be provided on the destination address. So it
> is not a spec requirement, but an SME implementation requirement.
> 

Ah, OK, that's clear now. Thanks Tom.

-Dov
diff mbox series

Patch

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index e65563bc8298..bf5167b2acfc 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -2017,6 +2017,38 @@  int sev_guest_df_flush(int *error)
 }
 EXPORT_SYMBOL_GPL(sev_guest_df_flush);
 
+int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error)
+{
+	struct sev_data_snp_dbg data = {0};
+	struct sev_device *sev;
+	int ret;
+
+	if (!psp_master || !psp_master->sev_data)
+		return -ENODEV;
+
+	sev = psp_master->sev_data;
+
+	if (!sev->snp_initialized)
+		return -EINVAL;
+
+	data.gctx_paddr = sme_me_mask | (gctx_pfn << PAGE_SHIFT);
+	data.src_addr = sme_me_mask | (src_pfn << PAGE_SHIFT);
+	data.dst_addr = sme_me_mask | (dst_pfn << PAGE_SHIFT);
+
+	/* The destination page must be in the firmware state. */
+	if (rmp_mark_pages_firmware(data.dst_addr, 1, false))
+		return -EIO;
+
+	ret = sev_do_cmd(SEV_CMD_SNP_DBG_DECRYPT, &data, error);
+
+	/* Restore the page state */
+	if (snp_reclaim_pages(data.dst_addr, 1, false))
+		ret = -EIO;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snp_guest_dbg_decrypt_page);
+
 int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
 				unsigned long vaddr, unsigned long *npages, unsigned long *fw_err)
 {
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 81bafc049eca..92116e2b74fd 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -710,7 +710,6 @@  struct sev_data_snp_dbg {
 	u64 gctx_paddr;				/* In */
 	u64 src_addr;				/* In */
 	u64 dst_addr;				/* In */
-	u32 len;				/* In */
 } __packed;
 
 /**
@@ -913,13 +912,27 @@  int sev_guest_decommission(struct sev_data_decommission *data, int *error);
  * @error: SEV command return code
  *
  * Returns:
+ * 0 if the sev successfully processed the command
+ * -%ENODEV    if the sev device is not available
+ * -%ENOTSUPP  if the sev does not support SEV
+ * -%ETIMEDOUT if the sev command timed out
+ * -%EIO       if the sev returned a non-zero return code
+ */
+int sev_do_cmd(int cmd, void *data, int *psp_ret);
+
+/**
+ * snp_guest_dbg_decrypt_page - perform SEV SNP_DBG_DECRYPT command
+ *
+ * @sev_ret: sev command return code
+ *
+ * Returns:
  * 0 if the SEV successfully processed the command
  * -%ENODEV    if the SEV device is not available
  * -%ENOTSUPP  if the SEV does not support SEV
  * -%ETIMEDOUT if the SEV command timed out
  * -%EIO       if the SEV returned a non-zero return code
  */
-int sev_do_cmd(int cmd, void *data, int *psp_ret);
+int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error);
 
 void *psp_copy_user_blob(u64 uaddr, u32 len);
 void *snp_alloc_firmware_page(gfp_t mask);
@@ -987,6 +1000,11 @@  static inline void *psp_copy_user_blob(u64 __user uaddr, u32 len) { return ERR_P
 
 void snp_mark_pages_offline(unsigned long pfn, unsigned int npages) {}
 
+static inline int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *error)
+{
+	return -ENODEV;
+}
+
 static inline void *snp_alloc_firmware_page(gfp_t mask)
 {
 	return NULL;