lkml.org 
[lkml]   [2022]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
    Date
    > From: Jason Gunthorpe <jgg@nvidia.com>
    > Sent: Monday, May 30, 2022 8:23 PM
    >
    > On Tue, May 24, 2022 at 08:17:27AM -0700, Jacob Pan wrote:
    > > Hi Jason,
    > >
    > > On Tue, 24 May 2022 10:50:34 -0300, Jason Gunthorpe <jgg@nvidia.com>
    > wrote:
    > >
    > > > On Wed, May 18, 2022 at 11:21:15AM -0700, Jacob Pan wrote:
    > > > > DMA requests tagged with PASID can target individual IOMMU domains.
    > > > > Introduce a domain-wide PASID for DMA API, it will be used on the
    > same
    > > > > mapping as legacy DMA without PASID. Let it be IOVA or PA in case of
    > > > > identity domain.
    > > >
    > > > Huh? I can't understand what this is trying to say or why this patch
    > > > makes sense.
    > > >
    > > > We really should not have pasid's like this attached to the domains..
    > > >
    > > This is the same "DMA API global PASID" you reviewed in v3, I just
    > > singled it out as a standalone patch and renamed it. Here is your previous
    > > review comment.
    > >
    > > > +++ b/include/linux/iommu.h
    > > > @@ -105,6 +105,8 @@ struct iommu_domain {
    > > > enum iommu_page_response_code (*iopf_handler)(struct
    > iommu_fault *fault,
    > > > void *data);
    > > > void *fault_data;
    > > > + ioasid_t pasid; /* Used for DMA requests with PASID */
    > > > + atomic_t pasid_users;
    > >
    > > These are poorly named, this is really the DMA API global PASID and
    > > shouldn't be used for other things.
    > >
    > >
    > >
    > > Perhaps I misunderstood, do you mind explaining more?
    >
    > You still haven't really explained what this is for in this patch,
    > maybe it just needs a better commit message, or maybe something is
    > wrong.
    >
    > I keep saying the DMA API usage is not special, so why do we need to
    > create a new global pasid and refcount? Realistically this is only
    > going to be used by IDXD, why can't we just allocate a PASID and
    > return it to the driver every time a driver asks for DMA API on PASI
    > mode? Why does the core need to do anything special?
    >

    Agree. I guess it was a mistake caused by treating ENQCMD as the
    only user although the actual semantics of the invented interfaces
    have already evolved to be quite general.

    This is very similar to what we have been discussing for iommufd.
    a PASID is just an additional routing info when attaching a device
    to an I/O address space (DMA API in this context) and by default
    it should be a per-device resource except when ENQCMD is
    explicitly opt in.

    Hence it's right time for us to develop common facility working
    for both this DMA API usage and iommufd, i.e.:

    for normal PASID attach to a domain, driver:

    allocates a local pasid from device local space;
    attaches the local pasid to a domain;

    for PASID attach in particular for ENQCMD, driver:

    allocates a global pasid in system-wide;
    attaches the global pasid to a domain;
    set the global pasid in PASID_MSR;

    In both cases the pasid is stored in the attach data instead of the
    domain.

    DMA API pasid is no special from above except it needs to allow
    one device attached to the same domain twice (one with RID
    and the other with RID+PASID).

    for iommufd those operations are initiated by userspace via
    iommufd uAPI.

    Thanks
    Kevin

    \
     
     \ /
      Last update: 2022-05-31 12:13    [W:2.330 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site