Messages in this thread | | | Date | Thu, 16 Feb 2023 03:02:56 -0800 | From | Jeremi Piotrowski <> | Subject | Re: [PATCH v2 2/8] x86/mtrr: support setting MTRR state for software defined MTRRs |
| |
On Thu, Feb 16, 2023 at 10:32:28AM +0100, Juergen Gross wrote: > On 13.02.23 12:39, Borislav Petkov wrote: > >On Thu, Feb 09, 2023 at 08:22:14AM +0100, Juergen Gross wrote: > >>When running virtualized, MTRR access can be reduced (e.g. in Xen PV > >>guests or when running as a SEV-SNP guest under Hyper-V). Typically > >>the hypervisor will reset the MTRR feature in cpuid data, resulting > >>in no MTRR memory type information being available for the kernel. > >> > >>This has turned out to result in problems: > >> > >>- Hyper-V SEV-SNP guests using uncached mappings where they shouldn't > >>- Xen PV dom0 mapping memory as WB which should be UC- instead > >> > >>Solve those problems by supporting to set a fixed MTRR state, > >>overwriting the empty state used today. In case such a state has been > >>set, don't call get_mtrr_state() in mtrr_bp_init(). The set state > >>will only be used by mtrr_type_lookup(), as in all other cases > >>mtrr_enabled() is being checked, which will return false. Accept the > >>overwrite call only in case of MTRRs being disabled in cpuid. > > > >s/cpuid/CPUID/g > > > >>Signed-off-by: Juergen Gross <jgross@suse.com> > >>--- > >>V2: > >>- new patch > >>--- > >> arch/x86/include/asm/mtrr.h | 2 ++ > >> arch/x86/kernel/cpu/mtrr/generic.c | 38 ++++++++++++++++++++++++++++++ > >> arch/x86/kernel/cpu/mtrr/mtrr.c | 9 +++++++ > >> 3 files changed, 49 insertions(+) > >> > >>diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h > >>index f0eeaf6e5f5f..0b8f51d683dc 100644 > >>--- a/arch/x86/include/asm/mtrr.h > >>+++ b/arch/x86/include/asm/mtrr.h > >>@@ -31,6 +31,8 @@ > >> */ > >> # ifdef CONFIG_MTRR > >> void mtrr_bp_init(void); > >>+void mtrr_overwrite_state(struct mtrr_var_range *var, unsigned int num_var, > >>+ mtrr_type *fixed, mtrr_type def_type); > >> extern u8 mtrr_type_lookup(u64 addr, u64 end, u8 *uniform); > >> extern void mtrr_save_fixed_ranges(void *); > >> extern void mtrr_save_state(void); > >>diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c > >>index ee09d359e08f..788bc16888a5 100644 > >>--- a/arch/x86/kernel/cpu/mtrr/generic.c > >>+++ b/arch/x86/kernel/cpu/mtrr/generic.c > >>@@ -240,6 +240,44 @@ static u8 mtrr_type_lookup_variable(u64 start, u64 end, u64 *partial_end, > >> return mtrr_state.def_type; > >> } > >>+/** > >>+ * mtrr_overwrite_state - set fixed MTRR state > > > >fixed only? You pass in variable too... > > > >>+ * > >>+ * Used to set MTRR state via different means (e.g. with data obtained from > >>+ * a hypervisor). > >>+ */ > >>+void mtrr_overwrite_state(struct mtrr_var_range *var, unsigned int num_var, > >>+ mtrr_type *fixed, mtrr_type def_type) > >>+{ > >>+ unsigned int i; > >>+ > >>+ if (boot_cpu_has(X86_FEATURE_MTRR)) > > > >check_for_deprecated_apis: WARNING: arch/x86/kernel/cpu/mtrr/generic.c:254: Do not use boot_cpu_has() - use cpu_feature_enabled() instead. > > > >>+ return; > > > >So this here needs to check: > > > > if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR) && > > !(cpu_feature_enabled(X86_FEATURE_SEV_SNP) || > > cpu_feature_enabled(X86_FEATURE_XENPV))) { > > WARN_ON_ONCE(1); > > return; > > } > > > >as we don't want this to be called somewhere or by something else. > > > >The SEV_SNP flag can be used from: > > > >https://lore.kernel.org/r/20221214194056.161492-14-michael.roth@amd.com > > > >I'm assuming here HyperV SEV-SNP guests really do set that feature flag > >(they better). We can expedite that patch ofc. > > Is that flag _really_ meant to indicate we are running as a SEV-SNP guest? > > Given that the referenced patch is part of the SEV-SNP host support series, > I'm inclined to suspect it won't be set for sure in HyperV SEV-SNP guests. > And who is setting it for KVM SEV-SNP guests? > > > Juergen
Initially both guest and host side have X86_FEATURE_SEV_SNP, but early_detect_mem_encrypt() clears it for the guest side.
You would want cc_platform_has(CC_ATTR_GUEST_SEV_SNP), but:
* there are two kinds of Hyper-V SEV-SNP VMs: "unenlightened" (vTOM+paravisor) and "enlightened" * the kernel currently supports the "unenlightened" kind which do not set CC_ATTR_GUEST_SEV_SNP (because it implies codepaths which do not apply to vTOM mode) * the patchset that adds support for "enlightened" Hyper-V SEV-SNP VMs sets CC_ATTR_GUEST_SEV_SNP [1]
The closest you can get is:
cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) && hypervisor_is_type(X86_HYPER_MS_HYPERV)
but that would soon cover TDX guests too so <shrug>...
[1]: https://lore.kernel.org/lkml/1673559753-94403-8-git-send-email-mikelley@microsoft.com/
| |