From: Like Xu <like.xu@linux.intel.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Kan Liang <kan.liang@linux.intel.com>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Borislav Petkov <bp@alien8.de>,
x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RESEND 1/2] perf/x86: Skip checking MSR for MSR 0x0
Date: Thu, 22 Apr 2021 09:18:47 +0800 [thread overview]
Message-ID: <8b799e26-f8b0-adeb-b8a6-331087c0d4be@linux.intel.com> (raw)
In-Reply-To: <YIBFCOz9R966xAVZ@google.com>
On 2021/4/21 23:30, Sean Christopherson wrote:
> On Wed, Apr 21, 2021, Like Xu wrote:
>> The Architecture LBR does not have MSR_LBR_TOS (0x000001c9).
>> When ARCH_LBR we don't set lbr_tos, the failure from the
>> check_msr() against MSR 0x000 will make x86_pmu.lbr_nr = 0,
>> thereby preventing the initialization of the guest LBR.
>>
>> Fixes: 47125db27e47 ("perf/x86/intel/lbr: Support Architectural LBR")
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
>> arch/x86/events/intel/core.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index 5272f349dca2..5036496caa60 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -4751,10 +4751,10 @@ static bool check_msr(unsigned long msr, u64 mask)
>> u64 val_old, val_new, val_tmp;
>>
>> /*
>> - * Disable the check for real HW, so we don't
>> + * Disable the check for real HW or non-sense msr, so we don't
>
> I think this should be "undefined MSR" or something along those lines. MSR 0x0
> is a "real" MSR, on Intel CPUs it's an alias for IA32_MC0_ADDR; at least it's
> supposed to be, most/all Intel CPUs incorrectly alias it to IA32_MC0_CTL.
Thank you, Sean.
<idle>-0 [000] dN.. 38980.032347: read_msr: 0, value fff
Do we have a historic story or specification for this kind of alias ?
#define MSR_IA32_MC0_ADDR 0x00000402
#define MSR_IA32_MC0_CTL 0x00000400
>
> Anyways, my point is that if your definition of "nonsense" is any MSR that is
> not a valid perf MSR, then this check is woefully incompletely. If your
> definition is a nonsensical value, then this comment is simply wrong.
>
> What you're really looking for is precisely the case where the MSR was zero
> initialized and never defined.
>
>> * mess with potentionaly enabled registers:
>> */
>> - if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
>> + if (!boot_cpu_has(X86_FEATURE_HYPERVISOR) || !msr)
>> return true;
>>
>> /*
>> --
>> 2.30.2
>>
next prev parent reply other threads:[~2021-04-22 1:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-21 2:18 [PATCH RESEND 1/2] perf/x86: Skip checking MSR for MSR 0x0 Like Xu
2021-04-21 2:18 ` [PATCH RESEND 2/2] perf/x86/lbr: Move cpuc->lbr_xsave allocation out of sleeping region Like Xu
2021-04-21 8:38 ` Peter Zijlstra
2021-04-21 8:48 ` Like Xu
2021-04-21 9:21 ` Peter Zijlstra
2021-04-21 15:30 ` [PATCH RESEND 1/2] perf/x86: Skip checking MSR for MSR 0x0 Sean Christopherson
2021-04-22 1:18 ` Like Xu [this message]
2021-04-22 1:38 ` Sean Christopherson
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=8b799e26-f8b0-adeb-b8a6-331087c0d4be@linux.intel.com \
--to=like.xu@linux.intel.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=bp@alien8.de \
--cc=jolsa@redhat.com \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=seanjc@google.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.