* [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs @ 2018-07-10 23:31 Masayoshi Mizuma 2018-07-15 22:34 ` Ingo Molnar 0 siblings, 1 reply; 9+ messages in thread From: Masayoshi Mizuma @ 2018-07-10 23:31 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86 Cc: Masayoshi Mizuma, linux-kernel, Masayoshi Mizuma From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs") introduced PCU.3 for Broadwell CPU. Unfortunately, the driver_data of PCU.3 conflicts to QPI Port 2 filter. { /* QPI Port 2 filter */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), { /* PCU.3 (for Capability registers) */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, HSWEP_PCI_PCU_3), // HSWEP_PCI_PCU_3 == 2 As the result, the index for each device gets same, so the pci device is overwrited in uncore_extra_pci_dev[pkg].dev[idx] when it is probed. static int uncore_pci_probe(struct pci_dev *pdev,... { ... if (UNCORE_PCI_DEV_TYPE(id->driver_data) == UNCORE_EXTRA_PCI_DEV) { int idx = UNCORE_PCI_DEV_IDX(id->driver_data); // HERE!! uncore_extra_pci_dev[pkg].dev[idx] = pdev; // HERE!! pci_set_drvdata(pdev, NULL); return 0; } Due to the overwriting, the following warning message are shown while CPU hot-removing. WARNING: CPU: 126 PID: 6 at arch/x86/events/intel/uncore.c:988 uncore_pci_remove+0x10b/0x150 Call Trace: pci_device_remove+0x42/0xd0 device_release_driver_internal+0x148/0x220 pci_stop_bus_device+0x76/0xa0 pci_stop_root_bus+0x44/0x60 acpi_pci_root_remove+0x1f/0x80 acpi_bus_trim+0x57/0x90 acpi_bus_trim+0x2e/0x90 acpi_device_hotplug+0x2bc/0x4b0 acpi_hotplug_work_fn+0x1a/0x30 process_one_work+0x174/0x3a0 worker_thread+0x4c/0x3d0 kthread+0xf8/0x130 To avoid the conflict, this patch changes the PCU.3 driver_data. And also increase UNCORE_EXTRA_PCI_DEV_MAX to handle the new index. Fixes: 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs") Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> --- arch/x86/events/intel/uncore.h | 2 +- arch/x86/events/intel/uncore_snbep.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h index c9e1e0b..e17ab88 100644 --- a/arch/x86/events/intel/uncore.h +++ b/arch/x86/events/intel/uncore.h @@ -28,7 +28,7 @@ #define UNCORE_PCI_DEV_TYPE(data) ((data >> 8) & 0xff) #define UNCORE_PCI_DEV_IDX(data) (data & 0xff) #define UNCORE_EXTRA_PCI_DEV 0xff -#define UNCORE_EXTRA_PCI_DEV_MAX 3 +#define UNCORE_EXTRA_PCI_DEV_MAX 4 #define UNCORE_EVENT_CONSTRAINT(c, n) EVENT_CONSTRAINT(c, n, 0xff) diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c index 87dc026..62f007c 100644 --- a/arch/x86/events/intel/uncore_snbep.c +++ b/arch/x86/events/intel/uncore_snbep.c @@ -1030,6 +1030,7 @@ enum { SNBEP_PCI_QPI_PORT0_FILTER, SNBEP_PCI_QPI_PORT1_FILTER, HSWEP_PCI_PCU_3, + BDX_PCI_PCU_3, }; static int snbep_qpi_hw_config(struct intel_uncore_box *box, struct perf_event *event) @@ -3070,11 +3071,11 @@ void bdx_uncore_cpu_init(void) if (boot_cpu_data.x86_model == 86) { uncore_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL; /* Detect systems with no SBOXes */ - } else if (uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3]) { + } else if (uncore_extra_pci_dev[pkg].dev[BDX_PCI_PCU_3]) { struct pci_dev *pdev; u32 capid4; - pdev = uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3]; + pdev = uncore_extra_pci_dev[pkg].dev[BDX_PCI_PCU_3]; pci_read_config_dword(pdev, 0x94, &capid4); if (((capid4 >> 6) & 0x3) == 0) bdx_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL; @@ -3299,7 +3300,7 @@ static const struct pci_device_id bdx_uncore_pci_ids[] = { { /* PCU.3 (for Capability registers) */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, - HSWEP_PCI_PCU_3), + BDX_PCI_PCU_3), }, { /* end: all zeroes */ } }; -- 2.16.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs 2018-07-10 23:31 [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs Masayoshi Mizuma @ 2018-07-15 22:34 ` Ingo Molnar 2018-07-16 14:29 ` Liang, Kan 0 siblings, 1 reply; 9+ messages in thread From: Ingo Molnar @ 2018-07-15 22:34 UTC (permalink / raw) To: Masayoshi Mizuma, Kan Liang Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Masayoshi Mizuma * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: > From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> > > commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for > Broadwell CPUs") introduced PCU.3 for Broadwell CPU. Unfortunately, > the driver_data of PCU.3 conflicts to QPI Port 2 filter. > > { /* QPI Port 2 filter */ > PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), > .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), > > { /* PCU.3 (for Capability registers) */ > PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), > .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, > HSWEP_PCI_PCU_3), > // HSWEP_PCI_PCU_3 == 2 > --- a/arch/x86/events/intel/uncore_snbep.c > +++ b/arch/x86/events/intel/uncore_snbep.c > @@ -1030,6 +1030,7 @@ enum { > SNBEP_PCI_QPI_PORT0_FILTER, > SNBEP_PCI_QPI_PORT1_FILTER, > HSWEP_PCI_PCU_3, > + BDX_PCI_PCU_3, > }; So we use a magic '2' enumerator in the 'QPI Port 2 filter', and that overlaps with HSWEP_PCI_PCU_3, right? Shouldn't we clean up all the enumerators and not use magic numbers, and this fix the conflict? Thanks, Ingo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs 2018-07-15 22:34 ` Ingo Molnar @ 2018-07-16 14:29 ` Liang, Kan 2018-07-16 15:07 ` Masayoshi Mizuma 0 siblings, 1 reply; 9+ messages in thread From: Liang, Kan @ 2018-07-16 14:29 UTC (permalink / raw) To: Ingo Molnar, Masayoshi Mizuma, Kan Liang Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Masayoshi Mizuma On 7/15/2018 6:34 PM, Ingo Molnar wrote: > > * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: > >> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> >> >> commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for >> Broadwell CPUs") introduced PCU.3 for Broadwell CPU. Unfortunately, >> the driver_data of PCU.3 conflicts to QPI Port 2 filter. >> >> { /* QPI Port 2 filter */ >> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), >> .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), >> >> { /* PCU.3 (for Capability registers) */ >> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), >> .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, >> HSWEP_PCI_PCU_3), >> // HSWEP_PCI_PCU_3 == 2 > >> --- a/arch/x86/events/intel/uncore_snbep.c >> +++ b/arch/x86/events/intel/uncore_snbep.c >> @@ -1030,6 +1030,7 @@ enum { >> SNBEP_PCI_QPI_PORT0_FILTER, >> SNBEP_PCI_QPI_PORT1_FILTER, >> HSWEP_PCI_PCU_3, >> + BDX_PCI_PCU_3, >> }; > > So we use a magic '2' enumerator in the 'QPI Port 2 filter', and that overlaps > with HSWEP_PCI_PCU_3, right? > > Shouldn't we clean up all the enumerators and not use magic numbers, and this fix > the conflict? > Yes, it should fix the conflict. I will clean up the code. Thanks, Kan > Thanks, > > Ingo > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs 2018-07-16 14:29 ` Liang, Kan @ 2018-07-16 15:07 ` Masayoshi Mizuma 2018-07-16 16:31 ` Liang, Kan 0 siblings, 1 reply; 9+ messages in thread From: Masayoshi Mizuma @ 2018-07-16 15:07 UTC (permalink / raw) To: kan.liang, mingo, kan.liang; +Cc: tglx, mingo, hpa, x86, linux-kernel, m.mizuma On 07/16/2018 10:29 AM, Liang, Kan wrote: > > > On 7/15/2018 6:34 PM, Ingo Molnar wrote: >> >> * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: >> >>> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> >>> >>> commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for >>> Broadwell CPUs") introduced PCU.3 for Broadwell CPU. Unfortunately, >>> the driver_data of PCU.3 conflicts to QPI Port 2 filter. >>> >>> { /* QPI Port 2 filter */ >>> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), >>> .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), >>> >>> { /* PCU.3 (for Capability registers) */ >>> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), >>> .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, >>> HSWEP_PCI_PCU_3), >>> // HSWEP_PCI_PCU_3 == 2 >> >>> --- a/arch/x86/events/intel/uncore_snbep.c >>> +++ b/arch/x86/events/intel/uncore_snbep.c >>> @@ -1030,6 +1030,7 @@ enum { >>> SNBEP_PCI_QPI_PORT0_FILTER, >>> SNBEP_PCI_QPI_PORT1_FILTER, >>> HSWEP_PCI_PCU_3, >>> + BDX_PCI_PCU_3, >>> }; >> >> So we use a magic '2' enumerator in the 'QPI Port 2 filter', and that overlaps >> with HSWEP_PCI_PCU_3, right? >> >> Shouldn't we clean up all the enumerators and not use magic numbers, and this fix >> the conflict? >> > > Yes, it should fix the conflict. I will clean up the code. Thanks a lot! I would appreciate if you could add CC to me when you post the patch. Thanks, Masa > > Thanks, > Kan > > >> Thanks, >> >> Ingo >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs 2018-07-16 15:07 ` Masayoshi Mizuma @ 2018-07-16 16:31 ` Liang, Kan 2018-07-17 16:37 ` Masayoshi Mizuma 0 siblings, 1 reply; 9+ messages in thread From: Liang, Kan @ 2018-07-16 16:31 UTC (permalink / raw) To: Masayoshi Mizuma, mingo, kan.liang Cc: tglx, mingo, hpa, x86, linux-kernel, m.mizuma On 7/16/2018 11:07 AM, Masayoshi Mizuma wrote: > > > On 07/16/2018 10:29 AM, Liang, Kan wrote: >> >> >> On 7/15/2018 6:34 PM, Ingo Molnar wrote: >>> >>> * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: >>> >>>> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> >>>> >>>> commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for >>>> Broadwell CPUs") introduced PCU.3 for Broadwell CPU. Unfortunately, >>>> the driver_data of PCU.3 conflicts to QPI Port 2 filter. >>>> >>>> { /* QPI Port 2 filter */ >>>> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), >>>> .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), >>>> >>>> { /* PCU.3 (for Capability registers) */ >>>> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), >>>> .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, >>>> HSWEP_PCI_PCU_3), >>>> // HSWEP_PCI_PCU_3 == 2 >>> >>>> --- a/arch/x86/events/intel/uncore_snbep.c >>>> +++ b/arch/x86/events/intel/uncore_snbep.c >>>> @@ -1030,6 +1030,7 @@ enum { >>>> SNBEP_PCI_QPI_PORT0_FILTER, >>>> SNBEP_PCI_QPI_PORT1_FILTER, >>>> HSWEP_PCI_PCU_3, >>>> + BDX_PCI_PCU_3, >>>> }; >>> >>> So we use a magic '2' enumerator in the 'QPI Port 2 filter', and that overlaps >>> with HSWEP_PCI_PCU_3, right? >>> >>> Shouldn't we clean up all the enumerators and not use magic numbers, and this fix >>> the conflict? >>> >> >> Yes, it should fix the conflict. I will clean up the code. > > Thanks a lot! > I would appreciate if you could add CC to me when you post the patch. > Here is the patch. Masa, could you please give it a try? Thanks, Kan From 688378a4003ec33156958a52dc822105c18075af Mon Sep 17 00:00:00 2001 From: Kan Liang <kan.liang@linux.intel.com> Date: Mon, 16 Jul 2018 04:57:51 -0400 Subject: [PATCH] perf/x86/intel/uncore: Fix hardcode index of Broadwell extra PCI DEV Masa reports that a warning message is shown while CPU hot-removing on Broadwell server. WARNING: CPU: 126 PID: 6 at arch/x86/events/intel/uncore.c:988 uncore_pci_remove+0x10b/0x150 Call Trace: pci_device_remove+0x42/0xd0 device_release_driver_internal+0x148/0x220 pci_stop_bus_device+0x76/0xa0 pci_stop_root_bus+0x44/0x60 acpi_pci_root_remove+0x1f/0x80 acpi_bus_trim+0x57/0x90 acpi_bus_trim+0x2e/0x90 acpi_device_hotplug+0x2bc/0x4b0 acpi_hotplug_work_fn+0x1a/0x30 process_one_work+0x174/0x3a0 worker_thread+0x4c/0x3d0 kthread+0xf8/0x130 This bug was introduced in: commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs") The index of "QPI Port 2 filter" was hardcode to 2. The index of "PCU.3" used enumerator "HSWEP_PCI_PCU_3", which equals to 2 as well. To fix the conflict, the hardcode index needs to be cleaned up. Introduce a new enumerator "BDX_PCI_QPI_PORT2_FILTER" for "QPI Port 2 filter" on Broadwell, and increase the UNCORE_EXTRA_PCI_DEV_MAX. Clean up hardcode index. Reported-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Suggested-by: Ingo Molnar <mingo@kernel.org> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> Fixes: 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs") --- arch/x86/events/intel/uncore.h | 2 +- arch/x86/events/intel/uncore_snbep.c | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h index c9e1e0b..e17ab88 100644 --- a/arch/x86/events/intel/uncore.h +++ b/arch/x86/events/intel/uncore.h @@ -28,7 +28,7 @@ #define UNCORE_PCI_DEV_TYPE(data) ((data >> 8) & 0xff) #define UNCORE_PCI_DEV_IDX(data) (data & 0xff) #define UNCORE_EXTRA_PCI_DEV 0xff -#define UNCORE_EXTRA_PCI_DEV_MAX 3 +#define UNCORE_EXTRA_PCI_DEV_MAX 4 #define UNCORE_EVENT_CONSTRAINT(c, n) EVENT_CONSTRAINT(c, n, 0xff) diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c index 87dc026..51d7c11 100644 --- a/arch/x86/events/intel/uncore_snbep.c +++ b/arch/x86/events/intel/uncore_snbep.c @@ -1029,6 +1029,7 @@ void snbep_uncore_cpu_init(void) enum { SNBEP_PCI_QPI_PORT0_FILTER, SNBEP_PCI_QPI_PORT1_FILTER, + BDX_PCI_QPI_PORT2_FILTER, HSWEP_PCI_PCU_3, }; @@ -3286,15 +3287,18 @@ static const struct pci_device_id bdx_uncore_pci_ids[] = { }, { /* QPI Port 0 filter */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f86), - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 0), + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, + SNBEP_PCI_QPI_PORT0_FILTER), }, { /* QPI Port 1 filter */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f96), - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 1), + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, + SNBEP_PCI_QPI_PORT1_FILTER), }, { /* QPI Port 2 filter */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, + BDX_PCI_QPI_PORT2_FILTER), }, { /* PCU.3 (for Capability registers) */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), -- 2.4.11 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs 2018-07-16 16:31 ` Liang, Kan @ 2018-07-17 16:37 ` Masayoshi Mizuma 2018-07-27 17:00 ` Masayoshi Mizuma 0 siblings, 1 reply; 9+ messages in thread From: Masayoshi Mizuma @ 2018-07-17 16:37 UTC (permalink / raw) To: kan.liang, mingo, kan.liang; +Cc: tglx, mingo, hpa, x86, linux-kernel, m.mizuma On 07/16/2018 12:31 PM, Liang, Kan wrote: > > > On 7/16/2018 11:07 AM, Masayoshi Mizuma wrote: >> >> >> On 07/16/2018 10:29 AM, Liang, Kan wrote: >>> >>> >>> On 7/15/2018 6:34 PM, Ingo Molnar wrote: >>>> >>>> * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: >>>> >>>>> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> >>>>> >>>>> commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for >>>>> Broadwell CPUs") introduced PCU.3 for Broadwell CPU. Unfortunately, >>>>> the driver_data of PCU.3 conflicts to QPI Port 2 filter. >>>>> >>>>> { /* QPI Port 2 filter */ >>>>> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), >>>>> .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), >>>>> >>>>> { /* PCU.3 (for Capability registers) */ >>>>> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), >>>>> .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, >>>>> HSWEP_PCI_PCU_3), >>>>> // HSWEP_PCI_PCU_3 == 2 >>>> >>>>> --- a/arch/x86/events/intel/uncore_snbep.c >>>>> +++ b/arch/x86/events/intel/uncore_snbep.c >>>>> @@ -1030,6 +1030,7 @@ enum { >>>>> SNBEP_PCI_QPI_PORT0_FILTER, >>>>> SNBEP_PCI_QPI_PORT1_FILTER, >>>>> HSWEP_PCI_PCU_3, >>>>> + BDX_PCI_PCU_3, >>>>> }; >>>> >>>> So we use a magic '2' enumerator in the 'QPI Port 2 filter', and that overlaps >>>> with HSWEP_PCI_PCU_3, right? >>>> >>>> Shouldn't we clean up all the enumerators and not use magic numbers, and this fix >>>> the conflict? >>>> >>> >>> Yes, it should fix the conflict. I will clean up the code. >> >> Thanks a lot! >> I would appreciate if you could add CC to me when you post the patch. >> > > Here is the patch. > > Masa, could you please give it a try? Thank you for the patch, it works well! Please feel free to add: Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks, Masa > > Thanks, > Kan > > From 688378a4003ec33156958a52dc822105c18075af Mon Sep 17 00:00:00 2001 > From: Kan Liang <kan.liang@linux.intel.com> > Date: Mon, 16 Jul 2018 04:57:51 -0400 > Subject: [PATCH] perf/x86/intel/uncore: Fix hardcode index of Broadwell extra PCI DEV > > Masa reports that a warning message is shown while CPU hot-removing on > Broadwell server. > > WARNING: CPU: 126 PID: 6 at arch/x86/events/intel/uncore.c:988 > uncore_pci_remove+0x10b/0x150 > Call Trace: > pci_device_remove+0x42/0xd0 > device_release_driver_internal+0x148/0x220 > pci_stop_bus_device+0x76/0xa0 > pci_stop_root_bus+0x44/0x60 > acpi_pci_root_remove+0x1f/0x80 > acpi_bus_trim+0x57/0x90 > acpi_bus_trim+0x2e/0x90 > acpi_device_hotplug+0x2bc/0x4b0 > acpi_hotplug_work_fn+0x1a/0x30 > process_one_work+0x174/0x3a0 > worker_thread+0x4c/0x3d0 > kthread+0xf8/0x130 > > This bug was introduced in: > > commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for > Broadwell CPUs") > > The index of "QPI Port 2 filter" was hardcode to 2. The index of > "PCU.3" used enumerator "HSWEP_PCI_PCU_3", which equals to 2 as well. > > To fix the conflict, the hardcode index needs to be cleaned up. > Introduce a new enumerator "BDX_PCI_QPI_PORT2_FILTER" for "QPI Port 2 > filter" on Broadwell, and increase the UNCORE_EXTRA_PCI_DEV_MAX. > Clean up hardcode index. > > Reported-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> > Suggested-by: Ingo Molnar <mingo@kernel.org> > Signed-off-by: Kan Liang <kan.liang@linux.intel.com> > Fixes: 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for > Broadwell CPUs") > --- > arch/x86/events/intel/uncore.h | 2 +- > arch/x86/events/intel/uncore_snbep.c | 10 +++++++--- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h > index c9e1e0b..e17ab88 100644 > --- a/arch/x86/events/intel/uncore.h > +++ b/arch/x86/events/intel/uncore.h > @@ -28,7 +28,7 @@ > #define UNCORE_PCI_DEV_TYPE(data) ((data >> 8) & 0xff) > #define UNCORE_PCI_DEV_IDX(data) (data & 0xff) > #define UNCORE_EXTRA_PCI_DEV 0xff > -#define UNCORE_EXTRA_PCI_DEV_MAX 3 > +#define UNCORE_EXTRA_PCI_DEV_MAX 4 > > #define UNCORE_EVENT_CONSTRAINT(c, n) EVENT_CONSTRAINT(c, n, 0xff) > > diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c > index 87dc026..51d7c11 100644 > --- a/arch/x86/events/intel/uncore_snbep.c > +++ b/arch/x86/events/intel/uncore_snbep.c > @@ -1029,6 +1029,7 @@ void snbep_uncore_cpu_init(void) > enum { > SNBEP_PCI_QPI_PORT0_FILTER, > SNBEP_PCI_QPI_PORT1_FILTER, > + BDX_PCI_QPI_PORT2_FILTER, > HSWEP_PCI_PCU_3, > }; > > @@ -3286,15 +3287,18 @@ static const struct pci_device_id bdx_uncore_pci_ids[] = { > }, > { /* QPI Port 0 filter */ > PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f86), > - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 0), > + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, > + SNBEP_PCI_QPI_PORT0_FILTER), > }, > { /* QPI Port 1 filter */ > PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f96), > - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 1), > + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, > + SNBEP_PCI_QPI_PORT1_FILTER), > }, > { /* QPI Port 2 filter */ > PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), > - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), > + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, > + BDX_PCI_QPI_PORT2_FILTER), > }, > { /* PCU.3 (for Capability registers) */ > PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs 2018-07-17 16:37 ` Masayoshi Mizuma @ 2018-07-27 17:00 ` Masayoshi Mizuma 2018-07-30 10:06 ` Ingo Molnar 0 siblings, 1 reply; 9+ messages in thread From: Masayoshi Mizuma @ 2018-07-27 17:00 UTC (permalink / raw) To: mingo; +Cc: kan.liang, kan.liang, tglx, mingo, hpa, x86, linux-kernel, m.mizuma Hi Ingo, Is the following Kan's patch ready to merge...? Thanks, Masa On 07/17/2018 12:37 PM, Masayoshi Mizuma wrote: ... >> Here is the patch. >> >> Masa, could you please give it a try? > > Thank you for the patch, it works well! > > Please feel free to add: > > Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> > > Thanks, > Masa > >> >> Thanks, >> Kan >> >> From 688378a4003ec33156958a52dc822105c18075af Mon Sep 17 00:00:00 2001 >> From: Kan Liang <kan.liang@linux.intel.com> >> Date: Mon, 16 Jul 2018 04:57:51 -0400 >> Subject: [PATCH] perf/x86/intel/uncore: Fix hardcode index of Broadwell extra PCI DEV >> >> Masa reports that a warning message is shown while CPU hot-removing on >> Broadwell server. >> >> WARNING: CPU: 126 PID: 6 at arch/x86/events/intel/uncore.c:988 >> uncore_pci_remove+0x10b/0x150 >> Call Trace: >> pci_device_remove+0x42/0xd0 >> device_release_driver_internal+0x148/0x220 >> pci_stop_bus_device+0x76/0xa0 >> pci_stop_root_bus+0x44/0x60 >> acpi_pci_root_remove+0x1f/0x80 >> acpi_bus_trim+0x57/0x90 >> acpi_bus_trim+0x2e/0x90 >> acpi_device_hotplug+0x2bc/0x4b0 >> acpi_hotplug_work_fn+0x1a/0x30 >> process_one_work+0x174/0x3a0 >> worker_thread+0x4c/0x3d0 >> kthread+0xf8/0x130 >> >> This bug was introduced in: >> >> commit 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for >> Broadwell CPUs") >> >> The index of "QPI Port 2 filter" was hardcode to 2. The index of >> "PCU.3" used enumerator "HSWEP_PCI_PCU_3", which equals to 2 as well. >> >> To fix the conflict, the hardcode index needs to be cleaned up. >> Introduce a new enumerator "BDX_PCI_QPI_PORT2_FILTER" for "QPI Port 2 >> filter" on Broadwell, and increase the UNCORE_EXTRA_PCI_DEV_MAX. >> Clean up hardcode index. >> >> Reported-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> >> Suggested-by: Ingo Molnar <mingo@kernel.org> >> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> >> Fixes: 15a3e845b01c ("perf/x86/intel/uncore: Fix SBOX support for >> Broadwell CPUs") >> --- >> arch/x86/events/intel/uncore.h | 2 +- >> arch/x86/events/intel/uncore_snbep.c | 10 +++++++--- >> 2 files changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h >> index c9e1e0b..e17ab88 100644 >> --- a/arch/x86/events/intel/uncore.h >> +++ b/arch/x86/events/intel/uncore.h >> @@ -28,7 +28,7 @@ >> #define UNCORE_PCI_DEV_TYPE(data) ((data >> 8) & 0xff) >> #define UNCORE_PCI_DEV_IDX(data) (data & 0xff) >> #define UNCORE_EXTRA_PCI_DEV 0xff >> -#define UNCORE_EXTRA_PCI_DEV_MAX 3 >> +#define UNCORE_EXTRA_PCI_DEV_MAX 4 >> >> #define UNCORE_EVENT_CONSTRAINT(c, n) EVENT_CONSTRAINT(c, n, 0xff) >> >> diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c >> index 87dc026..51d7c11 100644 >> --- a/arch/x86/events/intel/uncore_snbep.c >> +++ b/arch/x86/events/intel/uncore_snbep.c >> @@ -1029,6 +1029,7 @@ void snbep_uncore_cpu_init(void) >> enum { >> SNBEP_PCI_QPI_PORT0_FILTER, >> SNBEP_PCI_QPI_PORT1_FILTER, >> + BDX_PCI_QPI_PORT2_FILTER, >> HSWEP_PCI_PCU_3, >> }; >> >> @@ -3286,15 +3287,18 @@ static const struct pci_device_id bdx_uncore_pci_ids[] = { >> }, >> { /* QPI Port 0 filter */ >> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f86), >> - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 0), >> + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, >> + SNBEP_PCI_QPI_PORT0_FILTER), >> }, >> { /* QPI Port 1 filter */ >> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f96), >> - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 1), >> + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, >> + SNBEP_PCI_QPI_PORT1_FILTER), >> }, >> { /* QPI Port 2 filter */ >> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46), >> - .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2), >> + .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, >> + BDX_PCI_QPI_PORT2_FILTER), >> }, >> { /* PCU.3 (for Capability registers) */ >> PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0), ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs 2018-07-27 17:00 ` Masayoshi Mizuma @ 2018-07-30 10:06 ` Ingo Molnar 2018-07-30 12:13 ` Liang, Kan 0 siblings, 1 reply; 9+ messages in thread From: Ingo Molnar @ 2018-07-30 10:06 UTC (permalink / raw) To: Masayoshi Mizuma Cc: kan.liang, kan.liang, tglx, mingo, hpa, x86, linux-kernel, m.mizuma * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: > Hi Ingo, > > Is the following Kan's patch ready to merge...? Looks good at first sight - but it was whitespace damaged here so I couldn't apply it. Kan, mind re-sending it properly as a standalone patch, with Masayoshi's Reported-by, Debugged-by and Tested-by tags added? Thanks, Ingo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs 2018-07-30 10:06 ` Ingo Molnar @ 2018-07-30 12:13 ` Liang, Kan 0 siblings, 0 replies; 9+ messages in thread From: Liang, Kan @ 2018-07-30 12:13 UTC (permalink / raw) To: Ingo Molnar, Masayoshi Mizuma Cc: kan.liang, tglx, mingo, hpa, x86, linux-kernel, m.mizuma On 7/30/2018 6:06 AM, Ingo Molnar wrote: > > * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: > >> Hi Ingo, >> >> Is the following Kan's patch ready to merge...? > > Looks good at first sight - but it was whitespace damaged here so I couldn't apply it. > > Kan, mind re-sending it properly as a standalone patch, with Masayoshi's Reported-by, Debugged-by and Tested-by > tags added? > Sure, I will re-send the patch. Thanks, Kan ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-07-30 12:13 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-10 23:31 [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3 Broadwell CPUs Masayoshi Mizuma 2018-07-15 22:34 ` Ingo Molnar 2018-07-16 14:29 ` Liang, Kan 2018-07-16 15:07 ` Masayoshi Mizuma 2018-07-16 16:31 ` Liang, Kan 2018-07-17 16:37 ` Masayoshi Mizuma 2018-07-27 17:00 ` Masayoshi Mizuma 2018-07-30 10:06 ` Ingo Molnar 2018-07-30 12:13 ` Liang, Kan
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).