Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
On Fri, Dec 10, 2021 at 10:18:20AM -0800, Jacob Pan wrote: > > If one device has 10 PASID's pointing to this domain you must flush > > them all if that is what the HW requires. > > > Yes. My point is that other than PASID 0 is a given, we must track the 10 > PASIDs to avoid wasted flush. It also depend on how TLBs are tagged and > flush granularity available. But at the API level, should we support all the > cases? Yes, iommufd will expose all the cases to userspace and anything is possible. A scheme that can only attach a domain to one PASID is functionally useless except for this IDXD problem, so don't make something so broken. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
Hi Jason,
On Fri, 10 Dec 2021 13:48:48 -0400, Jason Gunthorpe wrote:
> On Fri, Dec 10, 2021 at 09:50:25AM -0800, Jacob Pan wrote:
>
> > > Tying pasid to an iommu_domain is not a good idea. An iommu_domain
> > > represents an I/O address translation table. It could be attached to a
> > > device or a PASID on the device.
> >
> > I don;t think we can avoid storing PASID at domain level or the group's
> > default domain. IOTLB flush is per domain. Default domain of DMA type
> > is already tying to PASID0, right?
>
> No, it is just wrong.
>
> If the HW requires a list of everything that is connected to the
> iommu_domain then it's private iommu_domain should have that list.
>
What I have in this patchset is in the private dmar_domain
struct dmar_domain {
...
u32 kernel_pasid; /* for in-kernel DMA w/
PASID */ atomic_t kernel_pasid_user; /* count of kernel_pasid users
*/ struct iommu_domain domain; /* generic domain data structure for
iommu core */
};
Perhaps I am missing the point. "private domain" is still "domain level" as
what I stated. Confused :(
> But it is a *list* not a single PASID.
>
We could have a list when real use case comes.
> If one device has 10 PASID's pointing to this domain you must flush
> them all if that is what the HW requires.
>
Yes. My point is that other than PASID 0 is a given, we must track the 10
PASIDs to avoid wasted flush. It also depend on how TLBs are tagged and
flush granularity available. But at the API level, should we support all the
cases?
> Jason
Thanks,
Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
On Fri, Dec 10, 2021 at 09:50:25AM -0800, Jacob Pan wrote: > > Tying pasid to an iommu_domain is not a good idea. An iommu_domain > > represents an I/O address translation table. It could be attached to a > > device or a PASID on the device. > > I don;t think we can avoid storing PASID at domain level or the group's > default domain. IOTLB flush is per domain. Default domain of DMA type > is already tying to PASID0, right? No, it is just wrong. If the HW requires a list of everything that is connected to the iommu_domain then it's private iommu_domain should have that list. But it is a *list* not a single PASID. If one device has 10 PASID's pointing to this domain you must flush them all if that is what the HW requires. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
Hi Lu,
On Fri, 10 Dec 2021 14:46:32 +0800, Lu Baolu
wrote:
> On 2021/12/10 7:21, Jacob Pan wrote:
> > On Thu, 9 Dec 2021 10:32:43 +0800, Lu Baolu
> > wrote:
> >
> >> On 12/9/21 3:16 AM, Jacob Pan wrote:
> >>> Hi Jason,
> >>>
> >>> On Wed, 8 Dec 2021 09:22:55 -0400, Jason Gunthorpe
> >>> wrote:
> On Tue, Dec 07, 2021 at 05:47:13AM -0800, Jacob Pan wrote:
> > Between DMA requests with and without PASID (legacy), DMA mapping
> > APIs are used indiscriminately on a device. Therefore, we should
> > always match the addressing mode of the legacy DMA when enabling
> > kernel PASID.
> >
> > This patch adds support for VT-d driver where the kernel PASID is
> > programmed to match RIDPASID. i.e. if the device is in pass-through,
> > the kernel PASID is also in pass-through; if the device is in IOVA
> > mode, the kernel PASID will also be using the same IOVA space.
> >
> > There is additional handling for IOTLB and device TLB flush w.r.t.
> > the kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a
> > per-domain basis; whereas device TLB flush is per device. Note that
> > IOTLBs are used even when devices are in pass-through mode. ATS is
> > enabled device-wide, but the device drivers can choose to manage ATS
> > at per PASID level whenever control is available.
> >
> > Signed-off-by: Jacob Pan
> >drivers/iommu/intel/iommu.c | 105
> > +++- drivers/iommu/intel/pasid.c |
> > 7 +++ include/linux/intel-iommu.h | 3 +-
> >3 files changed, 113 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c
> > b/drivers/iommu/intel/iommu.c index 60253bc436bb..a2ef6b9e4bfc
> > 100644 +++ b/drivers/iommu/intel/iommu.c
> > @@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
> > intel_iommu *iommu, if (domain->default_pasid)
> > qi_flush_piotlb(iommu, did,
> > domain->default_pasid, addr, npages, ih);
> > -
> > + if (domain->kernel_pasid && !domain_type_is_si(domain)) {
> > + /*
> > +* REVISIT: we only do PASID IOTLB inval for FL, we
> > could have SL
> > +* for PASID in the future such as vIOMMU PT. this
> > doesn't get hit.
> > +*/
> > + qi_flush_piotlb(iommu, did, domain->kernel_pasid,
> > + addr, npages, ih);
> > + }
> > if (!list_empty(&domain->devices))
> > qi_flush_piotlb(iommu, did, PASID_RID2PASID,
> > addr, npages, ih); }
> > @@ -5695,6 +5702,100 @@ static void
> > intel_iommu_iotlb_sync_map(struct iommu_domain *domain, }
> >}
> >
> > +static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
> > +{
> This seems like completely the wrong kind of op.
>
> At the level of the iommu driver things should be iommu_domain
> centric
>
> The op should be
>
> int attach_dev_pasid(struct iommu_domain *domain, struct device *dev,
> ioasid_t pasid)
>
> Where 'dev' purpose is to provide the RID
>
> The iommu_domain passed in should be the 'default domain' ie the
> table used for on-demand mapping, or the passthrough page table.
>
> >>> Makes sense. DMA API is device centric, iommu API is domain centric.
> >>> It should be the common IOMMU code to get the default domain then
> >>> pass to vendor drivers. Then we can enforce default domain behavior
> >>> across all vendor drivers.
> >>> i.e.
> >>> dom = iommu_get_dma_domain(dev);
> >>> attach_dev_pasid(dom, dev, pasid);
> >>>
> > + struct intel_iommu *iommu = device_to_iommu(dev, NULL,
> > NULL);
> > + struct device_domain_info *info;
> I don't even want to know why an iommu driver is tracking its own
> per-device state. That seems like completely wrong layering.
>
> >>> This is for IOTLB and deTLB flush. IOTLB is flushed at per domain
> >>> level, devTLB is per device.
> >>>
> >>> For multi-device groups, this is a need to track how many devices are
> >>> using the kernel DMA PASID.
> >>>
> >>> Are you suggesting we add the tracking info in the generic layer? i.e.
> >>> iommu_group.
> >>>
> >>> We could also have a generic device domain info to replace what is in
> >>> VT-d and FSL IOMMU driver, etc.
> >> The store place of per-device iommu driver private data has already
> >> been standardized. The iommu core provides below interfaces for this
> >> purpose:
> >>
> >> void dev_iommu_priv_set(struct device *dev, void *priv);
> >> void *dev_iommu_priv_get(struct device *dev);
> >>
> >> If we have anything generic among different vendor iommu drivers,
> >> perhaps we could move them into dev->iommu.
> >>
> > Yes, good suggestion. DMA PASID should be a generic feature, not
> > suit
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
On 2021/12/10 7:21, Jacob Pan wrote:
On Thu, 9 Dec 2021 10:32:43 +0800, Lu Baolu
wrote:
On 12/9/21 3:16 AM, Jacob Pan wrote:
Hi Jason,
On Wed, 8 Dec 2021 09:22:55 -0400, Jason Gunthorpe
wrote:
On Tue, Dec 07, 2021 at 05:47:13AM -0800, Jacob Pan wrote:
Between DMA requests with and without PASID (legacy), DMA mapping APIs
are used indiscriminately on a device. Therefore, we should always
match the addressing mode of the legacy DMA when enabling kernel
PASID.
This patch adds support for VT-d driver where the kernel PASID is
programmed to match RIDPASID. i.e. if the device is in pass-through,
the kernel PASID is also in pass-through; if the device is in IOVA
mode, the kernel PASID will also be using the same IOVA space.
There is additional handling for IOTLB and device TLB flush w.r.t. the
kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a
per-domain basis; whereas device TLB flush is per device. Note that
IOTLBs are used even when devices are in pass-through mode. ATS is
enabled device-wide, but the device drivers can choose to manage ATS
at per PASID level whenever control is available.
Signed-off-by: Jacob Pan
drivers/iommu/intel/iommu.c | 105
+++- drivers/iommu/intel/pasid.c |
7 +++ include/linux/intel-iommu.h | 3 +-
3 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 60253bc436bb..a2ef6b9e4bfc 100644
+++ b/drivers/iommu/intel/iommu.c
@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
intel_iommu *iommu, if (domain->default_pasid)
qi_flush_piotlb(iommu, did, domain->default_pasid,
addr, npages, ih);
-
+ if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+ /*
+* REVISIT: we only do PASID IOTLB inval for FL, we
could have SL
+* for PASID in the future such as vIOMMU PT. this
doesn't get hit.
+*/
+ qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+ addr, npages, ih);
+ }
if (!list_empty(&domain->devices))
qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,
npages, ih); }
@@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
iommu_domain *domain, }
}
+static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
+{
This seems like completely the wrong kind of op.
At the level of the iommu driver things should be iommu_domain centric
The op should be
int attach_dev_pasid(struct iommu_domain *domain, struct device *dev,
ioasid_t pasid)
Where 'dev' purpose is to provide the RID
The iommu_domain passed in should be the 'default domain' ie the table
used for on-demand mapping, or the passthrough page table.
Makes sense. DMA API is device centric, iommu API is domain centric. It
should be the common IOMMU code to get the default domain then pass to
vendor drivers. Then we can enforce default domain behavior across all
vendor drivers.
i.e.
dom = iommu_get_dma_domain(dev);
attach_dev_pasid(dom, dev, pasid);
+ struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+ struct device_domain_info *info;
I don't even want to know why an iommu driver is tracking its own
per-device state. That seems like completely wrong layering.
This is for IOTLB and deTLB flush. IOTLB is flushed at per domain level,
devTLB is per device.
For multi-device groups, this is a need to track how many devices are
using the kernel DMA PASID.
Are you suggesting we add the tracking info in the generic layer? i.e.
iommu_group.
We could also have a generic device domain info to replace what is in
VT-d and FSL IOMMU driver, etc.
The store place of per-device iommu driver private data has already been
standardized. The iommu core provides below interfaces for this purpose:
void dev_iommu_priv_set(struct device *dev, void *priv);
void *dev_iommu_priv_get(struct device *dev);
If we have anything generic among different vendor iommu drivers,
perhaps we could move them into dev->iommu.
Yes, good suggestion. DMA PASID should be a generic feature, not suitable
for the opaque private date. Can we agree on adding the following flag for
devTLB invalidation?
@@ -379,6 +379,7 @@ struct dev_iommu {
struct iommu_fwspec *fwspec;
struct iommu_device *iommu_dev;
void*priv;
+ u32 pasid_dma_enabled : 1;
};
For DMA PASID storage, can we store it in the iommu_domain instead of
iommu_group? In the end, this PASID is only used for the default domain. It
will be easier to refcount how many attached devices are using the PASID.
Destroy the PASID when no devices in the group are using PASID DMA. IOTLB
flush is per domain also.
Tying pasid to an iommu_domain is not a good idea. An iommu_domain
represents an I/O address translation table. It coul
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
On Thu, Dec 09, 2021 at 03:21:13PM -0800, Jacob Pan wrote: > For DMA PASID storage, can we store it in the iommu_domain instead of > iommu_group? It doesn't make sense to put in the domain, the domain should be only the page table and not have any relation to how things are matched to it Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
Hi Lu,
On Thu, 9 Dec 2021 10:32:43 +0800, Lu Baolu
wrote:
> On 12/9/21 3:16 AM, Jacob Pan wrote:
> > Hi Jason,
> >
> > On Wed, 8 Dec 2021 09:22:55 -0400, Jason Gunthorpe
> > wrote:
> >> On Tue, Dec 07, 2021 at 05:47:13AM -0800, Jacob Pan wrote:
> >>> Between DMA requests with and without PASID (legacy), DMA mapping APIs
> >>> are used indiscriminately on a device. Therefore, we should always
> >>> match the addressing mode of the legacy DMA when enabling kernel
> >>> PASID.
> >>>
> >>> This patch adds support for VT-d driver where the kernel PASID is
> >>> programmed to match RIDPASID. i.e. if the device is in pass-through,
> >>> the kernel PASID is also in pass-through; if the device is in IOVA
> >>> mode, the kernel PASID will also be using the same IOVA space.
> >>>
> >>> There is additional handling for IOTLB and device TLB flush w.r.t. the
> >>> kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a
> >>> per-domain basis; whereas device TLB flush is per device. Note that
> >>> IOTLBs are used even when devices are in pass-through mode. ATS is
> >>> enabled device-wide, but the device drivers can choose to manage ATS
> >>> at per PASID level whenever control is available.
> >>>
> >>> Signed-off-by: Jacob Pan
> >>> drivers/iommu/intel/iommu.c | 105
> >>> +++- drivers/iommu/intel/pasid.c |
> >>> 7 +++ include/linux/intel-iommu.h | 3 +-
> >>> 3 files changed, 113 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> >>> index 60253bc436bb..a2ef6b9e4bfc 100644
> >>> +++ b/drivers/iommu/intel/iommu.c
> >>> @@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
> >>> intel_iommu *iommu, if (domain->default_pasid)
> >>> qi_flush_piotlb(iommu, did, domain->default_pasid,
> >>> addr, npages, ih);
> >>> -
> >>> + if (domain->kernel_pasid && !domain_type_is_si(domain)) {
> >>> + /*
> >>> + * REVISIT: we only do PASID IOTLB inval for FL, we
> >>> could have SL
> >>> + * for PASID in the future such as vIOMMU PT. this
> >>> doesn't get hit.
> >>> + */
> >>> + qi_flush_piotlb(iommu, did, domain->kernel_pasid,
> >>> + addr, npages, ih);
> >>> + }
> >>> if (!list_empty(&domain->devices))
> >>> qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,
> >>> npages, ih); }
> >>> @@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
> >>> iommu_domain *domain, }
> >>> }
> >>>
> >>> +static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
> >>> +{
> >>
> >> This seems like completely the wrong kind of op.
> >>
> >> At the level of the iommu driver things should be iommu_domain centric
> >>
> >> The op should be
> >>
> >> int attach_dev_pasid(struct iommu_domain *domain, struct device *dev,
> >> ioasid_t pasid)
> >>
> >> Where 'dev' purpose is to provide the RID
> >>
> >> The iommu_domain passed in should be the 'default domain' ie the table
> >> used for on-demand mapping, or the passthrough page table.
> >>
> > Makes sense. DMA API is device centric, iommu API is domain centric. It
> > should be the common IOMMU code to get the default domain then pass to
> > vendor drivers. Then we can enforce default domain behavior across all
> > vendor drivers.
> > i.e.
> > dom = iommu_get_dma_domain(dev);
> > attach_dev_pasid(dom, dev, pasid);
> >
> >>> + struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
> >>> + struct device_domain_info *info;
> >>
> >> I don't even want to know why an iommu driver is tracking its own
> >> per-device state. That seems like completely wrong layering.
> >>
> > This is for IOTLB and deTLB flush. IOTLB is flushed at per domain level,
> > devTLB is per device.
> >
> > For multi-device groups, this is a need to track how many devices are
> > using the kernel DMA PASID.
> >
> > Are you suggesting we add the tracking info in the generic layer? i.e.
> > iommu_group.
> >
> > We could also have a generic device domain info to replace what is in
> > VT-d and FSL IOMMU driver, etc.
>
> The store place of per-device iommu driver private data has already been
> standardized. The iommu core provides below interfaces for this purpose:
>
> void dev_iommu_priv_set(struct device *dev, void *priv);
> void *dev_iommu_priv_get(struct device *dev);
>
> If we have anything generic among different vendor iommu drivers,
> perhaps we could move them into dev->iommu.
>
Yes, good suggestion. DMA PASID should be a generic feature, not suitable
for the opaque private date. Can we agree on adding the following flag for
devTLB invalidation?
@@ -379,6 +379,7 @@ struct dev_iommu {
struct iommu_fwspec *fwspec;
struct iommu_device *iommu_dev;
void*priv;
+ u32 pasid_dma_enabled : 1;
};
For DMA PASID storage
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
On 12/9/21 3:16 AM, Jacob Pan wrote:
Hi Jason,
On Wed, 8 Dec 2021 09:22:55 -0400, Jason Gunthorpe wrote:
On Tue, Dec 07, 2021 at 05:47:13AM -0800, Jacob Pan wrote:
Between DMA requests with and without PASID (legacy), DMA mapping APIs
are used indiscriminately on a device. Therefore, we should always match
the addressing mode of the legacy DMA when enabling kernel PASID.
This patch adds support for VT-d driver where the kernel PASID is
programmed to match RIDPASID. i.e. if the device is in pass-through, the
kernel PASID is also in pass-through; if the device is in IOVA mode, the
kernel PASID will also be using the same IOVA space.
There is additional handling for IOTLB and device TLB flush w.r.t. the
kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a
per-domain basis; whereas device TLB flush is per device. Note that
IOTLBs are used even when devices are in pass-through mode. ATS is
enabled device-wide, but the device drivers can choose to manage ATS at
per PASID level whenever control is available.
Signed-off-by: Jacob Pan
drivers/iommu/intel/iommu.c | 105 +++-
drivers/iommu/intel/pasid.c | 7 +++
include/linux/intel-iommu.h | 3 +-
3 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 60253bc436bb..a2ef6b9e4bfc 100644
+++ b/drivers/iommu/intel/iommu.c
@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
intel_iommu *iommu, if (domain->default_pasid)
qi_flush_piotlb(iommu, did, domain->default_pasid,
addr, npages, ih);
-
+ if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+ /*
+* REVISIT: we only do PASID IOTLB inval for FL, we
could have SL
+* for PASID in the future such as vIOMMU PT. this
doesn't get hit.
+*/
+ qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+ addr, npages, ih);
+ }
if (!list_empty(&domain->devices))
qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,
npages, ih); }
@@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
iommu_domain *domain, }
}
+static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
+{
This seems like completely the wrong kind of op.
At the level of the iommu driver things should be iommu_domain centric
The op should be
int attach_dev_pasid(struct iommu_domain *domain, struct device *dev,
ioasid_t pasid)
Where 'dev' purpose is to provide the RID
The iommu_domain passed in should be the 'default domain' ie the table
used for on-demand mapping, or the passthrough page table.
Makes sense. DMA API is device centric, iommu API is domain centric. It
should be the common IOMMU code to get the default domain then pass to
vendor drivers. Then we can enforce default domain behavior across all
vendor drivers.
i.e.
dom = iommu_get_dma_domain(dev);
attach_dev_pasid(dom, dev, pasid);
+ struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+ struct device_domain_info *info;
I don't even want to know why an iommu driver is tracking its own
per-device state. That seems like completely wrong layering.
This is for IOTLB and deTLB flush. IOTLB is flushed at per domain level,
devTLB is per device.
For multi-device groups, this is a need to track how many devices are using
the kernel DMA PASID.
Are you suggesting we add the tracking info in the generic layer? i.e.
iommu_group.
We could also have a generic device domain info to replace what is in VT-d
and FSL IOMMU driver, etc.
The store place of per-device iommu driver private data has already been
standardized. The iommu core provides below interfaces for this purpose:
void dev_iommu_priv_set(struct device *dev, void *priv);
void *dev_iommu_priv_get(struct device *dev);
If we have anything generic among different vendor iommu drivers,
perhaps we could move them into dev->iommu.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
Hi Jason,
On Wed, 8 Dec 2021 09:22:55 -0400, Jason Gunthorpe wrote:
> On Tue, Dec 07, 2021 at 05:47:13AM -0800, Jacob Pan wrote:
> > Between DMA requests with and without PASID (legacy), DMA mapping APIs
> > are used indiscriminately on a device. Therefore, we should always match
> > the addressing mode of the legacy DMA when enabling kernel PASID.
> >
> > This patch adds support for VT-d driver where the kernel PASID is
> > programmed to match RIDPASID. i.e. if the device is in pass-through, the
> > kernel PASID is also in pass-through; if the device is in IOVA mode, the
> > kernel PASID will also be using the same IOVA space.
> >
> > There is additional handling for IOTLB and device TLB flush w.r.t. the
> > kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a
> > per-domain basis; whereas device TLB flush is per device. Note that
> > IOTLBs are used even when devices are in pass-through mode. ATS is
> > enabled device-wide, but the device drivers can choose to manage ATS at
> > per PASID level whenever control is available.
> >
> > Signed-off-by: Jacob Pan
> > drivers/iommu/intel/iommu.c | 105 +++-
> > drivers/iommu/intel/pasid.c | 7 +++
> > include/linux/intel-iommu.h | 3 +-
> > 3 files changed, 113 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 60253bc436bb..a2ef6b9e4bfc 100644
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
> > intel_iommu *iommu, if (domain->default_pasid)
> > qi_flush_piotlb(iommu, did, domain->default_pasid,
> > addr, npages, ih);
> > -
> > + if (domain->kernel_pasid && !domain_type_is_si(domain)) {
> > + /*
> > +* REVISIT: we only do PASID IOTLB inval for FL, we
> > could have SL
> > +* for PASID in the future such as vIOMMU PT. this
> > doesn't get hit.
> > +*/
> > + qi_flush_piotlb(iommu, did, domain->kernel_pasid,
> > + addr, npages, ih);
> > + }
> > if (!list_empty(&domain->devices))
> > qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,
> > npages, ih); }
> > @@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
> > iommu_domain *domain, }
> > }
> >
> > +static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
> > +{
>
> This seems like completely the wrong kind of op.
>
> At the level of the iommu driver things should be iommu_domain centric
>
> The op should be
>
> int attach_dev_pasid(struct iommu_domain *domain, struct device *dev,
> ioasid_t pasid)
>
> Where 'dev' purpose is to provide the RID
>
> The iommu_domain passed in should be the 'default domain' ie the table
> used for on-demand mapping, or the passthrough page table.
>
Makes sense. DMA API is device centric, iommu API is domain centric. It
should be the common IOMMU code to get the default domain then pass to
vendor drivers. Then we can enforce default domain behavior across all
vendor drivers.
i.e.
dom = iommu_get_dma_domain(dev);
attach_dev_pasid(dom, dev, pasid);
> > + struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
> > + struct device_domain_info *info;
>
> I don't even want to know why an iommu driver is tracking its own
> per-device state. That seems like completely wrong layering.
>
This is for IOTLB and deTLB flush. IOTLB is flushed at per domain level,
devTLB is per device.
For multi-device groups, this is a need to track how many devices are using
the kernel DMA PASID.
Are you suggesting we add the tracking info in the generic layer? i.e.
iommu_group.
We could also have a generic device domain info to replace what is in VT-d
and FSL IOMMU driver, etc.
Thanks,
Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
On Tue, Dec 07, 2021 at 05:47:13AM -0800, Jacob Pan wrote:
> Between DMA requests with and without PASID (legacy), DMA mapping APIs
> are used indiscriminately on a device. Therefore, we should always match
> the addressing mode of the legacy DMA when enabling kernel PASID.
>
> This patch adds support for VT-d driver where the kernel PASID is
> programmed to match RIDPASID. i.e. if the device is in pass-through, the
> kernel PASID is also in pass-through; if the device is in IOVA mode, the
> kernel PASID will also be using the same IOVA space.
>
> There is additional handling for IOTLB and device TLB flush w.r.t. the
> kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a
> per-domain basis; whereas device TLB flush is per device. Note that
> IOTLBs are used even when devices are in pass-through mode. ATS is
> enabled device-wide, but the device drivers can choose to manage ATS at
> per PASID level whenever control is available.
>
> Signed-off-by: Jacob Pan
> drivers/iommu/intel/iommu.c | 105 +++-
> drivers/iommu/intel/pasid.c | 7 +++
> include/linux/intel-iommu.h | 3 +-
> 3 files changed, 113 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 60253bc436bb..a2ef6b9e4bfc 100644
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct intel_iommu
> *iommu,
> if (domain->default_pasid)
> qi_flush_piotlb(iommu, did, domain->default_pasid,
> addr, npages, ih);
> -
> + if (domain->kernel_pasid && !domain_type_is_si(domain)) {
> + /*
> + * REVISIT: we only do PASID IOTLB inval for FL, we could have
> SL
> + * for PASID in the future such as vIOMMU PT. this doesn't get
> hit.
> + */
> + qi_flush_piotlb(iommu, did, domain->kernel_pasid,
> + addr, npages, ih);
> + }
> if (!list_empty(&domain->devices))
> qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, npages, ih);
> }
> @@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
> iommu_domain *domain,
> }
> }
>
> +static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
> +{
This seems like completely the wrong kind of op.
At the level of the iommu driver things should be iommu_domain centric
The op should be
int attach_dev_pasid(struct iommu_domain *domain, struct device *dev, ioasid_t
pasid)
Where 'dev' purpose is to provide the RID
The iommu_domain passed in should be the 'default domain' ie the table
used for on-demand mapping, or the passthrough page table.
> + struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
> + struct device_domain_info *info;
I don't even want to know why an iommu driver is tracking its own
per-device state. That seems like completely wrong layering.
Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/4] iommu/vt-d: Support PASID DMA for in-kernel usage
Between DMA requests with and without PASID (legacy), DMA mapping APIs
are used indiscriminately on a device. Therefore, we should always match
the addressing mode of the legacy DMA when enabling kernel PASID.
This patch adds support for VT-d driver where the kernel PASID is
programmed to match RIDPASID. i.e. if the device is in pass-through, the
kernel PASID is also in pass-through; if the device is in IOVA mode, the
kernel PASID will also be using the same IOVA space.
There is additional handling for IOTLB and device TLB flush w.r.t. the
kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a
per-domain basis; whereas device TLB flush is per device. Note that
IOTLBs are used even when devices are in pass-through mode. ATS is
enabled device-wide, but the device drivers can choose to manage ATS at
per PASID level whenever control is available.
Signed-off-by: Jacob Pan
---
drivers/iommu/intel/iommu.c | 105 +++-
drivers/iommu/intel/pasid.c | 7 +++
include/linux/intel-iommu.h | 3 +-
3 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 60253bc436bb..a2ef6b9e4bfc 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct intel_iommu
*iommu,
if (domain->default_pasid)
qi_flush_piotlb(iommu, did, domain->default_pasid,
addr, npages, ih);
-
+ if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+ /*
+* REVISIT: we only do PASID IOTLB inval for FL, we could have
SL
+* for PASID in the future such as vIOMMU PT. this doesn't get
hit.
+*/
+ qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+ addr, npages, ih);
+ }
if (!list_empty(&domain->devices))
qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, npages, ih);
}
@@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
iommu_domain *domain,
}
}
+static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
+{
+ struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+ struct device_domain_info *info;
+ unsigned long flags;
+ int ret = 0;
+
+ info = get_domain_info(dev);
+ if (!info)
+ return -ENODEV;
+
+ if (!dev_is_pci(dev) || !sm_supported(info->iommu))
+ return -EINVAL;
+
+ if (intel_iommu_enable_pasid(info->iommu, dev))
+ return -ENODEV;
+
+ spin_lock_irqsave(&device_domain_lock, flags);
+ spin_lock(&iommu->lock);
+ /*
+* Store PASID for IOTLB flush, but only needed for non-passthrough
+* unmap case. For passthrough, we only need to do IOTLB flush during
+* PASID teardown. Flush covers all devices in the same domain as the
+* domain ID is the same for the same SL.
+*/
+ info->domain->kernel_pasid = pasid;
+
+ /*
+* Tracks how many attached devices are using the kernel PASID. Clear
+* the domain kernel PASID when all users called disable_pasid_dma().
+*/
+ atomic_inc(&info->domain->kernel_pasid_user);
+
+ /*
+* Addressing modes (IOVA vs. PA) is a per device choice made by the
+* platform code. We must treat legacy DMA (request w/o PASID) and
+* DMA w/ PASID identially in terms of mapping. Here we just set up
+* the kernel PASID to match the mapping of RID2PASID/PASID0.
+*/
+ if (hw_pass_through && domain_type_is_si(info->domain)) {
+ ret = intel_pasid_setup_pass_through(info->iommu, info->domain,
+ dev, pasid);
+ if (ret)
+ dev_err(dev, "Failed kernel PASID %d in BYPASS", pasid);
+
+ } else if (domain_use_first_level(info->domain)) {
+ /* We are using FL for IOVA, this is the default option */
+ ret = domain_setup_first_level(info->iommu, info->domain, dev,
+ pasid);
+ if (ret)
+ dev_err(dev, "Failed kernel PASID %d IOVA FL", pasid);
+ } else {
+ ret = intel_pasid_setup_second_level(info->iommu, info->domain,
+dev, pasid);
+ if (ret)
+ dev_err(dev, "Failed kernel SPASID %d IOVA SL", pasid);
+ }
+
+ spin_unlock(&iommu->lock);
+ spin_unlock_irqrestore(&device_domain_lock, flags);
+
+ return ret;
+}
+
+static int intel_disable_pasid_dma(struct device *dev)
+{
+ struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+ struct device_domain_info *info;
+ unsigned long flags;
+ int ret = 0;
+
+ info = get_domain_info(dev);
