Messages in this thread | | | Subject | Re: [PATCH 1/2] Revert "iommu/amd: Fix performance counter initialization" | From | Shuah Khan <> | Date | Fri, 9 Apr 2021 11:06:22 -0600 |
| |
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
| |