Hi Lennert,

On 7/18/2021 7:47 PM, Lennert Buytenhek wrote:
On an AMD system, I/O page faults are usually logged like this:
....
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 811a49a95d04..7ae426b092f2 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -483,7 +483,7 @@ static void amd_iommu_report_page_fault(u16 devid, u16 
domain_id,
        if (dev_data && __ratelimit(&dev_data->rs)) {
                pci_err(pdev, "Event logged [IO_PAGE_FAULT domain=0x%04x 
address=0x%llx flags=0x%04x]\n",
                        domain_id, address, flags);
-       } else if (printk_ratelimit()) {
+       } else if (!dev_data && printk_ratelimit()) {

This seems a bit confusing. Also, according to the following comment in 
include/linux/printk.h:

/*
 * Please don't use printk_ratelimit(), because it shares ratelimiting state
 * with all other unrelated printk_ratelimit() callsites.  Instead use
 * printk_ratelimited() or plain old __ratelimit().
 */

We probably should move away from using printk_ratelimit() here.
What about the following change instead?

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 811a49a95d04..8eb5d3519743 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -480,11 +480,12 @@ static void amd_iommu_report_page_fault(u16 devid, u16 
domain_id,
        if (pdev)
                dev_data = dev_iommu_priv_get(&pdev->dev);

-       if (dev_data && __ratelimit(&dev_data->rs)) {
-               pci_err(pdev, "Event logged [IO_PAGE_FAULT domain=0x%04x 
address=0x%llx flags=0x%04x]\n",
-                       domain_id, address, flags);
-       } else if (printk_ratelimit()) {
-               pr_err("Event logged [IO_PAGE_FAULT device=%02x:%02x.%x 
domain=0x%04x address=0x%llx flags=0x%04x]\n",
+       if (dev_data) {
+               if (__ratelimit(&dev_data->rs))
+                       pci_err(pdev, "Event logged [IO_PAGE_FAULT domain=0x%04x 
address=0x%llx flags=0x%04x]\n",
+                               domain_id, address, flags);
+       } else {
+               pr_err_ratelimited("Event logged [IO_PAGE_FAULT device=%02x:%02x.%x 
domain=0x%04x address=0x%llx flags=0x%04x]\n",
                        PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
                        domain_id, address, flags);
        }

Note also that there might be other places in this file that would need similar 
modification as well.

Thanks,
Suravee

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to