Re: [PATCH v3 03/15] hw/pci: Add a pci_device_iommu_memory_region() helper

From: Peter Xu <peterx@redhat.com>
To: Joao Martins <joao.m.martins@oracle.com>
Cc: qemu-devel@nongnu.org,
	Alex Williamson <alex.williamson@redhat.com>,
	Cedric Le Goater <clg@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	David Hildenbrand <david@redhat.com>,
	Philippe Mathieu-Daude <philmd@linaro.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Jason Wang <jasowang@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Eduardo Habkost <eduardo@habkost.net>,
	Avihai Horon <avihaih@nvidia.com>,
	Jason Gunthorpe <jgg@nvidia.com>
Subject: Re: [PATCH v3 03/15] hw/pci: Add a pci_device_iommu_memory_region() helper
Date: Tue, 6 Jun 2023 11:03:11 -0400	[thread overview]
Message-ID: <ZH9Kr6mrKNqUgcYs@x1n> (raw)
In-Reply-To: <8db546c9-395a-432e-ef07-ed6d6447a097@oracle.com>

On Tue, Jun 06, 2023 at 12:22:16PM +0100, Joao Martins wrote:
> On 05/06/2023 17:57, Peter Xu wrote:
> > On Tue, May 30, 2023 at 06:59:25PM +0100, Joao Martins wrote:
> >> Much like pci_device_iommu_address_space() fetches the IOMMU AS, add a
> >> pci_device_iommu_memory_region() which lets it return an the IOMMU MR
> >> associated with it. The IOMMU MR is returned correctly for vIOMMUs using
> >> pci_setup_iommu_info(). Note that today most vIOMMUs create the address
> >> space and IOMMU MR at the same time, it's just mainly that there's API
> >> to make the latter available.
> > 
> > Have you looked into other archs outside x86?  IIRC on some other arch one
> > address space can have >1 IOMMU memory regions.. at least with such AS and
> > MR layering it seems always possible?  Thanks,
> > 
> 
> I looked at all callers of pci_setup_iommu() restricting to those that actually
> track an IOMMUMemoryRegion when they create a address space... as this is where
> pci_device_iommu_memory_region() is applicable. From looking at those[*], I see
> always a 1:1 association between the AS and the IOMMU-MR in their initialization
> when iommu_fn is called. Unless I missed something... Is there an arch you were
> thinking specifically?

If only observing the ones that "track an IOMMUMemoryRegion when they
create a address space", probably we're fine.  I was thinking ppc but I
don't really know the details, and I assume that's not in the scope.
Copying David Gibson just in case he got some comments here.

> 
> [I am not sure we can track today an 1:N AS->IOMMU association today in Qemu]

IIUC we can?  The address space only have a root MR, and with that after
translate() upon the root mr (per address_space_translate_iommu(), it can
even be a few rounds of nested translations) it can go into whatever MR
under it IIUC.  Different ranges can map to a different IOMMU MR logically.

> 
> [*] alpha, arm smmu, ppc, s390, virtio, and some pci bridges (pnv_phb3 and pnv_phb4)

I just worried what we need here is not a MR object but a higher level
object like the vIOMMU object.  We used to have a requirement with Scalable
IOV (SVA) on Intel.  I tried to dig a bit in my inbox, not sure whether
it's the latest status, just to show what I meant:

https://lore.kernel.org/r/20210302203827.437645-6-yi.l.liu@intel.com

Copy Yi too for that too.  From that aspect it makes more sense to me to
fetching things from either an IOMMUops or "an iommu object", rather than
relying on a specific MR (it'll also make it even harder when we can have
>1 vIOMMUs so different MR can point to different IOMMUs in the future).

I assume the two goals have similar requirement, iiuc.  If that's the case,
we'd better make sure we'll have one way to work for both.

-- 
Peter Xu