lkml.org 
[lkml]   [2021]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] Revert "iommu/amd: Fix performance counter initialization"
From
Date
On 4/9/21 2:58 AM, Suravee Suthikulpanit wrote:
> From: Paul Menzel <pmenzel@molgen.mpg.de>
>
> This reverts commit 6778ff5b21bd8e78c8bd547fd66437cf2657fd9b.
>
> The original commit tries to address an issue, where PMC power-gating
> causing the IOMMU PMC pre-init test to fail on certain desktop/mobile
> platforms where the power-gating is normally enabled.
>
> There have been several reports that the workaround still does not
> guarantee to work, and can add up to 100 ms (on the worst case)
> to the boot process on certain platforms such as the MSI B350M MORTAR
> with AMD Ryzen 3 2200G.
>
> Therefore, revert this commit as a prelude to removing the pre-init
> test.
>
> Link: https://lore.kernel.org/linux-iommu/alpine.LNX.3.20.13.2006030935570.3181@monopod.intra.ispras.ru/
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201753
> Cc: Tj (Elloe Linux) <ml.linux@elloe.vision>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: Alexander Monakov <amonakov@ispras.ru>
> Cc: David Coe <david.coe@live.co.uk>
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> Note: I have revised the commit message to add more detail
> and remove uncessary information.
>
> drivers/iommu/amd/init.c | 45 ++++++++++------------------------------
> 1 file changed, 11 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 321f5906e6ed..648cdfd03074 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -12,7 +12,6 @@
> #include <linux/acpi.h>
> #include <linux/list.h>
> #include <linux/bitmap.h>
> -#include <linux/delay.h>
> #include <linux/slab.h>
> #include <linux/syscore_ops.h>
> #include <linux/interrupt.h>
> @@ -257,8 +256,6 @@ static enum iommu_init_state init_state = IOMMU_START_STATE;
> static int amd_iommu_enable_interrupts(void);
> static int __init iommu_go_to_state(enum iommu_init_state state);
> static void init_device_table_dma(void);
> -static int iommu_pc_get_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
> - u8 fxn, u64 *value, bool is_write);
>
> static bool amd_iommu_pre_enabled = true;
>
> @@ -1717,11 +1714,13 @@ static int __init init_iommu_all(struct acpi_table_header *table)
> return 0;
> }
>
> -static void __init init_iommu_perf_ctr(struct amd_iommu *iommu)
> +static int iommu_pc_get_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
> + u8 fxn, u64 *value, bool is_write);
> +
> +static void init_iommu_perf_ctr(struct amd_iommu *iommu)
> {
> - int retry;
> struct pci_dev *pdev = iommu->dev;
> - u64 val = 0xabcd, val2 = 0, save_reg, save_src;
> + u64 val = 0xabcd, val2 = 0, save_reg = 0;
>
> if (!iommu_feature(iommu, FEATURE_PC))
> return;
> @@ -1729,39 +1728,17 @@ static void __init init_iommu_perf_ctr(struct amd_iommu *iommu)
> amd_iommu_pc_present = true;
>
> /* save the value to restore, if writable */
> - if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, false) ||
> - iommu_pc_get_set_reg(iommu, 0, 0, 8, &save_src, false))
> - goto pc_false;
> -
> - /*
> - * Disable power gating by programing the performance counter
> - * source to 20 (i.e. counts the reads and writes from/to IOMMU
> - * Reserved Register [MMIO Offset 1FF8h] that are ignored.),
> - * which never get incremented during this init phase.
> - * (Note: The event is also deprecated.)
> - */
> - val = 20;
> - if (iommu_pc_get_set_reg(iommu, 0, 0, 8, &val, true))
> + if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, false))
> goto pc_false;
>
> /* Check if the performance counters can be written to */
> - val = 0xabcd;
> - for (retry = 5; retry; retry--) {
> - if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &val, true) ||
> - iommu_pc_get_set_reg(iommu, 0, 0, 0, &val2, false) ||
> - val2)
> - break;
> -
> - /* Wait about 20 msec for power gating to disable and retry. */
> - msleep(20);
> - }
> -
> - /* restore */
> - if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, true) ||
> - iommu_pc_get_set_reg(iommu, 0, 0, 8, &save_src, true))
> + if ((iommu_pc_get_set_reg(iommu, 0, 0, 0, &val, true)) ||
> + (iommu_pc_get_set_reg(iommu, 0, 0, 0, &val2, false)) ||
> + (val != val2))

Probably don't need parentheses around 'val != val2'

> goto pc_false;
>
> - if (val != val2)
> + /* restore */
> + if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, true))
> goto pc_false;
>
> pci_info(pdev, "IOMMU performance counters supported\n");
>

thanks,
-- Shuah

\
 
 \ /
  Last update: 2021-04-09 19:07    [W:0.149 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site