linux-kernel - RE: [PATCH v2 5/8] iommu: Add PASID support for DMA mapping API users

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:   Wed, 16 Mar 2022 10:32:52 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Jacob Pan <jacob.jun.pan@...ux.intel.com>,
        Jason Gunthorpe <jgg@...dia.com>
CC:     Robin Murphy <robin.murphy@....com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Joerg Roedel <joro@...tes.org>,
        Christoph Hellwig <hch@...radead.org>,
        "Lu Baolu" <baolu.lu@...ux.intel.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        "Jiang, Dave" <dave.jiang@...el.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Zanussi, Tom" <tom.zanussi@...el.com>,
        "Kumar, Sanjay K" <sanjay.k.kumar@...el.com>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
        "Williams, Dan J" <dan.j.williams@...el.com>
Subject: RE: [PATCH v2 5/8] iommu: Add PASID support for DMA mapping API users

> From: Jacob Pan <jacob.jun.pan@...ux.intel.com>
> Sent: Wednesday, March 16, 2022 5:24 AM
> 
> Hi Jason,
> 
> On Tue, 15 Mar 2022 14:05:07 -0300, Jason Gunthorpe <jgg@...dia.com>
> wrote:
> 
> > On Tue, Mar 15, 2022 at 09:31:35AM -0700, Jacob Pan wrote:
> >
> > > > IMHO it is a device mis-design of IDXD to require all DMA be PASID
> > > > tagged. Devices should be able to do DMA on their RID when the PCI
> >
> > > IDXD can do DMA w/ RID, the PASID requirement is only for shared WQ
> > > where ENQCMDS is used. ENQCMDS has the benefit of avoiding locking
> > > where work submission is done from multiple CPUs.
> > > Tony, Dave?
> >
> > This is what I mean, it has an operating mode you want to use from the
> > kernel driver that cannot do RID DMA. It is a HW mis-design, IMHO.

Well, that mode is configured per work queue and the device just provides
flexibility for the software to use it for a potential value (scalability). So in 
the end it is a software question whether we can find a clean way to manage
this mode (fortunately with your proposal it does 😊). From this angle I'd
not call it a mis-design because the software has other options if there is no
willing of using that mode. Even in the virtual IDXD case that I explained in
another thread, the admin should not share work queue between VMs unless
he knows that guest can support it. Otherwise the admin should just dedicate
a workqueue to the guest and then let the guest itself to decide whether to
use the shared mode capability for its own purpose.

Also in concept the IOVA space created with DMA API is not different from
other I/O address spaces. There is no architectural restriction why this space
cannot be attached by PASID.

> >
> > Something like PASID0 in the ENQCMDS should have triggered RID DMA.
> >
> That would simplify things a lot, it would be just a device change I think.
> From IA pov, only ENQCMD will #GP if PASID==0. I will bring this back to HW
> team to consider for future generations.
> 

Maybe you can have a quick try?

Per SDM CPU doesn't #GP on PASID0 for ENQCMDS.

Also I don't think the device should do such check since with RID2PASID
the actual PASID value used to mark RID requests in the IOMMU is 
configured by the iommu driver. In that regard it is not correct for any 
hardware outside of the iommu to assume that PASID0 is for RID.

Then the only uncertain thing is within VT-d. In a quick check I didn't 
find any VT-d fault specifically for a DMA request which contains
a value same as RID2PASID.	

There is one interest paragraph in 6.2.1 (Tagging of cached translations):

  In scalable mode, requests-without-PASID are treated as requests-with-
  PASID when looking up the paging-structure cache, and PASID-cache.
  Such lookups use the PASID value from the RID_PASID field in the
  scalable-mode context-entry used to process the request-withoutPASID.
  Refer to Section 9.4 for more details on scalable-mode context-entry.
  Additionally, after translation process when such requests fill into
  IOTLB, the entries are tagged with PASID value obtained from RID_PASID
  field but are still marked as entries for requests-without-PASID.
  Tagging of such entries with PASID value is required so that 
  PASID-selective P_IOTLB invalidation can correctly remove all stale
  mappings. Implementation may allow requests-with-PASID from a given
  Requester-ID to hit entries brought into IOTLB by requests-without-PASID
  from the same Requester-ID to improve performance.

The last sentence gives me the impression that there is no problem for
VT-d to receive a request which happens to contain RID2PASID except
there might be a performance issue (if duplicated entries are created).

Thanks
Kevin