| Message ID | 20221119034633.1728632-11-ltykernel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/hyperv/sev: Add AMD sev-snp enlightened guest support on hyperv | expand |
> 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;
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.
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
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 --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;