On 2023-04-21 13:25, Yoshihiro Shimoda wrote:
Add PCIe devices of R-Car Gen3/4 into devices_allowlist. For a PCI
device, ipmmu_attach_device() has to avoid enabling uTLB because
the uTLB has already been enabled by the parent device. Otherwise,
enable a wrong uTLB ID. Also ipmmu_device_is_allowed() has to
check whether the parent device is the PCIe host controller or not,
to use the IOMMU's dma_ops from a PCI device.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
---
drivers/iommu/ipmmu-vmsa.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 9f64c5c9f5b9..c635c9b192f4 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -19,6 +19,7 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_platform.h>
+#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/sizes.h>
#include <linux/slab.h>
@@ -624,6 +625,10 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
if (ret < 0)
return ret;
+ /* Avoid to enable utlb if this is a PCI device */
+ if (dev_is_pci(dev))
+ return 0;
Addressing that "TODO: Reference-count the microTLB..." comment instead
would be even nicer :)
+
for (i = 0; i < fwspec->num_ids; ++i)
ipmmu_utlb_enable(domain, fwspec->ids[i]);
@@ -702,10 +707,14 @@ static const struct soc_device_attribute soc_denylist[] = {
};
static const char * const devices_allowlist[] = {
+ "e65d0000.pcie", /* R-Car Gen4 */
+ "e65d8000.pcie", /* R-Car Gen4 */
"ee100000.mmc",
"ee120000.mmc",
"ee140000.mmc",
- "ee160000.mmc"
+ "ee160000.mmc",
+ "ee800000.pcie", /* R-Car Gen3 */
+ "fe000000.pcie", /* R-Car Gen3 */
This would seem to imply that you have not only an "iommu-map" property
representing the PCI devices, but also an "iommus" property representing
that the SoC side of the root complex has its own DMA path to an IOMMU,
distinct from the traffic coming over the PCI bridge. That can
occasionally be true (e.g. if the root complex IP includes a standalone
DMA engine), but more often it's just a mistake.
};
static bool ipmmu_device_is_allowed(struct device *dev)
@@ -723,12 +732,22 @@ static bool ipmmu_device_is_allowed(struct device *dev)
if (soc_device_match(soc_denylist))
return false;
+retry:
/* Check whether this device can work with the IPMMU */
for (i = 0; i < ARRAY_SIZE(devices_allowlist); i++) {
if (!strcmp(dev_name(dev), devices_allowlist[i]))
return true;
}
+ /*
+ * Check whether this device has the parent device like a PCI device
+ * except "soc".
+ */
+ if (dev->parent && strcmp(dev_name(dev->parent), "soc")) {
+ dev = dev->parent;
+ goto retry;
+ }
This is the place where a simple dev_is_pci() check would seem ideal.
Thanks,
Robin.
+
/* Otherwise, do not allow use of IPMMU */
return false;
}
