From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S942109AbcJFPZz (ORCPT ); Thu, 6 Oct 2016 11:25:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56736 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S942032AbcJFPZq (ORCPT ); Thu, 6 Oct 2016 11:25:46 -0400 Message-ID: <57F66CF7.7040204@redhat.com> Date: Thu, 06 Oct 2016 11:25:43 -0400 From: Prarit Bhargava User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Jiri Olsa , Thomas Gleixner CC: linux-kernel@vger.kernel.org, Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Peter Zijlstra , Len Brown , Borislav Petkov , Andi Kleen , Juergen Gross , dyoung@redhat.com, Eric Biederman , kexec@lists.infradead.org Subject: Re: [PATCH] arch/x86: Fix kdump on x86 with physically hotadded CPUs References: <1475514432-27682-1-git-send-email-prarit@redhat.com> <57F39C09.10001@redhat.com> <20161005161455.GA26029@krava> In-Reply-To: <20161005161455.GA26029@krava> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 06 Oct 2016 15:25:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/05/2016 12:14 PM, Jiri Olsa wrote: > 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; I thought about doing this but it seems like every time some driver uses topology_logical_package_id() the driver would have to replicate the error checking code. P. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx1.redhat.com ([209.132.183.28]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bsAYh-000304-5Y for kexec@lists.infradead.org; Thu, 06 Oct 2016 15:26:08 +0000 Message-ID: <57F66CF7.7040204@redhat.com> Date: Thu, 06 Oct 2016 11:25:43 -0400 From: Prarit Bhargava MIME-Version: 1.0 Subject: Re: [PATCH] arch/x86: Fix kdump on x86 with physically hotadded CPUs References: <1475514432-27682-1-git-send-email-prarit@redhat.com> <57F39C09.10001@redhat.com> <20161005161455.GA26029@krava> In-Reply-To: <20161005161455.GA26029@krava> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Jiri Olsa , Thomas Gleixner Cc: Juergen Gross , Len Brown , Andi Kleen , Peter Zijlstra , x86@kernel.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Ingo Molnar , Eric Biederman , "H. Peter Anvin" , Borislav Petkov , dyoung@redhat.com On 10/05/2016 12:14 PM, Jiri Olsa wrote: > 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; I thought about doing this but it seems like every time some driver uses topology_logical_package_id() the driver would have to replicate the error checking code. P. _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec