linux-kernel - Re: [PATCH RFCv1 08/14] iommufd: Add IOMMU_VIOMMU_SET_DEV

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 24 May 2024 10:19:12 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: "Tian, Kevin" <kevin.tian@...el.com>
Cc: Nicolin Chen <nicolinc@...dia.com>, "will@...nel.org" <will@...nel.org>,
	"robin.murphy@....com" <robin.murphy@....com>,
	"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
	"joro@...tes.org" <joro@...tes.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"Liu, Yi L" <yi.l.liu@...el.com>,
	"eric.auger@...hat.com" <eric.auger@...hat.com>,
	"vasant.hegde@....com" <vasant.hegde@....com>,
	"jon.grimm@....com" <jon.grimm@....com>,
	"santosh.shukla@....com" <santosh.shukla@....com>,
	"Dhaval.Giani@....com" <Dhaval.Giani@....com>,
	"shameerali.kolothum.thodi@...wei.com" <shameerali.kolothum.thodi@...wei.com>
Subject: Re: [PATCH RFCv1 08/14] iommufd: Add IOMMU_VIOMMU_SET_DEV_ID ioctl

On Fri, May 24, 2024 at 07:13:23AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@...dia.com>
> > 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