| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <687d351b-0165-fc87-1426-1a4c16ef048a@suse.com>
Date: Mon, 8 May 2023 15:00:09 +0200
From: Juergen Gross <jgross@...e.com>
To: Borislav Petkov <bp@...en8.de>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
mikelley@...rosoft.com, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v6 02/16] x86/mtrr: replace some constants with defines
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
Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)
Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)
Powered by blists - more mailing lists