Messages in this thread | | | Date | Tue, 14 Feb 2023 08:01:56 +0100 | Subject | Re: [PATCH v2 2/8] x86/mtrr: support setting MTRR state for software defined MTRRs | From | Juergen Gross <> |
| |
On 13.02.23 19:43, Borislav Petkov wrote: > 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
I will change this a little bit in order to avoid the #ifdef by using "WARN_ON(hypervisor_is_type() == X86_HYPER_NATIVE);"
Juergen [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |