RE: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
> From: Baolu Lu
> Sent: Wednesday, June 1, 2022 5:37 PM
>
> On 2022/6/1 09:43, Tian, Kevin wrote:
> >> From: Jacob Pan
> >> Sent: Wednesday, June 1, 2022 1:30 AM
> In both cases the pasid is stored in the attach data instead of the
> domain.
>
> >> So during IOTLB flush for the domain, do we loop through the attach data?
> > Yes and it's required.
>
> What does the attach data mean here? Do you mean group->pasid_array?
any structure describing the attaching relationship from {device, pasid} to
a domain
>
> Why not tracking the {device, pasid} info in the iommu driver when
> setting domain to {device, pasid}? We have tracked device in a list when
> setting a domain to device.
>
Yes, that tracking structure is the attach data.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
On 2022/6/1 09:43, Tian, Kevin wrote:
From: Jacob Pan
Sent: Wednesday, June 1, 2022 1:30 AM
In both cases the pasid is stored in the attach data instead of the
domain.
So during IOTLB flush for the domain, do we loop through the attach data?
Yes and it's required.
What does the attach data mean here? Do you mean group->pasid_array?
Why not tracking the {device, pasid} info in the iommu driver when
setting domain to {device, pasid}? We have tracked device in a list when
setting a domain to device.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
> From: Jacob Pan > Sent: Wednesday, June 1, 2022 4:44 AM > > Hi Jason, > > On Tue, 31 May 2022 16:05:50 -0300, Jason Gunthorpe > wrote: > > > On Tue, May 31, 2022 at 10:29:55AM -0700, Jacob Pan wrote: > > > > > The reason why I store PASID at IOMMU domain is for IOTLB flush within > > > the domain. Device driver is not aware of domain level IOTLB flush. We > > > also have iova_cookie for each domain which essentially is for > > > RIDPASID. > > > > You need to make the PASID stuff work generically. > > > > The domain needs to hold a list of all the places it needs to flush > > and that list needs to be maintained during attach/detach. > > > > A single PASID on the domain is obviously never going to work > > generically. > > > I agree, I did it this way really meant to be part of iommu_domain's > iommu_dma_cookie, not meant to be global. But for the lack of common > storage between identity domain and dma domain, I put it here as global. > > Then should we also extract RIDPASID to become part of the generic API? > i.e. set pasid, flush IOTLB etc. Right? RIDPASID is not in group's > pasid_array today. > RIDPASID is just an alias to RID in the PASID table (similar to pasid#0 on other platforms). it's reserved and not exposed outside the intel-iommu driver. So I don't think it should be managed via the generic iommu API. But probably you can generalize it with other pasids within intel-iommu driver if doing so can simplify the logic there. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
> From: Jacob Pan > Sent: Wednesday, June 1, 2022 1:30 AM > > > > > > In both cases the pasid is stored in the attach data instead of the > > > domain. > > > > So during IOTLB flush for the domain, do we loop through the attach data? Yes and it's required. > > > > 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. > > > > My understanding is that device driver owns its PASID policy. If ENQCMD > > is supported on the device, the PASIDs should be allocated through > > ioasid_alloc(). Otherwise, the whole PASID pool is managed by the device > > driver. > > > It seems the changes we want for this patchset are: > 1. move ioasid_alloc() from the core to device (allocation scope will be > based on whether ENQCMD is intended or not) yes, and the driver can specify whether the allocation is system-wide or per-device. > 2. store pasid in the attach data > 3. use the same iommufd api to attach/set pasid on its default domain s/iommufd/iommu/ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
Hi Jason, On Tue, 31 May 2022 16:05:50 -0300, Jason Gunthorpe wrote: > On Tue, May 31, 2022 at 10:29:55AM -0700, Jacob Pan wrote: > > > The reason why I store PASID at IOMMU domain is for IOTLB flush within > > the domain. Device driver is not aware of domain level IOTLB flush. We > > also have iova_cookie for each domain which essentially is for > > RIDPASID. > > You need to make the PASID stuff work generically. > > The domain needs to hold a list of all the places it needs to flush > and that list needs to be maintained during attach/detach. > > A single PASID on the domain is obviously never going to work > generically. > I agree, I did it this way really meant to be part of iommu_domain's iommu_dma_cookie, not meant to be global. But for the lack of common storage between identity domain and dma domain, I put it here as global. Then should we also extract RIDPASID to become part of the generic API? i.e. set pasid, flush IOTLB etc. Right? RIDPASID is not in group's pasid_array today. Thanks, Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
On Tue, May 31, 2022 at 10:29:55AM -0700, Jacob Pan wrote: > The reason why I store PASID at IOMMU domain is for IOTLB flush within the > domain. Device driver is not aware of domain level IOTLB flush. We also > have iova_cookie for each domain which essentially is for RIDPASID. You need to make the PASID stuff work generically. The domain needs to hold a list of all the places it needs to flush and that list needs to be maintained during attach/detach. A single PASID on the domain is obviously never going to work generically. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
Hi Baolu,
On Tue, 31 May 2022 20:45:28 +0800, Baolu Lu
wrote:
> On 2022/5/31 18:12, Tian, Kevin wrote:
> +++ 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?
> >>
The reason why I store PASID at IOMMU domain is for IOTLB flush within the
domain. Device driver is not aware of domain level IOTLB flush. We also
have iova_cookie for each domain which essentially is for RIDPASID.
> > 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.
> >
So during IOTLB flush for the domain, do we loop through the attach data?
> > 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.
>
> My understanding is that device driver owns its PASID policy. If ENQCMD
> is supported on the device, the PASIDs should be allocated through
> ioasid_alloc(). Otherwise, the whole PASID pool is managed by the device
> driver.
>
It seems the changes we want for this patchset are:
1. move ioasid_alloc() from the core to device (allocation scope will be
based on whether ENQCMD is intended or not)
2. store pasid in the attach data
3. use the same iommufd api to attach/set pasid on its default domain
Am I summarizing correctly?
> For kernel DMA w/ PASID, after the driver has a PASID for this purpose,
> it can just set the default domain to the PASID on device. There's no
> need for enable/disable() interfaces.
>
> Best regards,
> baolu
Thanks,
Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
On Tue, May 31, 2022 at 08:45:28PM +0800, Baolu Lu wrote: > My understanding is that device driver owns its PASID policy. I'm not sure this is actually useful, the core code should provide the allocator as I can't think of any device that actually needs a special allocator. > If ENQCMD is supported on the device, the PASIDs should be allocated > through ioasid_alloc(). Otherwise, the whole PASID pool is managed > by the device driver. This is OK only if we know in-advance that the device needs a global PASID. ENQCMD is one reason, maybe there are others down the road. Better to make this core code policy. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
On 2022/5/31 18:12, Tian, Kevin wrote:
+++ 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.
My understanding is that device driver owns its PASID policy. If ENQCMD
is supported on the device, the PASIDs should be allocated through
ioasid_alloc(). Otherwise, the whole PASID pool is managed by the device
driver.
For kernel DMA w/ PASID, after the driver has a PASID for this purpose,
it can just set the default domain to the PASID on device. There's no
need for enable/disable() interfaces.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
> From: Jason Gunthorpe
> 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
> 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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
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 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?
Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
Hi Jason,
On Tue, 24 May 2022 10:50:34 -0300, Jason Gunthorpe 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?
Thanks,
Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
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.. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API
On 2022/5/19 02:21, 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.
Signed-off-by: Jacob Pan
---
include/linux/iommu.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9405034e3013..36ad007084cc 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -106,6 +106,8 @@ struct iommu_domain {
enum iommu_page_response_code (*iopf_handler)(struct iommu_fault *fault,
void *data);
void *fault_data;
+ ioasid_t dma_pasid; /* Used for DMA requests with PASID */
This looks more suitable for iommu_dma_cookie?
+ atomic_t dma_pasid_users;
};
static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
