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. Juergen
Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
