> From: Zhi Wang <zhi.wang.linux@xxxxxxxxx>
> Sent: Friday, January 6, 2023 3:00 AM
> > diff --git a/arch/x86/mm/pat/set_memory.c
> > @@ -2120,7 +2120,7 @@ static int __set_memory_enc_pgtable(unsigned
> long
> > addr, int numpages, bool enc)
> > static int __set_memory_enc_dec(unsigned long addr, int numpages, bool
> > enc) {
> > - if (hv_is_isolation_supported())
> > + if (hv_is_isolation_supported() && !hv_isolation_type_tdx())
> > return hv_set_mem_host_visibility(addr, numpages, !enc);
> >
> > if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
The change here is kind of a hack to not call hv_set_mem_host_visibility()
for TDX guests on Hyper-V. The original change was also a hack to me to
call hv_set_mem_host_visibility() for SNP guests with pavavisor on Hyper-V.
> Let's say there will be four cases:
> ----
> case a. SEV-SNP guest with paravisor
>
> In the code, this case is represented by:
>
> hv_is_isolation_supported() && hv_isolation_type_snp()
> hv_is_isolation_supported() && !hv_isolation_type_tdx()
These look bad to me...
> case b. TDX guest with paravisor
> ?
As of now, this is not supported yet. I'll need to figure out how exactly
this scenario will look like.
> case c. SEV-SNP guest *without* paravisor
> ?
Tianyu Lan is working on this:
https://lwn.net/ml/linux-kernel/20221119034633.1728632-1-ltykernel@xxxxxxxxx/
set_memory_decrypted() calls __set_memory_enc_dec() directly. This
is the same as a SNP guest running on KVM.
> case d. TDX guest *without* paravisor
>
> In the code, this case is represented by:
>
> hv_is_isolation_supported() && hv_isolation_type_tdx()
This looks bad to me...
> ----
>
> 1. It would be better to use "hv_is_isolation_supported() &&
> hv_isolation_type_snp()" to represent case a to avoid confusion in the
> above patch.
>
> 2. For now, hv_is_isolation_supported() only shows if the guest is a CC
> guest or not. hv_isolation_type_*() only represent SNP or TDX but
> not "w/ or w/o paravisor".
>
> How are you going to represent case b and c in __set_memory_enc_dec()?
>
> I think you are looking for something to show if the guest is running
> with a paravisor or not here:
>
> if (hv_is_isolation_supported() && hv_is_isolation_with_paravisor())
> ...
>
> Thanks,
> Zhi.
Michael's patchset removes the special path for SNP with pavavisor on Hyper-V:
https://lwn.net/ml/linux-kernel/1669951831-4180-7-git-send-email-mikelley%40microsoft.com/
With Michael's patchset, I don't need the change to __set_memory_enc_dec()
at all. The plan was that Michael's patchset would be merged into the upstream
first and I would rebase my TDX patchset accordingly, but Michael's patchset
has been pending for almost 2 months...
so I probably need to post v3 with the below version, which looks a little
better to me because it hides the Hyper-V specific logic in a Hyper-V specific
file arch/x86/hyperv/ivm.c, and if necessary we can change the implementation
of hv_set_memory_enc_dec_needed() in future, e.g. Tianyu can change
hv_set_memory_enc_dec_needed() to distinguish between SNP with pavavisor
and SNP without pavavisor. Of course, I still hope Michael's patchset would
be merged soon so I can avoid this kind of mess...
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 07e4253b5809..4398042f10d5 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -258,6 +258,11 @@ bool hv_is_isolation_supported(void)
return hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE;
}
+bool hv_set_memory_enc_dec_needed(void)
+{
+ return hv_is_isolation_supported() && !hv_isolation_type_tdx();
+}
+
DEFINE_STATIC_KEY_FALSE(isolation_type_snp);
/*
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 2e5a045731de..5892196f8ade 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2120,7 +2120,7 @@ static int __set_memory_enc_pgtable(unsigned long addr, int numpages, bool enc)
static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
{
- if (hv_is_isolation_supported())
+ if (hv_set_memory_enc_dec_needed())
return hv_set_mem_host_visibility(addr, numpages, !enc);
if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index a9a03ab04b97..192dcf295dfc 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -262,6 +262,12 @@ bool __weak hv_is_isolation_supported(void)
}
EXPORT_SYMBOL_GPL(hv_is_isolation_supported);
+bool __weak hv_set_memory_enc_dec_needed(void)
+{
+ return false;
+}
+EXPORT_SYMBOL_GPL(hv_set_memory_enc_dec_needed);
+
bool __weak hv_isolation_type_snp(void)
{
return false;
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index bfb9eb9d7215..b7b1b18c9854 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -262,6 +262,7 @@ bool hv_is_hyperv_initialized(void);
bool hv_is_hibernation_supported(void);
enum hv_isolation_type hv_get_isolation_type(void);
bool hv_is_isolation_supported(void);
+bool hv_set_memory_enc_dec_needed(void);
bool hv_isolation_type_snp(void);
u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
void hyperv_cleanup(void);
@@ -274,6 +275,7 @@ static inline bool hv_is_hyperv_initialized(void) { return false; }
static inline bool hv_is_hibernation_supported(void) { return false; }
static inline void hyperv_cleanup(void) {}
static inline bool hv_is_isolation_supported(void) { return false; }
+static inline bool hv_set_memory_enc_dec_needed(void) { return false; }
static inline enum hv_isolation_type hv_get_isolation_type(void)
{
return HV_ISOLATION_TYPE_NONE;
