From: Jiri Olsa <jolsa@redhat.com> To: Thomas Gleixner <tglx@linutronix.de> Cc: Prarit Bhargava <prarit@redhat.com>, linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org, Peter Zijlstra <peterz@infradead.org>, Len Brown <len.brown@intel.com>, Borislav Petkov <bp@suse.de>, Andi Kleen <ak@linux.intel.com>, Juergen Gross <jgross@suse.com>, dyoung@redhat.com, Eric Biederman <ebiederm@xmission.com>, kexec@lists.infradead.org Subject: Re: [PATCH] arch/x86: Fix kdump on x86 with physically hotadded CPUs Date: Wed, 5 Oct 2016 18:14:55 +0200 [thread overview] Message-ID: <20161005161455.GA26029@krava> (raw) In-Reply-To: <alpine.DEB.2.20.1610041628120.5049@nanos> On Tue, Oct 04, 2016 at 04:38:16PM +0200, Thomas Gleixner wrote: > On Tue, 4 Oct 2016, Prarit Bhargava wrote: > > On 10/04/2016 06:58 AM, Thomas Gleixner wrote: > > > While it is the right thing to initialize the package map in that case, it > > > still papers over a robustness issue in the uncore code, which needs to be > > > fixed first. > > > > I will include a separate patch with an error check for pkg == 0xffff in the > > uncore code. > > 0xffff? That won't help. The id returned is -1 if the entry is not > initialized. And aside of that just patching that particular place is not > helping as the uncore code and also rapl is relying on the package map > being populated. > > So we need a sanity check in the initialization code which prevents any of > this being executed. I still need to test this, but how about something like this? thanks, jirka --- diff --git a/arch/x86/events/intel/rapl.c b/arch/x86/events/intel/rapl.c index 28865938aadf..61d087a2f25d 100644 --- a/arch/x86/events/intel/rapl.c +++ b/arch/x86/events/intel/rapl.c @@ -598,8 +598,13 @@ static int rapl_cpu_online(unsigned int cpu) static int rapl_cpu_prepare(unsigned int cpu) { - struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu); + struct rapl_pmu *pmu; + int pkg = topology_logical_package_id(cpu); + + if (WARN_ON(pkg == -1)) + return -EINVAL; + pmu = cpu_to_rapl_pmu(cpu); if (pmu) return 0; @@ -613,7 +618,7 @@ static int rapl_cpu_prepare(unsigned int cpu) pmu->timer_interval = ms_to_ktime(rapl_timer_ms); pmu->cpu = -1; rapl_hrtimer_init(pmu); - rapl_pmus->pmus[topology_logical_package_id(cpu)] = pmu; + rapl_pmus->pmus[pkg] = pmu; return 0; } diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c index 463dc7a5a6c3..3f657fbbf787 100644 --- a/arch/x86/events/intel/uncore.c +++ b/arch/x86/events/intel/uncore.c @@ -1352,7 +1352,7 @@ static int __init intel_uncore_init(void) { const struct x86_cpu_id *id; struct intel_uncore_init_fun *uncore_init; - int pret = 0, cret = 0, ret; + int pret = 0, cret = 0, ret, cpu; id = x86_match_cpu(intel_uncore_match); if (!id) @@ -1363,6 +1363,13 @@ static int __init intel_uncore_init(void) max_packages = topology_max_packages(); + for_each_present_cpu(cpu) { + int pkg = topology_logical_package_id(cpu); + + if (WARN_ON(pkg == -1)) + return -EINVAL; + } + uncore_init = (struct intel_uncore_init_fun *)id->driver_data; if (uncore_init->pci_init) { pret = uncore_init->pci_init();
WARNING: multiple messages have this Message-ID (diff)
From: Jiri Olsa <jolsa@redhat.com> To: Thomas Gleixner <tglx@linutronix.de> Cc: Prarit Bhargava <prarit@redhat.com>, Len Brown <len.brown@intel.com>, Andi Kleen <ak@linux.intel.com>, Juergen Gross <jgross@suse.com>, Peter Zijlstra <peterz@infradead.org>, x86@kernel.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>, Eric Biederman <ebiederm@xmission.com>, "H. Peter Anvin" <hpa@zytor.com>, Borislav Petkov <bp@suse.de>, dyoung@redhat.com Subject: Re: [PATCH] arch/x86: Fix kdump on x86 with physically hotadded CPUs Date: Wed, 5 Oct 2016 18:14:55 +0200 [thread overview] Message-ID: <20161005161455.GA26029@krava> (raw) In-Reply-To: <alpine.DEB.2.20.1610041628120.5049@nanos> On Tue, Oct 04, 2016 at 04:38:16PM +0200, Thomas Gleixner wrote: > On Tue, 4 Oct 2016, Prarit Bhargava wrote: > > On 10/04/2016 06:58 AM, Thomas Gleixner wrote: > > > While it is the right thing to initialize the package map in that case, it > > > still papers over a robustness issue in the uncore code, which needs to be > > > fixed first. > > > > I will include a separate patch with an error check for pkg == 0xffff in the > > uncore code. > > 0xffff? That won't help. The id returned is -1 if the entry is not > initialized. And aside of that just patching that particular place is not > helping as the uncore code and also rapl is relying on the package map > being populated. > > So we need a sanity check in the initialization code which prevents any of > this being executed. I still need to test this, but how about something like this? thanks, jirka --- diff --git a/arch/x86/events/intel/rapl.c b/arch/x86/events/intel/rapl.c index 28865938aadf..61d087a2f25d 100644 --- a/arch/x86/events/intel/rapl.c +++ b/arch/x86/events/intel/rapl.c @@ -598,8 +598,13 @@ static int rapl_cpu_online(unsigned int cpu) static int rapl_cpu_prepare(unsigned int cpu) { - struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu); + struct rapl_pmu *pmu; + int pkg = topology_logical_package_id(cpu); + + if (WARN_ON(pkg == -1)) + return -EINVAL; + pmu = cpu_to_rapl_pmu(cpu); if (pmu) return 0; @@ -613,7 +618,7 @@ static int rapl_cpu_prepare(unsigned int cpu) pmu->timer_interval = ms_to_ktime(rapl_timer_ms); pmu->cpu = -1; rapl_hrtimer_init(pmu); - rapl_pmus->pmus[topology_logical_package_id(cpu)] = pmu; + rapl_pmus->pmus[pkg] = pmu; return 0; } diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c index 463dc7a5a6c3..3f657fbbf787 100644 --- a/arch/x86/events/intel/uncore.c +++ b/arch/x86/events/intel/uncore.c @@ -1352,7 +1352,7 @@ static int __init intel_uncore_init(void) { const struct x86_cpu_id *id; struct intel_uncore_init_fun *uncore_init; - int pret = 0, cret = 0, ret; + int pret = 0, cret = 0, ret, cpu; id = x86_match_cpu(intel_uncore_match); if (!id) @@ -1363,6 +1363,13 @@ static int __init intel_uncore_init(void) max_packages = topology_max_packages(); + for_each_present_cpu(cpu) { + int pkg = topology_logical_package_id(cpu); + + if (WARN_ON(pkg == -1)) + return -EINVAL; + } + uncore_init = (struct intel_uncore_init_fun *)id->driver_data; if (uncore_init->pci_init) { pret = uncore_init->pci_init(); _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
next prev parent reply other threads:[~2016-10-05 16:15 UTC|newest] Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-10-03 17:07 [PATCH] arch/x86: Fix kdump on x86 with physically hotadded CPUs Prarit Bhargava 2016-10-03 17:07 ` Prarit Bhargava 2016-10-03 22:22 ` Jiri Olsa 2016-10-03 22:22 ` Jiri Olsa 2016-10-04 10:58 ` Thomas Gleixner 2016-10-04 10:58 ` Thomas Gleixner 2016-10-04 12:09 ` Prarit Bhargava 2016-10-04 12:09 ` Prarit Bhargava 2016-10-04 14:38 ` Thomas Gleixner 2016-10-04 14:38 ` Thomas Gleixner 2016-10-04 16:01 ` Prarit Bhargava 2016-10-04 16:01 ` Prarit Bhargava 2016-10-05 16:14 ` Jiri Olsa [this message] 2016-10-05 16:14 ` Jiri Olsa 2016-10-06 15:25 ` Prarit Bhargava 2016-10-06 15:25 ` Prarit Bhargava 2016-10-07 6:49 ` Jiri Olsa 2016-10-07 6:49 ` Jiri Olsa 2016-10-07 8:02 ` Thomas Gleixner 2016-10-07 8:02 ` Thomas Gleixner 2016-10-04 12:27 ` Jiri Olsa 2016-10-04 12:27 ` Jiri Olsa 2016-10-04 14:19 ` Thomas Gleixner 2016-10-04 14:19 ` Thomas Gleixner 2016-10-07 13:28 ` [tip:x86/urgent] arch/x86: Handle non enumerated CPU after physical hotplug tip-bot for Prarit Bhargava
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=20161005161455.GA26029@krava \ --to=jolsa@redhat.com \ --cc=ak@linux.intel.com \ --cc=bp@suse.de \ --cc=dyoung@redhat.com \ --cc=ebiederm@xmission.com \ --cc=hpa@zytor.com \ --cc=jgross@suse.com \ --cc=kexec@lists.infradead.org \ --cc=len.brown@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ --cc=prarit@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: linkBe 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.