Messages in this thread | | | Date | Mon, 8 May 2023 15:00:09 +0200 | Subject | Re: [PATCH v6 02/16] x86/mtrr: replace some constants with defines | From | Juergen Gross <> |
| |
On 05.05.23 20:49, Borislav Petkov wrote: > On Tue, May 02, 2023 at 02:09:17PM +0200, Juergen Gross wrote: >> Instead of using constants in MTRR code, use some new #defines. >> >> Replace size_or_mask and size_and_mask with the much easier concept of >> high reserved bits. > > This is the better commit title than what's there now because the patch > does more than just replacing some constants with defines. Therefore my > question to patch 1. Anyway, I'll fix it up. > > Also, some minor cleanups ontop: > > --- > > diff --git a/arch/x86/include/uapi/asm/mtrr.h b/arch/x86/include/uapi/asm/mtrr.h > index 376563f2bac1..e314969a5e6d 100644 > --- a/arch/x86/include/uapi/asm/mtrr.h > +++ b/arch/x86/include/uapi/asm/mtrr.h > @@ -84,8 +84,8 @@ typedef __u8 mtrr_type; > struct mtrr_state_type { > struct mtrr_var_range var_ranges[MTRR_MAX_VAR_RANGES]; > mtrr_type fixed_ranges[MTRR_NUM_FIXED_RANGES]; > - unsigned char enabled; > - unsigned char have_fixed; > + bool enabled; > + bool have_fixed; > mtrr_type def_type; > }; > > diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c > index 59b48bd8380c..3f0811b2fd18 100644 > --- a/arch/x86/kernel/cpu/mtrr/generic.c > +++ b/arch/x86/kernel/cpu/mtrr/generic.c > @@ -38,14 +38,8 @@ u64 mtrr_tom2; > struct mtrr_state_type mtrr_state; > EXPORT_SYMBOL_GPL(mtrr_state); > > -static u32 phys_hi_rsvd; > - > -void __init mtrr_set_mask(void) > -{ > - unsigned int phys_addr = boot_cpu_data.x86_phys_bits; > - > - phys_hi_rsvd = GENMASK(31, phys_addr - 32); > -} > +/* Reserved bits in the high portion of the MTRRphysBaseN MSR. */ > +u32 phys_hi_rsvd; > > /* > * BIOS is expected to clear MtrrFixDramModEn bit, see for example > @@ -231,8 +225,7 @@ static u8 mtrr_type_lookup_variable(u64 start, u64 end, u64 *partial_end, > if ((start & mask) != (base & mask)) > continue; > > - curr_match = mtrr_state.var_ranges[i].base_lo & > - MTRR_PHYSBASE_TYPE; > + curr_match = mtrr_state.var_ranges[i].base_lo & MTRR_PHYSBASE_TYPE; > if (prev_match == MTRR_TYPE_INVALID) { > prev_match = curr_match; > continue; > @@ -462,7 +455,7 @@ bool __init get_mtrr_state(void) > vrs = mtrr_state.var_ranges; > > rdmsr(MSR_MTRRcap, lo, dummy); > - mtrr_state.have_fixed = !!(lo & MTRR_CAP_FIX); > + mtrr_state.have_fixed = lo & MTRR_CAP_FIX; > > for (i = 0; i < num_var_ranges; i++) > get_mtrr_var_range(i, &vrs[i]); > @@ -471,8 +464,7 @@ bool __init get_mtrr_state(void) > > rdmsr(MSR_MTRRdefType, lo, dummy); > mtrr_state.def_type = lo & MTRR_DEF_TYPE_TYPE; > - mtrr_state.enabled = (lo & MTRR_DEF_TYPE_ENABLE) >> > - MTRR_STATE_SHIFT; > + mtrr_state.enabled = (lo & MTRR_DEF_TYPE_ENABLE) >> MTRR_STATE_SHIFT; > > if (amd_special_default_mtrr()) { > unsigned low, high; > @@ -585,7 +577,7 @@ static void generic_get_mtrr(unsigned int reg, unsigned long *base, > > rdmsr(MTRRphysMask_MSR(reg), mask_lo, mask_hi); > > - if ((mask_lo & MTRR_PHYSMASK_V) == 0) { > + if (!(mask_lo & MTRR_PHYSMASK_V)) { > /* Invalid (i.e. free) range */ > *base = 0; > *size = 0; > @@ -700,9 +692,8 @@ static unsigned long set_mtrr_state(void) > * Set_mtrr_restore restores the old value of MTRRdefType, > * so to set it we fiddle with the saved value: > */ > - if ((deftype_lo & MTRR_DEF_TYPE_TYPE) != mtrr_state.def_type > - || ((deftype_lo & MTRR_DEF_TYPE_ENABLE) >> MTRR_STATE_SHIFT) != > - mtrr_state.enabled) { > + if ((deftype_lo & MTRR_DEF_TYPE_TYPE) != mtrr_state.def_type || > + ((deftype_lo & MTRR_DEF_TYPE_ENABLE) >> MTRR_STATE_SHIFT) != mtrr_state.enabled) { > > deftype_lo = (deftype_lo & MTRR_DEF_TYPE_DISABLE) | > mtrr_state.def_type | > @@ -719,8 +710,7 @@ void mtrr_disable(void) > rdmsr(MSR_MTRRdefType, deftype_lo, deftype_hi); > > /* Disable MTRRs, and set the default type to uncached */ > - mtrr_wrmsr(MSR_MTRRdefType, deftype_lo & MTRR_DEF_TYPE_DISABLE, > - deftype_hi); > + mtrr_wrmsr(MSR_MTRRdefType, deftype_lo & MTRR_DEF_TYPE_DISABLE, deftype_hi); > } > > void mtrr_enable(void) > diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c > index fddc4e0c6626..1067f128bded 100644 > --- a/arch/x86/kernel/cpu/mtrr/mtrr.c > +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c > @@ -627,7 +627,7 @@ void __init mtrr_bp_init(void) > { > const char *why = "(not available)"; > > - mtrr_set_mask(); > + phys_hi_rsvd = GENMASK(31, boot_cpu_data.x86_phys_bits - 32); > > if (cpu_feature_enabled(X86_FEATURE_MTRR)) { > mtrr_if = &generic_mtrr_ops; > diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.h b/arch/x86/kernel/cpu/mtrr/mtrr.h > index a00987e6cc1c..1028a2b67961 100644 > --- a/arch/x86/kernel/cpu/mtrr/mtrr.h > +++ b/arch/x86/kernel/cpu/mtrr/mtrr.h > @@ -58,6 +58,7 @@ extern const struct mtrr_ops *mtrr_if; > extern unsigned int num_var_ranges; > extern u64 mtrr_tom2; > extern struct mtrr_state_type mtrr_state; > +extern u32 phys_hi_rsvd; > > void mtrr_set_mask(void);
I think you want to delete this^ prototype, too.
Juergen [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |