On Mon, Feb 13, 2023 at 04:36:12PM +0100, Juergen Gross wrote:
In the end I wouldn't mind dropping the fixed MTRRs from the interface, as
they are currently not needed at all.
Yes, the less the better.
I'd say we go with what is needed right now. And having a single interface
makes all the sanity checking you are asking for easier.
I guess I need to remember to finish designing this if more users
appear...
What are you especially asking for?
With my current patches Xen PV dom0 will call mtrr_overwrite_state() before
x86_hyper_type is set, while a Hyper-V SEV-SNP guest will make the call after
it has been set. Both calls happen before cache_bp_init().
So I could move the mtrr_overwrite_state() call for Xen PV dom0 into its
init_platform() callback and check in mtrr_overwrite_state() x86_hyper_type
to be set,
I believe that is good enough, see below.
or I could reject a call of mtrr_overwrite_state() after the call of
cache_bp_init() has happened, or I could do both.
I think one thing is enough as we'll be loud enough.
---
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c
index b73fe243c7fd..2dbe2c10e959 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.c
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.c
@@ -49,6 +49,7 @@
#include <asm/cacheinfo.h>
#include <asm/cpufeature.h>
#include <asm/e820/api.h>
+#include <asm/hypervisor.h>
#include <asm/mtrr.h>
#include <asm/msr.h>
#include <asm/memtype.h>
@@ -668,7 +669,12 @@ void __init mtrr_bp_init(void)
const char *why = "(not available)";
unsigned int phys_addr;
+#ifdef CONFIG_HYPERVISOR_GUEST
if (mtrr_state.enabled) {
+
+ /* This should not happen without a hypervisor present. */
+ WARN_ON_ONCE(!x86_hyper_type);
+
/* Software overwrite of MTRR state, only for generic case. */
mtrr_calc_physbits(true);
init_table();
@@ -676,6 +682,7 @@ void __init mtrr_bp_init(void)
return;
}
+#endif
Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature