Re: [PATCH RFCv1 08/14] iommufd: Add IOMMU_VIOMMU_SET_DEV_ID ioctl

[Jason Gunthorpe]

On Fri, May 24, 2024 at 07:13:23AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > Sent: Friday, May 24, 2024 1:40 PM
> > 
> > On Thu, May 23, 2024 at 06:42:56AM +0000, Tian, Kevin wrote:
> > > btw there is a check in the following code:
> > >
> > > +       if (viommu->iommu_dev != idev->dev->iommu->iommu_dev) {
> > > +               rc = -EINVAL;
> > > +               goto out_put_viommu;
> > > +       }
> > >
> > > I vaguely remember an earlier discussion about multiple vSMMU instances
> > > following the physical SMMU topology, but don't quite recall the exact
> > > reason.
> > >
> > > What is the actual technical obstacle prohibiting one to put multiple
> > > VCMDQ's from different SMMU's into one vIOMMU instance?
> > 
> > Because a VCMDQ passthrough involves a direct mmap of a HW MMIO
> > page to the guest-level MMIO region. The MMIO page provides the
> > read/write of queue's head and tail indexes.
> > 
> > With a single pSMMU and a single vSMMU, it's simply 1:1 mapping.
> > 
> > With a multi-pSMMU and a single vSMMU, the single vSMMU will see
> > one guest-level MMIO region backed by multiple physical pages.
> > Since we are talking about MMIO, technically it cannot select the
> > corresponding MMIO page to the device, not to mention we don't
> > really want VMM to involve, i.e. no VM exist, when using VCMDQ.
> 
> can a vSMMU report to support multiple CMDQ's then there are
> several MMIO regions each mapped to a different backend VCMDQ?

This is something I want to support in the API, regardless vCMDQ uses
it or not..

> but I guess even if it's supported there is still a problem describing
> the association between assigned devices and the CMDQ's of the
> single vIOMMU instance. 

CMDQ should be linked to the VIOMMU instance only and not per-device,
I think that is a very important property of the system.

This means if there are multiple pSMMU instances then we need a
matching set of vSMMU instances so that the devices are grouped on the
proper vSMMU instance so they are compatible with the vCMDQ.

> I'm curious to learn the real reason of that design. Is it because you
> want to do certain load-balance between viommu's or due to other
> reasons in the kernel smmuv3 driver which e.g. cannot support a
> viommu spanning multiple pSMMU?

Yeah, there is no concept of support for a SMMUv3 instance where it's
command Q's can only work on a subset of devices.

My expectation was that VIOMMU would be 1:1 with physical iommu
instances, I think AMD needs this too??

Jason