LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Luigi Rizzo <lrizzo@google.com>
To: Joerg Roedel <joro@8bytes.org>
Cc: Will Deacon <will@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	David Rientjes <rientjes@google.com>,
	Luigi Rizzo <rizzo.unipi@gmail.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Subject: Re: [PATCH] amd/iommu: fix logic bug in amd_iommu_report_page_fault()
Date: Mon, 2 Aug 2021 16:30:50 +0200	[thread overview]
Message-ID: <CAMOZA0LEr+xM6RrsJErPMqHP7-0GdLmNDqbGVKbKTn92=Ncejg@mail.gmail.com> (raw)
In-Reply-To: <YQf9h+qvWCx6D7XT@8bytes.org>

On Mon, Aug 2, 2021 at 4:13 PM Joerg Roedel <joro@8bytes.org> wrote:
>
> On Sat, Jul 31, 2021 at 12:26:37PM -0700, Luigi Rizzo wrote:
> > amd_iommu_report_page_fault() has two print paths, depending on whether or
> > not it can find a pci device. But the code erroneously enters the second
> > path if the rate limiter in the first path triggers:
> >   if (dev_data && ratelimit(A)) { A; } else if (ratelimit(B)) { B; }
> > The correct code should be
> >   if (dev_data) { if (ratelimit(A)) { A;} } else if (ratelimit(B)) { B; }
> >
> > Signed-off-by: Luigi Rizzo <lrizzo@google.com>
> > ---
> >  drivers/iommu/amd/iommu.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
>
> Thanks, but I queued this patch already:
>
>         https://lore.kernel.org/r/YPgk1dD1gPMhJXgY@wantstofly.org


Ah didn't realize that. Thank you!

Two questions on the topic:
1. how comes only the AMD driver is so verbose in reporting io page faults?
   Neither intel nor other iommu drivers seem to log anything

2. Would it make sense to have a control to disable such logging,
   either per-device or globally? Eg something like this (negative
   logic so it must be set explicitly to disable logging).


@ -985,6 +985,7 @@ struct device {
     defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
        bool                    dma_coherent:1;
 #endif
+       bool                    no_log_fault:1;

...
+               if (!pdev->dev.no_log_fault && __ratelimit(&dev_data->rs))
+                       dev_err(&pdev->dev, "Event logged [IO_PAGE_FAULT dom


cheers
luigi

  reply	other threads:[~2021-08-02 14:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-31 19:26 Luigi Rizzo
2021-08-02  3:52 ` David Rientjes
2021-08-02 14:13 ` Joerg Roedel
2021-08-02 14:30   ` Luigi Rizzo [this message]
2021-08-02 14:38     ` Joerg Roedel
2021-08-02 15:26       ` Luigi Rizzo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMOZA0LEr+xM6RrsJErPMqHP7-0GdLmNDqbGVKbKTn92=Ncejg@mail.gmail.com' \
    --to=lrizzo@google.com \
    --cc=joro@8bytes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rientjes@google.com \
    --cc=rizzo.unipi@gmail.com \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=will@kernel.org \
    --subject='Re: [PATCH] amd/iommu: fix logic bug in amd_iommu_report_page_fault()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).