| Message ID | 20211207075602.2452-3-ltykernel@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/Hyper-V: Add Hyper-V Isolation VM support(Second part) | expand |
On Tue, Dec 07, 2021 at 02:55:58AM -0500, Tianyu Lan wrote: > From: Tianyu Lan <Tianyu.Lan@microsoft.com> > > Hyper-V provides Isolation VM which has memory encrypt support. Add > hyperv_cc_platform_has() and return true for check of GUEST_MEM_ENCRYPT > attribute. You need to refresh on how to write commit messages - never say what the patch is doing - that's visible in the diff itself. Rather, you should talk about *why* it is doing what it is doing. > bool cc_platform_has(enum cc_attr attr) > { > + if (hv_is_isolation_supported()) > + return hyperv_cc_platform_has(attr); Is there any reason for the hv_is_.. check to come before... > + > if (sme_me_mask) > return amd_cc_platform_has(attr); ... the sme_me_mask check? What's in sme_me_mask on hyperv? Thx.
Hi Borislav: Thanks for your review. On 12/7/2021 5:47 PM, Borislav Petkov wrote: > On Tue, Dec 07, 2021 at 02:55:58AM -0500, Tianyu Lan wrote: >> From: Tianyu Lan <Tianyu.Lan@microsoft.com> >> >> Hyper-V provides Isolation VM which has memory encrypt support. Add >> hyperv_cc_platform_has() and return true for check of GUEST_MEM_ENCRYPT >> attribute. > > You need to refresh on how to write commit messages - never say what the > patch is doing - that's visible in the diff itself. Rather, you should > talk about *why* it is doing what it is doing. Sure. Will update. > >> bool cc_platform_has(enum cc_attr attr) >> { >> + if (hv_is_isolation_supported()) >> + return hyperv_cc_platform_has(attr); > > Is there any reason for the hv_is_.. check to come before... > Do you mean to check hyper-v before sev? If yes, no special reason. >> + >> if (sme_me_mask) >> return amd_cc_platform_has(attr); > > ... the sme_me_mask check? > > What's in sme_me_mask on hyperv? sme_me_mask is unset in this case.
From: Tianyu Lan <ltykernel@gmail.com> Sent: Monday, December 6, 2021 11:56 PM > > Hyper-V provides Isolation VM which has memory encrypt support. Add > hyperv_cc_platform_has() and return true for check of GUEST_MEM_ENCRYPT > attribute. > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> > --- > Change since v3: > * Change code style of checking GUEST_MEM attribute in the > hyperv_cc_platform_has(). > --- > arch/x86/kernel/cc_platform.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c > index 03bb2f343ddb..47db88c275d5 100644 > --- a/arch/x86/kernel/cc_platform.c > +++ b/arch/x86/kernel/cc_platform.c > @@ -11,6 +11,7 @@ > #include <linux/cc_platform.h> > #include <linux/mem_encrypt.h> > > +#include <asm/mshyperv.h> > #include <asm/processor.h> > > static bool __maybe_unused intel_cc_platform_has(enum cc_attr attr) > @@ -58,9 +59,16 @@ static bool amd_cc_platform_has(enum cc_attr attr) > #endif > } > > +static bool hyperv_cc_platform_has(enum cc_attr attr) > +{ > + return attr == CC_ATTR_GUEST_MEM_ENCRYPT; > +} > > bool cc_platform_has(enum cc_attr attr) > { > + if (hv_is_isolation_supported()) > + return hyperv_cc_platform_has(attr); > + > if (sme_me_mask) > return amd_cc_platform_has(attr); > Throughout Linux kernel code, there are about 20 calls to cc_platform_has() with CC_ATTR_GUEST_MEM_ENCRYPT as the argument. The original code (from v1 of this patch set) only dealt with the call in sev_setup_arch(). But with this patch, all the other calls that previously returned "false" will now return "true" in a Hyper-V Isolated VM. I didn't try to analyze all these other calls, so I think there's an open question about whether this is the behavior we want. Michael
On 12/10/2021 4:38 AM, Michael Kelley (LINUX) wrote: > From: Tianyu Lan <ltykernel@gmail.com> Sent: Monday, December 6, 2021 11:56 PM >> >> Hyper-V provides Isolation VM which has memory encrypt support. Add >> hyperv_cc_platform_has() and return true for check of GUEST_MEM_ENCRYPT >> attribute. >> >> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> >> --- >> Change since v3: >> * Change code style of checking GUEST_MEM attribute in the >> hyperv_cc_platform_has(). >> --- >> arch/x86/kernel/cc_platform.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c >> index 03bb2f343ddb..47db88c275d5 100644 >> --- a/arch/x86/kernel/cc_platform.c >> +++ b/arch/x86/kernel/cc_platform.c >> @@ -11,6 +11,7 @@ >> #include <linux/cc_platform.h> >> #include <linux/mem_encrypt.h> >> >> +#include <asm/mshyperv.h> >> #include <asm/processor.h> >> >> static bool __maybe_unused intel_cc_platform_has(enum cc_attr attr) >> @@ -58,9 +59,16 @@ static bool amd_cc_platform_has(enum cc_attr attr) >> #endif >> } >> >> +static bool hyperv_cc_platform_has(enum cc_attr attr) >> +{ >> + return attr == CC_ATTR_GUEST_MEM_ENCRYPT; >> +} >> >> bool cc_platform_has(enum cc_attr attr) >> { >> + if (hv_is_isolation_supported()) >> + return hyperv_cc_platform_has(attr); >> + >> if (sme_me_mask) >> return amd_cc_platform_has(attr); >> > > Throughout Linux kernel code, there are about 20 calls to cc_platform_has() > with CC_ATTR_GUEST_MEM_ENCRYPT as the argument. The original code > (from v1 of this patch set) only dealt with the call in sev_setup_arch(). But > with this patch, all the other calls that previously returned "false" will now > return "true" in a Hyper-V Isolated VM. I didn't try to analyze all these other > calls, so I think there's an open question about whether this is the behavior > we want. > CC_ATTR_GUEST_MEM_ENCRYPT is for SEV support so far. Hyper-V Isolation VM is based on SEV or software memory encrypt. Most checks can be reused. The difference is that SEV code use encrypt bit in the page table to encrypt and decrypt memory while Hyper-V uses vTOM. But the sev memory encrypt mask "sme_me_mask" is unset in the Hyper-V Isolation VM where claims sev and sme are unsupported. The rest of checks for mem enc bit are still safe. So reuse CC_ATTR_GUEST_MEM_ENCRYPT for Hyper-V.
diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c index 03bb2f343ddb..47db88c275d5 100644 --- a/arch/x86/kernel/cc_platform.c +++ b/arch/x86/kernel/cc_platform.c @@ -11,6 +11,7 @@ #include <linux/cc_platform.h> #include <linux/mem_encrypt.h> +#include <asm/mshyperv.h> #include <asm/processor.h> static bool __maybe_unused intel_cc_platform_has(enum cc_attr attr) @@ -58,9 +59,16 @@ static bool amd_cc_platform_has(enum cc_attr attr) #endif } +static bool hyperv_cc_platform_has(enum cc_attr attr) +{ + return attr == CC_ATTR_GUEST_MEM_ENCRYPT; +} bool cc_platform_has(enum cc_attr attr) { + if (hv_is_isolation_supported()) + return hyperv_cc_platform_has(attr); + if (sme_me_mask) return amd_cc_platform_has(attr);