| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 17 Jul 2018 12:37:49 -0400
From: Masayoshi Mizuma <msys.mizuma@...il.com>
To: kan.liang@...ux.intel.com, mingo@...nel.org, kan.liang@...el.com
Cc: tglx@...utronix.de, mingo@...hat.com, hpa@...or.com,
x86@...nel.org, linux-kernel@...r.kernel.org,
m.mizuma@...fujitsu.com
Subject: Re: [PATCH] [RESEND] perf/x86/intel/uncore: Fix the index of PCU.3
Broadwell CPUs
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@...il.com> wrote:
>>>>
>>>>> From: Masayoshi Mizuma <m.mizuma@...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@...fujitsu.com>
Thanks,
Masa
>
> Thanks,
> Kan
>
> From 688378a4003ec33156958a52dc822105c18075af Mon Sep 17 00:00:00 2001
> From: Kan Liang <kan.liang@...ux.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@...fujitsu.com>
> Suggested-by: Ingo Molnar <mingo@...nel.org>
> Signed-off-by: Kan Liang <kan.liang@...ux.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),
Powered by blists - more mailing lists