RE: [PATCH v2 1/1] iommu/vt-d: Fix RID2PASID setup failure
> From: Lu Baolu
> Sent: Wednesday, June 22, 2022 12:41 PM
>
> The IOMMU driver shares the pasid table for PCI alias devices. When the
> RID2PASID entry of the shared pasid table has been filled by the first
> device, the subsequent devices will encounter the "DMAR: Setup RID2PASID
> failed" failure as the pasid entry has already been marked as present. As
> the result, the IOMMU probing process will be aborted.
>
> This fixes it by skipping RID2PASID setting if the pasid entry has been
> populated. This works because the IOMMU core ensures that only the same
> IOMMU domain can be attached to all PCI alias devices at the same time.
> Therefore the subsequent devices just try to setup the RID2PASID entry
> with the same domain, which is negligible. This also adds domain validity
> checks for more confidence anyway.
>
> Fixes: ef848b7e5a6a0 ("iommu/vt-d: Setup pasid entry for RID2PASID
> support")
> Reported-by: Chenyi Qiang
> Cc: sta...@vger.kernel.org
> Signed-off-by: Lu Baolu
Reviewed-by: Kevin Tian
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Hi,
* Saravana Kannan [220621 19:29]:
> On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren wrote:
> >
> > Hi,
> >
> > * Saravana Kannan [700101 02:00]:
> > > Now that fw_devlink=on by default and fw_devlink supports
> > > "power-domains" property, the execution will never get to the point
> > > where driver_deferred_probe_check_state() is called before the supplier
> > > has probed successfully or before deferred probe timeout has expired.
> > >
> > > So, delete the call and replace it with -ENODEV.
> >
> > Looks like this causes omaps to not boot in Linux next.
>
> Can you please point me to an example DTS I could use for debugging
> this? I'm assuming you are leaving fw_devlink=on and not turning it
> off or putting it in permissive mode.
Sure, this seems to happen at least with simple-pm-bus as the top
level interconnect with a configured power-domains property:
$ git grep -A10 "ocp {" arch/arm/boot/dts/*.dtsi | grep -B3 -A4 simple-pm-bus
This issue is no directly related fw_devlink. It is a side effect of
removing driver_deferred_probe_check_state(). We no longer return
-EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > On platform_probe() genpd_get_from_provider() returns
> > -ENOENT.
>
> This error is with the series I assume?
On the first probe genpd_get_from_provider() will return -ENOENT in
both cases. The list is empty on the first probe and there are no
genpd providers at this point.
Earlier with driver_deferred_probe_check_state(), the initial -ENOENT
ends up getting changed to -EPROBE_DEFER at the end of
driver_deferred_probe_check_state(), we are now missing that.
Regards,
Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/1] iommu/vt-d: Fix RID2PASID setup failure
The IOMMU driver shares the pasid table for PCI alias devices. When the
RID2PASID entry of the shared pasid table has been filled by the first
device, the subsequent devices will encounter the "DMAR: Setup RID2PASID
failed" failure as the pasid entry has already been marked as present. As
the result, the IOMMU probing process will be aborted.
This fixes it by skipping RID2PASID setting if the pasid entry has been
populated. This works because the IOMMU core ensures that only the same
IOMMU domain can be attached to all PCI alias devices at the same time.
Therefore the subsequent devices just try to setup the RID2PASID entry
with the same domain, which is negligible. This also adds domain validity
checks for more confidence anyway.
Fixes: ef848b7e5a6a0 ("iommu/vt-d: Setup pasid entry for RID2PASID support")
Reported-by: Chenyi Qiang
Cc: sta...@vger.kernel.org
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/pasid.c | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
Change log:
v2:
- Add domain validity check in RID2PASID entry setup.
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index cb4c1d0cf25c..4f3525f3346f 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -575,6 +575,19 @@ static inline int pasid_enable_wpe(struct pasid_entry *pte)
return 0;
};
+/*
+ * Return true if @pasid is RID2PASID and the domain @did has already
+ * been setup to the @pte. Otherwise, return false. PCI alias devices
+ * probably share the single RID2PASID pasid entry in the shared pasid
+ * table. It's reasonable that those devices try to set a share domain
+ * in their probe paths.
+ */
+static inline bool
+rid2pasid_domain_valid(struct pasid_entry *pte, u32 pasid, u16 did)
+{
+ return pasid == PASID_RID2PASID && pasid_get_domain_id(pte) == did;
+}
+
/*
* Set up the scalable mode pasid table entry for first only
* translation type.
@@ -595,9 +608,8 @@ int intel_pasid_setup_first_level(struct intel_iommu *iommu,
if (WARN_ON(!pte))
return -EINVAL;
- /* Caller must ensure PASID entry is not in use. */
if (pasid_pte_is_present(pte))
- return -EBUSY;
+ return rid2pasid_domain_valid(pte, pasid, did) ? 0 : -EBUSY;
pasid_clear_entry(pte);
@@ -698,9 +710,8 @@ int intel_pasid_setup_second_level(struct intel_iommu
*iommu,
return -ENODEV;
}
- /* Caller must ensure PASID entry is not in use. */
if (pasid_pte_is_present(pte))
- return -EBUSY;
+ return rid2pasid_domain_valid(pte, pasid, did) ? 0 : -EBUSY;
pasid_clear_entry(pte);
pasid_set_domain_id(pte, did);
@@ -738,9 +749,8 @@ int intel_pasid_setup_pass_through(struct intel_iommu
*iommu,
return -ENODEV;
}
- /* Caller must ensure PASID entry is not in use. */
if (pasid_pte_is_present(pte))
- return -EBUSY;
+ return rid2pasid_domain_valid(pte, pasid, did) ? 0 : -EBUSY;
pasid_clear_entry(pte);
pasid_set_domain_id(pte, did);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/vt-d: Fix RID2PASID setup failure
On 2022/6/22 11:31, Tian, Kevin wrote:
From: Baolu Lu
Sent: Wednesday, June 22, 2022 11:28 AM
On 2022/6/22 11:06, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 21, 2022 5:04 PM
On 2022/6/21 13:48, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 21, 2022 12:28 PM
On 2022/6/21 11:46, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 21, 2022 11:39 AM
On 2022/6/21 10:54, Tian, Kevin wrote:
From: Lu Baolu
Sent: Monday, June 20, 2022 4:17 PM
@@ -2564,7 +2564,7 @@ static int domain_add_dev_info(struct
dmar_domain *domain, struct device *dev)
ret = intel_pasid_setup_second_level(iommu,
domain,
dev, PASID_RID2PASID);
spin_unlock_irqrestore(&iommu->lock, flags);
- if (ret) {
+ if (ret && ret != -EBUSY) {
dev_err(dev, "Setup RID2PASID failed\n");
dmar_remove_one_dev_info(dev);
return ret;
--
2.25.1
It's cleaner to avoid this error at the first place, i.e. only do the
setup when the first device is attached to the pasid table.
The logic that identifies the first device might introduce additional
unnecessary complexity. Devices that share a pasid table are rare. I
even prefer to give up sharing tables so that the code can be
simpler.:-)
It's not that complex if you simply move device_attach_pasid_table()
out of intel_pasid_alloc_table(). Then do the setup if
list_empty(&pasid_table->dev) and then attach device to the
pasid table in domain_add_dev_info().
The pasid table is part of the device, hence a better place to
allocate/free the pasid table is in the device probe/release paths.
Things will become more complicated if we change relationship
between
device and it's pasid table when attaching/detaching a domain. That's
the reason why I thought it was additional complexity.
If you do want to follow current route it’s still cleaner to check
whether the pasid entry has pointed to the domain in the individual
setup function instead of blindly returning -EBUSY and then ignoring
it even if a real busy condition occurs. The setup functions can
just return zero for this benign alias case.
Kevin, how do you like this one?
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index cb4c1d0cf25c..ecffd0129b2b 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -575,6 +575,16 @@ static inline int pasid_enable_wpe(struct
pasid_entry *pte)
return 0;
};
+/*
+ * Return true if @pasid is RID2PASID and the domain @did has already
+ * been setup to the @pte. Otherwise, return false.
+ */
+static inline bool
+rid2pasid_domain_valid(struct pasid_entry *pte, u32 pasid, u16 did)
+{
+ return pasid == PASID_RID2PASID && pasid_get_domain_id(pte) ==
did;
+}
better this is not restricted to RID2PASID only, e.g.
pasid_pte_match_domain()
and then read pasid from the pte to compare with the pasid argument.
The pasid value is not encoded in the pasid table entry. This validity
check is only for RID2PASID as alias devices share the single RID2PASID
entry. For other cases, we should always return -EBUSY as what the code
is doing now.
You are right.
Very appreciated for your input. I will update it with a v2.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/1] iommu/vt-d: Fix RID2PASID setup failure
> From: Baolu Lu
> Sent: Wednesday, June 22, 2022 11:28 AM
>
> On 2022/6/22 11:06, Tian, Kevin wrote:
> >> From: Baolu Lu
> >> Sent: Tuesday, June 21, 2022 5:04 PM
> >>
> >> On 2022/6/21 13:48, Tian, Kevin wrote:
> From: Baolu Lu
> Sent: Tuesday, June 21, 2022 12:28 PM
>
> On 2022/6/21 11:46, Tian, Kevin wrote:
> >> From: Baolu Lu
> >> Sent: Tuesday, June 21, 2022 11:39 AM
> >>
> >> On 2022/6/21 10:54, Tian, Kevin wrote:
> From: Lu Baolu
> Sent: Monday, June 20, 2022 4:17 PM
> @@ -2564,7 +2564,7 @@ static int domain_add_dev_info(struct
> dmar_domain *domain, struct device *dev)
> ret =
> intel_pasid_setup_second_level(iommu,
> domain,
> dev, PASID_RID2PASID);
> spin_unlock_irqrestore(&iommu->lock, flags);
> -if (ret) {
> +if (ret && ret != -EBUSY) {
> dev_err(dev, "Setup RID2PASID
> failed\n");
> dmar_remove_one_dev_info(dev);
> return ret;
> --
> 2.25.1
> >>> It's cleaner to avoid this error at the first place, i.e. only do the
> >>> setup when the first device is attached to the pasid table.
> >> The logic that identifies the first device might introduce additional
> >> unnecessary complexity. Devices that share a pasid table are rare. I
> >> even prefer to give up sharing tables so that the code can be
> >> simpler.:-)
> >>
> > It's not that complex if you simply move device_attach_pasid_table()
> > out of intel_pasid_alloc_table(). Then do the setup if
> > list_empty(&pasid_table->dev) and then attach device to the
> > pasid table in domain_add_dev_info().
> The pasid table is part of the device, hence a better place to
> allocate/free the pasid table is in the device probe/release paths.
> Things will become more complicated if we change relationship
> between
> device and it's pasid table when attaching/detaching a domain. That's
> the reason why I thought it was additional complexity.
>
> >>> If you do want to follow current route it’s still cleaner to check
> >>> whether the pasid entry has pointed to the domain in the individual
> >>> setup function instead of blindly returning -EBUSY and then ignoring
> >>> it even if a real busy condition occurs. The setup functions can
> >>> just return zero for this benign alias case.
> >> Kevin, how do you like this one?
> >>
> >> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> >> index cb4c1d0cf25c..ecffd0129b2b 100644
> >> --- a/drivers/iommu/intel/pasid.c
> >> +++ b/drivers/iommu/intel/pasid.c
> >> @@ -575,6 +575,16 @@ static inline int pasid_enable_wpe(struct
> >> pasid_entry *pte)
> >>return 0;
> >>};
> >>
> >> +/*
> >> + * Return true if @pasid is RID2PASID and the domain @did has already
> >> + * been setup to the @pte. Otherwise, return false.
> >> + */
> >> +static inline bool
> >> +rid2pasid_domain_valid(struct pasid_entry *pte, u32 pasid, u16 did)
> >> +{
> >> + return pasid == PASID_RID2PASID && pasid_get_domain_id(pte) ==
> >> did;
> >> +}
> > better this is not restricted to RID2PASID only, e.g.
> pasid_pte_match_domain()
> > and then read pasid from the pte to compare with the pasid argument.
> >
>
> The pasid value is not encoded in the pasid table entry. This validity
> check is only for RID2PASID as alias devices share the single RID2PASID
> entry. For other cases, we should always return -EBUSY as what the code
> is doing now.
>
You are right.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/vt-d: Fix RID2PASID setup failure
On 2022/6/22 11:06, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 21, 2022 5:04 PM
On 2022/6/21 13:48, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 21, 2022 12:28 PM
On 2022/6/21 11:46, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 21, 2022 11:39 AM
On 2022/6/21 10:54, Tian, Kevin wrote:
From: Lu Baolu
Sent: Monday, June 20, 2022 4:17 PM
@@ -2564,7 +2564,7 @@ static int domain_add_dev_info(struct
dmar_domain *domain, struct device *dev)
ret = intel_pasid_setup_second_level(iommu,
domain,
dev, PASID_RID2PASID);
spin_unlock_irqrestore(&iommu->lock, flags);
- if (ret) {
+ if (ret && ret != -EBUSY) {
dev_err(dev, "Setup RID2PASID failed\n");
dmar_remove_one_dev_info(dev);
return ret;
--
2.25.1
It's cleaner to avoid this error at the first place, i.e. only do the
setup when the first device is attached to the pasid table.
The logic that identifies the first device might introduce additional
unnecessary complexity. Devices that share a pasid table are rare. I
even prefer to give up sharing tables so that the code can be
simpler.:-)
It's not that complex if you simply move device_attach_pasid_table()
out of intel_pasid_alloc_table(). Then do the setup if
list_empty(&pasid_table->dev) and then attach device to the
pasid table in domain_add_dev_info().
The pasid table is part of the device, hence a better place to
allocate/free the pasid table is in the device probe/release paths.
Things will become more complicated if we change relationship between
device and it's pasid table when attaching/detaching a domain. That's
the reason why I thought it was additional complexity.
If you do want to follow current route it’s still cleaner to check
whether the pasid entry has pointed to the domain in the individual
setup function instead of blindly returning -EBUSY and then ignoring
it even if a real busy condition occurs. The setup functions can
just return zero for this benign alias case.
Kevin, how do you like this one?
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index cb4c1d0cf25c..ecffd0129b2b 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -575,6 +575,16 @@ static inline int pasid_enable_wpe(struct
pasid_entry *pte)
return 0;
};
+/*
+ * Return true if @pasid is RID2PASID and the domain @did has already
+ * been setup to the @pte. Otherwise, return false.
+ */
+static inline bool
+rid2pasid_domain_valid(struct pasid_entry *pte, u32 pasid, u16 did)
+{
+ return pasid == PASID_RID2PASID && pasid_get_domain_id(pte) ==
did;
+}
better this is not restricted to RID2PASID only, e.g. pasid_pte_match_domain()
and then read pasid from the pte to compare with the pasid argument.
The pasid value is not encoded in the pasid table entry. This validity
check is only for RID2PASID as alias devices share the single RID2PASID
entry. For other cases, we should always return -EBUSY as what the code
is doing now.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/vt-d: Fix RID2PASID setup failure
On 2022/6/22 10:56, Ethan Zhao wrote: 在 2022/6/20 16:17, Lu Baolu 写道: The IOMMU driver shares the pasid table for PCI alias devices. When the RID2PASID entry of the shared pasid table has been filled by the first device, the subsequent devices will encounter the "DMAR: Setup RID2PASID failed" failure as the pasid entry has already been marke as present. As the result, the IOMMU probing process will be aborted. This fixes it by skipping RID2PASID setting if the pasid entry has been populated. This works because the IOMMU core ensures that only the same IOMMU domain can be attached to all PCI alias devices at the same time. Therefore the subsequent devices just try to setup the RID2PASID entry with the same domain, which is negligible. We have two customers reported the issue "DMAR: Setup RID2PASID failed", Two ASPEED devices locate behind one PCIe-PCI bridge and iommu SM, PT mode is enabled. Most Interesting thing is the second device is only used by BIOS, and BIOS left it to OS without shutting down, and it is useless for OS. This sounds odd. Isn't this a bug? Is there practical case multi devices behind PCIe-PCI bridge share the same PASID entry without any security concern ? these two customer's case is not. The devices underneath the PCIe-PCI bridge are alias devices of the bridge. PCI alias devices always sit in the same group (the minimal unit that IOMMU guarantees isolation) and can only be attached with a same domain (managed I/O address space). Hence, there's no security concern if they further share the pasid table. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/vt-d: Fix RID2PASID setup failure
Hi，
在 2022/6/20 16:17, Lu Baolu 写道:
The IOMMU driver shares the pasid table for PCI alias devices. When the
RID2PASID entry of the shared pasid table has been filled by the first
device, the subsequent devices will encounter the "DMAR: Setup RID2PASID
failed" failure as the pasid entry has already been marke as present. As
the result, the IOMMU probing process will be aborted.
This fixes it by skipping RID2PASID setting if the pasid entry has been
populated. This works because the IOMMU core ensures that only the same
IOMMU domain can be attached to all PCI alias devices at the same time.
Therefore the subsequent devices just try to setup the RID2PASID entry
with the same domain, which is negligible.
We have two customers reported the issue "DMAR: Setup RID2PASID
failed",
Two ASPEED devices locate behind one PCIe-PCI bridge and iommu SM, PT
mode is enabled. Most
Interesting thing is the second device is only used by BIOS, and BIOS
left it to OS without shutting down,
and it is useless for OS. Is there practical case multi devices behind
PCIe-PCI bridge share the same
PASID entry without any security concern ? these two customer's case is
not.
Thanks,
Ethan
Fixes: ef848b7e5a6a0 ("iommu/vt-d: Setup pasid entry for RID2PASID support")
Reported-by: Chenyi Qiang
Cc:sta...@vger.kernel.org
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 44016594831d..b9966c01a2a2 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2564,7 +2564,7 @@ static int domain_add_dev_info(struct dmar_domain
*domain, struct device *dev)
ret = intel_pasid_setup_second_level(iommu, domain,
dev, PASID_RID2PASID);
spin_unlock_irqrestore(&iommu->lock, flags);
- if (ret) {
+ if (ret && ret != -EBUSY) {
dev_err(dev, "Setup RID2PASID failed\n");
dmar_remove_one_dev_info(dev);
return ret;
--
AFAIK = As Far As I Know
AKA = Also Known As
ASAP = As Soon As Possible
BTW = By The Way (used to introduce some piece of information or question that
is on a different topic but may be of interest)
COLA = comp.os.linux.announce (newsgroup)
ETA = Estimated Time of Arrival
FAQ = Frequently Asked Question
FUD = Fear, Uncertainty and Doubt
FWIW = For What It's Worth
FYI = For Your Information
IANAL = I Am Not A Lawyer
IIRC = If I Recall Correctly
IMHO = In My Humble Opinion
IMNSHO = In My Not-So-Humble Opinion
IOW = In Other Words
LART = Luser Attitude Readjustment Tool (quoting Al Viro: "Anything you use to
forcibly implant the clue into the place where luser's head is")
LUSER = pronounced "loser", a user who is considered to indeed be a loser
(idiot, drongo, wanker, dim-wit, fool, etc.)
OTOH = On The Other Hand
PEBKAC = Problem Exists Between Keyboard And Chair
ROTFL = Rolling On The Floor Laughing
RSN = Real Soon Now
RTFM = Read The Fucking Manual (original definition) or Read The Fine Manual
(if you want to pretend to be polite)
TANSTAAFL = There Ain't No Such Thing As A Free Lunch (contributed by David
Niemi, quoting Robert Heinlein in his science fiction novel 'The Moon is a
Harsh Mistress')
THX = Thanks (thank you)
TIA = Thanks In Advance
WIP = Work In Progress
WRT = With Respect To
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/1] iommu/vt-d: Fix RID2PASID setup failure
> From: Baolu Lu
> Sent: Tuesday, June 21, 2022 5:04 PM
>
> On 2022/6/21 13:48, Tian, Kevin wrote:
> >> From: Baolu Lu
> >> Sent: Tuesday, June 21, 2022 12:28 PM
> >>
> >> On 2022/6/21 11:46, Tian, Kevin wrote:
> From: Baolu Lu
> Sent: Tuesday, June 21, 2022 11:39 AM
>
> On 2022/6/21 10:54, Tian, Kevin wrote:
> >> From: Lu Baolu
> >> Sent: Monday, June 20, 2022 4:17 PM
> >> @@ -2564,7 +2564,7 @@ static int domain_add_dev_info(struct
> >> dmar_domain *domain, struct device *dev)
> >>ret = intel_pasid_setup_second_level(iommu,
> >> domain,
> >>dev, PASID_RID2PASID);
> >>spin_unlock_irqrestore(&iommu->lock, flags);
> >> - if (ret) {
> >> + if (ret && ret != -EBUSY) {
> >>dev_err(dev, "Setup RID2PASID failed\n");
> >>dmar_remove_one_dev_info(dev);
> >>return ret;
> >> --
> >> 2.25.1
> >
> > It's cleaner to avoid this error at the first place, i.e. only do the
> > setup when the first device is attached to the pasid table.
>
> The logic that identifies the first device might introduce additional
> unnecessary complexity. Devices that share a pasid table are rare. I
> even prefer to give up sharing tables so that the code can be
> simpler.:-)
>
> >>>
> >>> It's not that complex if you simply move device_attach_pasid_table()
> >>> out of intel_pasid_alloc_table(). Then do the setup if
> >>> list_empty(&pasid_table->dev) and then attach device to the
> >>> pasid table in domain_add_dev_info().
> >>
> >> The pasid table is part of the device, hence a better place to
> >> allocate/free the pasid table is in the device probe/release paths.
> >> Things will become more complicated if we change relationship between
> >> device and it's pasid table when attaching/detaching a domain. That's
> >> the reason why I thought it was additional complexity.
> >>
> >
> > If you do want to follow current route it’s still cleaner to check
> > whether the pasid entry has pointed to the domain in the individual
> > setup function instead of blindly returning -EBUSY and then ignoring
> > it even if a real busy condition occurs. The setup functions can
> > just return zero for this benign alias case.
>
> Kevin, how do you like this one?
>
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index cb4c1d0cf25c..ecffd0129b2b 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -575,6 +575,16 @@ static inline int pasid_enable_wpe(struct
> pasid_entry *pte)
> return 0;
> };
>
> +/*
> + * Return true if @pasid is RID2PASID and the domain @did has already
> + * been setup to the @pte. Otherwise, return false.
> + */
> +static inline bool
> +rid2pasid_domain_valid(struct pasid_entry *pte, u32 pasid, u16 did)
> +{
> + return pasid == PASID_RID2PASID && pasid_get_domain_id(pte) ==
> did;
> +}
better this is not restricted to RID2PASID only, e.g. pasid_pte_match_domain()
and then read pasid from the pte to compare with the pasid argument.
> +
> /*
>* Set up the scalable mode pasid table entry for first only
>* translation type.
> @@ -595,9 +605,8 @@ int intel_pasid_setup_first_level(struct intel_iommu
> *iommu,
> if (WARN_ON(!pte))
> return -EINVAL;
>
> - /* Caller must ensure PASID entry is not in use. */
> if (pasid_pte_is_present(pte))
> - return -EBUSY;
> + return rid2pasid_domain_valid(pte, pasid, did) ? 0: -EBUSY;
>
> pasid_clear_entry(pte);
>
> @@ -698,9 +707,8 @@ int intel_pasid_setup_second_level(struct
> intel_iommu *iommu,
> return -ENODEV;
> }
>
> - /* Caller must ensure PASID entry is not in use. */
> if (pasid_pte_is_present(pte))
> - return -EBUSY;
> + return rid2pasid_domain_valid(pte, pasid, did) ? 0: -EBUSY;
>
> pasid_clear_entry(pte);
> pasid_set_domain_id(pte, did);
> @@ -738,9 +746,8 @@ int intel_pasid_setup_pass_through(struct
> intel_iommu *iommu,
> return -ENODEV;
> }
>
> - /* Caller must ensure PASID entry is not in use. */
> if (pasid_pte_is_present(pte))
> - return -EBUSY;
> + return rid2pasid_domain_valid(pte, pasid, did) ? 0: -EBUSY;
>
> pasid_clear_entry(pte);
> pasid_set_domain_id(pte, did);
>
> --
> Best regards,
> baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/3] iommu: Clean up release_device checks
On 2022/6/21 23:14, Robin Murphy wrote: Since .release_device is now called through per-device ops, any call which gets as far as a driver definitely*is* for that driver, for a device which has successfully passed .probe_device, so all the checks to that effect are now redundant and can be removed. In the same vein we can also skip freeing fwspecs which are now managed by core code. Does this depend on any other series? I didn't see iommu_fwspec_free() called in the core code. Or I missed anything? Signed-off-by: Robin Murphy --- drivers/iommu/apple-dart.c | 3 --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 +--- drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 +++--- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 11 --- drivers/iommu/exynos-iommu.c| 3 --- drivers/iommu/mtk_iommu.c | 5 - drivers/iommu/mtk_iommu_v1.c| 5 - drivers/iommu/sprd-iommu.c | 11 --- drivers/iommu/virtio-iommu.c| 8 +--- 9 files changed, 5 insertions(+), 63 deletions(-) Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/3] iommu: Make .release_device optional
On 2022/6/21 23:14, Robin Murphy wrote:
Many drivers do nothing meaningful for .release_device, and it's neatly
abstracted to just two callsites in the core code, so let's make it
optional to implement.
Signed-off-by: Robin Murphy
---
drivers/iommu/fsl_pamu_domain.c | 5 -
drivers/iommu/iommu.c | 6 --
drivers/iommu/msm_iommu.c | 5 -
drivers/iommu/sun50i-iommu.c| 3 ---
drivers/iommu/tegra-gart.c | 5 -
drivers/iommu/tegra-smmu.c | 3 ---
6 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 94b4589dc67c..011f9ab7f743 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -447,15 +447,10 @@ static struct iommu_device *fsl_pamu_probe_device(struct
device *dev)
return &pamu_iommu;
}
-static void fsl_pamu_release_device(struct device *dev)
-{
-}
-
static const struct iommu_ops fsl_pamu_ops = {
.capable= fsl_pamu_capable,
.domain_alloc = fsl_pamu_domain_alloc,
.probe_device = fsl_pamu_probe_device,
- .release_device = fsl_pamu_release_device,
.device_group = fsl_pamu_device_group,
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = fsl_pamu_attach_device,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 06d6989f07f6..8b4fc7e62b99 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -259,7 +259,8 @@ static int __iommu_probe_device(struct device *dev, struct
list_head *group_list
return 0;
out_release:
- ops->release_device(dev);
+ if (ops->release_device)
+ ops->release_device(dev);
out_module_put:
module_put(ops->owner);
@@ -337,7 +338,8 @@ void iommu_release_device(struct device *dev)
iommu_device_unlink(dev->iommu->iommu_dev, dev);
ops = dev_iommu_ops(dev);
- ops->release_device(dev);
+ if (ops->release_device)
+ ops->release_device(dev);
iommu_group_remove_device(dev);
module_put(ops->owner);
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index f09aedfdd462..428919a474c1 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -394,10 +394,6 @@ static struct iommu_device *msm_iommu_probe_device(struct
device *dev)
return &iommu->iommu;
}
-static void msm_iommu_release_device(struct device *dev)
-{
-}
-
static int msm_iommu_attach_dev(struct iommu_domain *domain, struct device
*dev)
{
int ret = 0;
@@ -677,7 +673,6 @@ irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id)
static struct iommu_ops msm_iommu_ops = {
.domain_alloc = msm_iommu_domain_alloc,
.probe_device = msm_iommu_probe_device,
- .release_device = msm_iommu_release_device,
.device_group = generic_device_group,
.pgsize_bitmap = MSM_IOMMU_PGSIZES,
.of_xlate = qcom_iommu_of_xlate,
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index c54ab477b8fd..a84c63518773 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -738,8 +738,6 @@ static struct iommu_device
*sun50i_iommu_probe_device(struct device *dev)
return &iommu->iommu;
}
-static void sun50i_iommu_release_device(struct device *dev) {}
-
static struct iommu_group *sun50i_iommu_device_group(struct device *dev)
{
struct sun50i_iommu *iommu = sun50i_iommu_from_dev(dev);
@@ -764,7 +762,6 @@ static const struct iommu_ops sun50i_iommu_ops = {
.domain_alloc = sun50i_iommu_domain_alloc,
.of_xlate = sun50i_iommu_of_xlate,
.probe_device = sun50i_iommu_probe_device,
- .release_device = sun50i_iommu_release_device,
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = sun50i_iommu_attach_device,
.detach_dev = sun50i_iommu_detach_device,
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index a6700a40a6f8..e5ca3cf1a949 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -246,10 +246,6 @@ static struct iommu_device *gart_iommu_probe_device(struct
device *dev)
return &gart_handle->iommu;
}
-static void gart_iommu_release_device(struct device *dev)
-{
-}
-
static int gart_iommu_of_xlate(struct device *dev,
struct of_phandle_args *args)
{
@@ -273,7 +269,6 @@ static void gart_iommu_sync(struct iommu_domain *domain,
static const struct iommu_ops gart_iommu_ops = {
.domain_alloc = gart_iommu_domain_alloc,
.probe_device = gart_iommu_probe_device,
- .release_device = gart_iommu_release_device,
.device_group = generic_device_group,
.pgsize_bitmap = GART_IOMMU_PGSIZES,
.of_xlate = gart_iommu_of_xlate,
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iom
Re: [PATCH v10 1/2] iommu/io-pgtable-arm-v7s: Add a quirk to allow pgtable PA up to 35bit
On Thu, 2022-06-16 at 20:07 +0800, yf.w...@mediatek.com wrote: > From: Yunfei Wang > > Single memory zone feature will remove ZONE_DMA32 and ZONE_DMA and > cause pgtable PA size larger than 32bit. > > Since Mediatek IOMMU hardware support at most 35bit PA in pgtable, > so add a quirk to allow the PA of pgtables support up to bit35. > > Signed-off-by: Ning Li > Signed-off-by: Yunfei Wang > --- > drivers/iommu/io-pgtable-arm-v7s.c | 67 +++- > -- > include/linux/io-pgtable.h | 15 --- > 2 files changed, 63 insertions(+), 19 deletions(-) [...] > /* TTBR */ > - cfg->arm_v7s_cfg.ttbr = virt_to_phys(data->pgd) | > ARM_V7S_TTBR_S | > + paddr = virt_to_phys(data->pgd); > + cfg->arm_v7s_cfg.ttbr = paddr | ARM_V7S_TTBR_S | > (cfg->coherent_walk ? (ARM_V7S_TTBR_NOS > | >ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBW > A) | >ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_WBW > A)) : > (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_NC) > | >ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_NC) > )); > + > + if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT) > + cfg->arm_v7s_cfg.ttbr = (paddr & GENMASK(31, 7)) | > + upper_32_bits(paddr); If we keep ttbr u32, we have to put the special logic here. This line is ok for all the MediaTek cases, not only for this quirk. It means: if (arm_v7s_is_mtk_enabled(cfg)) cfg->arm_v7s_cfg.ttbr = (virt_to_phys(data->pgd) & GENMASK(31, 7)) | upper_32_bits(paddr); else xxx Then we don't need add "& MMU_PT_ADDR_MASK" in mtk_iommu.c since you have done it here. > + > return &data->iop; > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/5] vfio/iommu_type1: Prefer to reuse domains vs match enforced cache coherency
On Tue, Jun 21, 2022 at 04:46:02PM -0600, Alex Williamson wrote: > External email: Use caution opening links or attachments > > > On Wed, 15 Jun 2022 17:03:01 -0700 > Nicolin Chen wrote: > > > From: Jason Gunthorpe > > > > The KVM mechanism for controlling wbinvd is based on OR of the coherency > > property of all devices attached to a guest, no matter those devices are > > attached to a single domain or multiple domains. > > > > So, there is no value in trying to push a device that could do enforced > > cache coherency to a dedicated domain vs re-using an existing domain > > which is non-coherent since KVM won't be able to take advantage of it. > > This just wastes domain memory. > > > > Simplify this code and eliminate the test. This removes the only logic > > that needed to have a dummy domain attached prior to searching for a > > matching domain and simplifies the next patches. > > > > It's unclear whether we want to further optimize the Intel driver to > > update the domain coherency after a device is detached from it, at > > least not before KVM can be verified to handle such dynamics in related > > emulation paths (wbinvd, vcpu load, write_cr0, ept, etc.). In reality > > we don't see an usage requiring such optimization as the only device > > which imposes such non-coherency is Intel GPU which even doesn't > > support hotplug/hot remove. > > The 2nd paragraph above is quite misleading in this respect. I think > it would be more accurate to explain that the benefit to using separate > domains was that devices attached to domains supporting enforced cache > coherency always mapped with the attributes necessary to provide that > feature, therefore if a non-enforced domain was dropped, the associated > group removal would re-trigger an evaluation by KVM. We can then go on > to discuss that in practice the only known cases of such mixed domains > included an Intel IGD device behind an IOMMU lacking snoop control, > where such devices do not support hotplug, therefore this scenario lacks > testing and is not considered sufficiently relevant to support. Thanks, Thanks for the input. I integrated that into the commit log: vfio/iommu_type1: Prefer to reuse domains vs match enforced cache coherency The KVM mechanism for controlling wbinvd is based on OR of the coherency property of all devices attached to a guest, no matter whether those devices are attached to a single domain or multiple domains. On the other hand, the benefit to using separate domains was that those devices attached to domains supporting enforced cache coherency always mapped with the attributes necessary to provide that feature, therefore if a non-enforced domain was dropped, the associated group removal would re-trigger an evaluation by KVM. In practice however, the only known cases of such mixed domains included an Intel IGD device behind an IOMMU lacking snoop control, where such devices do not support hotplug, therefore this scenario lacks testing and is not considered sufficiently relevant to support. After all, KVM won't take advantage of trying to push a device that could do enforced cache coherency to a dedicated domain vs re-using an existing domain, which is non-coherent. Simplify this code and eliminate the test. This removes the only logic that needed to have a dummy domain attached prior to searching for a matching domain and simplifies the next patches. It's unclear whether we want to further optimize the Intel driver to update the domain coherency after a device is detached from it, at least not before KVM can be verified to handle such dynamics in related emulation paths (wbinvd, vcpu load, write_cr0, ept, etc.). In reality we don't see an usage requiring such optimization as the only device which imposes such non-coherency is Intel GPU which even doesn't support hotplug/hot remove. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/5] vfio/iommu_type1: Prefer to reuse domains vs match enforced cache coherency
On Wed, 15 Jun 2022 17:03:01 -0700
Nicolin Chen wrote:
> From: Jason Gunthorpe
>
> The KVM mechanism for controlling wbinvd is based on OR of the coherency
> property of all devices attached to a guest, no matter those devices are
> attached to a single domain or multiple domains.
>
> So, there is no value in trying to push a device that could do enforced
> cache coherency to a dedicated domain vs re-using an existing domain
> which is non-coherent since KVM won't be able to take advantage of it.
> This just wastes domain memory.
>
> Simplify this code and eliminate the test. This removes the only logic
> that needed to have a dummy domain attached prior to searching for a
> matching domain and simplifies the next patches.
>
> It's unclear whether we want to further optimize the Intel driver to
> update the domain coherency after a device is detached from it, at
> least not before KVM can be verified to handle such dynamics in related
> emulation paths (wbinvd, vcpu load, write_cr0, ept, etc.). In reality
> we don't see an usage requiring such optimization as the only device
> which imposes such non-coherency is Intel GPU which even doesn't
> support hotplug/hot remove.
The 2nd paragraph above is quite misleading in this respect. I think
it would be more accurate to explain that the benefit to using separate
domains was that devices attached to domains supporting enforced cache
coherency always mapped with the attributes necessary to provide that
feature, therefore if a non-enforced domain was dropped, the associated
group removal would re-trigger an evaluation by KVM. We can then go on
to discuss that in practice the only known cases of such mixed domains
included an Intel IGD device behind an IOMMU lacking snoop control,
where such devices do not support hotplug, therefore this scenario lacks
testing and is not considered sufficiently relevant to support. Thanks,
Alex
>
> Signed-off-by: Jason Gunthorpe
> Signed-off-by: Nicolin Chen
> ---
> drivers/vfio/vfio_iommu_type1.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index c13b9290e357..f4e3b423a453 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -2285,9 +2285,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>* testing if they're on the same bus_type.
>*/
> list_for_each_entry(d, &iommu->domain_list, next) {
> - if (d->domain->ops == domain->domain->ops &&
> - d->enforce_cache_coherency ==
> - domain->enforce_cache_coherency) {
> + if (d->domain->ops == domain->domain->ops) {
> iommu_detach_group(domain->domain, group->iommu_group);
> if (!iommu_attach_group(d->domain,
> group->iommu_group)) {
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/3] scsi: BusLogic remove bus_to_virt
On 6/17/22 06:57, Arnd Bergmann wrote:
From: Arnd Bergmann
The BusLogic driver is the last remaining driver that relies on the
deprecated bus_to_virt() function, which in turn only works on a few
architectures, and is incompatible with both swiotlb and iommu support.
Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."),
the driver had a dependency on x86-32, presumably because of this
problem. However, the change introduced another bug that made it still
impossible to use the driver on any 64-bit machine.
This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix
64-bit system enumeration error for Buslogic"), 8 years later, which
shows that there are not a lot of users.
Maciej is still using the driver on 32-bit hardware, and Khalid mentioned
that the driver works with the device emulation used in VirtualBox
and VMware. Both of those only emulate it for Windows 2000 and older
operating systems that did not ship with the better LSI logic driver.
Do a minimum fix that searches through the list of descriptors to find
one that matches the bus address. This is clearly as inefficient as
was indicated in the code comment about the lack of a bus_to_virt()
replacement. A better fix would likely involve changing out the entire
descriptor allocation for a simpler one, but that would be much
more invasive.
Cc: Maciej W. Rozycki
Cc: Matt Wang
Cc: Khalid Aziz
Signed-off-by: Arnd Bergmann
---
drivers/scsi/BusLogic.c | 27 ---
drivers/scsi/Kconfig| 2 +-
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index a897c8f914cf..d057abfcdd5c 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter
*adapter,
return (hoststatus << 16) | tgt_status;
}
+/*
+ * turn the dma address from an inbox into a ccb pointer
+ * This is rather inefficient.
+ */
+static struct blogic_ccb *
+blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox)
+{
+ struct blogic_ccb *ccb;
+
+ for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all)
+ if (inbox->ccb == ccb->dma_handle)
+ break;
+
+ return ccb;
+}
/*
blogic_scan_inbox scans the Incoming Mailboxes saving any
Incoming Mailbox entries for completion processing.
*/
-
static void blogic_scan_inbox(struct blogic_adapter *adapter)
{
/*
@@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter
*adapter)
enum blogic_cmplt_code comp_code;
while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
- /*
- We are only allowed to do this because we limit our
- architectures we run on to machines where bus_to_virt(
- actually works. There *needs* to be a dma_addr_to_virt()
- in the new PCI DMA mapping interface to replace
- bus_to_virt() or else this code is going to become very
- innefficient.
-*/
- struct blogic_ccb *ccb =
- (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
+ struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter,
adapter->next_inbox);
This change looks good enough as workaround to not use bus_to_virt() for
now. There are two problems I see though. One, I do worry about
blogic_inbox_to_ccb() returning NULL for ccb which should not happen
unless the mailbox pointer was corrupted which would indicate a bigger
problem. Nevertheless a NULL pointer causing kernel panic concerns me.
How about adding a check before we dereference ccb?
Second, with this patch applied, I am seeing errors from the driver:
=
[ 1623.902685] sdb: sdb1 sdb2
[ 1623.903245] sd 2:0:0:0: [sdb] Attached SCSI disk
[ 1623.911000] scsi2: Illegal CCB #76 status 2 in Incoming Mailbox
[ 1623.911005] scsi2: Illegal CCB #76 status 2 in Incoming Mailbox
[ 1623.911070] scsi2: Illegal CCB #79 status 2 in Incoming Mailbox
[ 1651.458008] scsi2: Warning: Partition Table appears to have Geometry
256/63 which is
[ 1651.458013] scsi2: not compatible with current BusLogic Host Adapter
Geometry 255/63
[ 1658.797609] scsi2: Resetting BusLogic BT-958D Failed
[ 1659.533208] sd 2:0:0:0: Device offlined - not ready after error recovery
[ 1659.51] sd 2:0:0:0: Device offlined - not ready after error recovery
[ 1659.53] sd 2:0:0:0: Device offlined - not ready after error recovery
[ 1659.533342] sd 2:0:0:0: [sdb] tag#101 FAILED Result:
hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK cmd_age=35s
[ 1659.533345] sd 2:0:0:0: [sdb] tag#101 CDB: Read(10) 28 00 00 00 00 28
00 00 10 00
[ 1659.533346] I/O error, dev sdb, sector 40 op 0x0:(READ) flags 0x80700
phys_seg 1 prio class 0
=
This is on VirtualBox using emulated BusLogic adapter.
This pa
Re: [PATCH v2 5/5] vfio/iommu_type1: Simplify group attachment
On Mon, Jun 20, 2022 at 11:11:01AM +0100, Robin Murphy wrote:
> External email: Use caution opening links or attachments
>
>
> On 2022-06-17 03:53, Tian, Kevin wrote:
> > > From: Nicolin Chen
> > > Sent: Friday, June 17, 2022 6:41 AM
> > >
> > > > ...
> > > > > - if (resv_msi) {
> > > > > + if (resv_msi && !domain->msi_cookie) {
> > > > >ret = iommu_get_msi_cookie(domain->domain,
> > > > > resv_msi_base);
> > > > >if (ret && ret != -ENODEV)
> > > > >goto out_detach;
> > > > > + domain->msi_cookie = true;
> > > > >}
> > > >
> > > > why not moving to alloc_attach_domain() then no need for the new
> > > > domain field? It's required only when a new domain is allocated.
> > >
> > > When reusing an existing domain that doesn't have an msi_cookie,
> > > we can do iommu_get_msi_cookie() if resv_msi is found. So it is
> > > not limited to a new domain.
> >
> > Looks msi_cookie requirement is per platform (currently only
> > for smmu. see arm_smmu_get_resv_regions()). If there is
> > no mixed case then above check is not required.
> >
> > But let's hear whether Robin has a different thought here.
>
> Yes, the cookie should logically be tied to the lifetime of the domain
> itself. In the relevant context, "an existing domain that doesn't have
> an msi_cookie" should never exist.
Thanks for the explanation. I will move the iommu_get_msi_cookie()
into alloc_attach_domain(), as Kevin suggested.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/5] vfio/iommu_type1: Simplify group attachment
On Mon, Jun 20, 2022 at 01:03:17AM -0300, Jason Gunthorpe wrote: > On Fri, Jun 17, 2022 at 04:07:20PM -0700, Nicolin Chen wrote: > > > > > > > + vfio_iommu_aper_expand(iommu, &iova_copy); > > > > > > > > > > but now it's done for every group detach. The aperture is decided > > > > > by domain geometry which is not affected by attached groups. > > > > > > > > Yea, I've noticed this part. Actually Jason did this change for > > > > simplicity, and I think it'd be safe to do so? > > > > > > Perhaps detach_destroy() can return a Boolean to indicate whether > > > a domain is destroyed. > > > > It could be a solution but doesn't feel that common for a clean > > function to have a return value indicating a special case. Maybe > > passing in "&domain" so that we can check if it's NULL after? > > It is harmless to do every time, it just burns a few CPU cycles on a > slow path. We don't need complexity to optmize it. OK. I will keep it simple then. If Kevin or other has a further objection, please let us know. Thanks Nic ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver
Hi Marek, On Fri, 21 Jan 2022 at 14:31, Marek Szyprowski wrote: [snip] > > Well, for starting point the existing exynos-iommu driver is really > enough. I've played a bit with newer Exyos SoCs some time ago. If I > remember right, if you limit the iommu functionality to the essential > things like mapping pages to IO-virtual space, the hardware differences > between SYSMMU v5 (already supported by the exynos-iommu driver) and v7 > are just a matter of changing a one register during the initialization > and different bits the page fault reason decoding. You must of course > rely on the DMA-mapping framework and its implementation based on > mainline DMA-IOMMU helpers. All the code for custom iommu group(s) > handling or extended fault management are not needed for the initial > version. > Thanks for the advice! Just implemented some testing driver, which uses "Emulated Translation" registers available on SysMMU v7. That's one way to verify the IOMMU driver with no actual users of it. It works fine with vendor SysMMU driver I ported to mainline earlier, and now I'm trying to use it with exynos-sysmmu driver (existing upstream). If you're curious -- I can share the testing driver somewhere on GitHub. I believe the register you mentioned is PT_BASE one, so I used REG_V7_FLPT_BASE_VM = 0x800C instead of REG_V5_PT_BASE_PFN. But I didn't manage to get that far, unfortunately, as exynos_iommu_domain_alloc() function fails in my case, with BUG_ON() at this line: /* For mapping page table entries we rely on dma == phys */ BUG_ON(handle != virt_to_phys(domain->pgtable)); One possible explanation for this BUG is that "dma-ranges" property is not provided in DTS (which seems to be the case right now for all users of "samsung,exynos-sysmmu" driver). Because of that the SWIOTLB is used for dma_map_single() call (in exynos_iommu_domain_alloc() function), which in turn leads to that BUG. At least that's what happens in my case. The call chain looks like this: exynos_iommu_domain_alloc() v dma_map_single() v dma_map_single_attrs() v dma_map_page_attrs() v dma_direct_map_page() // dma_capable() == false v swiotlb_map() v swiotlb_tbl_map_single() And the last call of course always returns the address different than the address for allocated pgtable. E.g. in my case I see this: handle = 0xfbfff000 virt_to_phys(domain->pgtable) = 0x000880d0c000 Do you know what might be the reason for that? I just wonder how the SysMMU driver work for all existing Exynos platforms right now. I feel I might be missing something, like some DMA option should be enabled so that SWIOTLB is not used, or something like that. Please let me know if you have any idea on possible cause. The vendor's SysMMU driver is kinda different in that regard, as it doesn't use dma_map_single(), so I don't see such issue there. Thanks! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren wrote: > > Hi, > > * Saravana Kannan [700101 02:00]: > > Now that fw_devlink=on by default and fw_devlink supports > > "power-domains" property, the execution will never get to the point > > where driver_deferred_probe_check_state() is called before the supplier > > has probed successfully or before deferred probe timeout has expired. > > > > So, delete the call and replace it with -ENODEV. > > Looks like this causes omaps to not boot in Linux next. Can you please point me to an example DTS I could use for debugging this? I'm assuming you are leaving fw_devlink=on and not turning it off or putting it in permissive mode. > With this > simple-pm-bus fails to probe initially as the power-domain is not > yet available. Before we get to late_initcall(), I'd expect this series to not have any impact because both fw_devlink and driver_deferred_probe_check_state() should be causing the device's probe to get deferred until the PM domain device comes up. To double check this, without this series, can you give me the list of "supplier:*" symlinks under a simple-pm-bus device's sysfs folder that's having problems with this series? And for all those symlinks, cat the "status" file under that directory? In the system where this is failing, is the PM domain driver loaded as a module at a later point? Couple of other things to try (with the patches) to narrow this down: * Can you set driver_probe_timeout=0 in the command line and see if that helps? * Can you set it to something high like 30 or even larger and see if it helps? > On platform_probe() genpd_get_from_provider() returns > -ENOENT. This error is with the series I assume? > Seems like other stuff is potentially broken too, any ideas on > how to fix this? I'll want to understand the issue first. It's not yet clear to me why fw_devlink isn't blocking the probe of the simple-pm-bus device until the PM domain device shows up. And if it is not blocking, then why and at what point in boot it's giving up and letting the probe get to this point where there's an error. -Saravana > > > > > > Signed-off-by: Saravana Kannan > > --- > > drivers/base/power/domain.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index 739e52cd4aba..3e86772d5fac 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -2730,7 +2730,7 @@ static int __genpd_dev_pm_attach(struct device *dev, > > struct device *base_dev, > > mutex_unlock(&gpd_list_lock); > > dev_dbg(dev, "%s() failed to find PM domain: %ld\n", > > __func__, PTR_ERR(pd)); > > - return driver_deferred_probe_check_state(base_dev); > > + return -ENODEV; > > } > > > > dev_dbg(dev, "adding to PM domain %s\n", pd->name); > > -- > > 2.36.1.255.ge46751e96f-goog > > > > -- > To unsubscribe from this group and stop receiving emails from it, send an > email to kernel-team+unsubscr...@android.com. > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] vfio/type1: Simplify bus_type determination
On 2022-06-10 01:03, Jason Gunthorpe via iommu wrote:
On Wed, Jun 08, 2022 at 03:25:49PM +0100, Robin Murphy wrote:
Since IOMMU groups are mandatory for drivers to support, it stands to
reason that any device which has been successfully be added to a group
must be on a bus supported by that IOMMU driver, and therefore a domain
viable for any device in the group must be viable for all devices in
the group. This already has to be the case for the IOMMU API's internal
default domain, for instance. Thus even if the group contains devices
on different buses, that can only mean that the IOMMU driver actually
supports such an odd topology, and so without loss of generality we can
expect the bus type of any arbitrary device in a group to be suitable
for IOMMU API calls.
Replace vfio_bus_type() with a trivial callback that simply returns any
device from which to then derive a usable bus type. This is also a step
towards removing the vague bus-based interfaces from the IOMMU API.
Furthermore, scrutiny reveals a lack of protection for the bus and/or
device being removed while .attach_group is inspecting them; the
reference we hold on the iommu_group ensures that data remains valid,
but does not prevent the group's membership changing underfoot. Holding
the vfio_goup's device_lock should be sufficient to block any relevant
device's VFIO driver from unregistering, and thus block unbinding and
any further stages of removal for the duration of the attach operation.
The device_lock only protects devices that are on the device_list from
concurrent unregistration, the device returned by
iommu_group_for_each_dev() is not guarented to be the on the device
list.
Sigh, you're quite right, and now I have a vague feeling that you called
that out in the previous discussion too, so apologies for forgetting.
@@ -760,8 +760,11 @@ static int __vfio_container_attach_groups(struct
vfio_container *container,
int ret = -ENODEV;
list_for_each_entry(group, &container->group_list, container_next) {
+ /* Prevent devices unregistering during attach */
+ mutex_lock(&group->device_lock);
ret = driver->ops->attach_group(data, group->iommu_group,
group->type);
+ mutex_unlock(&group->device_lock);
I still prefer the version where we pass in an arbitrary vfio_device
from the list the group maintains:
list_first_entry(group->device_list)
And don't call iommu_group_for_each_dev(), it is much simpler to
reason about how it works.
Agreed, trying to figure out which are the VFIO devices from within the
iommu_group iterator seems beyond the threshold of practicality.
Quick consensus then: does anyone have a particular preference between
changing the .attach_group signature vs. adding a helper based on
vfio_group_get_from_iommu() for type1 to call from within its callback?
They seem about equal (but opposite) in terms of the simplicity vs.
impact tradeoff to me, so I can't quite decide conclusively...
Thanks,
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/3] iommu: Clean up release_device checks
Since .release_device is now called through per-device ops, any call
which gets as far as a driver definitely *is* for that driver, for a
device which has successfully passed .probe_device, so all the checks to
that effect are now redundant and can be removed. In the same vein we
can also skip freeing fwspecs which are now managed by core code.
Signed-off-by: Robin Murphy
---
drivers/iommu/apple-dart.c | 3 ---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 +---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 +++---
drivers/iommu/arm/arm-smmu/qcom_iommu.c | 11 ---
drivers/iommu/exynos-iommu.c| 3 ---
drivers/iommu/mtk_iommu.c | 5 -
drivers/iommu/mtk_iommu_v1.c| 5 -
drivers/iommu/sprd-iommu.c | 11 ---
drivers/iommu/virtio-iommu.c| 8 +---
9 files changed, 5 insertions(+), 63 deletions(-)
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index 8af0242a90d9..e87d3cf54ed6 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -564,9 +564,6 @@ static void apple_dart_release_device(struct device *dev)
{
struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
- if (!cfg)
- return;
-
dev_iommu_priv_set(dev, NULL);
kfree(cfg);
}
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 88817a3376ef..382f3120e27b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2691,20 +2691,14 @@ static struct iommu_device
*arm_smmu_probe_device(struct device *dev)
static void arm_smmu_release_device(struct device *dev)
{
- struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
- struct arm_smmu_master *master;
+ struct arm_smmu_master *master = dev_iommu_priv_get(dev);
- if (!fwspec || fwspec->ops != &arm_smmu_ops)
- return;
-
- master = dev_iommu_priv_get(dev);
if (WARN_ON(arm_smmu_master_sva_enabled(master)))
iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
arm_smmu_detach_dev(master);
arm_smmu_disable_pasid(master);
arm_smmu_remove_master(master);
kfree(master);
- iommu_fwspec_free(dev);
}
static struct iommu_group *arm_smmu_device_group(struct device *dev)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 2ed3594f384e..7c2a99862fd3 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1432,27 +1432,19 @@ static struct iommu_device
*arm_smmu_probe_device(struct device *dev)
static void arm_smmu_release_device(struct device *dev)
{
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
- struct arm_smmu_master_cfg *cfg;
- struct arm_smmu_device *smmu;
+ struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
int ret;
- if (!fwspec || fwspec->ops != &arm_smmu_ops)
- return;
-
- cfg = dev_iommu_priv_get(dev);
- smmu = cfg->smmu;
-
- ret = arm_smmu_rpm_get(smmu);
+ ret = arm_smmu_rpm_get(cfg->smmu);
if (ret < 0)
return;
arm_smmu_master_free_smes(cfg, fwspec);
- arm_smmu_rpm_put(smmu);
+ arm_smmu_rpm_put(cfg->smmu);
dev_iommu_priv_set(dev, NULL);
kfree(cfg);
- iommu_fwspec_free(dev);
}
static void arm_smmu_probe_finalize(struct device *dev)
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 4c077c38fbd6..4a922c7b69ee 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -532,16 +532,6 @@ static struct iommu_device *qcom_iommu_probe_device(struct
device *dev)
return &qcom_iommu->iommu;
}
-static void qcom_iommu_release_device(struct device *dev)
-{
- struct qcom_iommu_dev *qcom_iommu = to_iommu(dev);
-
- if (!qcom_iommu)
- return;
-
- iommu_fwspec_free(dev);
-}
-
static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args
*args)
{
struct qcom_iommu_dev *qcom_iommu;
@@ -591,7 +581,6 @@ static const struct iommu_ops qcom_iommu_ops = {
.capable= qcom_iommu_capable,
.domain_alloc = qcom_iommu_domain_alloc,
.probe_device = qcom_iommu_probe_device,
- .release_device = qcom_iommu_release_device,
.device_group = generic_device_group,
.of_xlate = qcom_iommu_of_xlate,
.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M,
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 71f2018e23fe..1d6808d6e190 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1251,9 +1251,6 @@ static void exynos_iommu_release_device(struct de
[PATCH 1/3] iommu: Use dev_iommu_ops() for probe_finalize
The ->probe_finalize hook only runs after ->probe_device succeeds,
so we can move that over to the new dev_iommu_ops() as well.
Reviewed-by: Lu Baolu
Signed-off-by: Robin Murphy
---
drivers/iommu/iommu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 847ad47a2dfd..06d6989f07f6 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -272,7 +272,7 @@ static int __iommu_probe_device(struct device *dev, struct
list_head *group_list
int iommu_probe_device(struct device *dev)
{
- const struct iommu_ops *ops = dev->bus->iommu_ops;
+ const struct iommu_ops *ops;
struct iommu_group *group;
int ret;
@@ -313,6 +313,7 @@ int iommu_probe_device(struct device *dev)
mutex_unlock(&group->mutex);
iommu_group_put(group);
+ ops = dev_iommu_ops(dev);
if (ops->probe_finalize)
ops->probe_finalize(dev);
--
2.36.1.dirty
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] iommu: Make .release_device optional
Many drivers do nothing meaningful for .release_device, and it's neatly
abstracted to just two callsites in the core code, so let's make it
optional to implement.
Signed-off-by: Robin Murphy
---
drivers/iommu/fsl_pamu_domain.c | 5 -
drivers/iommu/iommu.c | 6 --
drivers/iommu/msm_iommu.c | 5 -
drivers/iommu/sun50i-iommu.c| 3 ---
drivers/iommu/tegra-gart.c | 5 -
drivers/iommu/tegra-smmu.c | 3 ---
6 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 94b4589dc67c..011f9ab7f743 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -447,15 +447,10 @@ static struct iommu_device *fsl_pamu_probe_device(struct
device *dev)
return &pamu_iommu;
}
-static void fsl_pamu_release_device(struct device *dev)
-{
-}
-
static const struct iommu_ops fsl_pamu_ops = {
.capable= fsl_pamu_capable,
.domain_alloc = fsl_pamu_domain_alloc,
.probe_device = fsl_pamu_probe_device,
- .release_device = fsl_pamu_release_device,
.device_group = fsl_pamu_device_group,
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = fsl_pamu_attach_device,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 06d6989f07f6..8b4fc7e62b99 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -259,7 +259,8 @@ static int __iommu_probe_device(struct device *dev, struct
list_head *group_list
return 0;
out_release:
- ops->release_device(dev);
+ if (ops->release_device)
+ ops->release_device(dev);
out_module_put:
module_put(ops->owner);
@@ -337,7 +338,8 @@ void iommu_release_device(struct device *dev)
iommu_device_unlink(dev->iommu->iommu_dev, dev);
ops = dev_iommu_ops(dev);
- ops->release_device(dev);
+ if (ops->release_device)
+ ops->release_device(dev);
iommu_group_remove_device(dev);
module_put(ops->owner);
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index f09aedfdd462..428919a474c1 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -394,10 +394,6 @@ static struct iommu_device *msm_iommu_probe_device(struct
device *dev)
return &iommu->iommu;
}
-static void msm_iommu_release_device(struct device *dev)
-{
-}
-
static int msm_iommu_attach_dev(struct iommu_domain *domain, struct device
*dev)
{
int ret = 0;
@@ -677,7 +673,6 @@ irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id)
static struct iommu_ops msm_iommu_ops = {
.domain_alloc = msm_iommu_domain_alloc,
.probe_device = msm_iommu_probe_device,
- .release_device = msm_iommu_release_device,
.device_group = generic_device_group,
.pgsize_bitmap = MSM_IOMMU_PGSIZES,
.of_xlate = qcom_iommu_of_xlate,
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index c54ab477b8fd..a84c63518773 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -738,8 +738,6 @@ static struct iommu_device
*sun50i_iommu_probe_device(struct device *dev)
return &iommu->iommu;
}
-static void sun50i_iommu_release_device(struct device *dev) {}
-
static struct iommu_group *sun50i_iommu_device_group(struct device *dev)
{
struct sun50i_iommu *iommu = sun50i_iommu_from_dev(dev);
@@ -764,7 +762,6 @@ static const struct iommu_ops sun50i_iommu_ops = {
.domain_alloc = sun50i_iommu_domain_alloc,
.of_xlate = sun50i_iommu_of_xlate,
.probe_device = sun50i_iommu_probe_device,
- .release_device = sun50i_iommu_release_device,
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = sun50i_iommu_attach_device,
.detach_dev = sun50i_iommu_detach_device,
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index a6700a40a6f8..e5ca3cf1a949 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -246,10 +246,6 @@ static struct iommu_device *gart_iommu_probe_device(struct
device *dev)
return &gart_handle->iommu;
}
-static void gart_iommu_release_device(struct device *dev)
-{
-}
-
static int gart_iommu_of_xlate(struct device *dev,
struct of_phandle_args *args)
{
@@ -273,7 +269,6 @@ static void gart_iommu_sync(struct iommu_domain *domain,
static const struct iommu_ops gart_iommu_ops = {
.domain_alloc = gart_iommu_domain_alloc,
.probe_device = gart_iommu_probe_device,
- .release_device = gart_iommu_release_device,
.device_group = generic_device_group,
.pgsize_bitmap = GART_IOMMU_PGSIZES,
.of_xlate = gart_iommu_of_xlate,
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 1fea68e551f1..2a8de975fe63 100644
--- a/drivers/
[PATCH 0/3] iommu: More internal ops cleanup
Hi all, Here are a few more thematically-related patches from my develompent stack that don't depend on the rest, so may as well get some exposure sooner rather than later. Thanks, Robin. Robin Murphy (3): iommu: Use dev_iommu_ops() for probe_finalize iommu: Make .release_device optional iommu: Clean up release_device checks drivers/iommu/apple-dart.c | 3 --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 +--- drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 +++--- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 11 --- drivers/iommu/exynos-iommu.c| 3 --- drivers/iommu/fsl_pamu_domain.c | 5 - drivers/iommu/iommu.c | 9 ++--- drivers/iommu/msm_iommu.c | 5 - drivers/iommu/mtk_iommu.c | 5 - drivers/iommu/mtk_iommu_v1.c| 5 - drivers/iommu/sprd-iommu.c | 11 --- drivers/iommu/sun50i-iommu.c| 3 --- drivers/iommu/tegra-gart.c | 5 - drivers/iommu/tegra-smmu.c | 3 --- drivers/iommu/virtio-iommu.c| 8 +--- 15 files changed, 11 insertions(+), 87 deletions(-) -- 2.36.1.dirty ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 02/10] dt-bindings: display: tegra: Convert to json-schema
From: Thierry Reding
Convert the Tegra host1x controller bindings from the free-form text
format to json-schema.
This also adds the missing display-hub DT bindings that were not
previously documented.
Reviewed-by: Rob Herring
Signed-off-by: Thierry Reding
---
.../display/tegra/nvidia,tegra114-mipi.txt| 41 --
.../display/tegra/nvidia,tegra114-mipi.yaml | 74 ++
.../display/tegra/nvidia,tegra124-dpaux.yaml | 149
.../display/tegra/nvidia,tegra124-sor.yaml| 206 ++
.../display/tegra/nvidia,tegra124-vic.yaml| 71 ++
.../display/tegra/nvidia,tegra186-dc.yaml | 85 +++
.../tegra/nvidia,tegra186-display.yaml| 310
.../tegra/nvidia,tegra186-dsi-padctl.yaml | 45 ++
.../display/tegra/nvidia,tegra20-dc.yaml | 181 +
.../display/tegra/nvidia,tegra20-dsi.yaml | 159 +
.../display/tegra/nvidia,tegra20-epp.yaml | 70 ++
.../display/tegra/nvidia,tegra20-gr2d.yaml| 73 ++
.../display/tegra/nvidia,tegra20-gr3d.yaml| 214 ++
.../display/tegra/nvidia,tegra20-hdmi.yaml| 126
.../display/tegra/nvidia,tegra20-host1x.txt | 675 --
.../display/tegra/nvidia,tegra20-host1x.yaml | 347 +
.../display/tegra/nvidia,tegra20-isp.yaml | 67 ++
.../display/tegra/nvidia,tegra20-mpe.yaml | 73 ++
.../display/tegra/nvidia,tegra20-tvo.yaml | 58 ++
.../display/tegra/nvidia,tegra20-vi.yaml | 163 +
.../display/tegra/nvidia,tegra210-csi.yaml| 52 ++
.../pinctrl/nvidia,tegra124-dpaux-padctl.txt | 59 --
22 files changed, 2523 insertions(+), 775 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.txt
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-dpaux.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-sor.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-dc.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-display.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-dsi-padctl.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dc.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dsi.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-epp.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr2d.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr3d.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-hdmi.yaml
delete mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-isp.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-mpe.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-tvo.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
create mode 100644
Documentation/devicetree/bindings/display/tegra/nvidia,tegra210-csi.yaml
delete mode 100644
Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
diff --git
a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.txt
b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.txt
deleted file mode 100644
index e4a25cedc5cf..
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.txt
+++ /dev/null
@@ -1,41 +0,0 @@
-NVIDIA Tegra MIPI pad calibration controller
-
-Required properties:
-- compatible: "nvidia,tegra-mipi"
-- reg: Physical base address and length of the controller's registers.
-- clocks: Must contain an entry for each entry in clock-names.
- See ../clocks/clock-bindings.txt for details.
-- clock-names: Must include the following entries:
- - mipi-cal
-- #nvidia,mipi-calibrate-cells: Should be 1. The cell is a bitmask of the pads
- that need to be calibrated for a given device.
-
-User nodes need to contain an nvidia,mipi-calibrate property that has a
-phandle to refer to the calibration controller node and a bitmask of the pads
-that need to be calibrated.
-
-Example:
-
- mipi: mipi@700e3000 {
- compatible = "nvidia,tegra114-mipi";
- reg = <0x700e3000 0x100>;
- clocks = <&tegra_car TEGRA114_CLK_MIPI_CAL>;
- clock-names = "mipi-cal";
- #nvidia,mipi-calibrate-cells = <1>;
- };
-
- .
[PATCH v6 09/10] drm/tegra: Support context isolation
From: Mikko Perttunen
For engines that support context isolation, allocate a context when
opening a channel, and set up stream ID offset and context fields
when submitting a job.
As of this commit, the stream ID offset and fallback stream ID
are not used when context isolation is disabled. However, with
upcoming patches that enable a full featured job opcode sequence,
these will be necessary.
Signed-off-by: Mikko Perttunen
---
v5:
* On supporting engines, always program stream ID offset and
new fallback stream ID.
* Rename host1x_context to host1x_memory_context
v4:
* Separate error and output values in get_streamid_offset API
* Improve error handling
* Rename job->context to job->memory_context for clarity
---
drivers/gpu/drm/tegra/drm.h| 3 +++
drivers/gpu/drm/tegra/submit.c | 48 +-
drivers/gpu/drm/tegra/uapi.c | 43 --
3 files changed, 91 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index fc0a19554eac..2acc8f2948ad 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -80,6 +80,7 @@ struct tegra_drm_context {
/* Only used by new UAPI. */
struct xarray mappings;
+ struct host1x_memory_context *memory_context;
};
struct tegra_drm_client_ops {
@@ -91,6 +92,8 @@ struct tegra_drm_client_ops {
int (*submit)(struct tegra_drm_context *context,
struct drm_tegra_submit *args, struct drm_device *drm,
struct drm_file *file);
+ int (*get_streamid_offset)(struct tegra_drm_client *client, u32
*offset);
+ int (*can_use_memory_ctx)(struct tegra_drm_client *client, bool
*supported);
};
int tegra_drm_submit(struct tegra_drm_context *context,
diff --git a/drivers/gpu/drm/tegra/submit.c b/drivers/gpu/drm/tegra/submit.c
index 6d6dd8c35475..b24738bdf3df 100644
--- a/drivers/gpu/drm/tegra/submit.c
+++ b/drivers/gpu/drm/tegra/submit.c
@@ -498,6 +498,9 @@ static void release_job(struct host1x_job *job)
struct tegra_drm_submit_data *job_data = job->user_data;
u32 i;
+ if (job->memory_context)
+ host1x_memory_context_put(job->memory_context);
+
for (i = 0; i < job_data->num_used_mappings; i++)
tegra_drm_mapping_put(job_data->used_mappings[i].mapping);
@@ -588,11 +591,51 @@ int tegra_drm_ioctl_channel_submit(struct drm_device
*drm, void *data,
goto put_job;
}
+ if (context->client->ops->get_streamid_offset) {
+ err = context->client->ops->get_streamid_offset(
+ context->client, &job->engine_streamid_offset);
+ if (err) {
+ SUBMIT_ERR(context, "failed to get streamid offset:
%d", err);
+ goto unpin_job;
+ }
+ }
+
+ if (context->memory_context &&
context->client->ops->can_use_memory_ctx) {
+ bool supported;
+
+ err = context->client->ops->can_use_memory_ctx(context->client,
&supported);
+ if (err) {
+ SUBMIT_ERR(context, "failed to detect if engine can use
memory context: %d", err);
+ goto unpin_job;
+ }
+
+ if (supported) {
+ job->memory_context = context->memory_context;
+ host1x_memory_context_get(job->memory_context);
+ }
+ } else if (context->client->ops->get_streamid_offset) {
+#ifdef CONFIG_IOMMU_API
+ struct iommu_fwspec *spec;
+
+ /*
+* Job submission will need to temporarily change stream ID,
+* so need to tell it what to change it back to.
+*/
+ spec = dev_iommu_fwspec_get(context->client->base.dev);
+ if (spec && spec->num_ids > 0)
+ job->engine_fallback_streamid = spec->ids[0] & 0x;
+ else
+ job->engine_fallback_streamid = 0x7f;
+#else
+ job->engine_fallback_streamid = 0x7f;
+#endif
+ }
+
/* Boot engine. */
err = pm_runtime_resume_and_get(context->client->base.dev);
if (err < 0) {
SUBMIT_ERR(context, "could not power up engine: %d", err);
- goto unpin_job;
+ goto put_memory_context;
}
job->user_data = job_data;
@@ -627,6 +670,9 @@ int tegra_drm_ioctl_channel_submit(struct drm_device *drm,
void *data,
goto put_job;
+put_memory_context:
+ if (job->memory_context)
+ host1x_memory_context_put(job->memory_context);
unpin_job:
host1x_job_unpin(job);
put_job:
diff --git a/drivers/gpu/drm/tegra/uapi.c b/drivers/gpu/drm/tegra/uapi.c
index 9ab9179d2026..a98239cb0e29 100644
--- a/drivers/gpu/drm/tegra/uapi.c
+++ b/drivers/gpu/drm/tegra/uapi.c
@@ -33,6 +33,9 @@ static void tegra_drm
[PATCH v6 04/10] gpu: host1x: Add context device management code
From: Mikko Perttunen
Add code to register context devices from device tree, allocate them
out and manage their refcounts.
Signed-off-by: Mikko Perttunen
---
v2:
* Directly set DMA mask instead of inheriting from Host1x.
* Use iommu-map instead of custom DT property.
v4:
* Use u64 instead of dma_addr_t for DMA mask
* Use unsigned ints for indexes and adjust error handling flow
* Parse iommu-map property at top level host1x DT node
* Use separate DMA mask per device
* Export symbols as GPL
v5:
* Rename host1x_context to host1x_memory_context
---
drivers/gpu/host1x/Makefile | 1 +
drivers/gpu/host1x/context.c | 160 +++
drivers/gpu/host1x/context.h | 27 ++
drivers/gpu/host1x/dev.c | 12 ++-
drivers/gpu/host1x/dev.h | 2 +
include/linux/host1x.h | 18
6 files changed, 219 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/host1x/context.c
create mode 100644 drivers/gpu/host1x/context.h
diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile
index c891a3e33844..8a65e13d113a 100644
--- a/drivers/gpu/host1x/Makefile
+++ b/drivers/gpu/host1x/Makefile
@@ -10,6 +10,7 @@ host1x-y = \
debug.o \
mipi.o \
fence.o \
+ context.o \
hw/host1x01.o \
hw/host1x02.o \
hw/host1x04.o \
diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c
new file mode 100644
index ..d7d95b69a72a
--- /dev/null
+++ b/drivers/gpu/host1x/context.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021, NVIDIA Corporation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "context.h"
+#include "dev.h"
+
+int host1x_memory_context_list_init(struct host1x *host1x)
+{
+ struct host1x_memory_context_list *cdl = &host1x->context_list;
+ struct device_node *node = host1x->dev->of_node;
+ struct host1x_memory_context *ctx;
+ unsigned int i;
+ int err;
+
+ cdl->devs = NULL;
+ cdl->len = 0;
+ mutex_init(&cdl->lock);
+
+ err = of_property_count_u32_elems(node, "iommu-map");
+ if (err < 0)
+ return 0;
+
+ cdl->devs = kcalloc(err, sizeof(*cdl->devs), GFP_KERNEL);
+ if (!cdl->devs)
+ return -ENOMEM;
+ cdl->len = err / 4;
+
+ for (i = 0; i < cdl->len; i++) {
+ struct iommu_fwspec *fwspec;
+
+ ctx = &cdl->devs[i];
+
+ ctx->host = host1x;
+
+ device_initialize(&ctx->dev);
+
+ /*
+* Due to an issue with T194 NVENC, only 38 bits can be used.
+* Anyway, 256GiB of IOVA ought to be enough for anyone.
+*/
+ ctx->dma_mask = DMA_BIT_MASK(38);
+ ctx->dev.dma_mask = &ctx->dma_mask;
+ ctx->dev.coherent_dma_mask = ctx->dma_mask;
+ dev_set_name(&ctx->dev, "host1x-ctx.%d", i);
+ ctx->dev.bus = &host1x_context_device_bus_type;
+ ctx->dev.parent = host1x->dev;
+
+ dma_set_max_seg_size(&ctx->dev, UINT_MAX);
+
+ err = device_add(&ctx->dev);
+ if (err) {
+ dev_err(host1x->dev, "could not add context device %d:
%d\n", i, err);
+ goto del_devices;
+ }
+
+ err = of_dma_configure_id(&ctx->dev, node, true, &i);
+ if (err) {
+ dev_err(host1x->dev, "IOMMU configuration failed for
context device %d: %d\n",
+ i, err);
+ device_del(&ctx->dev);
+ goto del_devices;
+ }
+
+ fwspec = dev_iommu_fwspec_get(&ctx->dev);
+ if (!fwspec) {
+ dev_err(host1x->dev, "Context device %d has no
IOMMU!\n", i);
+ device_del(&ctx->dev);
+ goto del_devices;
+ }
+
+ ctx->stream_id = fwspec->ids[0] & 0x;
+ }
+
+ return 0;
+
+del_devices:
+ while (i--)
+ device_del(&cdl->devs[i].dev);
+
+ kfree(cdl->devs);
+ cdl->len = 0;
+
+ return err;
+}
+
+void host1x_memory_context_list_free(struct host1x_memory_context_list *cdl)
+{
+ unsigned int i;
+
+ for (i = 0; i < cdl->len; i++)
+ device_del(&cdl->devs[i].dev);
+
+ kfree(cdl->devs);
+ cdl->len = 0;
+}
+
+struct host1x_memory_context *host1x_memory_context_alloc(struct host1x
*host1x,
+ struct pid *pid)
+{
+ struct host1x_memory_context_list *cdl = &host1x->context_list;
+ struct host1x_memory_context *free = NULL;
+ int i;
+
+ if (!cdl->len)
+ return ERR_PTR(-EOPNOTSUPP);
+
+ mutex_lock(&cdl->lock);
+
+ for (i = 0; i < cdl->len; i++) {
+ struct host1x_memor
[PATCH v6 08/10] drm/tegra: nvdec: Fix TRANSCFG register offset
From: Mikko Perttunen
NVDEC's TRANSCFG register is at a different offset than VIC.
This becomes a problem now when context isolation is enabled and
the reset value of the register is no longer sufficient.
Signed-off-by: Mikko Perttunen
---
v6:
* New patch
---
drivers/gpu/drm/tegra/nvdec.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/nvdec.c b/drivers/gpu/drm/tegra/nvdec.c
index 79e1e88203cf..386f9b2e78c4 100644
--- a/drivers/gpu/drm/tegra/nvdec.c
+++ b/drivers/gpu/drm/tegra/nvdec.c
@@ -21,6 +21,8 @@
#include "falcon.h"
#include "vic.h"
+#define NVDEC_TFBIF_TRANSCFG 0x2c44
+
struct nvdec_config {
const char *firmware;
unsigned int version;
@@ -63,7 +65,7 @@ static int nvdec_boot(struct nvdec *nvdec)
u32 value;
value = TRANSCFG_ATT(1, TRANSCFG_SID_FALCON) | TRANSCFG_ATT(0,
TRANSCFG_SID_HW);
- nvdec_writel(nvdec, value, VIC_TFBIF_TRANSCFG);
+ nvdec_writel(nvdec, value, NVDEC_TFBIF_TRANSCFG);
if (spec->num_ids > 0) {
value = spec->ids[0] & 0x;
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 10/10] drm/tegra: Implement stream ID related callbacks on engines
From: Mikko Perttunen
Implement the get_streamid_offset and can_use_memory_ctx callbacks
required for supporting context isolation. Since old firmware on VIC
cannot support context isolation without hacks that we don't want to
implement, check the firmware binary to see if context isolation
should be enabled.
Signed-off-by: Mikko Perttunen
---
v5:
* Split into two callbacks
* Add NVDEC support
v4:
* Add locking in vic_load_firmware
* Return -EOPNOTSUPP if context isolation is not available
* Update for changed get_streamid_offset declaration
* Add comment noting that vic_load_firmware is safe to call
without the hardware being powered on
Implement context isolation related callbacks in VIC, NVDEC
---
drivers/gpu/drm/tegra/drm.h | 8 +
drivers/gpu/drm/tegra/nvdec.c | 9 +
drivers/gpu/drm/tegra/vic.c | 67 ++-
3 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 2acc8f2948ad..845e60f144c7 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -100,6 +100,14 @@ int tegra_drm_submit(struct tegra_drm_context *context,
struct drm_tegra_submit *args, struct drm_device *drm,
struct drm_file *file);
+static inline int
+tegra_drm_get_streamid_offset_thi(struct tegra_drm_client *client, u32 *offset)
+{
+ *offset = 0x30;
+
+ return 0;
+}
+
struct tegra_drm_client {
struct host1x_client base;
struct list_head list;
diff --git a/drivers/gpu/drm/tegra/nvdec.c b/drivers/gpu/drm/tegra/nvdec.c
index 386f9b2e78c4..a84f61709679 100644
--- a/drivers/gpu/drm/tegra/nvdec.c
+++ b/drivers/gpu/drm/tegra/nvdec.c
@@ -306,10 +306,19 @@ static void nvdec_close_channel(struct tegra_drm_context
*context)
host1x_channel_put(context->channel);
}
+static int nvdec_can_use_memory_ctx(struct tegra_drm_client *client, bool
*supported)
+{
+ *supported = true;
+
+ return 0;
+}
+
static const struct tegra_drm_client_ops nvdec_ops = {
.open_channel = nvdec_open_channel,
.close_channel = nvdec_close_channel,
.submit = tegra_drm_submit,
+ .get_streamid_offset = tegra_drm_get_streamid_offset_thi,
+ .can_use_memory_ctx = nvdec_can_use_memory_ctx,
};
#define NVIDIA_TEGRA_210_NVDEC_FIRMWARE "nvidia/tegra210/nvdec.bin"
diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
index f56f5921a8c2..c5526bda88d6 100644
--- a/drivers/gpu/drm/tegra/vic.c
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -38,6 +38,8 @@ struct vic {
struct clk *clk;
struct reset_control *rst;
+ bool can_use_context;
+
/* Platform configuration */
const struct vic_config *config;
};
@@ -229,28 +231,38 @@ static int vic_load_firmware(struct vic *vic)
{
struct host1x_client *client = &vic->client.base;
struct tegra_drm *tegra = vic->client.drm;
+ static DEFINE_MUTEX(lock);
+ u32 fce_bin_data_offset;
dma_addr_t iova;
size_t size;
void *virt;
int err;
- if (vic->falcon.firmware.virt)
- return 0;
+ mutex_lock(&lock);
+
+ if (vic->falcon.firmware.virt) {
+ err = 0;
+ goto unlock;
+ }
err = falcon_read_firmware(&vic->falcon, vic->config->firmware);
if (err < 0)
- return err;
+ goto unlock;
size = vic->falcon.firmware.size;
if (!client->group) {
virt = dma_alloc_coherent(vic->dev, size, &iova, GFP_KERNEL);
- if (!virt)
- return -ENOMEM;
+ if (!virt) {
+ err = -ENOMEM;
+ goto unlock;
+ }
} else {
virt = tegra_drm_alloc(tegra, size, &iova);
- if (IS_ERR(virt))
- return PTR_ERR(virt);
+ if (IS_ERR(virt)) {
+ err = PTR_ERR(virt);
+ goto unlock;
+ }
}
vic->falcon.firmware.virt = virt;
@@ -277,7 +289,28 @@ static int vic_load_firmware(struct vic *vic)
vic->falcon.firmware.phys = phys;
}
- return 0;
+ /*
+* Check if firmware is new enough to not require mapping firmware
+* to data buffer domains.
+*/
+ fce_bin_data_offset = *(u32 *)(virt + VIC_UCODE_FCE_DATA_OFFSET);
+
+ if (!vic->config->supports_sid) {
+ vic->can_use_context = false;
+ } else if (fce_bin_data_offset != 0x0 && fce_bin_data_offset !=
0xa5a5a5a5) {
+ /*
+* Firmware will access FCE through STREAMID0, so context
+* isolation cannot be used.
+*/
+ vic->can_use_context = false;
+ dev_warn_once(vic->dev, "context isolation disabled due to old
firmware\n");
+ }
[PATCH v6 03/10] dt-bindings: host1x: Add iommu-map property
From: Mikko Perttunen Add schema information for specifying context stream IDs. This uses the standard iommu-map property. Signed-off-by: Mikko Perttunen Reviewed-by: Robin Murphy Acked-by: Rob Herring --- v3: * New patch v4: * Remove memory-contexts subnode. --- .../bindings/display/tegra/nvidia,tegra20-host1x.yaml| 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml index 4fd513efb0f7..0adeb03b9e3a 100644 --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml @@ -144,6 +144,11 @@ allOf: reset-names: maxItems: 1 +iommu-map: + description: Specification of stream IDs available for memory context device +use. Should be a mapping of IDs 0..n to IOMMU entries corresponding to +usable stream IDs. + required: - reg-names -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 07/10] drm/tegra: falcon: Set DMACTX field on DMA transactions
From: Mikko Perttunen The DMACTX field determines which context, as specified in the TRANSCFG register, is used. While during boot it doesn't matter which is used, later on it matters and this value is reused by the firmware. Signed-off-by: Mikko Perttunen --- drivers/gpu/drm/tegra/falcon.c | 8 drivers/gpu/drm/tegra/falcon.h | 1 + 2 files changed, 9 insertions(+) diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c index 3762d87759d9..c0d85463eb1a 100644 --- a/drivers/gpu/drm/tegra/falcon.c +++ b/drivers/gpu/drm/tegra/falcon.c @@ -48,6 +48,14 @@ static int falcon_copy_chunk(struct falcon *falcon, if (target == FALCON_MEMORY_IMEM) cmd |= FALCON_DMATRFCMD_IMEM; + /* +* Use second DMA context (i.e. the one for firmware). Strictly +* speaking, at this point both DMA contexts point to the firmware +* stream ID, but this register's value will be reused by the firmware +* for later DMA transactions, so we need to use the correct value. +*/ + cmd |= FALCON_DMATRFCMD_DMACTX(1); + falcon_writel(falcon, offset, FALCON_DMATRFMOFFS); falcon_writel(falcon, base, FALCON_DMATRFFBOFFS); falcon_writel(falcon, cmd, FALCON_DMATRFCMD); diff --git a/drivers/gpu/drm/tegra/falcon.h b/drivers/gpu/drm/tegra/falcon.h index c56ee32d92ee..1955cf11a8a6 100644 --- a/drivers/gpu/drm/tegra/falcon.h +++ b/drivers/gpu/drm/tegra/falcon.h @@ -50,6 +50,7 @@ #define FALCON_DMATRFCMD_IDLE (1 << 1) #define FALCON_DMATRFCMD_IMEM (1 << 4) #define FALCON_DMATRFCMD_SIZE_256B (6 << 8) +#define FALCON_DMATRFCMD_DMACTX(v) (((v) & 0x7) << 12) #define FALCON_DMATRFFBOFFS0x111c -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 00/10] Host1x context isolation support
From: Mikko Perttunen - Merging notes - The changes to DT bindings should be applied on top of Thierry's patch 'dt-bindings: display: tegra: Convert to json-schema'. The change to the arm-smmu driver should be omitted if Robin Murphy's IOMMU bus cleanup series is merged. *** New in v6: Rebased on 5.19-rc3 (-next is too broken) Added patch to fix TRANSCFG offset on NVDEC. *** *** New in v5: Rebased Renamed host1x_context to host1x_memory_context Small change in DRM side client driver ops to reduce churn with some upcoming changes Add NVDEC support *** *** New in v4: Addressed review comments. See individual patches. *** *** New in v3: Added device tree bindings for new property. *** *** New in v2: Added support for Tegra194 Use standard iommu-map property instead of custom mechanism *** This series adds support for Host1x 'context isolation'. Since when programming engines through Host1x, userspace can program in any addresses it wants, we need some way to isolate the engines' memory spaces. Traditionally this has either been done imperfectly with a single shared IOMMU domain, or by copying and verifying the programming command stream at submit time (Host1x firewall). Since Tegra186 there is a privileged (only usable by kernel) Host1x opcode that allows setting the stream ID sent by the engine to the SMMU. So, by allocating a number of context banks and stream IDs for this purpose, and using this opcode at the beginning of each job, we can implement isolation. Due to the limited number of context banks only each process gets its own context, and not each channel. This feature also allows sharing engines among multiple VMs when used with Host1x's hardware virtualization support - up to 8 VMs can be configured with a subset of allowed stream IDs, enforced at hardware level. To implement this, this series adds a new host1x context bus, which will contain the 'struct device's corresponding to each context bank / stream ID, changes to device tree and SMMU code to allow registering the devices and using the bus, as well as the Host1x stream ID programming code and support in TegraDRM. Thanks, Mikko Mikko Perttunen (9): iommu/arm-smmu: Attach to host1x context device bus dt-bindings: host1x: Add iommu-map property gpu: host1x: Add context device management code gpu: host1x: Program context stream ID on submission arm64: tegra: Add Host1x context stream IDs on Tegra186+ drm/tegra: falcon: Set DMACTX field on DMA transactions drm/tegra: nvdec: Fix TRANSCFG register offset drm/tegra: Support context isolation drm/tegra: Implement stream ID related callbacks on engines Thierry Reding (1): dt-bindings: display: tegra: Convert to json-schema .../display/tegra/nvidia,tegra114-mipi.txt| 41 -- .../display/tegra/nvidia,tegra114-mipi.yaml | 74 ++ .../display/tegra/nvidia,tegra124-dpaux.yaml | 149 .../display/tegra/nvidia,tegra124-sor.yaml| 206 ++ .../display/tegra/nvidia,tegra124-vic.yaml| 71 ++ .../display/tegra/nvidia,tegra186-dc.yaml | 85 +++ .../tegra/nvidia,tegra186-display.yaml| 310 .../tegra/nvidia,tegra186-dsi-padctl.yaml | 45 ++ .../display/tegra/nvidia,tegra20-dc.yaml | 181 + .../display/tegra/nvidia,tegra20-dsi.yaml | 159 + .../display/tegra/nvidia,tegra20-epp.yaml | 70 ++ .../display/tegra/nvidia,tegra20-gr2d.yaml| 73 ++ .../display/tegra/nvidia,tegra20-gr3d.yaml| 214 ++ .../display/tegra/nvidia,tegra20-hdmi.yaml| 126 .../display/tegra/nvidia,tegra20-host1x.txt | 675 -- .../display/tegra/nvidia,tegra20-host1x.yaml | 352 + .../display/tegra/nvidia,tegra20-isp.yaml | 67 ++ .../display/tegra/nvidia,tegra20-mpe.yaml | 73 ++ .../display/tegra/nvidia,tegra20-tvo.yaml | 58 ++ .../display/tegra/nvidia,tegra20-vi.yaml | 163 + .../display/tegra/nvidia,tegra210-csi.yaml| 52 ++ .../pinctrl/nvidia,tegra124-dpaux-padctl.txt | 59 -- arch/arm64/boot/dts/nvidia/tegra186.dtsi | 11 + arch/arm64/boot/dts/nvidia/tegra194.dtsi | 11 + drivers/gpu/drm/tegra/drm.h | 11 + drivers/gpu/drm/tegra/falcon.c| 8 + drivers/gpu/drm/tegra/falcon.h| 1 + drivers/gpu/drm/tegra/nvdec.c | 13 +- drivers/gpu/drm/tegra/submit.c| 48 +- drivers/gpu/drm/tegra/uapi.c | 43 +- drivers/gpu/drm/tegra/vic.c | 67 +- drivers/gpu/host1x/Makefile | 1 + drivers/gpu/host1x/context.c | 160 + drivers/gpu/host1x/context.h | 27 + drivers/gpu/host1x/dev.c | 12 +- drivers/gpu/host1x/dev.h | 2 + drivers/gpu/host1x/hw/channel_hw.c| 52 +- drivers/gpu/host1x/hw/host1x06_hardware.h | 10 + drivers/gpu/host1x/hw/host1x07_hardware.h | 10 + drivers/iommu/arm/arm-smm
[PATCH v6 06/10] arm64: tegra: Add Host1x context stream IDs on Tegra186+
From: Mikko Perttunen
Add Host1x context stream IDs on systems that support Host1x context
isolation. Host1x and attached engines can use these stream IDs to
allow isolation between memory used by different processes.
The specified stream IDs must match those configured by the hypervisor,
if one is present.
Signed-off-by: Mikko Perttunen
---
v2:
* Added context devices on T194.
* Use iommu-map instead of custom property.
v4:
* Remove memory-contexts subnode.
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 11 +++
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 11 +++
2 files changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 0e9afc3e2f26..5f560f13ed93 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1461,6 +1461,17 @@ host1x@13e0 {
iommus = <&smmu TEGRA186_SID_HOST1X>;
+ /* Context isolation domains */
+ iommu-map = <
+ 0 &smmu TEGRA186_SID_HOST1X_CTX0 1
+ 1 &smmu TEGRA186_SID_HOST1X_CTX1 1
+ 2 &smmu TEGRA186_SID_HOST1X_CTX2 1
+ 3 &smmu TEGRA186_SID_HOST1X_CTX3 1
+ 4 &smmu TEGRA186_SID_HOST1X_CTX4 1
+ 5 &smmu TEGRA186_SID_HOST1X_CTX5 1
+ 6 &smmu TEGRA186_SID_HOST1X_CTX6 1
+ 7 &smmu TEGRA186_SID_HOST1X_CTX7 1>;
+
dpaux1: dpaux@1504 {
compatible = "nvidia,tegra186-dpaux";
reg = <0x1504 0x1>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index d1f8248c00f4..613fd71dec25 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1769,6 +1769,17 @@ host1x@13e0 {
interconnect-names = "dma-mem";
iommus = <&smmu TEGRA194_SID_HOST1X>;
+ /* Context isolation domains */
+ iommu-map = <
+ 0 &smmu TEGRA194_SID_HOST1X_CTX0 1
+ 1 &smmu TEGRA194_SID_HOST1X_CTX1 1
+ 2 &smmu TEGRA194_SID_HOST1X_CTX2 1
+ 3 &smmu TEGRA194_SID_HOST1X_CTX3 1
+ 4 &smmu TEGRA194_SID_HOST1X_CTX4 1
+ 5 &smmu TEGRA194_SID_HOST1X_CTX5 1
+ 6 &smmu TEGRA194_SID_HOST1X_CTX6 1
+ 7 &smmu TEGRA194_SID_HOST1X_CTX7 1>;
+
nvdec@1514 {
compatible = "nvidia,tegra194-nvdec";
reg = <0x1514 0x0004>;
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 05/10] gpu: host1x: Program context stream ID on submission
From: Mikko Perttunen
Add code to do stream ID switching at the beginning of a job. The
stream ID is switched to the stream ID specified by the context
passed in the job structure.
Before switching the stream ID, an OP_DONE wait is done on the
channel's engine to ensure that there is no residual ongoing
work that might do DMA using the new stream ID.
Signed-off-by: Mikko Perttunen
---
v5:
* Add fallback stream ID. Not used yet, will be needed for
full featured opcode sequence.
* Rename host1x_context to host1x_memory_context
v4:
* Rename job->context to job->memory_context for clarity
---
drivers/gpu/host1x/hw/channel_hw.c| 52 +--
drivers/gpu/host1x/hw/host1x06_hardware.h | 10 +
drivers/gpu/host1x/hw/host1x07_hardware.h | 10 +
include/linux/host1x.h| 8
4 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/host1x/hw/channel_hw.c
b/drivers/gpu/host1x/hw/channel_hw.c
index 6b40e9af1e88..f84caf06621a 100644
--- a/drivers/gpu/host1x/hw/channel_hw.c
+++ b/drivers/gpu/host1x/hw/channel_hw.c
@@ -180,6 +180,45 @@ static void host1x_enable_gather_filter(struct
host1x_channel *ch)
#endif
}
+static void host1x_channel_program_engine_streamid(struct host1x_job *job)
+{
+#if HOST1X_HW >= 6
+ u32 fence;
+
+ if (!job->memory_context)
+ return;
+
+ fence = host1x_syncpt_incr_max(job->syncpt, 1);
+
+ /* First, increment a syncpoint on OP_DONE condition.. */
+
+ host1x_cdma_push(&job->channel->cdma,
+ host1x_opcode_nonincr(HOST1X_UCLASS_INCR_SYNCPT, 1),
+ HOST1X_UCLASS_INCR_SYNCPT_INDX_F(job->syncpt->id) |
+ HOST1X_UCLASS_INCR_SYNCPT_COND_F(1));
+
+ /* Wait for syncpoint to increment */
+
+ host1x_cdma_push(&job->channel->cdma,
+ host1x_opcode_setclass(HOST1X_CLASS_HOST1X,
+ host1x_uclass_wait_syncpt_r(), 1),
+ host1x_class_host_wait_syncpt(job->syncpt->id, fence));
+
+ /*
+* Now that we know the engine is idle, return to class and
+* change stream ID.
+*/
+
+ host1x_cdma_push(&job->channel->cdma,
+ host1x_opcode_setclass(job->class, 0, 0),
+ HOST1X_OPCODE_NOP);
+
+ host1x_cdma_push(&job->channel->cdma,
+ host1x_opcode_setpayload(job->memory_context->stream_id),
+ host1x_opcode_setstreamid(job->engine_streamid_offset / 4));
+#endif
+}
+
static int channel_submit(struct host1x_job *job)
{
struct host1x_channel *ch = job->channel;
@@ -236,18 +275,23 @@ static int channel_submit(struct host1x_job *job)
if (sp->base)
synchronize_syncpt_base(job);
- syncval = host1x_syncpt_incr_max(sp, user_syncpt_incrs);
-
host1x_hw_syncpt_assign_to_channel(host, sp, ch);
- job->syncpt_end = syncval;
-
/* add a setclass for modules that require it */
if (job->class)
host1x_cdma_push(&ch->cdma,
host1x_opcode_setclass(job->class, 0, 0),
HOST1X_OPCODE_NOP);
+ /*
+* Ensure engine DMA is idle and set new stream ID. May increment
+* syncpt max.
+*/
+ host1x_channel_program_engine_streamid(job);
+
+ syncval = host1x_syncpt_incr_max(sp, user_syncpt_incrs);
+ job->syncpt_end = syncval;
+
submit_gathers(job, syncval - user_syncpt_incrs);
/* end CDMA submit & stash pinned hMems into sync queue */
diff --git a/drivers/gpu/host1x/hw/host1x06_hardware.h
b/drivers/gpu/host1x/hw/host1x06_hardware.h
index 01a142a09800..5d515745eee7 100644
--- a/drivers/gpu/host1x/hw/host1x06_hardware.h
+++ b/drivers/gpu/host1x/hw/host1x06_hardware.h
@@ -127,6 +127,16 @@ static inline u32 host1x_opcode_gather_incr(unsigned
offset, unsigned count)
return (6 << 28) | (offset << 16) | BIT(15) | BIT(14) | count;
}
+static inline u32 host1x_opcode_setstreamid(unsigned streamid)
+{
+ return (7 << 28) | streamid;
+}
+
+static inline u32 host1x_opcode_setpayload(unsigned payload)
+{
+ return (9 << 28) | payload;
+}
+
static inline u32 host1x_opcode_gather_wide(unsigned count)
{
return (12 << 28) | count;
diff --git a/drivers/gpu/host1x/hw/host1x07_hardware.h
b/drivers/gpu/host1x/hw/host1x07_hardware.h
index e6582172ebfd..82c0cc9bb0b5 100644
--- a/drivers/gpu/host1x/hw/host1x07_hardware.h
+++ b/drivers/gpu/host1x/hw/host1x07_hardware.h
@@ -127,6 +127,16 @@ static inline u32 host1x_opcode_gather_incr(unsigned
offset, unsigned count)
return (6 << 28) | (offset << 16) | BIT(15) | BIT(14) | count;
}
+static inline u32 host1x_opcode_setstreamid(unsigned streamid)
+{
+ return (7 << 28) | streamid;
+}
+
+static inline u32 host1x_opcode_setpayload(unsigned payload)
+{
+ return (9 << 28) | payload;
+}
+
static inline u32 host1x_opcode_gather_wide(unsigned count)
{
[PATCH v6 01/10] iommu/arm-smmu: Attach to host1x context device bus
From: Mikko Perttunen
Set itself as the IOMMU for the host1x context device bus, containing
"dummy" devices used for Host1x context isolation.
Signed-off-by: Mikko Perttunen
---
This patch should only be applied if Robin Murphy's IOMMU bus
refactoring series doesn't get merged in time.
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 2ed3594f384e..74c4357f850b 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -39,6 +39,7 @@
#include
#include
+#include
#include "arm-smmu.h"
@@ -2056,8 +2057,20 @@ static int arm_smmu_bus_init(struct iommu_ops *ops)
goto err_reset_pci_ops;
}
#endif
+#ifdef CONFIG_TEGRA_HOST1X_CONTEXT_BUS
+ if (!iommu_present(&host1x_context_device_bus_type)) {
+ err = bus_set_iommu(&host1x_context_device_bus_type, ops);
+ if (err)
+ goto err_reset_fsl_mc_ops;
+ }
+#endif
+
return 0;
+err_reset_fsl_mc_ops: __maybe_unused;
+#ifdef CONFIG_FSL_MC_BUS
+ bus_set_iommu(&fsl_mc_bus_type, NULL);
+#endif
err_reset_pci_ops: __maybe_unused;
#ifdef CONFIG_PCI
bus_set_iommu(&pci_bus_type, NULL);
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v9 11/11] iommu: Rename iommu-sva-lib.{c,h}
Rename iommu-sva-lib.c[h] to iommu-sva.c[h] as it contains all code
for SVA implementation in iommu core.
Signed-off-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
---
drivers/iommu/{iommu-sva-lib.h => iommu-sva.h} | 6 +++---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/intel/iommu.c | 2 +-
drivers/iommu/intel/svm.c | 2 +-
drivers/iommu/io-pgfault.c | 2 +-
drivers/iommu/{iommu-sva-lib.c => iommu-sva.c} | 2 +-
drivers/iommu/iommu.c | 2 +-
drivers/iommu/Makefile | 2 +-
9 files changed, 11 insertions(+), 11 deletions(-)
rename drivers/iommu/{iommu-sva-lib.h => iommu-sva.h} (95%)
rename drivers/iommu/{iommu-sva-lib.c => iommu-sva.c} (99%)
diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva.h
similarity index 95%
rename from drivers/iommu/iommu-sva-lib.h
rename to drivers/iommu/iommu-sva.h
index 1b3ace4b5863..7215a761b962 100644
--- a/drivers/iommu/iommu-sva-lib.h
+++ b/drivers/iommu/iommu-sva.h
@@ -2,8 +2,8 @@
/*
* SVA library for IOMMU drivers
*/
-#ifndef _IOMMU_SVA_LIB_H
-#define _IOMMU_SVA_LIB_H
+#ifndef _IOMMU_SVA_H
+#define _IOMMU_SVA_H
#include
#include
@@ -72,4 +72,4 @@ iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
return IOMMU_PAGE_RESP_INVALID;
}
#endif /* CONFIG_IOMMU_SVA */
-#endif /* _IOMMU_SVA_LIB_H */
+#endif /* _IOMMU_SVA_H */
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index e36c689f56c5..b33bc592ccfa 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -10,7 +10,7 @@
#include
#include "arm-smmu-v3.h"
-#include "../../iommu-sva-lib.h"
+#include "../../iommu-sva.h"
#include "../../io-pgtable-arm.h"
struct arm_smmu_mmu_notifier {
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 8b9b78c7a67d..79e8991e9181 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -31,7 +31,7 @@
#include
#include "arm-smmu-v3.h"
-#include "../../iommu-sva-lib.h"
+#include "../../iommu-sva.h"
static bool disable_bypass = true;
module_param(disable_bypass, bool, 0444);
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 37d68eda1889..d16ab6d1cc05 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -27,7 +27,7 @@
#include
#include "../irq_remapping.h"
-#include "../iommu-sva-lib.h"
+#include "../iommu-sva.h"
#include "pasid.h"
#include "cap_audit.h"
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index db55b06cafdf..58656a93b201 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -25,7 +25,7 @@
#include "pasid.h"
#include "perf.h"
-#include "../iommu-sva-lib.h"
+#include "../iommu-sva.h"
static irqreturn_t prq_event_thread(int irq, void *d);
static void intel_svm_drain_prq(struct device *dev, u32 pasid);
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index 4f24ec703479..91b1c6bd01d4 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -11,7 +11,7 @@
#include
#include
-#include "iommu-sva-lib.h"
+#include "iommu-sva.h"
/**
* struct iopf_queue - IO Page Fault queue
diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva.c
similarity index 99%
rename from drivers/iommu/iommu-sva-lib.c
rename to drivers/iommu/iommu-sva.c
index dee8e2e42e06..1a4897a5697b 100644
--- a/drivers/iommu/iommu-sva-lib.c
+++ b/drivers/iommu/iommu-sva.c
@@ -6,7 +6,7 @@
#include
#include
-#include "iommu-sva-lib.h"
+#include "iommu-sva.h"
static DEFINE_MUTEX(iommu_sva_lock);
static DECLARE_IOASID_SET(iommu_sva_pasid);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a0e3d8083943..c766c852b647 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -29,7 +29,7 @@
#include
#include
-#include "iommu-sva-lib.h"
+#include "iommu-sva.h"
static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 44475a9b3eea..c1763476162b 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -27,6 +27,6 @@ obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
-obj-$(CONFIG_IOMMU_SVA) += iommu-sva-lib.o io-pgfault.o
+obj-$(CONFIG_IOMMU_SVA) += iommu-sva.o io-pgfault.o
obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o
obj-$(CONFIG_APPLE_DART) += apple-dart.o
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo
[PATCH v9 10/11] iommu: Per-domain I/O page fault handling
Tweak the I/O page fault handling framework to route the page faults to
the domain and call the page fault handler retrieved from the domain.
This makes the I/O page fault handling framework possible to serve more
usage scenarios as long as they have an IOMMU domain and install a page
fault handler in it. Some unused functions are also removed to avoid
dead code.
The iommu_get_domain_for_dev_pasid() which retrieves attached domain
for a {device, PASID} pair is used. It will be used by the page fault
handling framework which knows {device, PASID} reported from the iommu
driver. We have a guarantee that the SVA domain doesn't go away during
IOPF handling, because unbind() waits for pending faults with
iopf_queue_flush_dev() before freeing the domain. Hence, there's no need
to synchronize life cycle of the iommu domains between the unbind() and
the interrupt threads.
Signed-off-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
---
drivers/iommu/io-pgfault.c | 64 +-
1 file changed, 7 insertions(+), 57 deletions(-)
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index aee9e033012f..4f24ec703479 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -69,69 +69,18 @@ static int iopf_complete_group(struct device *dev, struct
iopf_fault *iopf,
return iommu_page_response(dev, &resp);
}
-static enum iommu_page_response_code
-iopf_handle_single(struct iopf_fault *iopf)
-{
- vm_fault_t ret;
- struct mm_struct *mm;
- struct vm_area_struct *vma;
- unsigned int access_flags = 0;
- unsigned int fault_flags = FAULT_FLAG_REMOTE;
- struct iommu_fault_page_request *prm = &iopf->fault.prm;
- enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
-
- if (!(prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID))
- return status;
-
- mm = iommu_sva_find(prm->pasid);
- if (IS_ERR_OR_NULL(mm))
- return status;
-
- mmap_read_lock(mm);
-
- vma = find_extend_vma(mm, prm->addr);
- if (!vma)
- /* Unmapped area */
- goto out_put_mm;
-
- if (prm->perm & IOMMU_FAULT_PERM_READ)
- access_flags |= VM_READ;
-
- if (prm->perm & IOMMU_FAULT_PERM_WRITE) {
- access_flags |= VM_WRITE;
- fault_flags |= FAULT_FLAG_WRITE;
- }
-
- if (prm->perm & IOMMU_FAULT_PERM_EXEC) {
- access_flags |= VM_EXEC;
- fault_flags |= FAULT_FLAG_INSTRUCTION;
- }
-
- if (!(prm->perm & IOMMU_FAULT_PERM_PRIV))
- fault_flags |= FAULT_FLAG_USER;
-
- if (access_flags & ~vma->vm_flags)
- /* Access fault */
- goto out_put_mm;
-
- ret = handle_mm_fault(vma, prm->addr, fault_flags, NULL);
- status = ret & VM_FAULT_ERROR ? IOMMU_PAGE_RESP_INVALID :
- IOMMU_PAGE_RESP_SUCCESS;
-
-out_put_mm:
- mmap_read_unlock(mm);
- mmput(mm);
-
- return status;
-}
-
static void iopf_handle_group(struct work_struct *work)
{
struct iopf_group *group;
+ struct iommu_domain *domain;
struct iopf_fault *iopf, *next;
enum iommu_page_response_code status = IOMMU_PAGE_RESP_SUCCESS;
group = container_of(work, struct iopf_group, work);
+ domain = iommu_get_domain_for_dev_pasid(group->dev,
+ group->last_fault.fault.prm.pasid);
+ if (!domain || !domain->iopf_handler)
+ status = IOMMU_PAGE_RESP_INVALID;
list_for_each_entry_safe(iopf, next, &group->faults, list) {
/*
@@ -139,7 +88,8 @@ static void iopf_handle_group(struct work_struct *work)
* faults in the group if there is an error.
*/
if (status == IOMMU_PAGE_RESP_SUCCESS)
- status = iopf_handle_single(iopf);
+ status = domain->iopf_handler(&iopf->fault,
+ domain->fault_data);
if (!(iopf->fault.prm.flags &
IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE))
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v9 09/11] iommu: Prepare IOMMU domain for IOPF
This adds some mechanisms around the iommu_domain so that the I/O page
fault handling framework could route a page fault to the domain and
call the fault handler from it.
Add pointers to the page fault handler and its private data in struct
iommu_domain. The fault handler will be called with the private data
as a parameter once a page fault is routed to the domain. Any kernel
component which owns an iommu domain could install handler and its
private parameter so that the page fault could be further routed and
handled.
This also prepares the SVA implementation to be the first consumer of
the per-domain page fault handling model. The I/O page fault handler
for SVA is copied to the SVA file with mmget_not_zero() added before
mmap_read_lock().
Suggested-by: Jean-Philippe Brucker
Signed-off-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
---
include/linux/iommu.h | 3 ++
drivers/iommu/iommu-sva-lib.h | 8 +
drivers/iommu/io-pgfault.c| 7
drivers/iommu/iommu-sva-lib.c | 60 +++
drivers/iommu/iommu.c | 4 +++
5 files changed, 82 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 17780537db6e..36c822a5b135 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -105,6 +105,9 @@ struct iommu_domain {
unsigned long pgsize_bitmap;/* Bitmap of page sizes in use */
struct iommu_domain_geometry geometry;
struct iommu_dma_cookie *iova_cookie;
+ enum iommu_page_response_code (*iopf_handler)(struct iommu_fault *fault,
+ void *data);
+ void *fault_data;
union {
struct {/* IOMMU_DOMAIN_DMA */
iommu_fault_handler_t handler;
diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva-lib.h
index 8909ea1094e3..1b3ace4b5863 100644
--- a/drivers/iommu/iommu-sva-lib.h
+++ b/drivers/iommu/iommu-sva-lib.h
@@ -26,6 +26,8 @@ int iopf_queue_flush_dev(struct device *dev);
struct iopf_queue *iopf_queue_alloc(const char *name);
void iopf_queue_free(struct iopf_queue *queue);
int iopf_queue_discard_partial(struct iopf_queue *queue);
+enum iommu_page_response_code
+iommu_sva_handle_iopf(struct iommu_fault *fault, void *data);
#else /* CONFIG_IOMMU_SVA */
static inline int iommu_queue_iopf(struct iommu_fault *fault, void *cookie)
@@ -63,5 +65,11 @@ static inline int iopf_queue_discard_partial(struct
iopf_queue *queue)
{
return -ENODEV;
}
+
+static inline enum iommu_page_response_code
+iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
+{
+ return IOMMU_PAGE_RESP_INVALID;
+}
#endif /* CONFIG_IOMMU_SVA */
#endif /* _IOMMU_SVA_LIB_H */
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index 1df8c1dcae77..aee9e033012f 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -181,6 +181,13 @@ static void iopf_handle_group(struct work_struct *work)
* request completes, outstanding faults will have been dealt with by the time
* the PASID is freed.
*
+ * Any valid page fault will be eventually routed to an iommu domain and the
+ * page fault handler installed there will get called. The users of this
+ * handling framework should guarantee that the iommu domain could only be
+ * freed after the device has stopped generating page faults (or the iommu
+ * hardware has been set to block the page faults) and the pending page faults
+ * have been flushed.
+ *
* Return: 0 on success and <0 on error.
*/
int iommu_queue_iopf(struct iommu_fault *fault, void *cookie)
diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c
index 1e3e2b395b1e..dee8e2e42e06 100644
--- a/drivers/iommu/iommu-sva-lib.c
+++ b/drivers/iommu/iommu-sva-lib.c
@@ -167,3 +167,63 @@ u32 iommu_sva_get_pasid(struct iommu_sva *handle)
return domain->mm->pasid;
}
EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
+
+/*
+ * I/O page fault handler for SVA
+ */
+enum iommu_page_response_code
+iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
+{
+ vm_fault_t ret;
+ struct mm_struct *mm;
+ struct vm_area_struct *vma;
+ unsigned int access_flags = 0;
+ struct iommu_domain *domain = data;
+ unsigned int fault_flags = FAULT_FLAG_REMOTE;
+ struct iommu_fault_page_request *prm = &fault->prm;
+ enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
+
+ if (!(prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID))
+ return status;
+
+ mm = domain->mm;
+ if (IS_ERR_OR_NULL(mm) || !mmget_not_zero(mm))
+ return status;
+
+ mmap_read_lock(mm);
+
+ vma = find_extend_vma(mm, prm->addr);
+ if (!vma)
+ /* Unmapped area */
+ goto out_put_mm;
+
+ if (prm->perm & IOMMU_FAULT_PERM_READ)
+ access_flags |= VM_READ;
+
+ if (prm->perm & IOMMU_FAULT_PERM_WRITE) {
+ a
[PATCH v9 08/11] iommu: Remove SVA related callbacks from iommu ops
These ops'es have been replaced with the dev_attach/detach_pasid domain
ops'es. There's no need for them anymore. Remove them to avoid dead
code.
Signed-off-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
Reviewed-by: Kevin Tian
---
include/linux/intel-iommu.h | 3 --
include/linux/iommu.h | 7 ---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 16 --
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 40 ---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 --
drivers/iommu/intel/iommu.c | 3 --
drivers/iommu/intel/svm.c | 49 ---
7 files changed, 121 deletions(-)
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 9007428a68f1..5bd19c95a926 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -738,9 +738,6 @@ struct intel_iommu *device_to_iommu(struct device *dev, u8
*bus, u8 *devfn);
extern void intel_svm_check(struct intel_iommu *iommu);
extern int intel_svm_enable_prq(struct intel_iommu *iommu);
extern int intel_svm_finish_prq(struct intel_iommu *iommu);
-struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm);
-void intel_svm_unbind(struct iommu_sva *handle);
-u32 intel_svm_get_pasid(struct iommu_sva *handle);
int intel_svm_page_response(struct device *dev, struct iommu_fault_event *evt,
struct iommu_page_response *msg);
struct iommu_domain *intel_svm_domain_alloc(void);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index c0c23d9fd8fe..17780537db6e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -227,9 +227,6 @@ struct iommu_iotlb_gather {
* @dev_has/enable/disable_feat: per device entries to check/enable/disable
* iommu specific features.
* @dev_feat_enabled: check enabled feature
- * @sva_bind: Bind process address space to device
- * @sva_unbind: Unbind process address space from device
- * @sva_get_pasid: Get PASID associated to a SVA handle
* @page_response: handle page request response
* @def_domain_type: device default domain type, return value:
* - IOMMU_DOMAIN_IDENTITY: must use an identity domain
@@ -263,10 +260,6 @@ struct iommu_ops {
int (*dev_enable_feat)(struct device *dev, enum iommu_dev_features f);
int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
- struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm);
- void (*sva_unbind)(struct iommu_sva *handle);
- u32 (*sva_get_pasid)(struct iommu_sva *handle);
-
int (*page_response)(struct device *dev,
struct iommu_fault_event *evt,
struct iommu_page_response *msg);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 96399dd3a67a..15dd4c7e6d3a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -754,9 +754,6 @@ bool arm_smmu_master_sva_enabled(struct arm_smmu_master
*master);
int arm_smmu_master_enable_sva(struct arm_smmu_master *master);
int arm_smmu_master_disable_sva(struct arm_smmu_master *master);
bool arm_smmu_master_iopf_supported(struct arm_smmu_master *master);
-struct iommu_sva *arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm);
-void arm_smmu_sva_unbind(struct iommu_sva *handle);
-u32 arm_smmu_sva_get_pasid(struct iommu_sva *handle);
void arm_smmu_sva_notifier_synchronize(void);
struct iommu_domain *arm_smmu_sva_domain_alloc(void);
#else /* CONFIG_ARM_SMMU_V3_SVA */
@@ -790,19 +787,6 @@ static inline bool arm_smmu_master_iopf_supported(struct
arm_smmu_master *master
return false;
}
-static inline struct iommu_sva *
-arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm)
-{
- return ERR_PTR(-ENODEV);
-}
-
-static inline void arm_smmu_sva_unbind(struct iommu_sva *handle) {}
-
-static inline u32 arm_smmu_sva_get_pasid(struct iommu_sva *handle)
-{
- return IOMMU_PASID_INVALID;
-}
-
static inline void arm_smmu_sva_notifier_synchronize(void) {}
static inline struct iommu_domain *arm_smmu_sva_domain_alloc(void)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index fc4555dac5b4..e36c689f56c5 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -344,11 +344,6 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct
*mm)
if (!bond)
return ERR_PTR(-ENOMEM);
- /* Allocate a PASID for this mm if necessary */
- ret = iommu_sva_alloc_pasid(mm, 1, (1U << master->ssid_bits) - 1);
- if (ret)
- goto err_free_bond;
-
bond->mm = mm;
bond->sva.dev = dev;
refcount_set(&bond->refs, 1);
@@ -367,41 +362,6 @@ __arm_smmu_sva_bind(struct device *
[PATCH v9 07/11] iommu/sva: Refactoring iommu_sva_bind/unbind_device()
The existing iommu SVA interfaces are implemented by calling the SVA
specific iommu ops provided by the IOMMU drivers. There's no need for
any SVA specific ops in iommu_ops vector anymore as we can achieve
this through the generic attach/detach_dev_pasid domain ops.
This refactors the IOMMU SVA interfaces implementation by using the
set/block_pasid_dev ops and align them with the concept of the SVA
iommu domain. Put the new SVA code in the sva related file in order
to make it self-contained.
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 67 +++
drivers/iommu/iommu-sva-lib.c | 98
drivers/iommu/iommu.c | 119 --
3 files changed, 165 insertions(+), 119 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b8b6b8c5e20e..c0c23d9fd8fe 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -39,7 +39,6 @@ struct device;
struct iommu_domain;
struct iommu_domain_ops;
struct notifier_block;
-struct iommu_sva;
struct iommu_fault_event;
struct iommu_dma_cookie;
@@ -57,6 +56,14 @@ struct iommu_domain_geometry {
bool force_aperture; /* DMA only allowed in mappable range? */
};
+/**
+ * struct iommu_sva - handle to a device-mm bond
+ */
+struct iommu_sva {
+ struct device *dev;
+ refcount_t users;
+};
+
/* Domain feature flags */
#define __IOMMU_DOMAIN_PAGING (1U << 0) /* Support for iommu_map/unmap */
#define __IOMMU_DOMAIN_DMA_API (1U << 1) /* Domain for use in DMA-API
@@ -105,6 +112,7 @@ struct iommu_domain {
};
struct {/* IOMMU_DOMAIN_SVA */
struct mm_struct *mm;
+ struct iommu_sva bond;
};
};
};
@@ -638,13 +646,6 @@ struct iommu_fwspec {
/* ATS is supported */
#define IOMMU_FWSPEC_PCI_RC_ATS(1 << 0)
-/**
- * struct iommu_sva - handle to a device-mm bond
- */
-struct iommu_sva {
- struct device *dev;
-};
-
int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
const struct iommu_ops *ops);
void iommu_fwspec_free(struct device *dev);
@@ -685,11 +686,6 @@ int iommu_dev_enable_feature(struct device *dev, enum
iommu_dev_features f);
int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features f);
bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features f);
-struct iommu_sva *iommu_sva_bind_device(struct device *dev,
- struct mm_struct *mm);
-void iommu_sva_unbind_device(struct iommu_sva *handle);
-u32 iommu_sva_get_pasid(struct iommu_sva *handle);
-
int iommu_device_use_default_domain(struct device *dev);
void iommu_device_unuse_default_domain(struct device *dev);
@@ -703,6 +699,8 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
struct device *dev,
ioasid_t pasid);
void iommu_detach_device_pasid(struct iommu_domain *domain, struct device *dev,
ioasid_t pasid);
+struct iommu_domain *
+iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid);
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
@@ -1033,21 +1031,6 @@ iommu_dev_disable_feature(struct device *dev, enum
iommu_dev_features feat)
return -ENODEV;
}
-static inline struct iommu_sva *
-iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
-{
- return NULL;
-}
-
-static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
-{
-}
-
-static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
-{
- return IOMMU_PASID_INVALID;
-}
-
static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
{
return NULL;
@@ -1093,6 +1076,12 @@ static inline void iommu_detach_device_pasid(struct
iommu_domain *domain,
struct device *dev, ioasid_t pasid)
{
}
+
+static inline struct iommu_domain *
+iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid)
+{
+ return NULL;
+}
#endif /* CONFIG_IOMMU_API */
/**
@@ -1118,4 +1107,26 @@ void iommu_debugfs_setup(void);
static inline void iommu_debugfs_setup(void) {}
#endif
+#ifdef CONFIG_IOMMU_SVA
+struct iommu_sva *iommu_sva_bind_device(struct device *dev,
+ struct mm_struct *mm);
+void iommu_sva_unbind_device(struct iommu_sva *handle);
+u32 iommu_sva_get_pasid(struct iommu_sva *handle);
+#else
+static inline struct iommu_sva *
+iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
+{
+ return NULL;
+}
+
+static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
+{
+}
+
+static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
+{
+ return IOMMU_PASID_INVALID;
+}
+#endif /* CONFIG_IOMMU_SVA */
+
#endif /* __LINUX_IOMMU_H */
diff --git a/drivers/iommu/iommu-s
[PATCH v9 06/11] arm-smmu-v3/sva: Add SVA domain support
Add support for SVA domain allocation and provide an SVA-specific
iommu_domain_ops.
Signed-off-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 6 ++
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 69 +++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 +
3 files changed, 78 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index d2ba86470c42..96399dd3a67a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -758,6 +758,7 @@ struct iommu_sva *arm_smmu_sva_bind(struct device *dev,
struct mm_struct *mm);
void arm_smmu_sva_unbind(struct iommu_sva *handle);
u32 arm_smmu_sva_get_pasid(struct iommu_sva *handle);
void arm_smmu_sva_notifier_synchronize(void);
+struct iommu_domain *arm_smmu_sva_domain_alloc(void);
#else /* CONFIG_ARM_SMMU_V3_SVA */
static inline bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
{
@@ -803,5 +804,10 @@ static inline u32 arm_smmu_sva_get_pasid(struct iommu_sva
*handle)
}
static inline void arm_smmu_sva_notifier_synchronize(void) {}
+
+static inline struct iommu_domain *arm_smmu_sva_domain_alloc(void)
+{
+ return NULL;
+}
#endif /* CONFIG_ARM_SMMU_V3_SVA */
#endif /* _ARM_SMMU_V3_H */
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index f155d406c5d5..fc4555dac5b4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -549,3 +549,72 @@ void arm_smmu_sva_notifier_synchronize(void)
*/
mmu_notifier_synchronize();
}
+
+static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t id)
+{
+ int ret = 0;
+ struct mm_struct *mm;
+ struct iommu_sva *handle;
+
+ if (domain->type != IOMMU_DOMAIN_SVA)
+ return -EINVAL;
+
+ mm = domain->mm;
+ if (WARN_ON(!mm))
+ return -ENODEV;
+
+ mutex_lock(&sva_lock);
+ handle = __arm_smmu_sva_bind(dev, mm);
+ if (IS_ERR(handle))
+ ret = PTR_ERR(handle);
+ mutex_unlock(&sva_lock);
+
+ return ret;
+}
+
+static void arm_smmu_sva_block_dev_pasid(struct iommu_domain *domain,
+struct device *dev, ioasid_t id)
+{
+ struct mm_struct *mm = domain->mm;
+ struct arm_smmu_bond *bond = NULL, *t;
+ struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+
+ mutex_lock(&sva_lock);
+ list_for_each_entry(t, &master->bonds, list) {
+ if (t->mm == mm) {
+ bond = t;
+ break;
+ }
+ }
+
+ if (!WARN_ON(!bond) && refcount_dec_and_test(&bond->refs)) {
+ list_del(&bond->list);
+ arm_smmu_mmu_notifier_put(bond->smmu_mn);
+ kfree(bond);
+ }
+ mutex_unlock(&sva_lock);
+}
+
+static void arm_smmu_sva_domain_free(struct iommu_domain *domain)
+{
+ kfree(domain);
+}
+
+static const struct iommu_domain_ops arm_smmu_sva_domain_ops = {
+ .set_dev_pasid = arm_smmu_sva_set_dev_pasid,
+ .block_dev_pasid= arm_smmu_sva_block_dev_pasid,
+ .free = arm_smmu_sva_domain_free,
+};
+
+struct iommu_domain *arm_smmu_sva_domain_alloc(void)
+{
+ struct iommu_domain *domain;
+
+ domain = kzalloc(sizeof(*domain), GFP_KERNEL);
+ if (!domain)
+ return NULL;
+ domain->ops = &arm_smmu_sva_domain_ops;
+
+ return domain;
+}
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index ae8ec8df47c1..a30b252e2f95 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1999,6 +1999,9 @@ static struct iommu_domain
*arm_smmu_domain_alloc(unsigned type)
{
struct arm_smmu_domain *smmu_domain;
+ if (type == IOMMU_DOMAIN_SVA)
+ return arm_smmu_sva_domain_alloc();
+
if (type != IOMMU_DOMAIN_UNMANAGED &&
type != IOMMU_DOMAIN_DMA &&
type != IOMMU_DOMAIN_DMA_FQ &&
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v9 05/11] iommu/vt-d: Add SVA domain support
Add support for SVA domain allocation and provide an SVA-specific
iommu_domain_ops.
Signed-off-by: Lu Baolu
---
include/linux/intel-iommu.h | 5
drivers/iommu/intel/iommu.c | 2 ++
drivers/iommu/intel/svm.c | 49 +
3 files changed, 56 insertions(+)
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 31e3edc0fc7e..9007428a68f1 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -743,6 +743,7 @@ void intel_svm_unbind(struct iommu_sva *handle);
u32 intel_svm_get_pasid(struct iommu_sva *handle);
int intel_svm_page_response(struct device *dev, struct iommu_fault_event *evt,
struct iommu_page_response *msg);
+struct iommu_domain *intel_svm_domain_alloc(void);
struct intel_svm_dev {
struct list_head list;
@@ -768,6 +769,10 @@ struct intel_svm {
};
#else
static inline void intel_svm_check(struct intel_iommu *iommu) {}
+static inline struct iommu_domain *intel_svm_domain_alloc(void)
+{
+ return NULL;
+}
#endif
#ifdef CONFIG_INTEL_IOMMU_DEBUGFS
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 44016594831d..993a1ce509a8 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4298,6 +4298,8 @@ static struct iommu_domain
*intel_iommu_domain_alloc(unsigned type)
return domain;
case IOMMU_DOMAIN_IDENTITY:
return &si_domain->domain;
+ case IOMMU_DOMAIN_SVA:
+ return intel_svm_domain_alloc();
default:
return NULL;
}
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index d04880a291c3..7d4f9d173013 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -931,3 +931,52 @@ int intel_svm_page_response(struct device *dev,
mutex_unlock(&pasid_mutex);
return ret;
}
+
+static int intel_svm_set_dev_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid)
+{
+ struct device_domain_info *info = dev_iommu_priv_get(dev);
+ struct intel_iommu *iommu = info->iommu;
+ struct mm_struct *mm = domain->mm;
+ struct iommu_sva *sva;
+ int ret = 0;
+
+ mutex_lock(&pasid_mutex);
+ sva = intel_svm_bind_mm(iommu, dev, mm);
+ if (IS_ERR(sva))
+ ret = PTR_ERR(sva);
+ mutex_unlock(&pasid_mutex);
+
+ return ret;
+}
+
+static void intel_svm_block_dev_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid)
+{
+ mutex_lock(&pasid_mutex);
+ intel_svm_unbind_mm(dev, pasid);
+ mutex_unlock(&pasid_mutex);
+}
+
+static void intel_svm_domain_free(struct iommu_domain *domain)
+{
+ kfree(to_dmar_domain(domain));
+}
+
+static const struct iommu_domain_ops intel_svm_domain_ops = {
+ .set_dev_pasid = intel_svm_set_dev_pasid,
+ .block_dev_pasid= intel_svm_block_dev_pasid,
+ .free = intel_svm_domain_free,
+};
+
+struct iommu_domain *intel_svm_domain_alloc(void)
+{
+ struct dmar_domain *domain;
+
+ domain = kzalloc(sizeof(*domain), GFP_KERNEL);
+ if (!domain)
+ return NULL;
+ domain->domain.ops = &intel_svm_domain_ops;
+
+ return &domain->domain;
+}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v9 04/11] iommu: Add sva iommu_domain support
The sva iommu_domain represents a hardware pagetable that the IOMMU
hardware could use for SVA translation. This adds some infrastructure
to support SVA domain in the iommu common layer. It includes:
- Extend the iommu_domain to support a new IOMMU_DOMAIN_SVA domain
type. The IOMMU drivers that support SVA should provide the sva
domain specific iommu_domain_ops.
- Add a helper to allocate an SVA domain. The iommu_domain_free()
is still used to free an SVA domain.
- Add helpers to attach an SVA domain to a device and the reverse
operation.
Some buses, like PCI, route packets without considering the PASID value.
Thus a DMA target address with PASID might be treated as P2P if the
address falls into the MMIO BAR of other devices in the group. To make
things simple, the attach/detach interfaces only apply to devices
belonging to the singleton groups, and the singleton is immutable in
fabric i.e. not affected by hotplug.
The iommu_attach/detach_device_pasid() can be used for other purposes,
such as kernel DMA with pasid, mediation device, etc.
Suggested-by: Jean-Philippe Brucker
Suggested-by: Jason Gunthorpe
Signed-off-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
---
include/linux/iommu.h | 45 -
drivers/iommu/iommu.c | 93 +++
2 files changed, 136 insertions(+), 2 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 3fbad42c0bf8..b8b6b8c5e20e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -64,6 +64,8 @@ struct iommu_domain_geometry {
#define __IOMMU_DOMAIN_PT (1U << 2) /* Domain is identity mapped */
#define __IOMMU_DOMAIN_DMA_FQ (1U << 3) /* DMA-API uses flush queue*/
+#define __IOMMU_DOMAIN_SVA (1U << 4) /* Shared process address space */
+
/*
* This are the possible domain-types
*
@@ -77,6 +79,8 @@ struct iommu_domain_geometry {
* certain optimizations for these domains
* IOMMU_DOMAIN_DMA_FQ - As above, but definitely using batched TLB
* invalidation.
+ * IOMMU_DOMAIN_SVA- DMA addresses are shared process address
+ * spaces represented by mm_struct's.
*/
#define IOMMU_DOMAIN_BLOCKED (0U)
#define IOMMU_DOMAIN_IDENTITY (__IOMMU_DOMAIN_PT)
@@ -86,15 +90,23 @@ struct iommu_domain_geometry {
#define IOMMU_DOMAIN_DMA_FQ(__IOMMU_DOMAIN_PAGING |\
__IOMMU_DOMAIN_DMA_API | \
__IOMMU_DOMAIN_DMA_FQ)
+#define IOMMU_DOMAIN_SVA (__IOMMU_DOMAIN_SVA)
struct iommu_domain {
unsigned type;
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap;/* Bitmap of page sizes in use */
- iommu_fault_handler_t handler;
- void *handler_token;
struct iommu_domain_geometry geometry;
struct iommu_dma_cookie *iova_cookie;
+ union {
+ struct {/* IOMMU_DOMAIN_DMA */
+ iommu_fault_handler_t handler;
+ void *handler_token;
+ };
+ struct {/* IOMMU_DOMAIN_SVA */
+ struct mm_struct *mm;
+ };
+ };
};
static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
@@ -262,6 +274,8 @@ struct iommu_ops {
* struct iommu_domain_ops - domain specific operations
* @attach_dev: attach an iommu domain to a device
* @detach_dev: detach an iommu domain from a device
+ * @set_dev_pasid: set an iommu domain to a pasid of device
+ * @block_dev_pasid: block pasid of device from using iommu domain
* @map: map a physically contiguous memory region to an iommu domain
* @map_pages: map a physically contiguous set of pages of the same size to
* an iommu domain.
@@ -282,6 +296,10 @@ struct iommu_ops {
struct iommu_domain_ops {
int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
+ int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
+ioasid_t pasid);
+ void (*block_dev_pasid)(struct iommu_domain *domain, struct device *dev,
+ ioasid_t pasid);
int (*map)(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
@@ -679,6 +697,12 @@ int iommu_group_claim_dma_owner(struct iommu_group *group,
void *owner);
void iommu_group_release_dma_owner(struct iommu_group *group);
bool iommu_group_dma_owner_claimed(struct iommu_group *group);
+struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
+ struct mm_struct *mm);
+int iommu_attach_device_pasid(struct iommu_domain *domain, struct device *dev,
+ ioasid_t pasid);
+void iommu_detach_devic
[PATCH v9 03/11] iommu: Remove SVM_FLAG_SUPERVISOR_MODE support
The current kernel DMA with PASID support is based on the SVA with a flag
SVM_FLAG_SUPERVISOR_MODE. The IOMMU driver binds the kernel memory address
space to a PASID of the device. The device driver programs the device with
kernel virtual address (KVA) for DMA access. There have been security and
functional issues with this approach:
- The lack of IOTLB synchronization upon kernel page table updates.
(vmalloc, module/BPF loading, CONFIG_DEBUG_PAGEALLOC etc.)
- Other than slight more protection, using kernel virtual address (KVA)
has little advantage over physical address. There are also no use
cases yet where DMA engines need kernel virtual addresses for in-kernel
DMA.
This removes SVM_FLAG_SUPERVISOR_MODE support from the IOMMU interface.
The device drivers are suggested to handle kernel DMA with PASID through
the kernel DMA APIs.
The drvdata parameter in iommu_sva_bind_device() and all callbacks is not
needed anymore. Cleanup them as well.
Link: https://lore.kernel.org/linux-iommu/20210511194726.gp1002...@nvidia.com/
Signed-off-by: Jacob Pan
Signed-off-by: Lu Baolu
Reviewed-by: Jason Gunthorpe
Reviewed-by: Jean-Philippe Brucker
Reviewed-by: Kevin Tian
---
include/linux/intel-iommu.h | 3 +-
include/linux/intel-svm.h | 13 -
include/linux/iommu.h | 8 +--
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 +-
drivers/dma/idxd/cdev.c | 3 +-
drivers/dma/idxd/init.c | 25 +---
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +-
drivers/iommu/intel/svm.c | 57 +--
drivers/iommu/iommu.c | 5 +-
drivers/misc/uacce/uacce.c| 2 +-
10 files changed, 26 insertions(+), 98 deletions(-)
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index e065cbe3c857..31e3edc0fc7e 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -738,8 +738,7 @@ struct intel_iommu *device_to_iommu(struct device *dev, u8
*bus, u8 *devfn);
extern void intel_svm_check(struct intel_iommu *iommu);
extern int intel_svm_enable_prq(struct intel_iommu *iommu);
extern int intel_svm_finish_prq(struct intel_iommu *iommu);
-struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm,
-void *drvdata);
+struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm);
void intel_svm_unbind(struct iommu_sva *handle);
u32 intel_svm_get_pasid(struct iommu_sva *handle);
int intel_svm_page_response(struct device *dev, struct iommu_fault_event *evt,
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index 207ef06ba3e1..f9a0d44f6fdb 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -13,17 +13,4 @@
#define PRQ_RING_MASK ((0x1000 << PRQ_ORDER) - 0x20)
#define PRQ_DEPTH ((0x1000 << PRQ_ORDER) >> 5)
-/*
- * The SVM_FLAG_SUPERVISOR_MODE flag requests a PASID which can be used only
- * for access to kernel addresses. No IOTLB flushes are automatically done
- * for kernel mappings; it is valid only for access to the kernel's static
- * 1:1 mapping of physical memory — not to vmalloc or even module mappings.
- * A future API addition may permit the use of such ranges, by means of an
- * explicit IOTLB flush call (akin to the DMA API's unmap method).
- *
- * It is unlikely that we will ever hook into flush_tlb_kernel_range() to
- * do such IOTLB flushes automatically.
- */
-#define SVM_FLAG_SUPERVISOR_MODE BIT(0)
-
#endif /* __INTEL_SVM_H__ */
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d50afb2c9a09..3fbad42c0bf8 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -243,8 +243,7 @@ struct iommu_ops {
int (*dev_enable_feat)(struct device *dev, enum iommu_dev_features f);
int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
- struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm,
- void *drvdata);
+ struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm);
void (*sva_unbind)(struct iommu_sva *handle);
u32 (*sva_get_pasid)(struct iommu_sva *handle);
@@ -669,8 +668,7 @@ int iommu_dev_disable_feature(struct device *dev, enum
iommu_dev_features f);
bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features f);
struct iommu_sva *iommu_sva_bind_device(struct device *dev,
- struct mm_struct *mm,
- void *drvdata);
+ struct mm_struct *mm);
void iommu_sva_unbind_device(struct iommu_sva *handle);
u32 iommu_sva_get_pasid(struct iommu_sva *handle);
@@ -1012,7 +1010,7 @@ iommu_dev_disable_feature(struct device *dev, enum
iommu_dev_features feat)
}
static inline struct iommu_sva *
-iommu_sva_bind_device
[PATCH v9 02/11] iommu: Add max_pasids field in struct dev_iommu
Use this field to save the number of PASIDs that a device is able to
consume. It is a generic attribute of a device and lifting it into the
per-device dev_iommu struct could help to avoid the boilerplate code
in various IOMMU drivers.
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 2 ++
drivers/iommu/iommu.c | 20
2 files changed, 22 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 03fbb1b71536..d50afb2c9a09 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -364,6 +364,7 @@ struct iommu_fault_param {
* @fwspec: IOMMU fwspec data
* @iommu_dev: IOMMU device this device is linked to
* @priv: IOMMU Driver private data
+ * @max_pasids: number of PASIDs device can consume
*
* TODO: migrate other per device data pointers under iommu_dev_data, e.g.
* struct iommu_group *iommu_group;
@@ -375,6 +376,7 @@ struct dev_iommu {
struct iommu_fwspec *fwspec;
struct iommu_device *iommu_dev;
void*priv;
+ u32 max_pasids;
};
int iommu_device_register(struct iommu_device *iommu,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 847ad47a2dfd..6b731568efff 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -218,6 +219,24 @@ static void dev_iommu_free(struct device *dev)
kfree(param);
}
+static u32 dev_iommu_get_max_pasids(struct device *dev)
+{
+ u32 max_pasids = 0, bits = 0;
+ int ret;
+
+ if (dev_is_pci(dev)) {
+ ret = pci_max_pasids(to_pci_dev(dev));
+ if (ret > 0)
+ max_pasids = ret;
+ } else {
+ ret = device_property_read_u32(dev, "pasid-num-bits", &bits);
+ if (!ret)
+ max_pasids = 1UL << bits;
+ }
+
+ return min_t(u32, max_pasids, dev->iommu->iommu_dev->max_pasids);
+}
+
static int __iommu_probe_device(struct device *dev, struct list_head
*group_list)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
@@ -243,6 +262,7 @@ static int __iommu_probe_device(struct device *dev, struct
list_head *group_list
}
dev->iommu->iommu_dev = iommu_dev;
+ dev->iommu->max_pasids = dev_iommu_get_max_pasids(dev);
group = iommu_group_get_for_dev(dev);
if (IS_ERR(group)) {
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v9 00/11] iommu: SVA and IOPF refactoring
Hi folks, The former part of this series refactors the IOMMU SVA code by assigning an SVA type of iommu_domain to a shared virtual address and replacing sva_bind/unbind iommu ops with set/block_dev_pasid domain ops. The latter part changes the existing I/O page fault handling framework from only serving SVA to a generic one. Any driver or component could handle the I/O page faults for its domain in its own way by installing an I/O page fault handler. This series has been functionally tested on an x86 machine and compile tested for all architectures. This series is also available on github: [2] https://github.com/LuBaolu/intel-iommu/commits/iommu-sva-refactoring-v9 Please review and suggest. Best regards, baolu Change log: v9: - Some minor changes on comments and function names. - Simplify dev_iommu_get_max_pasids(). v8: - https://lore.kernel.org/linux-iommu/20220607014942.3954894-1-baolu...@linux.intel.com/ - Add support for calculating the max pasids that a device could consume. - Replace container_of_safe() with container_of. - Remove iommu_ops->sva_domain_ops and make sva support through the generic domain_alloc/free() interfaces. - [Robin] It would be logical to pass IOMMU_DOMAIN_SVA to the normal domain_alloc call, so that driver-internal stuff like context descriptors can be still be hung off the domain as usual (rather than all drivers having to implement some extra internal lookup mechanism to handle all the SVA domain ops). - [Robin] I'd just stick the mm pointer in struct iommu_domain, in a union with the fault handler stuff those are mutually exclusive with SVA. - https://lore.kernel.org/linux-iommu/f3170016-4d7f-e78e-db48-68305f683...@arm.com/ v7: - https://lore.kernel.org/linux-iommu/20220519072047.2996983-1-baolu...@linux.intel.com/ - Remove duplicate array for sva domain. - Rename detach_dev_pasid to block_dev_pasid. - Add raw device driver interfaces for iommufd. - Other misc refinements and patch reorganization. - Drop "dmaengine: idxd: Separate user and kernel pasid enabling" which has been picked for dmaengine tree. v6: - https://lore.kernel.org/linux-iommu/20220510061738.2761430-1-baolu...@linux.intel.com/ - Refine the SVA basic data structures. Link: https://lore.kernel.org/linux-iommu/YnFv0ps0Ad8v+7uH@myrica/ - Refine arm smmuv3 sva domain allocation. - Fix a possible lock issue. Link: https://lore.kernel.org/linux-iommu/YnFydE8j8l7Q4m+b@myrica/ v5: - https://lore.kernel.org/linux-iommu/20220502014842.991097-1-baolu...@linux.intel.com/ - Address review comments from Jean-Philippe Brucker. Very appreciated! - Remove redundant pci aliases check in device_group_immutable_singleton(). - Treat all buses except PCI as static in immutable singleton check. - As the sva_bind/unbind() have already guaranteed sva domain free only after iopf_queue_flush_dev(), remove the unnecessary domain refcount. - Move domain get() out of the list iteration in iopf_handle_group(). v4: - https://lore.kernel.org/linux-iommu/20220421052121.3464100-1-baolu...@linux.intel.com/ - Solve the overlap with another series and make this series self-contained. - No objection to the abstraction of data structure during v3 review. Hence remove the RFC subject prefix. - Refine the immutable singleton group code according to Kevin's comments. v3: - https://lore.kernel.org/linux-iommu/20220410102443.294128-1-baolu...@linux.intel.com/ - Rework iommu_group_singleton_lockdown() by adding a flag to the group that positively indicates the group can never have more than one member, even after hot plug. - Abstract the data structs used for iommu sva in a separated patches to make it easier for review. - I still keep the RFC prefix in this series as above two significant changes need at least another round review to be finalized. - Several misc refinements. v2: - https://lore.kernel.org/linux-iommu/20220329053800.3049561-1-baolu...@linux.intel.com/ - Add sva domain life cycle management to avoid race between unbind and page fault handling. - Use a single domain for each mm. - Return a single sva handler for the same binding. - Add a new helper to meet singleton group requirement. - Rework the SVA domain allocation for arm smmu v3 driver and move the pasid_bit initialization to device probe. - Drop the patch "iommu: Handle IO page faults directly". - Add mmget_not_zero(mm) in SVA page fault handler. v1: - https://lore.kernel.org/linux-iommu/20220320064030.2936936-1-baolu...@linux.intel.com/ - Initial post. *** BLURB HERE *** Lu Baolu (11): iommu: Add max_pasids field in struct iommu_device iommu: Add max_pasids field in struct dev_iommu iommu: Remove SVM_FLAG_SUPERVISOR_MODE support iommu: Add sva iommu_domain support iommu/vt-d: Add SVA domain support arm-smmu-v3/sva: Add SVA domain support iommu/sva: Refactoring iommu_sva_bind/unbind_device() iommu: Remove SVA related callbacks from
[PATCH v9 01/11] iommu: Add max_pasids field in struct iommu_device
Use this field to keep the number of supported PASIDs that an IOMMU
hardware is able to support. This is a generic attribute of an IOMMU
and lifting it into the per-IOMMU device structure makes it possible
to allocate a PASID for device without calls into the IOMMU drivers.
Any iommu driver that supports PASID related features should set this
field before enabling them on the devices.
In the Intel IOMMU driver, intel_iommu_sm is moved to CONFIG_INTEL_IOMMU
enclave so that the pasid_supported() helper could be used in dmar.c
without compilation errors.
Signed-off-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
---
include/linux/intel-iommu.h | 3 ++-
include/linux/iommu.h | 2 ++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
drivers/iommu/intel/dmar.c | 7 +++
4 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 4f29139bbfc3..e065cbe3c857 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -479,7 +479,6 @@ enum {
#define VTD_FLAG_IRQ_REMAP_PRE_ENABLED (1 << 1)
#define VTD_FLAG_SVM_CAPABLE (1 << 2)
-extern int intel_iommu_sm;
extern spinlock_t device_domain_lock;
#define sm_supported(iommu)(intel_iommu_sm && ecap_smts((iommu)->ecap))
@@ -786,6 +785,7 @@ struct context_entry *iommu_context_addr(struct intel_iommu
*iommu, u8 bus,
extern const struct iommu_ops intel_iommu_ops;
#ifdef CONFIG_INTEL_IOMMU
+extern int intel_iommu_sm;
extern int iommu_calculate_agaw(struct intel_iommu *iommu);
extern int iommu_calculate_max_sagaw(struct intel_iommu *iommu);
extern int dmar_disabled;
@@ -802,6 +802,7 @@ static inline int iommu_calculate_max_sagaw(struct
intel_iommu *iommu)
}
#define dmar_disabled (1)
#define intel_iommu_enabled (0)
+#define intel_iommu_sm (0)
#endif
static inline const char *decode_prq_descriptor(char *str, size_t size,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5e1afe169549..03fbb1b71536 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -318,12 +318,14 @@ struct iommu_domain_ops {
* @list: Used by the iommu-core to keep a list of registered iommus
* @ops: iommu-ops for talking to this iommu
* @dev: struct device for sysfs handling
+ * @max_pasids: number of supported PASIDs
*/
struct iommu_device {
struct list_head list;
const struct iommu_ops *ops;
struct fwnode_handle *fwnode;
struct device *dev;
+ u32 max_pasids;
};
/**
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 88817a3376ef..ae8ec8df47c1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3546,6 +3546,7 @@ static int arm_smmu_device_hw_probe(struct
arm_smmu_device *smmu)
/* SID/SSID sizes */
smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, reg);
smmu->sid_bits = FIELD_GET(IDR1_SIDSIZE, reg);
+ smmu->iommu.max_pasids = 1UL << smmu->ssid_bits;
/*
* If the SMMU supports fewer bits than would fill a single L2 stream
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 592c1e1a5d4b..6c33061a 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1123,6 +1123,13 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
raw_spin_lock_init(&iommu->register_lock);
+ /*
+* A value of N in PSS field of eCap register indicates hardware
+* supports PASID field of N+1 bits.
+*/
+ if (pasid_supported(iommu))
+ iommu->iommu.max_pasids = 2UL << ecap_pss(iommu->ecap);
+
/*
* This is only for hotplug; at boot time intel_iommu_enabled won't
* be set yet. When intel_iommu_init() runs, it registers the units
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/vt-d: Fix RID2PASID setup failure
On 2022/6/21 13:48, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 21, 2022 12:28 PM
On 2022/6/21 11:46, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 21, 2022 11:39 AM
On 2022/6/21 10:54, Tian, Kevin wrote:
From: Lu Baolu
Sent: Monday, June 20, 2022 4:17 PM
@@ -2564,7 +2564,7 @@ static int domain_add_dev_info(struct
dmar_domain *domain, struct device *dev)
ret = intel_pasid_setup_second_level(iommu,
domain,
dev, PASID_RID2PASID);
spin_unlock_irqrestore(&iommu->lock, flags);
- if (ret) {
+ if (ret && ret != -EBUSY) {
dev_err(dev, "Setup RID2PASID failed\n");
dmar_remove_one_dev_info(dev);
return ret;
--
2.25.1
It's cleaner to avoid this error at the first place, i.e. only do the
setup when the first device is attached to the pasid table.
The logic that identifies the first device might introduce additional
unnecessary complexity. Devices that share a pasid table are rare. I
even prefer to give up sharing tables so that the code can be
simpler.:-)
It's not that complex if you simply move device_attach_pasid_table()
out of intel_pasid_alloc_table(). Then do the setup if
list_empty(&pasid_table->dev) and then attach device to the
pasid table in domain_add_dev_info().
The pasid table is part of the device, hence a better place to
allocate/free the pasid table is in the device probe/release paths.
Things will become more complicated if we change relationship between
device and it's pasid table when attaching/detaching a domain. That's
the reason why I thought it was additional complexity.
If you do want to follow current route it’s still cleaner to check
whether the pasid entry has pointed to the domain in the individual
setup function instead of blindly returning -EBUSY and then ignoring
it even if a real busy condition occurs. The setup functions can
just return zero for this benign alias case.
Kevin, how do you like this one?
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index cb4c1d0cf25c..ecffd0129b2b 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -575,6 +575,16 @@ static inline int pasid_enable_wpe(struct
pasid_entry *pte)
return 0;
};
+/*
+ * Return true if @pasid is RID2PASID and the domain @did has already
+ * been setup to the @pte. Otherwise, return false.
+ */
+static inline bool
+rid2pasid_domain_valid(struct pasid_entry *pte, u32 pasid, u16 did)
+{
+ return pasid == PASID_RID2PASID && pasid_get_domain_id(pte) == did;
+}
+
/*
* Set up the scalable mode pasid table entry for first only
* translation type.
@@ -595,9 +605,8 @@ int intel_pasid_setup_first_level(struct intel_iommu
*iommu,
if (WARN_ON(!pte))
return -EINVAL;
- /* Caller must ensure PASID entry is not in use. */
if (pasid_pte_is_present(pte))
- return -EBUSY;
+ return rid2pasid_domain_valid(pte, pasid, did) ? 0: -EBUSY;
pasid_clear_entry(pte);
@@ -698,9 +707,8 @@ int intel_pasid_setup_second_level(struct
intel_iommu *iommu,
return -ENODEV;
}
- /* Caller must ensure PASID entry is not in use. */
if (pasid_pte_is_present(pte))
- return -EBUSY;
+ return rid2pasid_domain_valid(pte, pasid, did) ? 0: -EBUSY;
pasid_clear_entry(pte);
pasid_set_domain_id(pte, did);
@@ -738,9 +746,8 @@ int intel_pasid_setup_pass_through(struct
intel_iommu *iommu,
return -ENODEV;
}
- /* Caller must ensure PASID entry is not in use. */
if (pasid_pte_is_present(pte))
- return -EBUSY;
+ return rid2pasid_domain_valid(pte, pasid, did) ? 0: -EBUSY;
pasid_clear_entry(pte);
pasid_set_domain_id(pte, did);
--
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/3] scsi: BusLogic remove bus_to_virt
On 6/17/22 14:57, Arnd Bergmann wrote:
From: Arnd Bergmann
The BusLogic driver is the last remaining driver that relies on the
deprecated bus_to_virt() function, which in turn only works on a few
architectures, and is incompatible with both swiotlb and iommu support.
Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."),
the driver had a dependency on x86-32, presumably because of this
problem. However, the change introduced another bug that made it still
impossible to use the driver on any 64-bit machine.
This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix
64-bit system enumeration error for Buslogic"), 8 years later, which
shows that there are not a lot of users.
Maciej is still using the driver on 32-bit hardware, and Khalid mentioned
that the driver works with the device emulation used in VirtualBox
and VMware. Both of those only emulate it for Windows 2000 and older
operating systems that did not ship with the better LSI logic driver.
Do a minimum fix that searches through the list of descriptors to find
one that matches the bus address. This is clearly as inefficient as
was indicated in the code comment about the lack of a bus_to_virt()
replacement. A better fix would likely involve changing out the entire
descriptor allocation for a simpler one, but that would be much
more invasive.
Cc: Maciej W. Rozycki
Cc: Matt Wang
Cc: Khalid Aziz
Signed-off-by: Arnd Bergmann
---
drivers/scsi/BusLogic.c | 27 ---
drivers/scsi/Kconfig| 2 +-
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index a897c8f914cf..d057abfcdd5c 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter
*adapter,
return (hoststatus << 16) | tgt_status;
}
+/*
+ * turn the dma address from an inbox into a ccb pointer
+ * This is rather inefficient.
+ */
+static struct blogic_ccb *
+blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox)
+{
+ struct blogic_ccb *ccb;
+
+ for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all)
+ if (inbox->ccb == ccb->dma_handle)
+ break;
+
+ return ccb;
+}
/*
blogic_scan_inbox scans the Incoming Mailboxes saving any
Incoming Mailbox entries for completion processing.
*/
-
static void blogic_scan_inbox(struct blogic_adapter *adapter)
{
/*
@@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter
*adapter)
enum blogic_cmplt_code comp_code;
while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
- /*
- We are only allowed to do this because we limit our
- architectures we run on to machines where bus_to_virt(
- actually works. There *needs* to be a dma_addr_to_virt()
- in the new PCI DMA mapping interface to replace
- bus_to_virt() or else this code is going to become very
- innefficient.
-*/
- struct blogic_ccb *ccb =
- (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
+ struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter,
adapter->next_inbox);
if (comp_code != BLOGIC_CMD_NOTFOUND) {
if (ccb->status == BLOGIC_CCB_ACTIVE ||
ccb->status == BLOGIC_CCB_RESET) {
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index cf75588a2587..56bdc08d0b77 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -513,7 +513,7 @@ config SCSI_HPTIOP
config SCSI_BUSLOGIC
tristate "BusLogic SCSI support"
- depends on PCI && SCSI && VIRT_TO_BUS
+ depends on PCI && SCSI
help
This is support for BusLogic MultiMaster and FlashPoint SCSI Host
Adapters. Consult the SCSI-HOWTO, available from
CCB handling in the driver is ugly anyway, so that'll be good enough.
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeKernel Storage Architect
h...@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/3] scsi: dpt_i2o: drop stale VIRT_TO_BUS dependency
On 6/17/22 14:57, Arnd Bergmann wrote:
From: Arnd Bergmann
The dpt_i2o driver was fixed to stop using virt_to_bus() in 2008, but
it still has a stale reference in an error handling code path that could
never work.
Fix it up to build without VIRT_TO_BUS and remove the Kconfig dependency.
The alternative to this would be to just remove the driver, as it is
clearly obsolete. The i2o driver layer was removed in 2015 with commit
4a72a7af462d ("staging: remove i2o subsystem"), but the even older
dpt_i2o scsi driver stayed around.
The last non-cleanup patches I could find were from Miquel van Smoorenburg
and Mark Salyzyn back in 2008, they might know if there is any chance
of the hardware still being used anywhere.
Fixes: 67af2b060e02 ("[SCSI] dpt_i2o: move from virt_to_bus/bus_to_virt to
dma_alloc_coherent")
Cc: Miquel van Smoorenburg
Cc: Mark Salyzyn
Signed-off-by: Arnd Bergmann
---
drivers/scsi/Kconfig | 2 +-
drivers/scsi/dpt_i2o.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index a9fe5152addd..cf75588a2587 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -460,7 +460,7 @@ config SCSI_MVUMI
config SCSI_DPT_I2O
tristate "Adaptec I2O RAID support "
- depends on SCSI && PCI && VIRT_TO_BUS
+ depends on SCSI && PCI
help
This driver supports all of Adaptec's I2O based RAID controllers as
well as the DPT SmartRaid V cards. This is an Adaptec maintained
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 2e9155ba7408..55dfe7011912 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -52,11 +52,11 @@ MODULE_DESCRIPTION("Adaptec I2O RAID Driver");
#include
#include
+#include
#include
#include
#include /* for boot_cpu_data */
-#include /* for virt_to_bus, etc. */
#include
#include
@@ -2112,7 +2112,7 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
} else {
/* Ick, we should *never* be here */
printk(KERN_ERR "dpti: reply frame not from pool\n");
- reply = (u8 *)bus_to_virt(m);
+ goto out;
}
if (readl(reply) & MSG_FAIL) {
Reviewed-by: Hannes Reinecke
Personally I wouldn't mind to see this driver gone, as it's being built
upon the (long-defunct) I2O specification. We already deleted the i2o
subsystem years ago, so maybe it's time to consign this driver to
history, too.
Cheers,
Hannes
--
Dr. Hannes ReineckeKernel Storage Architect
h...@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] uacce: fix concurrency of fops_open and uacce_remove
On Tue, Jun 21, 2022 at 03:37:31PM +0800, Zhangfei Gao wrote: > > > On 2022/6/20 下午9:36, Greg Kroah-Hartman wrote: > > On Mon, Jun 20, 2022 at 02:24:31PM +0100, Jean-Philippe Brucker wrote: > > > On Fri, Jun 17, 2022 at 02:05:21PM +0800, Zhangfei Gao wrote: > > > > > The refcount only ensures that the uacce_device object is not freed as > > > > > long as there are open fds. But uacce_remove() can run while there are > > > > > open fds, or fds in the process of being opened. And atfer > > > > > uacce_remove() > > > > > runs, the uacce_device object still exists but is mostly unusable. For > > > > > example once the module is freed, uacce->ops is not valid anymore. But > > > > > currently uacce_fops_open() may dereference the ops in this case: > > > > > > > > > > uacce_fops_open() > > > > >if (!uacce->parent->driver) > > > > >/* Still valid, keep going */ > > > > >...rmmod > > > > >uacce_remove() > > > > >... free_module() > > > > >uacce->ops->get_queue() /* BUG */ > > > > uacce_remove should wait for uacce->queues_lock, until fops_open > > > > release the > > > > lock. > > > > If open happen just after the uacce_remove: unlock, uacce_bind_queue in > > > > open > > > > should fail. > > > Ah yes sorry, I lost sight of what this patch was adding. But we could > > > have the same issue with the patch, just in a different order, no? > > > > > > uacce_fops_open() > > >uacce = xa_load() > > >...rmmod > > Um, how is rmmod called if the file descriptor is open? > > > > That should not be possible if the owner of the file descriptor is > > properly set. Please fix that up. > Thanks Greg > > Set cdev owner or use module_get/put can block rmmod once fops_open. > - uacce->cdev->owner = THIS_MODULE; > + uacce->cdev->owner = uacce->parent->driver->owner; > > However, still not find good method to block removing parent pci device. > > $ echo 1 > /sys/bus/pci/devices/:00:02.0/remove & > > [ 32.563350] uacce_remove+0x6c/0x148 > [ 32.563353] hisi_qm_uninit+0x12c/0x178 > [ 32.563356] hisi_zip_remove+0xa0/0xd0 [hisi_zip] > [ 32.563361] pci_device_remove+0x44/0xd8 > [ 32.563364] device_remove+0x54/0x88 > [ 32.563367] device_release_driver_internal+0xec/0x1a0 > [ 32.563370] device_release_driver+0x20/0x30 > [ 32.563372] pci_stop_bus_device+0x8c/0xe0 > [ 32.563375] pci_stop_and_remove_bus_device_locked+0x28/0x60 > [ 32.563378] remove_store+0x9c/0xb0 > [ 32.563379] dev_attr_store+0x20/0x38 Removing the parent pci device does not remove the module code, it removes the device itself. Don't confuse code vs. data here. thanks, greg k-h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] uacce: fix concurrency of fops_open and uacce_remove
On 2022/6/20 下午9:36, Greg Kroah-Hartman wrote: On Mon, Jun 20, 2022 at 02:24:31PM +0100, Jean-Philippe Brucker wrote: On Fri, Jun 17, 2022 at 02:05:21PM +0800, Zhangfei Gao wrote: The refcount only ensures that the uacce_device object is not freed as long as there are open fds. But uacce_remove() can run while there are open fds, or fds in the process of being opened. And atfer uacce_remove() runs, the uacce_device object still exists but is mostly unusable. For example once the module is freed, uacce->ops is not valid anymore. But currently uacce_fops_open() may dereference the ops in this case: uacce_fops_open() if (!uacce->parent->driver) /* Still valid, keep going */ ...rmmod uacce_remove() ... free_module() uacce->ops->get_queue() /* BUG */ uacce_remove should wait for uacce->queues_lock, until fops_open release the lock. If open happen just after the uacce_remove: unlock, uacce_bind_queue in open should fail. Ah yes sorry, I lost sight of what this patch was adding. But we could have the same issue with the patch, just in a different order, no? uacce_fops_open() uacce = xa_load() ...rmmod Um, how is rmmod called if the file descriptor is open? That should not be possible if the owner of the file descriptor is properly set. Please fix that up. Thanks Greg Set cdev owner or use module_get/put can block rmmod once fops_open. - uacce->cdev->owner = THIS_MODULE; + uacce->cdev->owner = uacce->parent->driver->owner; However, still not find good method to block removing parent pci device. $ echo 1 > /sys/bus/pci/devices/:00:02.0/remove & [ 32.563350] uacce_remove+0x6c/0x148 [ 32.563353] hisi_qm_uninit+0x12c/0x178 [ 32.563356] hisi_zip_remove+0xa0/0xd0 [hisi_zip] [ 32.563361] pci_device_remove+0x44/0xd8 [ 32.563364] device_remove+0x54/0x88 [ 32.563367] device_release_driver_internal+0xec/0x1a0 [ 32.563370] device_release_driver+0x20/0x30 [ 32.563372] pci_stop_bus_device+0x8c/0xe0 [ 32.563375] pci_stop_and_remove_bus_device_locked+0x28/0x60 [ 32.563378] remove_store+0x9c/0xb0 [ 32.563379] dev_attr_store+0x20/0x38 mutex_lock(&dev->device_lock) can be used, which used in device_release_driver_internal. Or use internal mutex. Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Hi, * Saravana Kannan [700101 02:00]: > Now that fw_devlink=on by default and fw_devlink supports > "power-domains" property, the execution will never get to the point > where driver_deferred_probe_check_state() is called before the supplier > has probed successfully or before deferred probe timeout has expired. > > So, delete the call and replace it with -ENODEV. Looks like this causes omaps to not boot in Linux next. With this simple-pm-bus fails to probe initially as the power-domain is not yet available. On platform_probe() genpd_get_from_provider() returns -ENOENT. Seems like other stuff is potentially broken too, any ideas on how to fix this? Regards, Tony > > Signed-off-by: Saravana Kannan > --- > drivers/base/power/domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 739e52cd4aba..3e86772d5fac 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2730,7 +2730,7 @@ static int __genpd_dev_pm_attach(struct device *dev, > struct device *base_dev, > mutex_unlock(&gpd_list_lock); > dev_dbg(dev, "%s() failed to find PM domain: %ld\n", > __func__, PTR_ERR(pd)); > - return driver_deferred_probe_check_state(base_dev); > + return -ENODEV; > } > > dev_dbg(dev, "adding to PM domain %s\n", pd->name); > -- > 2.36.1.255.ge46751e96f-goog > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 2/5] iommu/io-pgtable: Move Apple DART support to its own file
The pte format used by the DARTs found in the Apple M1 (t8103) is not
fully compatible with io-pgtable-arm. The 24 MSB are used for subpage
protection (mapping only parts of page) and conflict with the address
mask. In addition bit 1 is not available for tagging entries but disables
subpage protection. Subpage protection could be useful to support a CPU
granule of 4k with the fixed IOMMU page size of 16k.
The DARTs found on Apple M1 Pro/Max/Ultra use another different pte
format which is even less compatible. To support an output address size
of 42 bit the address is shifted down by 4. Subpage protection is
mandatory and bit 1 signifies uncached mappings used by the display
controller.
It would be advantageous to share code for all known Apple DART
variants to support common features. The page table allocator for DARTs
is less complex since it uses a two levels of translation table without
support for huge pages.
Signed-off-by: Janne Grunau
---
Changes in v3:
- move APPLE_DART to its own io-pgtable implementation, copied from
io-pgtable-arm and simplified
Changes in v2:
- add APPLE_DART2 io-pgtable format
MAINTAINERS | 1 +
drivers/iommu/Kconfig | 1 -
drivers/iommu/Makefile | 2 +-
drivers/iommu/io-pgtable-arm.c | 63
drivers/iommu/io-pgtable-dart.c | 580
drivers/iommu/io-pgtable.c | 2 +
6 files changed, 584 insertions(+), 65 deletions(-)
create mode 100644 drivers/iommu/io-pgtable-dart.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 1fc9ead83d2a..028b7e31e589 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1848,6 +1848,7 @@ F:drivers/clk/clk-apple-nco.c
F: drivers/i2c/busses/i2c-pasemi-core.c
F: drivers/i2c/busses/i2c-pasemi-platform.c
F: drivers/iommu/apple-dart.c
+F: drivers/iommu/io-pgtable-dart.c
F: drivers/irqchip/irq-apple-aic.c
F: drivers/mailbox/apple-mailbox.c
F: drivers/nvme/host/apple.c
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c79a0df090c0..ed6d8260f330 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -294,7 +294,6 @@ config APPLE_DART
tristate "Apple DART IOMMU Support"
depends on ARCH_APPLE || (COMPILE_TEST && !GENERIC_ATOMIC64)
select IOMMU_API
- select IOMMU_IO_PGTABLE_LPAE
default ARCH_APPLE
help
Support for Apple DART (Device Address Resolution Table) IOMMUs
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 44475a9b3eea..bd68c93bbd62 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -29,4 +29,4 @@ obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
obj-$(CONFIG_IOMMU_SVA) += iommu-sva-lib.o io-pgfault.o
obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o
-obj-$(CONFIG_APPLE_DART) += apple-dart.o
+obj-$(CONFIG_APPLE_DART) += apple-dart.o io-pgtable-dart.o
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 94ff319ae8ac..d7f5e23da643 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -130,9 +130,6 @@
#define ARM_MALI_LPAE_MEMATTR_IMP_DEF 0x88ULL
#define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL
-#define APPLE_DART_PTE_PROT_NO_WRITE (1<<7)
-#define APPLE_DART_PTE_PROT_NO_READ (1<<8)
-
/* IOPTE accessors */
#define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d))
@@ -406,15 +403,6 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
arm_lpae_io_pgtable *data,
{
arm_lpae_iopte pte;
- if (data->iop.fmt == APPLE_DART) {
- pte = 0;
- if (!(prot & IOMMU_WRITE))
- pte |= APPLE_DART_PTE_PROT_NO_WRITE;
- if (!(prot & IOMMU_READ))
- pte |= APPLE_DART_PTE_PROT_NO_READ;
- return pte;
- }
-
if (data->iop.fmt == ARM_64_LPAE_S1 ||
data->iop.fmt == ARM_32_LPAE_S1) {
pte = ARM_LPAE_PTE_nG;
@@ -1107,52 +1095,6 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg,
void *cookie)
return NULL;
}
-static struct io_pgtable *
-apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
-{
- struct arm_lpae_io_pgtable *data;
- int i;
-
- if (cfg->oas > 36)
- return NULL;
-
- data = arm_lpae_alloc_pgtable(cfg);
- if (!data)
- return NULL;
-
- /*
-* The table format itself always uses two levels, but the total VA
-* space is mapped by four separate tables, making the MMIO registers
-* an effective "level 1". For simplicity, though, we treat this
-* equivalently to LPAE stage 2 concatenation at level 2, with the
-* additional TTBRs each just pointing at consecutive pages.
-*/
- if (data->start_level < 1)
- goto out_free_data;
- if (data->start_level == 1 && data->pgd_bits > 2)
- goto out_free_data;
- if (data-
[PATCH v3 1/5] dt-bindings: iommu: dart: add t6000 compatible
From: Sven Peter The M1 Max/Pro SoCs come with a new DART variant that is incompatible with the previous one. Add a new compatible for those. Signed-off-by: Sven Peter Acked-by: Rob Herring Signed-off-by: Janne Grunau --- (no changes since v2) Changes in v2: - added Rob's Acked-by: Documentation/devicetree/bindings/iommu/apple,dart.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/iommu/apple,dart.yaml b/Documentation/devicetree/bindings/iommu/apple,dart.yaml index 82ad669feef7..06af2bacbe97 100644 --- a/Documentation/devicetree/bindings/iommu/apple,dart.yaml +++ b/Documentation/devicetree/bindings/iommu/apple,dart.yaml @@ -22,7 +22,9 @@ description: |+ properties: compatible: -const: apple,t8103-dart +enum: + - apple,t8103-dart + - apple,t6000-dart reg: maxItems: 1 -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 5/5] iommu: dart: Support t6000 variant
From: Sven Peter
The M1 Pro/Max/Ultra SoCs come with a new variant of DART which
supports a larger physical address space with a different PTE format.
Pass through the correct paddr address space size and the PTE format
to the io-pgtable code which will take care of the rest.
Signed-off-by: Sven Peter
Co-developed-by: Janne Grunau
Signed-off-by: Janne Grunau
---
Changes in v3:
- apply change to io-pgtable-dart.c
Changes in v2:
- use APPLE_DART2 PTE format for dart-t6000
drivers/iommu/apple-dart.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index 8af0242a90d9..e5793c0d08b4 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -81,10 +81,16 @@
#define DART_TTBR_VALID BIT(31)
#define DART_TTBR_SHIFT 12
+struct apple_dart_hw {
+ u32 oas;
+ enum io_pgtable_fmt fmt;
+};
+
/*
* Private structure associated with each DART device.
*
* @dev: device struct
+ * @hw: SoC-specific hardware data
* @regs: mapped MMIO region
* @irq: interrupt number, can be shared with other DARTs
* @clks: clocks associated with this DART
@@ -98,6 +104,7 @@
*/
struct apple_dart {
struct device *dev;
+ const struct apple_dart_hw *hw;
void __iomem *regs;
@@ -421,13 +428,13 @@ static int apple_dart_finalize_domain(struct iommu_domain
*domain,
pgtbl_cfg = (struct io_pgtable_cfg){
.pgsize_bitmap = dart->pgsize,
.ias = 32,
- .oas = 36,
+ .oas = dart->hw->oas,
.coherent_walk = 1,
.iommu_dev = dart->dev,
};
dart_domain->pgtbl_ops =
- alloc_io_pgtable_ops(APPLE_DART, &pgtbl_cfg, domain);
+ alloc_io_pgtable_ops(dart->hw->fmt, &pgtbl_cfg, domain);
if (!dart_domain->pgtbl_ops) {
ret = -ENOMEM;
goto done;
@@ -858,6 +865,7 @@ static int apple_dart_probe(struct platform_device *pdev)
return -ENOMEM;
dart->dev = dev;
+ dart->hw = of_device_get_match_data(dev);
spin_lock_init(&dart->lock);
dart->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
@@ -946,8 +954,18 @@ static int apple_dart_remove(struct platform_device *pdev)
return 0;
}
+static const struct apple_dart_hw apple_dart_hw_t8103 = {
+ .oas = 36,
+ .fmt = APPLE_DART,
+};
+static const struct apple_dart_hw apple_dart_hw_t6000 = {
+ .oas = 42,
+ .fmt = APPLE_DART2,
+};
+
static const struct of_device_id apple_dart_of_match[] = {
- { .compatible = "apple,t8103-dart", .data = NULL },
+ { .compatible = "apple,t8103-dart", .data = &apple_dart_hw_t8103 },
+ { .compatible = "apple,t6000-dart", .data = &apple_dart_hw_t6000 },
{},
};
MODULE_DEVICE_TABLE(of, apple_dart_of_match);
--
2.35.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 3/5] iommu/io-pgtable: Add DART subpage protection support
From: Sven Peter DART allows to only expose a subpage to the device. While this is an optional feature on the M1 DARTs the new ones present on the Pro/Max models require this field in every PTE. Signed-off-by: Sven Peter Signed-off-by: Janne Grunau --- Changes in v3: - apply change to io-pgtable-dart.c drivers/iommu/io-pgtable-dart.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/iommu/io-pgtable-dart.c b/drivers/iommu/io-pgtable-dart.c index 0c5222942c65..fa8025c03bb5 100644 --- a/drivers/iommu/io-pgtable-dart.c +++ b/drivers/iommu/io-pgtable-dart.c @@ -14,6 +14,7 @@ #define pr_fmt(fmt)"dart io-pgtable: " fmt #include +#include #include #include #include @@ -63,6 +64,9 @@ /* Calculate the block/page mapping size at level l for pagetable in d. */ #define DART_BLOCK_SIZE(l, d) (1ULL << DART_LVL_SHIFT(l, d)) +#define APPLE_DART_PTE_SUBPAGE_START GENMASK_ULL(63, 52) +#define APPLE_DART_PTE_SUBPAGE_END GENMASK_ULL(51, 40) + #define APPLE_DART1_PADDR_MASK GENMASK_ULL(35, 12) /* Apple DART1 protection bits */ @@ -140,6 +144,10 @@ static void __dart_init_pte(struct dart_io_pgtable *data, pte |= APPLE_DART_PTE_VALID; + /* subpage protection: always allow access to the entire page */ + pte |= FIELD_PREP(APPLE_DART_PTE_SUBPAGE_START, 0); + pte |= FIELD_PREP(APPLE_DART_PTE_SUBPAGE_END, 0xfff); + for (i = 0; i < num_entries; i++) ptep[i] = pte | paddr_to_iopte(paddr + i * sz, data); } -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 4/5] iommu/io-pgtable-dart: Add DART PTE support for t6000
From: Sven Peter
The DARTs present in the M1 Pro/Max/Ultra SoC use a diffent PTE format.
They support a 42bit physical address space by shifting the paddr and
extending its mask inside the PTE.
They also come with mandatory sub-page protection now which we just
configure to always allow access to the entire page. This feature is
already present but optional on the previous DARTs which allows to
unconditionally configure it.
Signed-off-by: Sven Peter
Co-developed-by: Janne Grunau
Signed-off-by: Janne Grunau
---
Changes in v3:
- apply change to io-pgtable-dart.c
- handle pte <> paddr conversion based on the pte format instead of
the output address size
Changes in v2:
- add APPLE_DART2 PTE format
drivers/iommu/io-pgtable-dart.c | 51 +++--
drivers/iommu/io-pgtable.c | 1 +
include/linux/io-pgtable.h | 1 +
3 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/io-pgtable-dart.c b/drivers/iommu/io-pgtable-dart.c
index fa8025c03bb5..9c3c2505f3dc 100644
--- a/drivers/iommu/io-pgtable-dart.c
+++ b/drivers/iommu/io-pgtable-dart.c
@@ -68,12 +68,19 @@
#define APPLE_DART_PTE_SUBPAGE_END GENMASK_ULL(51, 40)
#define APPLE_DART1_PADDR_MASK GENMASK_ULL(35, 12)
+#define APPLE_DART2_PADDR_MASK GENMASK_ULL(37, 10)
+#define APPLE_DART2_PADDR_SHIFT(4)
/* Apple DART1 protection bits */
#define APPLE_DART1_PTE_PROT_NO_READ BIT(8)
#define APPLE_DART1_PTE_PROT_NO_WRITE BIT(7)
#define APPLE_DART1_PTE_PROT_SP_DISBIT(1)
+/* Apple DART2 protection bits */
+#define APPLE_DART2_PTE_PROT_NO_READ BIT(3)
+#define APPLE_DART2_PTE_PROT_NO_WRITE BIT(2)
+#define APPLE_DART2_PTE_PROT_NO_CACHE BIT(1)
+
/* marks PTE as valid */
#define APPLE_DART_PTE_VALID BIT(0)
@@ -101,13 +108,31 @@ static inline bool iopte_leaf(dart_iopte pte, int lvl,
static dart_iopte paddr_to_iopte(phys_addr_t paddr,
struct dart_io_pgtable *data)
{
- return paddr & APPLE_DART1_PADDR_MASK;
+ dart_iopte pte;
+
+ if (data->iop.fmt == APPLE_DART)
+ return paddr & APPLE_DART1_PADDR_MASK;
+
+ /* format is APPLE_DART2 */
+ pte = paddr >> APPLE_DART2_PADDR_SHIFT;
+ pte &= APPLE_DART2_PADDR_MASK;
+
+ return pte;
}
static phys_addr_t iopte_to_paddr(dart_iopte pte,
struct dart_io_pgtable *data)
{
- return pte & APPLE_DART1_PADDR_MASK;
+ u64 paddr;
+
+ if (data->iop.fmt == APPLE_DART)
+ return pte & APPLE_DART1_PADDR_MASK;
+
+ /* format is APPLE_DART2 */
+ paddr = pte & APPLE_DART2_PADDR_MASK;
+ paddr <<= APPLE_DART2_PADDR_SHIFT;
+
+ return paddr;
}
static void *__dart_alloc_pages(size_t size, gfp_t gfp,
@@ -139,7 +164,7 @@ static void __dart_init_pte(struct dart_io_pgtable *data,
size_t sz = DART_BLOCK_SIZE(lvl, data);
int i;
- if (lvl == DART_MAX_LEVELS - 1)
+ if (lvl == DART_MAX_LEVELS - 1 && data->iop.fmt == APPLE_DART)
pte |= APPLE_DART1_PTE_PROT_SP_DIS;
pte |= APPLE_DART_PTE_VALID;
@@ -251,10 +276,20 @@ static dart_iopte dart_prot_to_pte(struct dart_io_pgtable
*data,
{
dart_iopte pte = 0;
- if (!(prot & IOMMU_WRITE))
- pte |= APPLE_DART1_PTE_PROT_NO_WRITE;
- if (!(prot & IOMMU_READ))
- pte |= APPLE_DART1_PTE_PROT_NO_READ;
+ if (data->iop.fmt == APPLE_DART) {
+ if (!(prot & IOMMU_WRITE))
+ pte |= APPLE_DART1_PTE_PROT_NO_WRITE;
+ if (!(prot & IOMMU_READ))
+ pte |= APPLE_DART1_PTE_PROT_NO_READ;
+ }
+ if (data->iop.fmt == APPLE_DART2) {
+ if (!(prot & IOMMU_WRITE))
+ pte |= APPLE_DART2_PTE_PROT_NO_WRITE;
+ if (!(prot & IOMMU_READ))
+ pte |= APPLE_DART2_PTE_PROT_NO_READ;
+ if (!(prot & IOMMU_CACHE))
+ pte |= APPLE_DART2_PTE_PROT_NO_CACHE;
+ }
return pte;
}
@@ -536,7 +571,7 @@ apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void
*cookie)
if (!cfg->coherent_walk)
return NULL;
- if (cfg->oas > 36)
+ if (cfg->oas != 36 && cfg->oas != 42)
return NULL;
data = dart_alloc_pgtable(cfg);
diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
index e6edc6686859..a5d0f01afa7b 100644
--- a/drivers/iommu/io-pgtable.c
+++ b/drivers/iommu/io-pgtable.c
@@ -23,6 +23,7 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = {
#endif
#ifdef CONFIG_APPLE_DART
[APPLE_DART] = &io_pgtable_apple_dart_init_fns,
+ [APPLE_DART2] = &io_pgtable_apple_dart_init_fns,
#endif
#ifdef CONFIG_IOMMU_IO_PGTABLE_ARMV7S
[ARM_V7S] = &io_pgtable_arm_v7s_init_fns,
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index 86af6f0a00a2..76b98511cbc8 100644
--- a/include/linux/io-pgt
[PATCH v3 0/5] iommu: M1 Pro/Max DART support
Hej, this is the next attempt adding support for the DART found in Apple's M1 Pro/Max/Ultra. This adds a separate io-pgtable implementation for DART. As already mentioned in v2 the pte format is not fully compatible with io-pgtable-arm. Especially the 2nd least significant bit is used and is not available to tag tables/pages. io-pgtable-dart.c is copied from io-pgtable-arm.c and support for unused features is removed. Support for 4k IO pages is left for A7 to A11 SoCs as there's work underway to run Linux on them. The incompatibilities between both Apple DART pte seems manageable in their own io-pgtable implementation. A short list of the known differences: - the physical addresses are shifted left by 4 bits and and have 2 more bits inside the PTE entries - the read/write protection flags are at a different position - the subpage protection feature is now mandatory. For Linux we can just configure it to always allow access to the entire page. - BIT(1) tags "uncached" mappings (used for the display controller) There is second type of DART (t8110) present on M1 Pro/Max SoCs which uses the same PTE format as t6000. Changes in v3: - move APPLE_DART to its own io-pgtable implementation, copied from io-pgtable-arm and simplified Changes in v2: - added Rob's Acked-by: - add APPLE_DART2 io-pgtable format Janne Grunau (1): iommu/io-pgtable: Move Apple DART support to its own file Sven Peter (4): dt-bindings: iommu: dart: add t6000 compatible iommu/io-pgtable: Add DART subpage protection support iommu/io-pgtable-dart: Add DART PTE support for t6000 iommu: dart: Support t6000 variant .../devicetree/bindings/iommu/apple,dart.yaml | 4 +- MAINTAINERS | 1 + drivers/iommu/Kconfig | 1 - drivers/iommu/Makefile| 2 +- drivers/iommu/apple-dart.c| 24 +- drivers/iommu/io-pgtable-arm.c| 63 -- drivers/iommu/io-pgtable-dart.c | 623 ++ drivers/iommu/io-pgtable.c| 3 + include/linux/io-pgtable.h| 1 + 9 files changed, 653 insertions(+), 69 deletions(-) create mode 100644 drivers/iommu/io-pgtable-dart.c -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
