All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: Jason Wang <jasowang@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Virtio-Dev <virtio-dev@lists.oasis-open.org>,
	eperezma <eperezma@redhat.com>, Cindy Lu <lulu@redhat.com>
Subject: Re: [virtio-dev] Re: [PATCH V2 2/2] virtio-pci: add PASID configuration extended capability
Date: Mon, 24 Jan 2022 17:15:33 -0500	[thread overview]
Message-ID: <20220124170534-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <Ye7FK6BLPSNLunPv@myrica>

On Mon, Jan 24, 2022 at 03:26:35PM +0000, Jean-Philippe Brucker wrote:
> On Wed, Jan 19, 2022 at 06:53:17PM -0500, Michael S. Tsirkin wrote:
> > > I think we should be explicit about this particular case because someone
> > > implementing this extension might get it wrong. MSIs should not have a
> > > PASID because IOMMUs don't support it:
> > > 
> > > * VT-d treats Requests-with-PASID to the special range 0xfeexxxxx as
> > >   normal DMA (3.14 Handling Requests to Interrupt Address Range)
> > > * AMD IOMMU reports an error for MSIs with PASID (2.2.7.7 PCIe® TLP PASID
> > >   Prefix).
> > 
> > Ouch. I didn't realize. Really weird, of the "what were they thinking"
> > variety. The natural thing would be to have PASID==VM and scope
> > both DMA and MSI within PASID. I guess that is precluded on these
> > platforms then.
> > 
> 
> I think the problem is mainly that MSIs appear as normal memory writes
> requests. If PCI had given them a special type, IOMMU/IRQ remapping
> hardware could route them more easily, but as is they only have the
> transaction's address to work with. So vendors made different choices for
> supporting MSIs with their IRQ remapping hardware. Intel and AMD define
> reserved address ranges 0xfeexxxxx, where any memory transaction is
> treated as MSI.
> 
> This is fine when the kernel is in charge of allocating the I/O address
> space for DMA, it just needs to make sure IOVAs are not allocated within
> the address region reserved for MSIs. But it's more complicated with SVA.
> 
> One of the use of PASID is assigning a partition of device (here a group
> of virtqueues) to a process. With this approach (SVA) the PASID accesses
> the process' page tables, so the virtual address space is shared between
> CPU and IOMMU, IOVA==VA. How would we deal with MSIs in this case, if they
> had a PASID?

Basically what I would do is what ARM has done ;)


>  On x86, if the IOMMU performed IRQ remapping instead of
> address translation, on a memory write with PASID to address 0xfeexxxxx,
> the process couldn't use any address in that range for DMA. The process
> would need to filter addresses returned by malloc() and treat some of them
> as non-DMA'able, which defeats the purpose of SVA. To avoid having to do
> this Intel treats all DMA with PASID as normal memory access, and the
> process can't accidentally trigger MSIs by using the wrong address. I
> think AMD behaves the same way but am not entirely sure - the wording in
> one part of the spec (2.2.7.7) seems to imply that PASID access to the MSI
> range would trigger an IOMMU error report, in which case the range would
> need carving out anyway.
> 
> > > * Arm requires creating mappings to the MSI controller in the SMMU. This
> > >   has implications for SVA where the PASID accesses the whole process
> > >   address space: if MSI transactions have a PASID prefix, that requires
> > >   mapping the MSI controller into the process address space on Arm, which
> > >   isn't convenient.
> > 
> > I'd like to understand this part a bit better. Can you explain?
> > We are talking about the IOMMU address space right?
> > What is wrong with mapping the MSI controller there?
> > Isn't convenient in what sense?
> 
> On Arm the IRQ remapping device is separate from the IOMMU, so there is no
> special address range as far as the IOMMU is concerned. An MSI goes
> through IOMMU address translation like any other memory write, the IOVA
> gets translated into a PA that corresponds to the doorbell (MMIO) address
> of the IRQ remapping device. The IOVA of the MSI is chosen by the OS and
> can have any value. Now if MSIs had a PASID, then the IOMMU would
> translate MSI addresses with the page tables corresponding to that PASID,
> which with SVA correspond to the process page tables. So the kernel would
> need to create a mapping inside the process' address space, to the PA of
> the IRQ remapping device. That's the part that is inconvenient (would not
> be a security issue because accesses to the doorbell from CPUs are ignored
> according to Arm's platform spec, but it would certainly be awkward to set
> up).
> 
> Thanks,
> Jean

The way I'd do it, is probably by an mmap call. Nothing too tricky about
that. Gives you a VA or it can get a VA with MMAP_FIXED if process
wants to allocate a range. And for a VM, we just naturally use
the regular doorbell address.

In fact, as you describe it it seems that we do have a similar problem
on AMD, we need to preclude VAs in the MSI range somehow. so I guess 2
vendors out of 3 need to carve up the process address range.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


  reply	other threads:[~2022-01-24 22:15 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-12  5:57 [virtio-dev] [PATCH V2 0/2] virito-pci: PASID support Jason Wang
2022-01-12  5:57 ` [virtio-dev] [PATCH V2 1/2] virtio-pci: introduce virtio structure PCI Extended Capability Jason Wang
2022-01-12 10:10   ` [virtio-dev] " Stefan Hajnoczi
2022-01-13  0:55     ` Jason Wang
2022-01-13 10:19       ` Stefan Hajnoczi
2022-01-14  3:23         ` Jason Wang
2022-01-17 10:03           ` Stefan Hajnoczi
2022-01-12  5:57 ` [virtio-dev] [PATCH V2 2/2] virtio-pci: add PASID configuration extended capability Jason Wang
2022-01-12 10:41   ` [virtio-dev] " Stefan Hajnoczi
2022-01-13  1:24     ` Jason Wang
2022-01-13 10:32       ` Stefan Hajnoczi
2022-01-13 10:45         ` Michael S. Tsirkin
2022-01-13 14:53           ` Stefan Hajnoczi
2022-01-13 15:17             ` Michael S. Tsirkin
2022-01-14  3:15               ` Jason Wang
2022-01-14 10:38                 ` Jean-Philippe Brucker
2022-01-17  5:58                   ` Jason Wang
2022-01-14  9:43       ` Jean-Philippe Brucker
2022-01-17  5:57         ` Jason Wang
2022-01-19 18:01           ` Jean-Philippe Brucker
2022-01-19 23:53         ` Michael S. Tsirkin
2022-01-24 15:26           ` Jean-Philippe Brucker
2022-01-24 22:15             ` Michael S. Tsirkin [this message]
2022-01-12 10:44 ` [virtio-dev] Re: [PATCH V2 0/2] virito-pci: PASID support Stefan Hajnoczi
2022-01-13  1:28   ` Jason Wang
2022-01-13 10:36     ` Stefan Hajnoczi
2022-01-13 10:40       ` Michael S. Tsirkin
2022-01-14  2:53         ` Jason Wang
2022-01-13 15:18 ` Michael S. Tsirkin
2022-01-14  2:55   ` Jason Wang

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=20220124170534-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=jean-philippe@linaro.org \
    --cc=lulu@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=virtio-dev@lists.oasis-open.org \
    /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.