From: Jason Gunthorpe <jgg@nvidia.com>
To: Pranjal Shrivastava <praan@google.com>
Cc: iommu@lists.linux.dev, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Joerg Roedel <joro@8bytes.org>,
Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
Vasant Hegde <vasant.hegde@amd.com>,
Ankit Soni <ankit.soni@amd.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Samiullah Khawaja <skhawaja@google.com>,
sashiko-bot@kernel.org
Subject: Re: [PATCH 2/6] iommu/amd: Clear DTE with update_dte256 in iommu_ignore_device()
Date: Mon, 1 Jun 2026 11:19:56 -0300 [thread overview]
Message-ID: <20260601141956.GQ3195266@nvidia.com> (raw)
In-Reply-To: <20260601134204.2150602-3-praan@google.com>
On Mon, Jun 01, 2026 at 01:42:00PM +0000, Pranjal Shrivastava wrote:
> The iommu_ignore_device() function currently uses memset() to clear
> the Device Table Entry (DTE), which risks torn writes because the
> hardware reads DTEs as atomic 256-bit qwords. Fix this by using
> update_dte256() to perform a hardware-safe atomic clear when a live
> dev_data entry is available.
>
> Fixes: 99fc4ac3d297 ("iommu/amd: Introduce per PCI segment alias_table")
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/all/20260529153216.2AD1E1F00899@smtp.kernel.org/
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
> drivers/iommu/amd/iommu.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index a94de66a885e..9b5861e241d7 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -750,7 +750,16 @@ static void iommu_ignore_device(struct amd_iommu *iommu, struct device *dev)
> setup_aliases(iommu, dev);
>
> pci_seg->rlookup_table[devid] = NULL;
> - memset(&dev_table[devid], 0, sizeof(struct dev_table_entry));
> +
> + /* Clear DTE if we have a live entry */
> + if (dev_data) {
> + struct dev_table_entry new = {};
> +
> + amd_iommu_make_clear_dte(dev_data, &new);
> + update_dte256(iommu, dev_data, &new);
> + } else {
> + memset(&dev_table[devid], 0, sizeof(struct dev_table_entry));
> + }
This seems a little weird, an ignored device shouldn't have a dev_data
really, or it will soon be freed.
I think you are better to replace the memset with a dedicated function
/* Cannot not be used on a probe'd device with a live dev_data */
disable_dte(..)
{
struct dev_table_entry new = {};
write_dte_lower128(ptr, new);
write_dte_upper128(ptr, new);
}
And then this new ordering breaks the clone_aliases flow, it was
supposed to copy the 0 from the current DTE to the aliases..
Jason
next prev parent reply other threads:[~2026-06-01 14:20 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-01 13:41 [PATCH 0/6] iommu/amd: Refactors for ATS updates Pranjal Shrivastava
2026-06-01 13:41 ` [PATCH 1/6] iommu/amd: Clear aliases before setting the rlookup_table to NULL Pranjal Shrivastava
2026-06-01 13:57 ` sashiko-bot
2026-06-01 13:42 ` [PATCH 2/6] iommu/amd: Clear DTE with update_dte256 in iommu_ignore_device() Pranjal Shrivastava
2026-06-01 14:09 ` sashiko-bot
2026-06-01 14:19 ` Jason Gunthorpe [this message]
2026-06-02 4:01 ` Pranjal Shrivastava
2026-06-02 8:55 ` Vasant Hegde
2026-06-01 13:42 ` [PATCH 3/6] iommu/amd: Split probe error paths to preserve IRQ remapping Pranjal Shrivastava
2026-06-01 14:30 ` sashiko-bot
2026-06-01 13:42 ` [PATCH 4/6] iommu/amd: Fix Use-After-Free in non-fatal probe error path Pranjal Shrivastava
2026-06-01 14:44 ` sashiko-bot
2026-06-01 13:42 ` [PATCH 5/6] iommu/amd: Fail probe on ATS configuration failure Pranjal Shrivastava
2026-06-01 15:01 ` sashiko-bot
2026-06-01 15:05 ` Pranjal Shrivastava
2026-06-01 13:42 ` [PATCH 6/6] PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats() Pranjal Shrivastava
2026-06-01 19:06 ` sashiko-bot
2026-06-02 8:49 ` [PATCH 0/6] iommu/amd: Refactors for ATS updates Vasant Hegde
2026-06-02 12:08 ` Jason Gunthorpe
2026-06-04 8:23 ` Vasant Hegde
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=20260601141956.GQ3195266@nvidia.com \
--to=jgg@nvidia.com \
--cc=ankit.soni@amd.com \
--cc=bhelgaas@google.com \
--cc=iommu@lists.linux.dev \
--cc=joro@8bytes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=praan@google.com \
--cc=sashiko-bot@kernel.org \
--cc=skhawaja@google.com \
--cc=suravee.suthikulpanit@amd.com \
--cc=vasant.hegde@amd.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.