All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.