On Mon, 2023-03-20 at 14:47 +0100, Juergen Gross wrote: > On 20.03.23 13:59, Huang, Kai wrote: > > On Mon, 2023-03-06 at 17:34 +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 static 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 for selected cases when running as a guest. > > > Disable X86_FEATURE_MTRR in order to avoid any MTRR modifications by > > > just refusing them. > > > > > > > > [...] > > > > > diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c > > > index ee09d359e08f..49b4cc923312 100644 > > > --- a/arch/x86/kernel/cpu/mtrr/generic.c > > > +++ b/arch/x86/kernel/cpu/mtrr/generic.c > > > @@ -8,10 +8,12 @@ > > > #include <linux/init.h> > > > #include <linux/io.h> > > > #include <linux/mm.h> > > > - > > > +#include <linux/cc_platform.h> > > > #include <asm/processor-flags.h> > > > #include <asm/cacheinfo.h> > > > #include <asm/cpufeature.h> > > > +#include <asm/hypervisor.h> > > > +#include <asm/mshyperv.h> > > > > Is <asm/mshyperv.h> needed here? > > Yes, for hv_is_isolation_supported(). > > > > > > #include <asm/tlbflush.h> > > > #include <asm/mtrr.h> > > > #include <asm/msr.h> > > > @@ -240,6 +242,48 @@ static u8 mtrr_type_lookup_variable(u64 start, u64 end, u64 *partial_end, > > > return mtrr_state.def_type; > > > } > > > > > > +/** > > > + * mtrr_overwrite_state - set static MTRR state > > > + * > > > + * Used to set MTRR state via different means (e.g. with data obtained from > > > + * a hypervisor). > > > > +KVM list and KVM maintainers, > > > > IIUC in the next patch, SEV-SNP guest only sets a synthetic MTRR w/o telling the > > hypervisor (hyperv). I think this works for SEV-SNP running on top of hyperv > > because they have mutual understanding? > > > > What about the SNP guest running on other hypervisors such as KVM? > > > > Since this code covers TDX guest too, I think eventually it makes sense for TDX > > guest to use this function too (to avoid #VE IIUC). If want to do that, then I > > think TDX guest should have the same mutual understanding with *ALL* hypervisor, > > as I am not sure what's the point of making the TDX guest's MTRR behaviour > > depending on specific hypervisor. > > This series tries to fix the current fallout. > > Boris Petkov asked for the hypervisor specific tests to be added, so I've > added them after discussing the topic with him (he is the maintainer of > this code after all). > > > For now I don't see there's any use case for TDX guest to use non-WB memory type > > (in fact, KVM always maps guest memory as WB if there's no non-coherent DMA to > > the guest memory), so to me it seems it's OK to make a universal mutual > > understanding that TDX guest will always have WB memory type for all memory. > > I agree. > > > But, I am not sure whether it's better to have a standard hypercall between > > guest & hypervisor for this purpose so things can be more flexible? > > Maybe. But for now we need to handle the current situation. > > Agreed. Thanks for explaining.
