linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Juergen Gross <jgross@suse.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH v4 06/12] x86/mtrr: replace vendor tests in MTRR code
Date: Fri, 24 Mar 2023 17:56:11 +0100	[thread overview]
Message-ID: <20230324165611.GIZB3WK13NdjceLWnN@fat_crate.local> (raw)
In-Reply-To: <20230306163425.8324-7-jgross@suse.com>

On Mon, Mar 06, 2023 at 05:34:19PM +0100, Juergen Gross wrote:
> Modern CPUs all share the same MTRR interface implemented via
> generic_mtrr_ops.
> 
> At several places in MTRR code this generic interface is deduced via
> is_cpu(INTEL) tests, which is only working due to X86_VENDOR_INTEL
> being 0 (the is_cpu() macro is testing mtrr_if->vendor, which isn't
> explicitly set in generic_mtrr_ops).
> 
> Fix that by replacing the is_cpu(INTEL) tests with testing for mtrr_if
> to be &generic_mtrr_ops.

Two things:

* is_cpu() checks also whether mtrr_if is set. And we don't set it for
all vendors. I wanted to replace that thing with a vendor check recently
but there's that little issue.

I guess for the cases where we have the generic MTRR implementation, we
can safely assume that mtrr_if is set. Which leads me to the second
thing:

* If you're going to test for &generic_mtrr_ops, then you can just as
well do

	cpu_feature_enabled(X86_FEATURE_MTRR)

which is a lot more telling.

> diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c
> index 5fe62ee0361b..0c83990501f5 100644
> --- a/arch/x86/kernel/cpu/mtrr/mtrr.c
> +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c
> @@ -108,14 +108,12 @@ static int have_wrcomb(void)
>  /*  This function returns the number of variable MTRRs  */
>  static void __init set_num_var_ranges(bool use_generic)
>  {
> -	unsigned long config = 0, dummy;
> +	unsigned long config, dummy;
>  
>  	if (use_generic)
>  		rdmsr(MSR_MTRRcap, config, dummy);
> -	else if (is_cpu(AMD) || is_cpu(HYGON))
> -		config = 2;
> -	else if (is_cpu(CYRIX) || is_cpu(CENTAUR))
> -		config = 8;
> +	else
> +		config = mtrr_if->var_regs;
>  
>  	num_var_ranges = config & 0xff;
>  }

Since you're touching this function, you might simply expand its body in
its only call site in mtrr_bp_init(), put a comment above the expanded
code and remove that function.

That is, if we're going to do the ->var_regs thing.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

  reply	other threads:[~2023-03-24 16:56 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-06 16:34 [PATCH v4 00/12] x86/mtrr: fix handling with PAT but without MTRR Juergen Gross
2023-03-06 16:34 ` [PATCH v4 01/12] x86/mtrr: split off physical address size calculation Juergen Gross
2023-03-06 16:34 ` [PATCH v4 02/12] x86/mtrr: optimize mtrr_calc_physbits() Juergen Gross
2023-03-20 12:50   ` Borislav Petkov
2023-03-06 16:34 ` [PATCH v4 03/12] x86/mtrr: support setting MTRR state for software defined MTRRs Juergen Gross
2023-03-20 12:59   ` Huang, Kai
2023-03-20 13:47     ` Juergen Gross
2023-03-20 21:34       ` Huang, Kai
2023-03-20 22:42       ` Borislav Petkov
2023-03-21  6:01         ` Juergen Gross
2023-03-20 19:05   ` Borislav Petkov
2023-03-21  6:00     ` Juergen Gross
2023-03-21 10:30       ` Borislav Petkov
2023-03-21 15:49         ` Juergen Gross
2023-03-06 16:34 ` [PATCH v4 04/12] x86/hyperv: set MTRR state when running as SEV-SNP Hyper-V guest Juergen Gross
2023-03-06 16:34 ` [PATCH v4 05/12] x86/xen: set MTRR state when running as Xen PV initial domain Juergen Gross
2023-03-07 21:47   ` Boris Ostrovsky
2023-03-23 12:43   ` Borislav Petkov
2023-03-06 16:34 ` [PATCH v4 06/12] x86/mtrr: replace vendor tests in MTRR code Juergen Gross
2023-03-24 16:56   ` Borislav Petkov [this message]
2023-03-27  5:43     ` Juergen Gross
2023-03-27  7:14       ` Borislav Petkov
2023-03-06 16:34 ` [PATCH v4 07/12] x86/mtrr: allocate mtrr_value array dynamically Juergen Gross
2023-03-20 12:25   ` Huang, Kai
2023-03-20 13:49     ` Juergen Gross
2023-03-20 15:31       ` Dave Hansen
2023-03-20 15:49         ` Juergen Gross
2023-03-26 22:05   ` Borislav Petkov
2023-03-27  5:44     ` Juergen Gross
2023-03-06 16:34 ` [PATCH v4 08/12] x86/mtrr: add get_effective_type() service function Juergen Gross
2023-03-06 16:34 ` [PATCH v4 09/12] x86/mtrr: construct a memory map with cache modes Juergen Gross
2023-03-29 12:51   ` Borislav Petkov
2023-03-29 13:39     ` Juergen Gross
2023-03-31 12:55       ` Borislav Petkov
2023-03-31 13:23         ` Juergen Gross
2023-04-01 14:24           ` Borislav Petkov
2023-04-03  6:57             ` Juergen Gross
2023-03-31 12:57   ` Borislav Petkov
2023-03-31 13:35     ` Juergen Gross
2023-04-01 14:26       ` Borislav Petkov
2023-04-03  7:02         ` Juergen Gross
2023-03-06 16:34 ` [PATCH v4 10/12] x86/mtrr: use new cache_map in mtrr_type_lookup() Juergen Gross
2023-03-06 16:34 ` [PATCH v4 11/12] x86/mtrr: don't let mtrr_type_lookup() return MTRR_TYPE_INVALID Juergen Gross
2023-03-06 16:34 ` [PATCH v4 12/12] x86/mm: only check uniform after calling mtrr_type_lookup() Juergen Gross
2023-03-07 21:09 ` [PATCH v4 00/12] x86/mtrr: fix handling with PAT but without MTRR Michael Kelley (LINUX)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230324165611.GIZB3WK13NdjceLWnN@fat_crate.local \
    --to=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).