From: Joerg Roedel <[email protected]>
The polling loop for the register change in iommu_ga_log_enable() needs
to have a udelay() in it. Otherwise the CPU might be faster than the
IOMMU hardware and wrongly trigger the WARN_ON() further down the code
stream.
Fixes: 8bda0cfbdc1a ("iommu/amd: Detect and initialize guest vAPIC log")
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/amd/init.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index dc338acf3338..d2e09d53851f 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -834,6 +834,7 @@ static int iommu_ga_log_enable(struct amd_iommu *iommu)
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
if (status & (MMIO_STATUS_GALOG_RUN_MASK))
break;
+ udelay(1);
}
if (WARN_ON(i >= LOOP_TIMEOUT))
--
2.34.1
On 31/01/2022 16:17, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> The polling loop for the register change in iommu_ga_log_enable() needs
> to have a udelay() in it. Otherwise the CPU might be faster than the
> IOMMU hardware and wrongly trigger the WARN_ON() further down the code
> stream.
>
> Fixes: 8bda0cfbdc1a ("iommu/amd: Detect and initialize guest vAPIC log")
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/amd/init.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index dc338acf3338..d2e09d53851f 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -834,6 +834,7 @@ static int iommu_ga_log_enable(struct amd_iommu *iommu)
> status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> if (status & (MMIO_STATUS_GALOG_RUN_MASK))
> break;
> + udelay(1);
Maybe readl_relaxed_poll_timeout_atomic() could be used instead
Thanks,
John
> }
>
> if (WARN_ON(i >= LOOP_TIMEOUT))
>
Hi Joerg,
I love your patch! Yet something to improve:
[auto build test ERROR on joro-iommu/next]
[also build test ERROR on v5.17-rc2 next-20220131]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Joerg-Roedel/iommu-amd-Fix-loop-timeout-issue-in-iommu_ga_log_enable/20220201-002214
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220201/[email protected]/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/5b5785526da0e2a6e793d2107d09afc9f310f17f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Joerg-Roedel/iommu-amd-Fix-loop-timeout-issue-in-iommu_ga_log_enable/20220201-002214
git checkout 5b5785526da0e2a6e793d2107d09afc9f310f17f
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/iommu/amd/init.c: In function 'iommu_ga_log_enable':
>> drivers/iommu/amd/init.c:837:3: error: implicit declaration of function 'udelay' [-Werror=implicit-function-declaration]
837 | udelay(1);
| ^~~~~~
cc1: some warnings being treated as errors
vim +/udelay +837 drivers/iommu/amd/init.c
804
805 static int iommu_ga_log_enable(struct amd_iommu *iommu)
806 {
807 #ifdef CONFIG_IRQ_REMAP
808 u32 status, i;
809 u64 entry;
810
811 if (!iommu->ga_log)
812 return -EINVAL;
813
814 /* Check if already running */
815 status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
816 if (WARN_ON(status & (MMIO_STATUS_GALOG_RUN_MASK)))
817 return 0;
818
819 entry = iommu_virt_to_phys(iommu->ga_log) | GA_LOG_SIZE_512;
820 memcpy_toio(iommu->mmio_base + MMIO_GA_LOG_BASE_OFFSET,
821 &entry, sizeof(entry));
822 entry = (iommu_virt_to_phys(iommu->ga_log_tail) &
823 (BIT_ULL(52)-1)) & ~7ULL;
824 memcpy_toio(iommu->mmio_base + MMIO_GA_LOG_TAIL_OFFSET,
825 &entry, sizeof(entry));
826 writel(0x00, iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
827 writel(0x00, iommu->mmio_base + MMIO_GA_TAIL_OFFSET);
828
829
830 iommu_feature_enable(iommu, CONTROL_GAINT_EN);
831 iommu_feature_enable(iommu, CONTROL_GALOG_EN);
832
833 for (i = 0; i < LOOP_TIMEOUT; ++i) {
834 status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
835 if (status & (MMIO_STATUS_GALOG_RUN_MASK))
836 break;
> 837 udelay(1);
838 }
839
840 if (WARN_ON(i >= LOOP_TIMEOUT))
841 return -EINVAL;
842 #endif /* CONFIG_IRQ_REMAP */
843 return 0;
844 }
845
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Mon, Jan 31, 2022 at 05:06:03PM +0000, John Garry wrote:
> > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> > index dc338acf3338..d2e09d53851f 100644
> > --- a/drivers/iommu/amd/init.c
> > +++ b/drivers/iommu/amd/init.c
> > @@ -834,6 +834,7 @@ static int iommu_ga_log_enable(struct amd_iommu *iommu)
> > status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> > if (status & (MMIO_STATUS_GALOG_RUN_MASK))
> > break;
> > + udelay(1);
>
> Maybe readl_relaxed_poll_timeout_atomic() could be used instead
I sent another version of this patch which uses
readl_poll_timeout_atomic(), but it didn't fix the issue. I take this
approach for now and leave using the helper as a future improvement.
Thanks,
Joerg