diff mbox series

[RFC,V2,10/18] drivers: hv: Decrypt percpu hvcall input arg page in sev-snp enlightened guest

Message ID 20221119034633.1728632-11-ltykernel@gmail.com
State New
Headers
Series x86/hyperv/sev: Add AMD sev-snp enlightened guest support on hyperv |

Commit Message

Tianyu Lan Nov. 19, 2022, 3:46 a.m. UTC 
  From: Tianyu Lan <tiala@microsoft.com>

Hypervisor needs to access iput arg page and guest should decrypt
the page.

Signed-off-by: Tianyu Lan <tiala@microsoft.com>
---
 drivers/hv/hv_common.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Dexuan Cui Dec. 8, 2022, 9:52 p.m. UTC | #1 
> From: Tianyu Lan <ltykernel@gmail.com>
> Sent: Friday, November 18, 2022 7:46 PM
> [...]
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> @@ -125,6 +126,7 @@ int hv_common_cpu_init(unsigned int cpu)
>  	u64 msr_vp_index;
>  	gfp_t flags;
>  	int pgcount = hv_root_partition ? 2 : 1;
> +	int ret;
> 
>  	/* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
>  	flags = irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL;
> @@ -134,6 +136,16 @@ int hv_common_cpu_init(unsigned int cpu)
>  	if (!(*inputarg))
>  		return -ENOMEM;
> 
> +	if (hv_isolation_type_en_snp()) {
> +		ret = set_memory_decrypted((unsigned long)*inputarg, 1);

Is it possible hv_root_partition==1 here? If yes, the pgcount is 2.

> +		if (ret) {
> +			kfree(*inputarg);
> +			return ret;
> +		}
> +
> +		memset(*inputarg, 0x00, PAGE_SIZE);
> +	}
> +
>  	if (hv_root_partition) {
>  		outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
>  		*outputarg = (char *)(*inputarg) + HV_HYP_PAGE_SIZE;
> @@ -168,6 +180,9 @@ int hv_common_cpu_die(unsigned int cpu)
> 
>  	local_irq_restore(flags);
> 
> +	if (hv_isolation_type_en_snp())
> +		set_memory_encrypted((unsigned long)mem, 1);

If set_memory_encrypted() fails, we should not free the 'mem'.

> +
>  	kfree(mem);
> 
>  	return 0;
Tianyu Lan Dec. 9, 2022, 2:26 a.m. UTC | #2 
On 12/9/2022 5:52 AM, Dexuan Cui wrote:
>> @@ -134,6 +136,16 @@ int hv_common_cpu_init(unsigned int cpu)
>>   	if (!(*inputarg))
>>   		return -ENOMEM;
>>
>> +	if (hv_isolation_type_en_snp()) {
>> +		ret = set_memory_decrypted((unsigned long)*inputarg, 1);
> Is it possible hv_root_partition==1 here? If yes, the pgcount is 2.
>
Hi Dexuan:
	Thanks for review. So far, root partition doesn't support sev 
enlightened guest and so here assume pgcount is always 1. We may use 
pgcount variable here instead of the number.

>> +		if (ret) {
>> +			kfree(*inputarg);
>> +			return ret;
>> +		}
>> +
>> +		memset(*inputarg, 0x00, PAGE_SIZE);
>> +	}
>> +
>>   	if (hv_root_partition) {
>>   		outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
>>   		*outputarg = (char *)(*inputarg) + HV_HYP_PAGE_SIZE;
>> @@ -168,6 +180,9 @@ int hv_common_cpu_die(unsigned int cpu)
>>
>>   	local_irq_restore(flags);
>>
>> +	if (hv_isolation_type_en_snp())
>> +		set_memory_encrypted((unsigned long)mem, 1);
> If set_memory_encrypted() fails, we should not free the 'mem'.

Good point. Will update in the next version.
Michael Kelley (LINUX) Dec. 14, 2022, 6:16 p.m. UTC | #3 
From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, November 18, 2022 7:46 PM
> 
> Hypervisor needs to access iput arg page and guest should decrypt
> the page.
> 
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
>  drivers/hv/hv_common.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 2c6602571c47..c16961e686a0 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -21,6 +21,7 @@
>  #include <linux/ptrace.h>
>  #include <linux/slab.h>
>  #include <linux/dma-map-ops.h>
> +#include <linux/set_memory.h>
>  #include <asm/hyperv-tlfs.h>
>  #include <asm/mshyperv.h>
> 
> @@ -125,6 +126,7 @@ int hv_common_cpu_init(unsigned int cpu)
>  	u64 msr_vp_index;
>  	gfp_t flags;
>  	int pgcount = hv_root_partition ? 2 : 1;
> +	int ret;
> 
>  	/* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
>  	flags = irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL;
> @@ -134,6 +136,16 @@ int hv_common_cpu_init(unsigned int cpu)
>  	if (!(*inputarg))
>  		return -ENOMEM;
> 
> +	if (hv_isolation_type_en_snp()) {
> +		ret = set_memory_decrypted((unsigned long)*inputarg, 1);
> +		if (ret) {
> +			kfree(*inputarg);

After the kfree(), set *inputarg back to NULL.  There's other code that
tests the value of *inputarg to know if the per-CPU hypercall page has
been successfully allocated.

> +			return ret;
> +		}
> +
> +		memset(*inputarg, 0x00, PAGE_SIZE);
> +	}
> +
>  	if (hv_root_partition) {
>  		outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
>  		*outputarg = (char *)(*inputarg) + HV_HYP_PAGE_SIZE;
> @@ -168,6 +180,9 @@ int hv_common_cpu_die(unsigned int cpu)
> 
>  	local_irq_restore(flags);
> 
> +	if (hv_isolation_type_en_snp())
> +		set_memory_encrypted((unsigned long)mem, 1);
> +
>  	kfree(mem);
> 
>  	return 0;
> --
> 2.25.1
Tianyu Lan Dec. 26, 2022, 7:26 a.m. UTC | #4 
On 12/15/2022 2:16 AM, Michael Kelley (LINUX) wrote:
>> @@ -134,6 +136,16 @@ int hv_common_cpu_init(unsigned int cpu)
>>   	if (!(*inputarg))
>>   		return -ENOMEM;
>>
>> +	if (hv_isolation_type_en_snp()) {
>> +		ret = set_memory_decrypted((unsigned long)*inputarg, 1);
>> +		if (ret) {
>> +			kfree(*inputarg);
> After the kfree(), set *inputarg back to NULL.  There's other code that
> tests the value of *inputarg to know if the per-CPU hypercall page has
> been successfully allocated.
> 

Good point! Will add in the next version.
diff mbox series

Patch

diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 2c6602571c47..c16961e686a0 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -21,6 +21,7 @@ 
 #include <linux/ptrace.h>
 #include <linux/slab.h>
 #include <linux/dma-map-ops.h>
+#include <linux/set_memory.h>
 #include <asm/hyperv-tlfs.h>
 #include <asm/mshyperv.h>
 
@@ -125,6 +126,7 @@  int hv_common_cpu_init(unsigned int cpu)
 	u64 msr_vp_index;
 	gfp_t flags;
 	int pgcount = hv_root_partition ? 2 : 1;
+	int ret;
 
 	/* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
 	flags = irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL;
@@ -134,6 +136,16 @@  int hv_common_cpu_init(unsigned int cpu)
 	if (!(*inputarg))
 		return -ENOMEM;
 
+	if (hv_isolation_type_en_snp()) {
+		ret = set_memory_decrypted((unsigned long)*inputarg, 1);
+		if (ret) {
+			kfree(*inputarg);
+			return ret;
+		}
+
+		memset(*inputarg, 0x00, PAGE_SIZE);
+	}
+
 	if (hv_root_partition) {
 		outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
 		*outputarg = (char *)(*inputarg) + HV_HYP_PAGE_SIZE;
@@ -168,6 +180,9 @@  int hv_common_cpu_die(unsigned int cpu)
 
 	local_irq_restore(flags);
 
+	if (hv_isolation_type_en_snp())
+		set_memory_encrypted((unsigned long)mem, 1);
+
 	kfree(mem);
 
 	return 0;