linux-kernel - Re: [PATCH RFCv1 07/14] iommufd: Add viommu set/unset_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 for Android: free password hash cracker in your pocket

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

Date: Wed, 22 May 2024 10:59:44 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: will@...nel.org, robin.murphy@....com, kevin.tian@...el.com,
	suravee.suthikulpanit@....com, joro@...tes.org,
	linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-tegra@...r.kernel.org,
	yi.l.liu@...el.com, eric.auger@...hat.com, vasant.hegde@....com,
	jon.grimm@....com, santosh.shukla@....com, Dhaval.Giani@....com,
	shameerali.kolothum.thodi@...wei.com
Subject: Re: [PATCH RFCv1 07/14] iommufd: Add viommu set/unset_dev_id ops

On Tue, May 21, 2024 at 03:27:02PM -0700, Nicolin Chen wrote:
> On Tue, May 21, 2024 at 03:24:48PM -0300, Jason Gunthorpe wrote:
> > On Tue, May 14, 2024 at 06:59:07PM -0700, Nicolin Chen wrote:
> > > So, you want a proxy S1 domain for a device to attach, in case
> > > of a stage-2 only setup, because an S2 domain will no longer has
> > > a VMID, since it's shared among viommus. In the SMMU driver case,
> > > an arm_smmu_domain won't have an smmu pointer, so a device can't
> > > attach to an S2 domain but always an nested S1 domain, right?
> > 
> > That seems like a simple solution to the VMID lifetime, but it means
> > the kernel has to decode more types of vSTE.
> 
> Yea. For vSTE=abort, likely we need a nested block domain too?

Sure, it is easy to do
 
> > I don't know if there is merit one way or the other. A more specific
> > API surface is nice, but the two APIs are completely duplicating.
> > 
> > So maybe:
> > 
> > #define IOMMU_VIOMMU_INVALIDATE IOMMU_HWPT_INVALIDATE
> > 
> > As documentation and have the kernel just detect based on the type of
> > the passed ID?
> 
> Yea, the only difference is viommu_id v.s. hwpt_id that we can
> document.
> 
> Then in this case, we have two mostly identical uAPIs for the
> SMMU driver to use. Should we implement both?

I suspect it will turn out nicely naturally, lets try and see
 
> > > > We can add ATS invalidation after either as an enhancement as part of
> > > > adding the VIOMMU either as DEV_INVALIDATE or VIOMMU_INVALIDATE (or
> > > > both)
> > > 
> > > Yea, maybe step by step like this:
> > > 
> > > Part-1 VIOMMU_ALLOC and VIOMMU_ATTACH
> > > Part-2 VIOMMU_SET/UNSET_VDEV_ID
> > > Part-3 VIOMMU_INVALIDATE
> > > Part-4 VQUEUE_ALLOC
> > > ...
> > 
> > So we have this stuff still open:
> >  - Identity STE with PASID (part 2b)
> >  - IOMMU_GET_HW_INFO (part 3)
> >  - IOMMU_HWPT_ALLOC_NEST_PARENT (part 3)
> >  - IOMMU_HWPT_DATA_ARM_SMMUV3 (part 3)
> >  - IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3
> >  - VIOMMU_ALLOC, VIOMMU_ATTACH
> >  - VIOMMU_INVALIDATE
> >  - VIOMMU_SET/UNSET_VDEV_ID
> >  - VQUEUE_ALLOC / vCMDQ
> > 
> > I feel like IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3 is a reasonable fit
> > to part 3. Then part 4 would be VIOMMU_ALLOC -> VIOMMU_SET/UNSET_VDEV_ID
> > which brings ATS support the API.
> 
> There is some conflict at passing in viommu_id/viommu v.s. parent
> hwpt_id/domain for a nested domain allocation. Do you think that
> should be addressed later in VIOMMU series v.s. part3?
>
> More specifically, I have two drafts in my viommu series:
> 87a659e65229 WAR: iommufd: Allow pt_it to carry viommu_id
> 7c5fd8f50bc9 WAR pass in viommu pointer to domain_alloc_user op

It would be good for viommu to come with all the uAPI changes in one
shot, so all the pt_ids should be updated to accept viommu to pass the
S2 HWPT.

Then whatever driver changes are needed to make ATS work should come
together too.

> I know that these two only make sense with VIOMMU_ALOC. Yet, will
> there be a problem, if we establish nested domain allocation with
> parent domain/hwpt by part3, in the uAPI, and then change later?
> Will we end up with supporting two for backward compatibility?

I think this is fairly minor compatability, let's see.

Jason