From: Bjorn Helgaas <helgaas@kernel.org>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: "Baolu Lu" <baolu.lu@linux.intel.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Joerg Roedel" <jroedel@suse.de>,
"Matt Fagnani" <matt.fagnani@bell.net>,
"Christian König" <christian.koenig@amd.com>,
"Kevin Tian" <kevin.tian@intel.com>,
"Vasant Hegde" <vasant.hegde@amd.com>,
"Tony Zhu" <tony.zhu@intel.com>,
linux-pci@vger.kernel.org, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org,
"Alex Williamson" <alex.williamson@redhat.com>
Subject: Re: [PATCH v3 1/1] PCI: Add translated request only flag for pci_enable_pasid()
Date: Fri, 3 Feb 2023 12:20:45 -0600 [thread overview]
Message-ID: <20230203182045.GA1972366@bhelgaas> (raw)
In-Reply-To: <Y9wg0Znc0tRWj4O9@nvidia.com>
[+cc Alex in case you're interested in the ACS angle]
On Thu, Feb 02, 2023 at 04:45:05PM -0400, Jason Gunthorpe wrote:
> On Thu, Feb 02, 2023 at 02:12:49PM -0600, Bjorn Helgaas wrote:
> > On Thu, Feb 02, 2023 at 11:08:25AM +0800, Baolu Lu wrote:
> > > ...
> >
> > > ACS is unnecessary for the devices that only use translated
> > > memory request for PASID. All translated addresses are granted
> > > by the Linux kernel which ensures that such addresses will never
> > > be in a P2P address, i.e., it's not contained in any bridge
> > > aperture, will *always* be routed toward the RC.
> >
> > Re 201007ef707a ("PCI: Enable PASID only when ACS RR & UF enabled
> > on upstream path"), does that commit actually *fix* anything? I
> > wonder whether we could revert it completely.
> >
> > The intent of 201007ef707a is to use ACS to prevent misrouting,
> > which would happen if a TLP contained an address that *looked*
> > like a PCI bus address, i.e., it was inside a host bridge
> > aperture, but was *intended* to reach an IOMMU or main memory
> > directly.
>
> Yes.
>
> > 201007ef707a only affects pci_enable_pasid(), so I think we
> > already avoid this misrouting by restricting DMA address
> > allocation for both non-IOMMU scenarios and non-PASID IOMMU
> > scenarios.
>
> There is no restriction on DMA address allocation with PASID.
>
> The typical PASID use case is to point the PASID at the CPU page
> table and then all VA's are fair game by userspace. There is no
> carve out like the DMA API has to protect from bus address
> confusion.
I think you're saying that for (Requester ID, PASID, Untranslated
Address), the Untranslated Address is not restricted at all, and it
may look like a PCI bus address.
> > So what about PASID mappings, e.g., consider a mapping of
> > (Requester ID, PASID, Untranslated Address) -> Translated Address?
> > If either the Untranslated Address or the Translated Address looks
> > like a PCI bus address, a Memory Request or Translation Request
> > could be misrouted.
>
> The PCI rules are a bit complicated:
> - A simple MemRd/Wr with a PASID will be routed according to the
> address. This can be mis-routed
> - A ATS translation request with a PASID is always routed to the host
> bridge
From a PCIe point of view, I think these cases are equivalent because
a PASID prefix doesn't affect routing (sec 2.2.10.4). A Translation
Request includes an Untranslated Address, and if that happens to look
like a PCI bus address, I think it will be mis-routed just like a
MemRd/Wr would be.
> - A MemRd/Wr with Translated set and no PASID is always routed to the
> correct destination, even if that is not the host bridge
I don't think Address Type 10b ("Translated") affects routing. A
MemRd/Wr should be routed to a PCI peer if the Translated Address is
inside a host bridge aperture, or to the host bridge otherwise.
> > Do IOMMUs allocate (PASID, Untranslated Addresses) that look like
> > PCI bus addresses?
>
> Yes, because it is mapped to a mm_struct userspace can use any mmap
> to access any valid address as an IOVA and thus PASID tagged
> translation must never become confused with bus addresses.
If PCI bus addresses are carved out of the Translated Address arena,
the MemRd/Wr TLPs should be fine, but I think the Translation Requests
that include Untranslated Addresses are still a problem.
> Further, and worse, the common use model for PASID SVA is for
> userspace to directly submit IOVA to the device for operation. If
> userspace can submit a hostile IOVA and cause DMA to reach something
> that is not the host bridge then system security is completely
> wrecked.
>
> So, as an API in Linux we felt it was best to only enable PASID if
> PASID is secure and truely isolated, otherwise leave PASID off. The
> use cases for insecure PASID seem small.
The patch under discussion is intended to fix a v6.2-rc1 regression
added by 201007ef707a ("PCI: Enable PASID only when ACS RR & UF
enabled on upstream path").
Are we on track to fix this before v6.2? I don't have a clear
understanding of how we know this change is safe and it only affects
AMD GPU and not other devices below the same IOMMU.
Bjorn
next prev parent reply other threads:[~2023-02-03 18:20 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-14 7:34 [PATCH v3 1/1] PCI: Add translated request only flag for pci_enable_pasid() Lu Baolu
2023-01-16 15:42 ` Jason Gunthorpe
2023-01-27 11:30 ` Linux kernel regression tracking (Thorsten Leemhuis)
2023-01-27 17:30 ` Bjorn Helgaas
2023-01-28 7:52 ` Tian, Kevin
2023-01-29 8:42 ` Baolu Lu
2023-01-30 18:38 ` Bjorn Helgaas
2023-01-30 18:47 ` Jason Gunthorpe
2023-01-31 23:50 ` Bjorn Helgaas
2023-02-01 2:28 ` Jason Gunthorpe
2023-01-31 12:25 ` Baolu Lu
2023-02-01 16:58 ` Bjorn Helgaas
2023-02-02 3:08 ` Baolu Lu
2023-02-02 20:12 ` Bjorn Helgaas
2023-02-02 20:45 ` Jason Gunthorpe
2023-02-03 18:20 ` Bjorn Helgaas [this message]
2023-02-03 18:52 ` Jason Gunthorpe
2023-02-06 4:28 ` Tian, Kevin
2023-01-31 12:56 ` Baolu Lu
2023-02-01 0:14 ` Bjorn Helgaas
2023-02-01 2:36 ` Jason Gunthorpe
2023-02-01 14:09 ` Jonathan Cameron
2023-02-01 5:18 ` Vasant Hegde
2023-02-01 5:51 ` Baolu Lu
2023-02-01 5:59 ` Baolu Lu
2023-02-01 6:31 ` Baolu Lu
2023-02-01 14:22 ` Jason Gunthorpe
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=20230203182045.GA1972366@bhelgaas \
--to=helgaas@kernel.org \
--cc=alex.williamson@redhat.com \
--cc=baolu.lu@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=christian.koenig@amd.com \
--cc=iommu@lists.linux.dev \
--cc=jgg@nvidia.com \
--cc=jroedel@suse.de \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=matt.fagnani@bell.net \
--cc=tony.zhu@intel.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 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).