Re: [RFC v1 PATCH 1/3] drivers: soc: add support for soc_device_match returning -EPROBE_DEFER
First comment overall for the whole serie:
Since it is the solution I had suggested when I reported the problem[1]
I have no qualm on the approach, comments for individual patches
follow.
[1] http://lore.kernel.org/r/YGGZJjAxA1IO+/v...@atmark-techno.com
Alice Guo (OSS) wrote on Mon, Apr 19, 2021 at 12:27:20PM +0800:
> From: Alice Guo
>
> In i.MX8M boards, the registration of SoC device is later than caam
> driver which needs it. Caam driver needs soc_device_match to provide
> -EPROBE_DEFER when no SoC device is registered and no
> early_soc_dev_attr.
This patch should be last in the set: you can't have soc_device_match
return an error before its callers handle it.
> Signed-off-by: Alice Guo
As the one who reported the problem I would have been appreciated being
at least added to Ccs... I only happened to notice you posted this by
chance.
There is also not a single Fixes tag -- I believe this commit should
have Fixes: 7d981405d0fd ("soc: imx8m: change to use platform driver")
but I'm not sure how such tags should be handled in case of multiple
patches fixing something.
--
Dominique
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC v1 PATCH 1/3] drivers: soc: add support for soc_device_match returning -EPROBE_DEFER
From: Alice Guo
In i.MX8M boards, the registration of SoC device is later than caam
driver which needs it. Caam driver needs soc_device_match to provide
-EPROBE_DEFER when no SoC device is registered and no
early_soc_dev_attr.
Signed-off-by: Alice Guo
---
drivers/base/soc.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index 0af5363a582c..12a22f9cf5c8 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -110,6 +110,7 @@ static void soc_release(struct device *dev)
}
static struct soc_device_attribute *early_soc_dev_attr;
+static bool soc_dev_attr_init_done = false;
struct soc_device *soc_device_register(struct soc_device_attribute
*soc_dev_attr)
{
@@ -157,6 +158,7 @@ struct soc_device *soc_device_register(struct
soc_device_attribute *soc_dev_attr
return ERR_PTR(ret);
}
+ soc_dev_attr_init_done = true;
return soc_dev;
out3:
@@ -246,6 +248,9 @@ const struct soc_device_attribute *soc_device_match(
if (!matches)
return NULL;
+ if (!soc_dev_attr_init_done && !early_soc_dev_attr)
+ return ERR_PTR(-EPROBE_DEFER);
+
while (!ret) {
if (!(matches->machine || matches->family ||
matches->revision || matches->soc_id))
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: Use passthrough mode for the Intel IPUs
Intel IPU(Image Processing Unit) has its own (IO)MMU hardware,
The IPU driver allocates its own page table that is not mapped
via the DMA, and thus the Intel IOMMU driver blocks access giving
this error:
DMAR: DRHD: handling fault status reg 3
DMAR: [DMA Read] Request device [00:05.0] PASID
fault addr 76406000 [fault reason 06] PTE Read access is not set
As IPU is not an external facing device which is not risky, so use
IOMMU passthrough mode for Intel IPUs.
Signed-off-by: Bingbu Cao
---
drivers/iommu/intel/iommu.c | 35 +++
drivers/staging/media/ipu3/ipu3.c | 2 +-
include/linux/pci_ids.h | 5 +
3 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..59222d2fe73f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -55,6 +55,12 @@
#define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
#define IS_USB_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_SERIAL_USB)
#define IS_ISA_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA)
+#define IS_IPU_DEVICE(pdev) ((pdev)->vendor == PCI_VENDOR_ID_INTEL &&
\
+((pdev)->device == PCI_DEVICE_ID_INTEL_IPU3 ||
\
+(pdev)->device == PCI_DEVICE_ID_INTEL_IPU6 ||
\
+(pdev)->device == PCI_DEVICE_ID_INTEL_IPU6SE ||
\
+(pdev)->device == PCI_DEVICE_ID_INTEL_IPU6SE_P ||
\
+(pdev)->device == PCI_DEVICE_ID_INTEL_IPU6EP))
#define IS_AZALIA(pdev) ((pdev)->vendor == 0x8086 && (pdev)->device == 0x3a3e)
#define IOAPIC_RANGE_START (0xfee0)
@@ -360,6 +366,7 @@ int intel_iommu_enabled = 0;
EXPORT_SYMBOL_GPL(intel_iommu_enabled);
static int dmar_map_gfx = 1;
+static int dmar_map_ipu = 1;
static int dmar_forcedac;
static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
@@ -368,6 +375,7 @@ static int iommu_skip_te_disable;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
+#define IDENTMAP_IPU 8
int intel_iommu_gfx_mapped;
EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
@@ -2839,6 +2847,9 @@ static int device_def_domain_type(struct device *dev)
if ((iommu_identity_mapping & IDENTMAP_GFX) &&
IS_GFX_DEVICE(pdev))
return IOMMU_DOMAIN_IDENTITY;
+
+ if ((iommu_identity_mapping & IDENTMAP_IPU) &&
IS_IPU_DEVICE(pdev))
+ return IOMMU_DOMAIN_IDENTITY;
}
return 0;
@@ -3278,6 +3289,9 @@ static int __init init_dmars(void)
if (!dmar_map_gfx)
iommu_identity_mapping |= IDENTMAP_GFX;
+ if (!dmar_map_ipu)
+ iommu_identity_mapping |= IDENTMAP_IPU;
+
check_tylersburg_isoch();
ret = si_domain_init(hw_pass_through);
@@ -5622,6 +5636,15 @@ static void quirk_iommu_igfx(struct pci_dev *dev)
dmar_map_gfx = 0;
}
+static void quirk_iommu_ipu(struct pci_dev *dev)
+{
+ if (risky_device(dev))
+ return;
+
+ pci_info(dev, "Passthrough IOMMU for integrated Intel IPU\n");
+ dmar_map_ipu = 0;
+}
+
/* G4x/GM45 integrated gfx dmar support is totally busted. */
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_igfx);
@@ -5657,6 +5680,18 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1632,
quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx);
+/* disable IPU dmar support */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU3,
+quirk_iommu_ipu);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU6EP,
+quirk_iommu_ipu);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU6SE_P,
+quirk_iommu_ipu);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU6,
+quirk_iommu_ipu);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU6SE,
+quirk_iommu_ipu);
+
static void quirk_iommu_rwbf(struct pci_dev *dev)
{
if (risky_device(dev))
diff --git a/drivers/staging/media/ipu3/ipu3.c
b/drivers/staging/media/ipu3/ipu3.c
index ee1bba6bdcac..aee1130ac042 100644
--- a/drivers/staging/media/ipu3/ipu3.c
+++ b/drivers/staging/media/ipu3/ipu3.c
@@ -16,7 +16,7 @@
#include "ipu3-dmamap.h"
#include "ipu3-mmu.h"
-#define IMGU_PCI_ID0x1919
+#define IMGU_PCI_IDPCI_DEVICE_ID_INTEL_IPU3
#define IMGU_PCI_BAR 0
#define IMGU_DMA_MASK DMA_BIT_MASK(39)
#define IMGU_MAX_QUEUE_DEPTH (2 + 2)
diff --git a/include/linux/p
[RFC v1 PATCH 2/3] caam: add defer probe when the caam driver cannot identify SoC
From: Alice Guo
When imx8_soc_info_driver uses module_platform_driver() to regitser
itself, the caam driver cannot identify the SoC in the machine because
the SoC driver is probed later, so that add return -EPROBE_DEFER.
Signed-off-by: Alice Guo
---
drivers/crypto/caam/ctrl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..d08f8cc4131f 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -635,6 +635,9 @@ static int caam_probe(struct platform_device *pdev)
nprop = pdev->dev.of_node;
imx_soc_match = soc_device_match(caam_imx_soc_table);
+ if (IS_ERR(imx_soc_match))
+ return PTR_ERR(imx_soc_match);
+
caam_imx = (bool)imx_soc_match;
if (imx_soc_match) {
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v1 PATCH 1/3] drivers: soc: add support for soc_device_match returning -EPROBE_DEFER
> -Original Message-
> From: Dominique MARTINET
> Sent: 2021年4月19日 12:49
> To: Alice Guo (OSS)
> Cc: gre...@linuxfoundation.org; raf...@kernel.org; Horia Geanta
> ; Aymen Sghaier ;
> herb...@gondor.apana.org.au; da...@davemloft.net; t...@atomide.com;
> geert+rene...@glider.be; mturque...@baylibre.com; sb...@kernel.org;
> vk...@kernel.org; peter.ujfal...@gmail.com; a.ha...@samsung.com;
> narmstr...@baylibre.com; robert.f...@linaro.org; airl...@linux.ie;
> dan...@ffwll.ch; khil...@baylibre.com; to...@kernel.org; jyri.sa...@iki.fi;
> j...@8bytes.org; w...@kernel.org; mche...@kernel.org;
> ulf.hans...@linaro.org; adrian.hun...@intel.com; kis...@ti.com;
> k...@kernel.org; linus.wall...@linaro.org; Roy Pledge ;
> Leo Li ; ssant...@kernel.org; matthias@gmail.com;
> edubez...@gmail.com; j-keer...@ti.com; ba...@kernel.org;
> li...@prisktech.co.nz; st...@rowland.harvard.edu; w...@linux-watchdog.org;
> li...@roeck-us.net; linux-ker...@vger.kernel.org;
> linux-cry...@vger.kernel.org;
> linux-o...@vger.kernel.org; linux-renesas-...@vger.kernel.org;
> linux-...@vger.kernel.org; dmaeng...@vger.kernel.org;
> dri-de...@lists.freedesktop.org; linux-amlo...@lists.infradead.org;
> linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org;
> linux-me...@vger.kernel.org; linux-...@vger.kernel.org;
> net...@vger.kernel.org; linux-...@lists.infradead.org;
> linux-g...@vger.kernel.org; linuxppc-...@lists.ozlabs.org;
> linux-stag...@lists.linux.dev; linux-media...@lists.infradead.org;
> linux...@vger.kernel.org; linux-...@vger.kernel.org;
> linux-watch...@vger.kernel.org
> Subject: Re: [RFC v1 PATCH 1/3] drivers: soc: add support for soc_device_match
> returning -EPROBE_DEFER
>
> First comment overall for the whole serie:
> Since it is the solution I had suggested when I reported the problem[1] I
> have no
> qualm on the approach, comments for individual patches follow.
>
> [1] http://lore.kernel.org/r/YGGZJjAxA1IO+/v...@atmark-techno.com
>
>
> Alice Guo (OSS) wrote on Mon, Apr 19, 2021 at 12:27:20PM +0800:
> > From: Alice Guo
> >
> > In i.MX8M boards, the registration of SoC device is later than caam
> > driver which needs it. Caam driver needs soc_device_match to provide
> > -EPROBE_DEFER when no SoC device is registered and no
> > early_soc_dev_attr.
>
> This patch should be last in the set: you can't have soc_device_match return
> an
> error before its callers handle it.
>
> > Signed-off-by: Alice Guo
>
> As the one who reported the problem I would have been appreciated being at
> least added to Ccs... I only happened to notice you posted this by chance.
Sorry. I will Cc you next time.
> There is also not a single Fixes tag -- I believe this commit should have
> Fixes:
> 7d981405d0fd ("soc: imx8m: change to use platform driver") but I'm not sure
> how such tags should be handled in case of multiple patches fixing something.
I only mentioned "soc: imx8m: change to use platform driver" in cover letter.
If it is acceptable to make such a modification, I will send non-RFC and add
Fixes tag.
Best Regards,
Alice
> --
> Dominique
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC v1 PATCH 0/3] support soc_device_match to return -EPROBE_DEFER
From: Alice Guo In patch "soc: imx8m: change to use platform driver", change soc-imx8m.c to use module platform driver and use NVMEM APIs to ocotp register, the reason is that directly reading ocotp egister causes kexec kernel hang because kernel will disable unused clks after kernel boots up. This patch makes the SoC driver ready. This patch makes the SoC driver ready later than before, and causes device depends on soc_device_match() for initialization are affected, resulting in kernel boot error. CAAM driver is one of these affected drivers. It uses soc_device_match() to find the first matching entry of caam_imx_soc_table, if none of them match, the next instruction will be executed without any processing because CAAM driver is used not only on i.MX and LS, but also PPC and Vybrid. We hope that soc_device_match() could support to return -EPROBE_DEFER(or some other error code, e.g. -ENODEV, but not NULL) in case of “no SoC device registered” to SoC bus. We tried it and updated all the code that is using soc_device_match() throughout the tree. Alice Guo (3): drivers: soc: add support for soc_device_match returning -EPROBE_DEFER caam: add defer probe when the caam driver cannot identify SoC driver: update all the code that use soc_device_match drivers/base/soc.c| 5 + drivers/bus/ti-sysc.c | 2 +- drivers/clk/renesas/r8a7795-cpg-mssr.c| 4 +++- drivers/clk/renesas/rcar-gen2-cpg.c | 2 +- drivers/clk/renesas/rcar-gen3-cpg.c | 2 +- drivers/crypto/caam/ctrl.c| 3 +++ drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c | 7 ++- drivers/dma/ti/k3-psil.c | 3 +++ drivers/dma/ti/k3-udma.c | 2 +- drivers/gpu/drm/bridge/nwl-dsi.c | 2 +- drivers/gpu/drm/meson/meson_drv.c | 4 +++- drivers/gpu/drm/omapdrm/dss/dispc.c | 2 +- drivers/gpu/drm/omapdrm/dss/dpi.c | 4 +++- drivers/gpu/drm/omapdrm/dss/dsi.c | 3 +++ drivers/gpu/drm/omapdrm/dss/dss.c | 3 +++ drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 3 +++ drivers/gpu/drm/omapdrm/dss/venc.c| 4 +++- drivers/gpu/drm/omapdrm/omap_drv.c| 3 +++ drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 4 +++- drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +- drivers/gpu/drm/tidss/tidss_dispc.c | 4 +++- drivers/iommu/ipmmu-vmsa.c| 7 +-- drivers/media/platform/rcar-vin/rcar-core.c | 2 +- drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +- drivers/media/platform/vsp1/vsp1_uif.c| 4 +++- drivers/mmc/host/renesas_sdhi_core.c | 2 +- drivers/mmc/host/renesas_sdhi_internal_dmac.c | 2 +- drivers/mmc/host/sdhci-of-esdhc.c | 21 ++- drivers/mmc/host/sdhci-omap.c | 2 +- drivers/mmc/host/sdhci_am654.c| 2 +- drivers/net/ethernet/renesas/ravb_main.c | 4 +++- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 +- drivers/net/ethernet/ti/cpsw.c| 2 +- drivers/net/ethernet/ti/cpsw_new.c| 2 +- drivers/phy/ti/phy-omap-usb2.c| 4 +++- drivers/pinctrl/renesas/core.c| 2 +- drivers/pinctrl/renesas/pfc-r8a7790.c | 5 - drivers/pinctrl/renesas/pfc-r8a7794.c | 5 - drivers/soc/fsl/dpio/dpio-driver.c| 13 drivers/soc/renesas/r8a774c0-sysc.c | 5 - drivers/soc/renesas/r8a7795-sysc.c| 2 +- drivers/soc/renesas/r8a77990-sysc.c | 5 - drivers/soc/ti/k3-ringacc.c | 2 +- drivers/staging/mt7621-pci/pci-mt7621.c | 2 +- drivers/thermal/rcar_gen3_thermal.c | 4 +++- drivers/thermal/ti-soc-thermal/ti-bandgap.c | 10 +++-- drivers/usb/gadget/udc/renesas_usb3.c | 2 +- drivers/usb/host/ehci-platform.c | 4 +++- drivers/usb/host/xhci-rcar.c | 2 +- drivers/watchdog/renesas_wdt.c| 2 +- 50 files changed, 139 insertions(+), 52 deletions(-) -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
> -Original Message- > From: Leon Romanovsky > Sent: 2021年4月19日 13:02 > To: Alice Guo (OSS) > Cc: gre...@linuxfoundation.org; raf...@kernel.org; Horia Geanta > ; Aymen Sghaier ; > herb...@gondor.apana.org.au; da...@davemloft.net; t...@atomide.com; > geert+rene...@glider.be; mturque...@baylibre.com; sb...@kernel.org; > vk...@kernel.org; peter.ujfal...@gmail.com; a.ha...@samsung.com; > narmstr...@baylibre.com; robert.f...@linaro.org; airl...@linux.ie; > dan...@ffwll.ch; khil...@baylibre.com; to...@kernel.org; jyri.sa...@iki.fi; > j...@8bytes.org; w...@kernel.org; mche...@kernel.org; > ulf.hans...@linaro.org; adrian.hun...@intel.com; kis...@ti.com; > k...@kernel.org; linus.wall...@linaro.org; Roy Pledge ; > Leo Li ; ssant...@kernel.org; matthias@gmail.com; > edubez...@gmail.com; j-keer...@ti.com; ba...@kernel.org; > li...@prisktech.co.nz; st...@rowland.harvard.edu; w...@linux-watchdog.org; > li...@roeck-us.net; linux-ker...@vger.kernel.org; > linux-cry...@vger.kernel.org; > linux-o...@vger.kernel.org; linux-renesas-...@vger.kernel.org; > linux-...@vger.kernel.org; dmaeng...@vger.kernel.org; > dri-de...@lists.freedesktop.org; linux-amlo...@lists.infradead.org; > linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org; > linux-me...@vger.kernel.org; linux-...@vger.kernel.org; > net...@vger.kernel.org; linux-...@lists.infradead.org; > linux-g...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; > linux-stag...@lists.linux.dev; linux-media...@lists.infradead.org; > linux...@vger.kernel.org; linux-...@vger.kernel.org; > linux-watch...@vger.kernel.org > Subject: Re: [RFC v1 PATCH 3/3] driver: update all the code that use > soc_device_match > > On Mon, Apr 19, 2021 at 12:27:22PM +0800, Alice Guo (OSS) wrote: > > From: Alice Guo > > > > Update all the code that use soc_device_match because add support for > > soc_device_match returning -EPROBE_DEFER. > > > > Signed-off-by: Alice Guo > > --- > > drivers/bus/ti-sysc.c | 2 +- > > drivers/clk/renesas/r8a7795-cpg-mssr.c| 4 +++- > > drivers/clk/renesas/rcar-gen2-cpg.c | 2 +- > > drivers/clk/renesas/rcar-gen3-cpg.c | 2 +- > > drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c | 7 ++- > > drivers/dma/ti/k3-psil.c | 3 +++ > > drivers/dma/ti/k3-udma.c | 2 +- > > drivers/gpu/drm/bridge/nwl-dsi.c | 2 +- > > drivers/gpu/drm/meson/meson_drv.c | 4 +++- > > drivers/gpu/drm/omapdrm/dss/dispc.c | 2 +- > > drivers/gpu/drm/omapdrm/dss/dpi.c | 4 +++- > > drivers/gpu/drm/omapdrm/dss/dsi.c | 3 +++ > > drivers/gpu/drm/omapdrm/dss/dss.c | 3 +++ > > drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 3 +++ > > drivers/gpu/drm/omapdrm/dss/venc.c| 4 +++- > > drivers/gpu/drm/omapdrm/omap_drv.c| 3 +++ > > drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 4 +++- > > drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +- > > drivers/gpu/drm/tidss/tidss_dispc.c | 4 +++- > > drivers/iommu/ipmmu-vmsa.c| 7 +-- > > drivers/media/platform/rcar-vin/rcar-core.c | 2 +- > > drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +- > > drivers/media/platform/vsp1/vsp1_uif.c| 4 +++- > > drivers/mmc/host/renesas_sdhi_core.c | 2 +- > > drivers/mmc/host/renesas_sdhi_internal_dmac.c | 2 +- > > drivers/mmc/host/sdhci-of-esdhc.c | 21 > ++- > > drivers/mmc/host/sdhci-omap.c | 2 +- > > drivers/mmc/host/sdhci_am654.c| 2 +- > > drivers/net/ethernet/renesas/ravb_main.c | 4 +++- > > drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 +- > > drivers/net/ethernet/ti/cpsw.c| 2 +- > > drivers/net/ethernet/ti/cpsw_new.c| 2 +- > > drivers/phy/ti/phy-omap-usb2.c| 4 +++- > > drivers/pinctrl/renesas/core.c| 2 +- > > drivers/pinctrl/renesas/pfc-r8a7790.c | 5 - > > drivers/pinctrl/renesas/pfc-r8a7794.c | 5 - > > drivers/soc/fsl/dpio/dpio-driver.c| 13 > > drivers/soc/renesas/r8a774c0-sysc.c | 5 - > > drivers/soc/renesas/r8a7795-sysc.c| 2 +- > > drivers/soc/renesas/r8a77990-sysc.c | 5 - > > drivers/soc/ti/k3-ringacc.c | 2 +- > > drivers/staging/mt7621-pci/pci-mt7621.c | 2 +- > > drivers/thermal/rcar_gen3_thermal.c | 4 +++- > > drivers/thermal/ti-soc-thermal/ti-bandgap.c | 10 +++-- > > drivers/usb/gadget/udc/renesas_usb3.c | 2 +- > > drivers/usb/host/ehci-platform.c | 4 +++- > > drivers/usb/host/xhci-rcar.c | 2 +- > > drivers/watchdog/renesas_wdt.c| 2 +- > > 48 files changed, 131 insertions(+), 52 deletions(-) > > > > diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c index > > 5
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
Alice Guo (OSS) wrote on Mon, Apr 19, 2021 at 12:27:22PM +0800:
> From: Alice Guo
>
> Update all the code that use soc_device_match
A single patch might be difficult to accept for all components, a each
maintainer will probably want to have a say on their subsystem?
I would suggest to split these for a non-RFC version; a this will really
need to be case-by-case handling.
> because add support for soc_device_match returning -EPROBE_DEFER.
(English does not parse here for me)
I've only commented a couple of places in the code itself, but this
doesn't seem to add much support for errors, just sweep the problem
under the rug.
> Signed-off-by: Alice Guo
> ---
>
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> index 5fae60f8c135..00c59aa217c1 100644
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -2909,7 +2909,7 @@ static int sysc_init_soc(struct sysc *ddata)
> }
>
> match = soc_device_match(sysc_soc_feat_match);
> - if (!match)
> + if (!match || IS_ERR(match))
> return 0;
This function handles errors, I would recommend returning the error as
is if soc_device_match returned one so the probe can be retried later.
>
> if (match->data)
> diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> index c32d2c678046..90a18336a4c3 100644
> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> @@ -439,6 +439,7 @@ static const unsigned int r8a7795es2_mod_nullify[]
> __initconst = {
>
> static int __init r8a7795_cpg_mssr_init(struct device *dev)
> {
> + const struct soc_device_attribute *match;
> const struct rcar_gen3_cpg_pll_config *cpg_pll_config;
> u32 cpg_mode;
> int error;
> @@ -453,7 +454,8 @@ static int __init r8a7795_cpg_mssr_init(struct device
> *dev)
> return -EINVAL;
> }
>
> - if (soc_device_match(r8a7795es1)) {
> + match = soc_device_match(r8a7795es1);
> + if (!IS_ERR(match) && match) {
Same, return the error.
Assuming an error means no match will just lead to hard to debug
problems because the driver potentially assumed the wrong device when
it's just not ready yet.
> cpg_core_nullify_range(r8a7795_core_clks,
> ARRAY_SIZE(r8a7795_core_clks),
> R8A7795_CLK_S0D2, R8A7795_CLK_S0D12);
> [...]
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index eaaec0a55cc6..13a06b613379 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -757,17 +757,20 @@ static const char * const devices_allowlist[] = {
>
> static bool ipmmu_device_is_allowed(struct device *dev)
> {
> + const struct soc_device_attribute *match1, *match2;
> unsigned int i;
>
> /*
>* R-Car Gen3 and RZ/G2 use the allow list to opt-in devices.
>* For Other SoCs, this returns true anyway.
>*/
> - if (!soc_device_match(soc_needs_opt_in))
> + match1 = soc_device_match(soc_needs_opt_in);
> + if (!IS_ERR(match1) && !match1)
I'm not sure what you intended to do, but !match1 already means there is
no error so the original code is identical.
In this case ipmmu_device_is_allowed does not allow errors so this is
one of the "difficult" drivers that require slightly more thinking.
It is only called in ipmmu_of_xlate which does return errors properly,
so in this case the most straightforward approach would be to make
ipmmu_device_is_allowed return an int and forward errors as well.
...
This is going to need quite some more work to be acceptable, in my
opinion, but I think it should be possible.
Thanks,
--
Dominique
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
From: Alice Guo
Update all the code that use soc_device_match because add support for
soc_device_match returning -EPROBE_DEFER.
Signed-off-by: Alice Guo
---
drivers/bus/ti-sysc.c | 2 +-
drivers/clk/renesas/r8a7795-cpg-mssr.c| 4 +++-
drivers/clk/renesas/rcar-gen2-cpg.c | 2 +-
drivers/clk/renesas/rcar-gen3-cpg.c | 2 +-
drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c | 7 ++-
drivers/dma/ti/k3-psil.c | 3 +++
drivers/dma/ti/k3-udma.c | 2 +-
drivers/gpu/drm/bridge/nwl-dsi.c | 2 +-
drivers/gpu/drm/meson/meson_drv.c | 4 +++-
drivers/gpu/drm/omapdrm/dss/dispc.c | 2 +-
drivers/gpu/drm/omapdrm/dss/dpi.c | 4 +++-
drivers/gpu/drm/omapdrm/dss/dsi.c | 3 +++
drivers/gpu/drm/omapdrm/dss/dss.c | 3 +++
drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 3 +++
drivers/gpu/drm/omapdrm/dss/venc.c| 4 +++-
drivers/gpu/drm/omapdrm/omap_drv.c| 3 +++
drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 4 +++-
drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
drivers/gpu/drm/tidss/tidss_dispc.c | 4 +++-
drivers/iommu/ipmmu-vmsa.c| 7 +--
drivers/media/platform/rcar-vin/rcar-core.c | 2 +-
drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
drivers/media/platform/vsp1/vsp1_uif.c| 4 +++-
drivers/mmc/host/renesas_sdhi_core.c | 2 +-
drivers/mmc/host/renesas_sdhi_internal_dmac.c | 2 +-
drivers/mmc/host/sdhci-of-esdhc.c | 21 ++-
drivers/mmc/host/sdhci-omap.c | 2 +-
drivers/mmc/host/sdhci_am654.c| 2 +-
drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 +-
drivers/net/ethernet/ti/cpsw.c| 2 +-
drivers/net/ethernet/ti/cpsw_new.c| 2 +-
drivers/phy/ti/phy-omap-usb2.c| 4 +++-
drivers/pinctrl/renesas/core.c| 2 +-
drivers/pinctrl/renesas/pfc-r8a7790.c | 5 -
drivers/pinctrl/renesas/pfc-r8a7794.c | 5 -
drivers/soc/fsl/dpio/dpio-driver.c| 13
drivers/soc/renesas/r8a774c0-sysc.c | 5 -
drivers/soc/renesas/r8a7795-sysc.c| 2 +-
drivers/soc/renesas/r8a77990-sysc.c | 5 -
drivers/soc/ti/k3-ringacc.c | 2 +-
drivers/staging/mt7621-pci/pci-mt7621.c | 2 +-
drivers/thermal/rcar_gen3_thermal.c | 4 +++-
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 10 +++--
drivers/usb/gadget/udc/renesas_usb3.c | 2 +-
drivers/usb/host/ehci-platform.c | 4 +++-
drivers/usb/host/xhci-rcar.c | 2 +-
drivers/watchdog/renesas_wdt.c| 2 +-
48 files changed, 131 insertions(+), 52 deletions(-)
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index 5fae60f8c135..00c59aa217c1 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -2909,7 +2909,7 @@ static int sysc_init_soc(struct sysc *ddata)
}
match = soc_device_match(sysc_soc_feat_match);
- if (!match)
+ if (!match || IS_ERR(match))
return 0;
if (match->data)
diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c
b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index c32d2c678046..90a18336a4c3 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -439,6 +439,7 @@ static const unsigned int r8a7795es2_mod_nullify[]
__initconst = {
static int __init r8a7795_cpg_mssr_init(struct device *dev)
{
+ const struct soc_device_attribute *match;
const struct rcar_gen3_cpg_pll_config *cpg_pll_config;
u32 cpg_mode;
int error;
@@ -453,7 +454,8 @@ static int __init r8a7795_cpg_mssr_init(struct device *dev)
return -EINVAL;
}
- if (soc_device_match(r8a7795es1)) {
+ match = soc_device_match(r8a7795es1);
+ if (!IS_ERR(match) && match) {
cpg_core_nullify_range(r8a7795_core_clks,
ARRAY_SIZE(r8a7795_core_clks),
R8A7795_CLK_S0D2, R8A7795_CLK_S0D12);
diff --git a/drivers/clk/renesas/rcar-gen2-cpg.c
b/drivers/clk/renesas/rcar-gen2-cpg.c
index edae874fa2b6..946f82634d23 100644
--- a/drivers/clk/renesas/rcar-gen2-cpg.c
+++ b/drivers/clk/renesas/rcar-gen2-cpg.c
@@ -383,7 +383,7 @@ int __init rcar_gen2_cpg_init(const struct
rcar_gen2_cpg_pll_config *config,
cpg_pll0_div = pll0_div;
cpg_mode = mode;
attr = soc_device_match(cpg_quirks_match);
- if (attr)
+ if (!IS_ERR(attr) && attr)
cpg_quirks = (uintptr_t)attr->data;
pr_debug("%s: mode = 0x%x quirks = 0x%x\n", __func__, mode, cpg_quirks);
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c
b/drivers/clk/
RE: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
> -Original Message-
> From: Dominique MARTINET
> Sent: 2021年4月19日 13:03
> To: Alice Guo (OSS)
> Subject: Re: [RFC v1 PATCH 3/3] driver: update all the code that use
> soc_device_match
>
> Alice Guo (OSS) wrote on Mon, Apr 19, 2021 at 12:27:22PM +0800:
> > From: Alice Guo
> >
> > Update all the code that use soc_device_match
>
> A single patch might be difficult to accept for all components, a each
> maintainer
> will probably want to have a say on their subsystem?
>
> I would suggest to split these for a non-RFC version; a this will really need
> to be
> case-by-case handling.
>
> > because add support for soc_device_match returning -EPROBE_DEFER.
>
> (English does not parse here for me)
>
> I've only commented a couple of places in the code itself, but this doesn't
> seem
> to add much support for errors, just sweep the problem under the rug.
>
> > Signed-off-by: Alice Guo
> > ---
> >
> > diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c index
> > 5fae60f8c135..00c59aa217c1 100644
> > --- a/drivers/bus/ti-sysc.c
> > +++ b/drivers/bus/ti-sysc.c
> > @@ -2909,7 +2909,7 @@ static int sysc_init_soc(struct sysc *ddata)
> > }
> >
> > match = soc_device_match(sysc_soc_feat_match);
> > - if (!match)
> > + if (!match || IS_ERR(match))
> > return 0;
>
> This function handles errors, I would recommend returning the error as is if
> soc_device_match returned one so the probe can be retried later.
>
> >
> > if (match->data)
> > diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > index c32d2c678046..90a18336a4c3 100644
> > --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > @@ -439,6 +439,7 @@ static const unsigned int r8a7795es2_mod_nullify[]
> > __initconst = {
> >
> > static int __init r8a7795_cpg_mssr_init(struct device *dev) {
> > + const struct soc_device_attribute *match;
> > const struct rcar_gen3_cpg_pll_config *cpg_pll_config;
> > u32 cpg_mode;
> > int error;
> > @@ -453,7 +454,8 @@ static int __init r8a7795_cpg_mssr_init(struct device
> *dev)
> > return -EINVAL;
> > }
> >
> > - if (soc_device_match(r8a7795es1)) {
> > + match = soc_device_match(r8a7795es1);
> > + if (!IS_ERR(match) && match) {
>
> Same, return the error.
> Assuming an error means no match will just lead to hard to debug problems
> because the driver potentially assumed the wrong device when it's just not
> ready yet.
>
> > cpg_core_nullify_range(r8a7795_core_clks,
> >ARRAY_SIZE(r8a7795_core_clks),
> >R8A7795_CLK_S0D2, R8A7795_CLK_S0D12);
> > [...]
> diff --git
> > a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c index
> > eaaec0a55cc6..13a06b613379 100644
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -757,17 +757,20 @@ static const char * const devices_allowlist[] =
> > {
> >
> > static bool ipmmu_device_is_allowed(struct device *dev) {
> > + const struct soc_device_attribute *match1, *match2;
> > unsigned int i;
> >
> > /*
> > * R-Car Gen3 and RZ/G2 use the allow list to opt-in devices.
> > * For Other SoCs, this returns true anyway.
> > */
> > - if (!soc_device_match(soc_needs_opt_in))
> > + match1 = soc_device_match(soc_needs_opt_in);
> > + if (!IS_ERR(match1) && !match1)
>
> I'm not sure what you intended to do, but !match1 already means there is no
> error so the original code is identical.
>
> In this case ipmmu_device_is_allowed does not allow errors so this is one of
> the
> "difficult" drivers that require slightly more thinking.
> It is only called in ipmmu_of_xlate which does return errors properly, so in
> this
> case the most straightforward approach would be to make
> ipmmu_device_is_allowed return an int and forward errors as well.
>
I will reconsider the existing problems. Thank you for your advice
>
> ...
> This is going to need quite some more work to be acceptable, in my opinion,
> but
> I think it should be possible.
>
> Thanks,
> --
> Dominique
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock
From: Xiang Chen
It is not necessary to put free_iova_mem() inside of spinlock/unlock
iova_rbtree_lock which only leads to more completion for the spinlock.
It has a small promote on the performance after the change. And also
rename private_free_iova() as remove_iova() because the function will not
free iova after that change.
Signed-off-by: Xiang Chen
---
drivers/iommu/iova.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index b7ecd5b..b6cf5f1 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -412,12 +412,11 @@ private_find_iova(struct iova_domain *iovad, unsigned
long pfn)
return NULL;
}
-static void private_free_iova(struct iova_domain *iovad, struct iova *iova)
+static void remove_iova(struct iova_domain *iovad, struct iova *iova)
{
assert_spin_locked(&iovad->iova_rbtree_lock);
__cached_rbnode_delete_update(iovad, iova);
rb_erase(&iova->node, &iovad->rbroot);
- free_iova_mem(iova);
}
/**
@@ -452,8 +451,9 @@ __free_iova(struct iova_domain *iovad, struct iova *iova)
unsigned long flags;
spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);
- private_free_iova(iovad, iova);
+ remove_iova(iovad, iova);
spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
+ free_iova_mem(iova);
}
EXPORT_SYMBOL_GPL(__free_iova);
@@ -472,10 +472,13 @@ free_iova(struct iova_domain *iovad, unsigned long pfn)
spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);
iova = private_find_iova(iovad, pfn);
- if (iova)
- private_free_iova(iovad, iova);
+ if (!iova) {
+ spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
+ return;
+ }
+ remove_iova(iovad, iova);
spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
-
+ free_iova_mem(iova);
}
EXPORT_SYMBOL_GPL(free_iova);
@@ -825,7 +828,8 @@ iova_magazine_free_pfns(struct iova_magazine *mag, struct
iova_domain *iovad)
if (WARN_ON(!iova))
continue;
- private_free_iova(iovad, iova);
+ remove_iova(iovad, iova);
+ free_iova_mem(iova);
}
spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
--
2.8.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock
On 19/04/2021 08:13, chenxiang wrote: From: Xiang Chen It is not necessary to put free_iova_mem() inside of spinlock/unlock iova_rbtree_lock which only leads to more completion for the spinlock. It has a small promote on the performance after the change. And also rename private_free_iova() as remove_iova() because the function will not free iova after that change. Signed-off-by: Xiang Chen I think that you will need to resend after v5.13-rc1 comes out, however: Reviewed-by: John Garry --- drivers/iommu/ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v1 PATCH 1/3] drivers: soc: add support for soc_device_match returning -EPROBE_DEFER
Hi Alice,
CC Arnd (soc_device_match() author)
On Mon, Apr 19, 2021 at 6:28 AM Alice Guo (OSS) wrote:
> From: Alice Guo
>
> In i.MX8M boards, the registration of SoC device is later than caam
> driver which needs it. Caam driver needs soc_device_match to provide
> -EPROBE_DEFER when no SoC device is registered and no
> early_soc_dev_attr.
I'm wondering if this is really a good idea: soc_device_match() is a
last-resort low-level check, and IMHO should be made available early on,
so there is no need for -EPROBE_DEFER.
>
> Signed-off-by: Alice Guo
Thanks for your patch!
> --- a/drivers/base/soc.c
> +++ b/drivers/base/soc.c
> @@ -110,6 +110,7 @@ static void soc_release(struct device *dev)
> }
>
> static struct soc_device_attribute *early_soc_dev_attr;
> +static bool soc_dev_attr_init_done = false;
Do you need this variable?
>
> struct soc_device *soc_device_register(struct soc_device_attribute
> *soc_dev_attr)
> {
> @@ -157,6 +158,7 @@ struct soc_device *soc_device_register(struct
> soc_device_attribute *soc_dev_attr
> return ERR_PTR(ret);
> }
>
> + soc_dev_attr_init_done = true;
> return soc_dev;
>
> out3:
> @@ -246,6 +248,9 @@ const struct soc_device_attribute *soc_device_match(
> if (!matches)
> return NULL;
>
> + if (!soc_dev_attr_init_done && !early_soc_dev_attr)
if (!soc_bus_type.p && !early_soc_dev_attr)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> while (!ret) {
> if (!(matches->machine || matches->family ||
> matches->revision || matches->soc_id))
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
Hi Dominique,
CC Arnd (soc_device_match() author)
On Mon, Apr 19, 2021 at 7:03 AM Dominique MARTINET
wrote:
> Alice Guo (OSS) wrote on Mon, Apr 19, 2021 at 12:27:22PM +0800:
> > From: Alice Guo
> > Update all the code that use soc_device_match
>
> A single patch might be difficult to accept for all components, a each
> maintainer will probably want to have a say on their subsystem?
>
> I would suggest to split these for a non-RFC version; a this will really
> need to be case-by-case handling.
>
> > because add support for soc_device_match returning -EPROBE_DEFER.
>
> (English does not parse here for me)
>
> I've only commented a couple of places in the code itself, but this
> doesn't seem to add much support for errors, just sweep the problem
> under the rug.
>
> > Signed-off-by: Alice Guo
> > ---
> >
> > diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> > index 5fae60f8c135..00c59aa217c1 100644
> > --- a/drivers/bus/ti-sysc.c
> > +++ b/drivers/bus/ti-sysc.c
> > @@ -2909,7 +2909,7 @@ static int sysc_init_soc(struct sysc *ddata)
> > }
> >
> > match = soc_device_match(sysc_soc_feat_match);
> > - if (!match)
> > + if (!match || IS_ERR(match))
> > return 0;
>
> This function handles errors, I would recommend returning the error as
> is if soc_device_match returned one so the probe can be retried later.
Depends...
> > --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > @@ -439,6 +439,7 @@ static const unsigned int r8a7795es2_mod_nullify[]
> > __initconst = {
> >
> > static int __init r8a7795_cpg_mssr_init(struct device *dev)
> > {
> > + const struct soc_device_attribute *match;
> > const struct rcar_gen3_cpg_pll_config *cpg_pll_config;
> > u32 cpg_mode;
> > int error;
> > @@ -453,7 +454,8 @@ static int __init r8a7795_cpg_mssr_init(struct device
> > *dev)
> > return -EINVAL;
> > }
> >
> > - if (soc_device_match(r8a7795es1)) {
> > + match = soc_device_match(r8a7795es1);
> > + if (!IS_ERR(match) && match) {
>
> Same, return the error.
> Assuming an error means no match will just lead to hard to debug
> problems because the driver potentially assumed the wrong device when
> it's just not ready yet.
When running on R-Car H3, there will always be a match device, as
the SoC device is registered early.
>
> > cpg_core_nullify_range(r8a7795_core_clks,
> > ARRAY_SIZE(r8a7795_core_clks),
> > R8A7795_CLK_S0D2, R8A7795_CLK_S0D12);
> > [...]
> > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > index eaaec0a55cc6..13a06b613379 100644
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -757,17 +757,20 @@ static const char * const devices_allowlist[] = {
> >
> > static bool ipmmu_device_is_allowed(struct device *dev)
> > {
> > + const struct soc_device_attribute *match1, *match2;
> > unsigned int i;
> >
> > /*
> >* R-Car Gen3 and RZ/G2 use the allow list to opt-in devices.
> >* For Other SoCs, this returns true anyway.
> >*/
> > - if (!soc_device_match(soc_needs_opt_in))
> > + match1 = soc_device_match(soc_needs_opt_in);
> > + if (!IS_ERR(match1) && !match1)
>
> I'm not sure what you intended to do, but !match1 already means there is
> no error so the original code is identical.
>
> In this case ipmmu_device_is_allowed does not allow errors so this is
> one of the "difficult" drivers that require slightly more thinking.
> It is only called in ipmmu_of_xlate which does return errors properly,
> so in this case the most straightforward approach would be to make
> ipmmu_device_is_allowed return an int and forward errors as well.
>
> ...
> This is going to need quite some more work to be acceptable, in my
> opinion, but I think it should be possible.
In general, this is very hard to do, IMHO. Some drivers may be used on
multiple platforms, some of them registering an SoC device, some of
them not registering an SoC device. So there is no way to know the
difference between "SoC device not registered, intentionally", and
"SoC device not yet registered".
soc_device_match() should only be used as a last resort, to identify
systems that cannot be identified otherwise. Typically this is used for
quirks, which should only be enabled on a very specific subset of
systems. IMHO such systems should make sure soc_device_match()
is available early, by registering their SoC device early.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu
Re: [PATCH v3 02/12] iommu: Add iommu_split_block interface
Hi Baolu,
On 2021/4/14 15:14, Lu Baolu wrote:
> On 4/13/21 4:54 PM, Keqian Zhu wrote:
>> Block(largepage) mapping is not a proper granule for dirty log tracking.
>> Take an extreme example, if DMA writes one byte, under 1G mapping, the
>> dirty amount reported is 1G, but under 4K mapping, the dirty amount is
>> just 4K.
>>
>> This adds a new interface named iommu_split_block in IOMMU base layer.
>> A specific IOMMU driver can invoke it during start dirty log. If so, the
>> driver also need to realize the split_block iommu ops.
>>
>> We flush all iotlbs after the whole procedure is completed to ease the
>> pressure of IOMMU, as we will hanle a huge range of mapping in general.
>>
>> Signed-off-by: Keqian Zhu
>> Signed-off-by: Kunkun Jiang
>> ---
>> drivers/iommu/iommu.c | 41 +
>> include/linux/iommu.h | 11 +++
>> 2 files changed, 52 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 667b2d6d2fc0..bb413a927870 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -2721,6 +2721,47 @@ int iommu_domain_set_attr(struct iommu_domain *domain,
>> }
>> EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
>> +int iommu_split_block(struct iommu_domain *domain, unsigned long iova,
>> + size_t size)
>> +{
>> +const struct iommu_ops *ops = domain->ops;
>> +unsigned int min_pagesz;
>> +size_t pgsize;
>> +bool flush = false;
>> +int ret = 0;
>> +
>> +if (unlikely(!ops || !ops->split_block))
>> +return -ENODEV;
>> +
>> +min_pagesz = 1 << __ffs(domain->pgsize_bitmap);
>> +if (!IS_ALIGNED(iova | size, min_pagesz)) {
>> +pr_err("unaligned: iova 0x%lx size 0x%zx min_pagesz 0x%x\n",
>> + iova, size, min_pagesz);
>> +return -EINVAL;
>> +}
>> +
>> +while (size) {
>> +flush = true;
>> +
>> +pgsize = iommu_pgsize(domain, iova, size);
>> +
>> +ret = ops->split_block(domain, iova, pgsize);
>> +if (ret)
>> +break;
>> +
>> +pr_debug("split handled: iova 0x%lx size 0x%zx\n", iova, pgsize);
>> +
>> +iova += pgsize;
>> +size -= pgsize;
>> +}
>> +
>> +if (flush)
>> +iommu_flush_iotlb_all(domain);
>> +
>> +return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_split_block);
>
> Do you really have any consumers of this interface other than the dirty
> bit tracking? If not, I don't suggest to make this as a generic IOMMU
> interface.
>
> There is an implicit requirement for such interfaces. The
> iommu_map/unmap(iova, size) shouldn't be called at the same time.
> Currently there's no such sanity check in the iommu core. A poorly
> written driver could mess up the kernel by misusing this interface.
Yes, I don't think up a scenario except dirty tracking.
Indeed, we'd better not make them as a generic interface.
Do you have any suggestion that underlying iommu drivers can share these code
but
not make it as a generic iommu interface?
I have a not so good idea. Make the "split" interfaces as a static function, and
transfer the function pointer to start_dirty_log. But it looks weird and
inflexible.
On the other hand, if a driver calls map/unmap with split/merge at the same
time,
it's a bug of driver, it should follow the rule.
>
> This also applies to iommu_merge_page().
>
Thanks,
Keqian
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
Geert Uytterhoeven wrote on Mon, Apr 19, 2021 at 11:03:24AM +0200:
> > This is going to need quite some more work to be acceptable, in my
> > opinion, but I think it should be possible.
>
> In general, this is very hard to do, IMHO. Some drivers may be used on
> multiple platforms, some of them registering an SoC device, some of
> them not registering an SoC device. So there is no way to know the
> difference between "SoC device not registered, intentionally", and
> "SoC device not yet registered".
Hm, good point, I was probably a bit too optimistic if there are devices
which don't register any soc yet have drivers which want one; I don't
see how to make the difference indeed... And that does mean we can't
just defer all the time.
> soc_device_match() should only be used as a last resort, to identify
> systems that cannot be identified otherwise. Typically this is used for
> quirks, which should only be enabled on a very specific subset of
> systems. IMHO such systems should make sure soc_device_match()
> is available early, by registering their SoC device early.
I definitely agree there, my suggestion to defer was only because I know
of no other way to influence the ordering of drivers loading reliably
and gave up on soc being init'd early.
In this particular case the problem is that since 7d981405d0fd ("soc:
imx8m: change to use platform driver") the soc probe tries to use the
nvmem driver for ocotp fuses for imx8m devices, which isn't ready yet.
So soc loading gets pushed back to the end of the list because it gets
defered and other drivers relying on soc_device_match get confused
because they wrongly think a device doesn't match a quirk when it
actually does.
If there is a way to ensure the nvmem driver gets loaded before the soc,
that would also solve the problem nicely, and avoid the need to mess
with all the ~50 drivers which use it.
Is there a way to control in what order drivers get loaded? Something in
the dtb perhaps?
Thanks,
--
Dominique
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
On Mon, Apr 19, 2021 at 11:33 AM Dominique MARTINET
wrote:
> Geert Uytterhoeven wrote on Mon, Apr 19, 2021 at 11:03:24AM +0200:
>
> > soc_device_match() should only be used as a last resort, to identify
> > systems that cannot be identified otherwise. Typically this is used for
> > quirks, which should only be enabled on a very specific subset of
> > systems. IMHO such systems should make sure soc_device_match()
> > is available early, by registering their SoC device early.
>
> I definitely agree there, my suggestion to defer was only because I know
> of no other way to influence the ordering of drivers loading reliably
> and gave up on soc being init'd early.
In some cases, you can use the device_link infrastructure to deal
with dependencies between devices. Not sure if this would help
in your case, but have a look at device_link_add() etc in drivers/base/core.c
> In this particular case the problem is that since 7d981405d0fd ("soc:
> imx8m: change to use platform driver") the soc probe tries to use the
> nvmem driver for ocotp fuses for imx8m devices, which isn't ready yet.
> So soc loading gets pushed back to the end of the list because it gets
> defered and other drivers relying on soc_device_match get confused
> because they wrongly think a device doesn't match a quirk when it
> actually does.
>
> If there is a way to ensure the nvmem driver gets loaded before the soc,
> that would also solve the problem nicely, and avoid the need to mess
> with all the ~50 drivers which use it.
>
> Is there a way to control in what order drivers get loaded? Something in
> the dtb perhaps?
For built-in drivers, load order depends on the initcall level and
link order (how things are lined listed in the Makefile hierarchy).
For loadable modules, this is up to user space in the end.
Which of the drivers in this scenario are loadable modules?
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 02/12] iommu: Add iommu_split_block interface
Hi Keqian, On 2021/4/19 17:32, Keqian Zhu wrote: +EXPORT_SYMBOL_GPL(iommu_split_block); Do you really have any consumers of this interface other than the dirty bit tracking? If not, I don't suggest to make this as a generic IOMMU interface. There is an implicit requirement for such interfaces. The iommu_map/unmap(iova, size) shouldn't be called at the same time. Currently there's no such sanity check in the iommu core. A poorly written driver could mess up the kernel by misusing this interface. Yes, I don't think up a scenario except dirty tracking. Indeed, we'd better not make them as a generic interface. Do you have any suggestion that underlying iommu drivers can share these code but not make it as a generic iommu interface? I have a not so good idea. Make the "split" interfaces as a static function, and transfer the function pointer to start_dirty_log. But it looks weird and inflexible. I understand splitting/merging super pages is an optimization, but not a functional requirement. So is it possible to let the vendor iommu driver decide whether splitting super pages when starting dirty bit tracking and the opposite operation during when stopping it? The requirement for upper layer is that starting/stopping dirty bit tracking and mapping/unmapping are mutually exclusive. On the other hand, if a driver calls map/unmap with split/merge at the same time, it's a bug of driver, it should follow the rule. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
On 4/18/21 9:27 PM, Alice Guo (OSS) wrote:
> From: Alice Guo
>
> Update all the code that use soc_device_match because add support for
> soc_device_match returning -EPROBE_DEFER.
>
> Signed-off-by: Alice Guo
> ---
[ ... ]
> drivers/watchdog/renesas_wdt.c| 2 +-
> 48 files changed, 131 insertions(+), 52 deletions(-)
>
[ ... ]
> diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
> index 5791198960e6..fdc534dc4024 100644
> --- a/drivers/watchdog/renesas_wdt.c
> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -197,7 +197,7 @@ static bool rwdt_blacklisted(struct device *dev)
> const struct soc_device_attribute *attr;
>
> attr = soc_device_match(rwdt_quirks_match);
> - if (attr && setup_max_cpus > (uintptr_t)attr->data) {
> + if (!IS_ERR(attr) && attr && setup_max_cpus > (uintptr_t)attr->data) {
This is wrong. We can not make the decision below without having access
to attr. The function may wrongly return false if soc_device_match()
returns an error.
Guenter
> dev_info(dev, "Watchdog blacklisted on %s %s\n", attr->soc_id,
>attr->revision);
> return true;
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier
On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote:
> Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
> both implement "arm,mmu-500" in some QTI SoCs and to run through
> adreno smmu specific implementation such as enabling split pagetables
> support, we need to match the "qcom,adreno-smmu" compatible first
> before apss smmu or else we will be running apps smmu implementation
> for adreno smmu and the additional features for adreno smmu is never
> set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
> and adreno smmu implementing "arm,mmu-500", so the adreno smmu
> implementation is never reached because the current sequence checks
> for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
> specific impl and we never reach adreno smmu specific implementation.
>
> Suggested-by: Akhil P Oommen
> Signed-off-by: Sai Prakash Ranjan
Sorry for taking my time thinking about this.
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index bea3ee0dabc2..03f048aebb80 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct
> arm_smmu_device *smmu)
> {
> const struct device_node *np = smmu->dev->of_node;
>
> - if (of_match_node(qcom_smmu_impl_of_match, np))
> - return qcom_smmu_create(smmu, &qcom_smmu_impl);
> -
> + /*
> + * Do not change this order of implementation, i.e., first adreno
> + * smmu impl and then apss smmu since we can have both implementing
> + * arm,mmu-500 in which case we will miss setting adreno smmu specific
> + * features if the order is changed.
> + */
> if (of_device_is_compatible(np, "qcom,adreno-smmu"))
> return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl);
>
> + if (of_match_node(qcom_smmu_impl_of_match, np))
> + return qcom_smmu_create(smmu, &qcom_smmu_impl);
> +
> return smmu;
> }
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/3] iommu: io-pgtable: add DART pagetable format
On Fri, Apr 09, 2021 at 09:38:15PM +0200, Arnd Bergmann wrote: > On Fri, Apr 9, 2021 at 6:56 PM Sven Peter wrote: > > On Wed, Apr 7, 2021, at 12:44, Will Deacon wrote: > > > On Sun, Mar 28, 2021 at 09:40:07AM +0200, Sven Peter wrote: > > > > > > > + cfg->pgsize_bitmap &= SZ_16K; > > > > + if (!cfg->pgsize_bitmap) > > > > + return NULL; > > > > > > This is worrying (and again, I don't think this belongs here). How is this > > > thing supposed to work if the CPU is using 4k pages? > > > > This SoC is just full of fun surprises! > > I didn't even think about that case since I've always been using 16k pages > > so far. > > > > I've checked again and wasn't able to find any way to configure the pagesize > > of the IOMMU. There seem to be variants of this IP in older iPhones which > > support a 4k pagesize but to the best of my knowledge this is hard wired > > and not configurable in software. > > > > When booting with 4k pages I hit the BUG_ON in iova.c that ensures that the > > iommu pagesize has to be <= the cpu page size. > > > > I see two options here and I'm not sure I like either of them: > > > > 1) Just don't support 4k CPU pages together with IOMMU translations and only > >allow full bypass mode there. > >This would however mean that PCIe (i.e. ethernet, usb ports on the Mac > >mini) and possibly Thunderbolt support would not be possible since these > >devices don't seem to like iommu bypass mode at all. > > It should be possible to do a fake bypass mode by just programming a > static page table for as much address space as you can, and then > use swiotlb to address any memory beyond that. This won't perform > well because it requires bounce buffers for any high memory, but it > can be a last resort if a dart instance cannot do normal bypass mode. > > > 2) I've had a brief discussion on IRC with Arnd about this [1] and he > > pointed > >out that the dma_map_sg API doesn't make any guarantees about the > > returned > >iovas and that it might be possible to make this work at least for > > devices > >that go through the normal DMA API. > > > >I've then replaced the page size check with a WARN_ON in iova.c just to > > see > >what happens. At least normal devices that go through the DMA API seem to > >work with my configuration. iommu_dma_alloc took the > > iommu_dma_alloc_remap > >path which was called with the cpu page size but then used > >domain->pgsize_bitmap to increase that to 16k. So this kinda works out, > > but > >there are other functions in dma-iommu.c that I believe rely on the fact > > that > >the iommu can map single cpu pages. This feels very fragile right now and > >would probably require some rather invasive changes. > > The other second-to-last resort here would be to duplicate the code from > the dma-iommu code and implement the dma-mapping API directly on > top of the dart hardware instead of the iommu layer. This would probably > be much faster than the swiotlb on top of a bypass or a linear map, > but it's a really awful abstraction that would require adding special cases > into a lot of generic code. > > >Any driver that tries to use the iommu API directly could be trouble > >as well if they make similar assumptions. > > I think pretty much all drivers using the iommu API directly already > depends on having a matching page size. I don't see any way to use > e.g. PCI device assignment using vfio, or a GPU driver with per-process > contexts when the iotlb page size is larger than the CPU's. > > >Is this something you would even want to support in the iommu subsytem > >and is it even possible to do this in a sane way? > > I don't know how hard it is to do adjust the dma-iommu implementation > to allow this, but as long as we can work out the DT binding to support > both normal dma-iommu mode with 16KB pages and some kind of > passthrough mode (emulated or not) with 4KB pages, it can be left > as a possible optimization for later. I think one of the main things to modify is the IOVA allocator (drivers/iommu/iova.c). Once that is happy with pages bigger than the CPU page size, then you could probably fake everything else in the DMA layer by offsetting the returned DMA addresses into the 16K page which got mapped. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Fix extended features logging
On Sun, 11 Apr 2021, Joe Perches wrote:
> > v2: avoid pr_info(""), change pci_info() to pr_info() for a nicer
> > solution
> >
> > drivers/iommu/amd/init.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> > index 596d0c413473..62913f82a21f 100644
> > --- a/drivers/iommu/amd/init.c
> > +++ b/drivers/iommu/amd/init.c
> > @@ -1929,8 +1929,8 @@ static void print_iommu_info(void)
> > pci_info(pdev, "Found IOMMU cap 0x%hx\n", iommu->cap_ptr);
> >
> >
> > if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> > - pci_info(pdev, "Extended features (%#llx):",
> > -iommu->features);
> > + pr_info("Extended features (%#llx):", iommu->features);
> > +
> > for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
> > if (iommu_feature(iommu, (1ULL << i)))
> > pr_cont(" %s", feat_str[i]);
>
> How about avoiding all of this by using a temporary buffer
> and a single pci_info.
I think it is mostly up to the maintainers, but from my perspective, it's not
good to conflate such a simple bugfix with the substantial rewrite you are
proposing (which also increases code complexity).
My two-line patch is a straightforward fix to a bug that people already agreed
needs to be fixed (just the previous attempt turned out to be insufficient). If
there's a desire to eliminate pr_cont calls (which I wouldn't support in this
instance), the rewrite can go in separately from the bugfix.
A major problem with landing a simple bugfix together with a rewrite in a big
patch is that if a rewrite causes a problem, the whole patch gets reverted and
we end up without a trivial bugfix.
And, once again: can we please not emit the feature list via pci_info, the line
is long enough already even without the pci bus location info.
Joerg, are you satisfied with my v2 patch, are you waiting for anything before
picking it up?
Alexander
>
> Miscellanea:
> o Move the feat_str and i declarations into the if block for locality
>
> ---
> drivers/iommu/amd/init.c | 29 ++---
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 321f5906e6ed..0d219044726e 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -1943,30 +1943,37 @@ static int __init iommu_init_pci(struct amd_iommu
> *iommu)
>
> static void print_iommu_info(void)
> {
> - static const char * const feat_str[] = {
> - "PreF", "PPR", "X2APIC", "NX", "GT", "[5]",
> - "IA", "GA", "HE", "PC"
> - };
> struct amd_iommu *iommu;
>
> for_each_iommu(iommu) {
> struct pci_dev *pdev = iommu->dev;
> - int i;
>
> pci_info(pdev, "Found IOMMU cap 0x%x\n", iommu->cap_ptr);
>
> if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> - pci_info(pdev, "Extended features (%#llx):",
> - iommu->features);
> + static const char * const feat_str[] = {
> + "PreF", "PPR", "X2APIC", "NX", "GT", "[5]",
> + "IA", "GA", "HE", "PC"
> + };
> + int i;
> + char features[128] = "";
> + int len = 0;
> +
> for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
> - if (iommu_feature(iommu, (1ULL << i)))
> - pr_cont(" %s", feat_str[i]);
> + if (!iommu_feature(iommu, BIT_ULL(i)))
> + continue;
> + len += scnprintf(features + len,
> + sizeof(features) - len,
> + " %s", feat_str[i]);
> }
>
> if (iommu->features & FEATURE_GAM_VAPIC)
> - pr_cont(" GA_vAPIC");
> + len += scnprintf(features + len,
> + sizeof(features) - len,
> + " %s", "GA_vPIC");
>
> - pr_cont("\n");
> + pci_info(pdev, "Extended features (%#llx):%s\n",
> + iommu->features, features);
> }
> }
> if (irq_remapping_enabled) {
>
>
> ___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Fix extended features logging
On Mon, 2021-04-19 at 22:23 +0300, Alexander Monakov wrote:
> On Sun, 11 Apr 2021, Joe Perches wrote:
>
> > > v2: avoid pr_info(""), change pci_info() to pr_info() for a nicer
> > > solution
> > >
> > > drivers/iommu/amd/init.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> > > index 596d0c413473..62913f82a21f 100644
> > > --- a/drivers/iommu/amd/init.c
> > > +++ b/drivers/iommu/amd/init.c
> > > @@ -1929,8 +1929,8 @@ static void print_iommu_info(void)
> > > pci_info(pdev, "Found IOMMU cap 0x%hx\n", iommu->cap_ptr);
> > >
> > >
> > > if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> > > - pci_info(pdev, "Extended features (%#llx):",
> > > - iommu->features);
> > > + pr_info("Extended features (%#llx):", iommu->features);
> > > +
> > > for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
> > > if (iommu_feature(iommu, (1ULL << i)))
> > > pr_cont(" %s", feat_str[i]);
> >
> > How about avoiding all of this by using a temporary buffer
> > and a single pci_info.
>
> I think it is mostly up to the maintainers, but from my perspective, it's not
> good to conflate such a simple bugfix with the substantial rewrite you are
> proposing (which also increases code complexity).
You and I have _significant_ differences in the definition of substantial.
Buffering the output is the preferred code style in preference to
pr_cont.
Do remember pr_cont should _only_ be used when absolutely necessary
as interleaving of other messages from other processes/threads can
and does occur.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
Arnd Bergmann wrote on Mon, Apr 19, 2021 at 02:16:36PM +0200:
> In some cases, you can use the device_link infrastructure to deal
> with dependencies between devices. Not sure if this would help
> in your case, but have a look at device_link_add() etc in drivers/base/core.c
I'll need to actually try to convince myself but if creating the link
forces driver registration then it should be workable.
> > In this particular case the problem is that since 7d981405d0fd ("soc:
> > imx8m: change to use platform driver") the soc probe tries to use the
> > nvmem driver for ocotp fuses for imx8m devices, which isn't ready yet.
> > So soc loading gets pushed back to the end of the list because it gets
> > defered and other drivers relying on soc_device_match get confused
> > because they wrongly think a device doesn't match a quirk when it
> > actually does.
> >
> > If there is a way to ensure the nvmem driver gets loaded before the soc,
> > that would also solve the problem nicely, and avoid the need to mess
> > with all the ~50 drivers which use it.
> >
> > Is there a way to control in what order drivers get loaded? Something in
> > the dtb perhaps?
>
> For built-in drivers, load order depends on the initcall level and
> link order (how things are lined listed in the Makefile hierarchy).
>
> For loadable modules, this is up to user space in the end.
>
> Which of the drivers in this scenario are loadable modules?
All the drivers involved in my case are built-in (nvmem, soc and final
soc_device_match consumer e.g. caam_jr that crashes the kernel if soc is
not identified properly).
I frankly don't like the idea of moving nvmem/ above soc/ in
drivers/Makefile as a "solution" to this (especially as there is one
that seems to care about what soc they run on...), so I'll have a look
at links first, hopefully that will work out.
Thanks,
--
Dominique
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] pci: Rename pci_dev->untrusted to pci_dev->external
The current flag name "untrusted" is not correct as it is populated
using the firmware property "external-facing" for the parent ports. In
other words, the firmware only says which ports are external facing, so
the field really identifies the devices as external (vs internal).
Only field renaming. No functional change intended.
Signed-off-by: Rajat Jain
---
drivers/iommu/dma-iommu.c | 2 +-
drivers/iommu/intel/iommu.c | 2 +-
drivers/iommu/iommu.c | 2 +-
drivers/pci/ats.c | 2 +-
drivers/pci/pci.c | 2 +-
drivers/pci/probe.c | 12 ++--
drivers/pci/quirks.c| 2 +-
include/linux/pci.h | 9 +
8 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index af765c813cc8..14769c213f30 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -313,7 +313,7 @@ static void iommu_dma_flush_iotlb_all(struct iova_domain
*iovad)
static bool dev_is_untrusted(struct device *dev)
{
- return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
+ return dev_is_pci(dev) && to_pci_dev(dev)->external;
}
/**
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..965cc78e0dde 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -5511,7 +5511,7 @@ intel_iommu_domain_get_attr(struct iommu_domain *domain,
*/
static bool risky_device(struct pci_dev *pdev)
{
- if (pdev->untrusted) {
+ if (pdev->external) {
pci_info(pdev,
"Skipping IOMMU quirk for dev [%04X:%04X] on untrusted
PCI link\n",
pdev->vendor, pdev->device);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d0b0a15dba84..abdedc98f3c0 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1482,7 +1482,7 @@ static int iommu_get_def_domain_type(struct device *dev)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
- if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
+ if (dev_is_pci(dev) && to_pci_dev(dev)->external)
return IOMMU_DOMAIN_DMA;
if (ops->def_domain_type)
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 0d3719407b8b..0d17fb4a15bf 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -42,7 +42,7 @@ bool pci_ats_supported(struct pci_dev *dev)
if (!dev->ats_cap)
return false;
- return (dev->untrusted == 0);
+ return (dev->external == 0);
}
EXPORT_SYMBOL_GPL(pci_ats_supported);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 16a17215f633..ec98892389d7 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -886,7 +886,7 @@ static void pci_std_enable_acs(struct pci_dev *dev)
ctrl |= (cap & PCI_ACS_UF);
/* Enable Translation Blocking for external devices */
- if (dev->external_facing || dev->untrusted)
+ if (dev->external_facing || dev->external)
ctrl |= (cap & PCI_ACS_TB);
pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 953f15abc850..ae4800e82914 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1562,17 +1562,17 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
}
}
-static void set_pcie_untrusted(struct pci_dev *dev)
+static void set_pcie_external(struct pci_dev *dev)
{
struct pci_dev *parent;
/*
-* If the upstream bridge is untrusted we treat this device
-* untrusted as well.
+* If the upstream bridge is external we mark this device
+* external as well.
*/
parent = pci_upstream_bridge(dev);
- if (parent && (parent->untrusted || parent->external_facing))
- dev->untrusted = true;
+ if (parent && (parent->external || parent->external_facing))
+ dev->external = true;
}
/**
@@ -1814,7 +1814,7 @@ int pci_setup_device(struct pci_dev *dev)
/* Need to have dev->cfg_size ready */
set_pcie_thunderbolt(dev);
- set_pcie_untrusted(dev);
+ set_pcie_external(dev);
/* "Unknown power state" */
dev->current_state = PCI_UNKNOWN;
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e3ba9e..1054b7d9ee89 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4942,7 +4942,7 @@ static int pci_quirk_enable_intel_spt_pch_acs(struct
pci_dev *dev)
ctrl |= (cap & PCI_ACS_CR);
ctrl |= (cap & PCI_ACS_UF);
- if (dev->external_facing || dev->untrusted)
+ if (dev->external_facing || dev->external)
ctrl |= (cap & PCI_ACS_TB);
pci_write_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, ctrl);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..a535e2c2b690 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -431,14 +431,7 @@ struct pci_dev {
Re: [PATCH v3 02/12] iommu: Add iommu_split_block interface
Hi Baolu, On 2021/4/19 21:33, Lu Baolu wrote: > Hi Keqian, > > On 2021/4/19 17:32, Keqian Zhu wrote: +EXPORT_SYMBOL_GPL(iommu_split_block); >>> Do you really have any consumers of this interface other than the dirty >>> bit tracking? If not, I don't suggest to make this as a generic IOMMU >>> interface. >>> >>> There is an implicit requirement for such interfaces. The >>> iommu_map/unmap(iova, size) shouldn't be called at the same time. >>> Currently there's no such sanity check in the iommu core. A poorly >>> written driver could mess up the kernel by misusing this interface. >> Yes, I don't think up a scenario except dirty tracking. >> >> Indeed, we'd better not make them as a generic interface. >> >> Do you have any suggestion that underlying iommu drivers can share these >> code but >> not make it as a generic iommu interface? >> >> I have a not so good idea. Make the "split" interfaces as a static function, >> and >> transfer the function pointer to start_dirty_log. But it looks weird and >> inflexible. > > I understand splitting/merging super pages is an optimization, but not a > functional requirement. So is it possible to let the vendor iommu driver > decide whether splitting super pages when starting dirty bit tracking > and the opposite operation during when stopping it? The requirement for Right. If I understand you correct, actually that is what this series does. We realized split/merge in IOMMU core layer, but don't force vendor driver to use it. The problem is that when we expose these interfaces to vendor IOMMU driver, will also expose them to upper driver. > upper layer is that starting/stopping dirty bit tracking and > mapping/unmapping are mutually exclusive. OK, I will explicitly add the hints. Thanks. Thanks, Keqian > >> >> On the other hand, if a driver calls map/unmap with split/merge at the same >> time, >> it's a bug of driver, it should follow the rule. >> > > Best regards, > baolu > . > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 02/12] iommu: Add iommu_split_block interface
Hi Keqian, On 4/20/21 9:25 AM, Keqian Zhu wrote: Hi Baolu, On 2021/4/19 21:33, Lu Baolu wrote: Hi Keqian, On 2021/4/19 17:32, Keqian Zhu wrote: +EXPORT_SYMBOL_GPL(iommu_split_block); Do you really have any consumers of this interface other than the dirty bit tracking? If not, I don't suggest to make this as a generic IOMMU interface. There is an implicit requirement for such interfaces. The iommu_map/unmap(iova, size) shouldn't be called at the same time. Currently there's no such sanity check in the iommu core. A poorly written driver could mess up the kernel by misusing this interface. Yes, I don't think up a scenario except dirty tracking. Indeed, we'd better not make them as a generic interface. Do you have any suggestion that underlying iommu drivers can share these code but not make it as a generic iommu interface? I have a not so good idea. Make the "split" interfaces as a static function, and transfer the function pointer to start_dirty_log. But it looks weird and inflexible. I understand splitting/merging super pages is an optimization, but not a functional requirement. So is it possible to let the vendor iommu driver decide whether splitting super pages when starting dirty bit tracking and the opposite operation during when stopping it? The requirement for Right. If I understand you correct, actually that is what this series does. I mean to say no generic APIs, jut do it by the iommu subsystem itself. It's totally transparent to the upper level, just like what map() does. The upper layer doesn't care about either super page or small page is in use when do a mapping, right? If you want to consolidate some code, how about putting them in start/stop_tracking()? Best regards, baolu We realized split/merge in IOMMU core layer, but don't force vendor driver to use it. The problem is that when we expose these interfaces to vendor IOMMU driver, will also expose them to upper driver. upper layer is that starting/stopping dirty bit tracking and mapping/unmapping are mutually exclusive. OK, I will explicitly add the hints. Thanks. Thanks, Keqian On the other hand, if a driver calls map/unmap with split/merge at the same time, it's a bug of driver, it should follow the rule. Best regards, baolu . ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RESEND v2] iommu/vt-d: Use passthrough mode for the Intel IPUs
Intel IPU(Image Processing Unit) has its own (IO)MMU hardware,
The IPU driver allocates its own page table that is not mapped
via the DMA, and thus the Intel IOMMU driver blocks access giving
this error:
DMAR: DRHD: handling fault status reg 3
DMAR: [DMA Read] Request device [00:05.0] PASID
fault addr 76406000 [fault reason 06] PTE Read access is not set
As IPU is not an external facing device which is not risky, so use
IOMMU passthrough mode for Intel IPUs.
Fixes: 26f5689592e2 ("media: staging/intel-ipu3: mmu: Implement driver")
Signed-off-by: Bingbu Cao
---
drivers/iommu/intel/iommu.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..7e2fbdae467e 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -55,6 +55,12 @@
#define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
#define IS_USB_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_SERIAL_USB)
#define IS_ISA_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA)
+#define IS_INTEL_IPU(pdev) ((pdev)->vendor == PCI_VENDOR_ID_INTEL && \
+ ((pdev)->device == 0x9a19 ||\
+(pdev)->device == 0x9a39 ||\
+(pdev)->device == 0x4e19 ||\
+(pdev)->device == 0x465d ||\
+(pdev)->device == 0x1919))
#define IS_AZALIA(pdev) ((pdev)->vendor == 0x8086 && (pdev)->device == 0x3a3e)
#define IOAPIC_RANGE_START (0xfee0)
@@ -360,6 +366,7 @@ int intel_iommu_enabled = 0;
EXPORT_SYMBOL_GPL(intel_iommu_enabled);
static int dmar_map_gfx = 1;
+static int dmar_map_ipu = 1;
static int dmar_forcedac;
static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
@@ -368,6 +375,7 @@ static int iommu_skip_te_disable;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
+#define IDENTMAP_IPU 8
int intel_iommu_gfx_mapped;
EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
@@ -2839,6 +2847,9 @@ static int device_def_domain_type(struct device *dev)
if ((iommu_identity_mapping & IDENTMAP_GFX) &&
IS_GFX_DEVICE(pdev))
return IOMMU_DOMAIN_IDENTITY;
+
+ if ((iommu_identity_mapping & IDENTMAP_IPU) &&
IS_INTEL_IPU(pdev))
+ return IOMMU_DOMAIN_IDENTITY;
}
return 0;
@@ -3278,6 +3289,9 @@ static int __init init_dmars(void)
if (!dmar_map_gfx)
iommu_identity_mapping |= IDENTMAP_GFX;
+ if (!dmar_map_ipu)
+ iommu_identity_mapping |= IDENTMAP_IPU;
+
check_tylersburg_isoch();
ret = si_domain_init(hw_pass_through);
@@ -5622,6 +5636,18 @@ static void quirk_iommu_igfx(struct pci_dev *dev)
dmar_map_gfx = 0;
}
+static void quirk_iommu_ipu(struct pci_dev *dev)
+{
+ if (!IS_INTEL_IPU(dev))
+ return;
+
+ if (risky_device(dev))
+ return;
+
+ pci_info(dev, "Passthrough IOMMU for integrated Intel IPU\n");
+ dmar_map_ipu = 0;
+}
+
/* G4x/GM45 integrated gfx dmar support is totally busted. */
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_igfx);
@@ -5657,6 +5683,9 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1632,
quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx);
+/* disable IPU dmar support */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_iommu_ipu);
+
static void quirk_iommu_rwbf(struct pci_dev *dev)
{
if (risky_device(dev))
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v5 11/15] iommu/io-pgtable-arm: Implement arm_lpae_map_pages()
Hi Isaac,
在 2021/4/9 1:13, Isaac J. Manjarres 写道:
Implement the map_pages() callback for the ARM LPAE io-pgtable
format.
Signed-off-by: Isaac J. Manjarres
---
drivers/iommu/io-pgtable-arm.c | 42 ++
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 1b690911995a..92978dd9c885 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -344,20 +344,30 @@ static arm_lpae_iopte
arm_lpae_install_table(arm_lpae_iopte *table,
}
static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,
- phys_addr_t paddr, size_t size, arm_lpae_iopte prot,
- int lvl, arm_lpae_iopte *ptep, gfp_t gfp)
+ phys_addr_t paddr, size_t size, size_t pgcount,
+ arm_lpae_iopte prot, int lvl, arm_lpae_iopte *ptep,
+ gfp_t gfp, size_t *mapped)
{
arm_lpae_iopte *cptep, pte;
size_t block_size = ARM_LPAE_BLOCK_SIZE(lvl, data);
size_t tblsz = ARM_LPAE_GRANULE(data);
struct io_pgtable_cfg *cfg = &data->iop.cfg;
+ int ret = 0, num_entries, max_entries, map_idx_start;
/* Find our entry at the current level */
- ptep += ARM_LPAE_LVL_IDX(iova, lvl, data);
+ map_idx_start = ARM_LPAE_LVL_IDX(iova, lvl, data);
+ ptep += map_idx_start;
/* If we can install a leaf entry at this level, then do so */
- if (size == block_size)
- return arm_lpae_init_pte(data, iova, paddr, prot, lvl, 1, ptep);
+ if (size == block_size) {
+ max_entries = ARM_LPAE_PTES_PER_TABLE(data) - map_idx_start;
+ num_entries = min_t(int, pgcount, max_entries);
+ ret = arm_lpae_init_pte(data, iova, paddr, prot, lvl,
num_entries, ptep);
+ if (!ret && mapped)
+ *mapped += num_entries * size;
+
+ return ret;
+ }
/* We can't allocate tables at the final level */
if (WARN_ON(lvl >= ARM_LPAE_MAX_LEVELS - 1))
@@ -386,7 +396,8 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable *data,
unsigned long iova,
}
/* Rinse, repeat */
- return __arm_lpae_map(data, iova, paddr, size, prot, lvl + 1, cptep,
gfp);
+ return __arm_lpae_map(data, iova, paddr, size, pgcount, prot, lvl + 1,
+ cptep, gfp, mapped);
}
static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data,
@@ -453,8 +464,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
arm_lpae_io_pgtable *data,
return pte;
}
-static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
- phys_addr_t paddr, size_t size, int iommu_prot, gfp_t
gfp)
+static int arm_lpae_map_pages(struct io_pgtable_ops *ops, unsigned long iova,
+ phys_addr_t paddr, size_t pgsize, size_t pgcount,
+ int iommu_prot, gfp_t gfp, size_t *mapped)
{
struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
struct io_pgtable_cfg *cfg = &data->iop.cfg;
@@ -463,7 +475,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops,
unsigned long iova,
arm_lpae_iopte prot;
long iaext = (s64)iova >> cfg->ias;
- if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
+ if (WARN_ON(!pgsize || (pgsize & cfg->pgsize_bitmap) != pgsize))
return -EINVAL;
if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
@@ -476,7 +488,8 @@ static int arm_lpae_map(struct io_pgtable_ops *ops,
unsigned long iova,
return 0;
prot = arm_lpae_prot_to_pte(data, iommu_prot);
- ret = __arm_lpae_map(data, iova, paddr, size, prot, lvl, ptep, gfp);
+ ret = __arm_lpae_map(data, iova, paddr, pgsize, pgcount, prot, lvl,
+ptep, gfp, NULL);
The last input parameter should be mapped instead of NULL.
/*
* Synchronise all PTE updates for the new mapping before there's
* a chance for anything to kick off a table walk for the new iova.
@@ -486,6 +499,14 @@ static int arm_lpae_map(struct io_pgtable_ops *ops,
unsigned long iova,
return ret;
}
+
+static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
+ phys_addr_t paddr, size_t size, int iommu_prot, gfp_t
gfp)
+{
+ return arm_lpae_map_pages(ops, iova, paddr, size, 1, iommu_prot, gfp,
+ NULL);
+}
+
static void __arm_lpae_free_pgtable(struct arm_lpae_io_pgtable *data, int lvl,
arm_lpae_iopte *ptep)
{
@@ -782,6 +803,7 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg)
data->iop.ops = (struct io_pgtable_ops) {
.map= arm_lpae_map,
+ .map_pages = arm_lpae_map_pages,
[PATCHv3 0/2] iommu/arm-smmu-qcom: Add SC7280 support
Patch 1 adds the sc7280 smmu compatible. Patch 2 moves the adreno smmu check before apss smmu to enable adreno smmu specific implementation. Note that dt-binding for sc7280 is already merged. Changes in v3: * Collect acks and reviews * Rebase on top of for-joerg/arm-smmu/updates Changes in v2: * Add a comment to make sure this order is not changed in future (Jordan) Sai Prakash Ranjan (2): iommu/arm-smmu-qcom: Add SC7280 SMMU compatible iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCHv3 1/2] iommu/arm-smmu-qcom: Add SC7280 SMMU compatible
Add compatible for SC7280 SMMU to use the Qualcomm Technologies, Inc.
specific implementation.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 98b3a1c2a181..bea3ee0dabc2 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -166,6 +166,7 @@ static const struct of_device_id
qcom_smmu_client_of_match[] __maybe_unused = {
{ .compatible = "qcom,mdss" },
{ .compatible = "qcom,sc7180-mdss" },
{ .compatible = "qcom,sc7180-mss-pil" },
+ { .compatible = "qcom,sc7280-mdss" },
{ .compatible = "qcom,sc8180x-mdss" },
{ .compatible = "qcom,sdm845-mdss" },
{ .compatible = "qcom,sdm845-mss-pil" },
@@ -330,6 +331,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct
arm_smmu_device *smmu,
static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
{ .compatible = "qcom,msm8998-smmu-v2" },
{ .compatible = "qcom,sc7180-smmu-500" },
+ { .compatible = "qcom,sc7280-smmu-500" },
{ .compatible = "qcom,sc8180x-smmu-500" },
{ .compatible = "qcom,sdm630-smmu-v2" },
{ .compatible = "qcom,sdm845-smmu-500" },
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCHv3 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier
Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
both implement "arm,mmu-500" in some QTI SoCs and to run through
adreno smmu specific implementation such as enabling split pagetables
support, we need to match the "qcom,adreno-smmu" compatible first
before apss smmu or else we will be running apps smmu implementation
for adreno smmu and the additional features for adreno smmu is never
set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
and adreno smmu implementing "arm,mmu-500", so the adreno smmu
implementation is never reached because the current sequence checks
for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
specific impl and we never reach adreno smmu specific implementation.
Suggested-by: Akhil P Oommen
Signed-off-by: Sai Prakash Ranjan
Reviewed-by: Bjorn Andersson
Acked-by: Jordan Crouse
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index bea3ee0dabc2..03f048aebb80 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct
arm_smmu_device *smmu)
{
const struct device_node *np = smmu->dev->of_node;
- if (of_match_node(qcom_smmu_impl_of_match, np))
- return qcom_smmu_create(smmu, &qcom_smmu_impl);
-
+ /*
+* Do not change this order of implementation, i.e., first adreno
+* smmu impl and then apss smmu since we can have both implementing
+* arm,mmu-500 in which case we will miss setting adreno smmu specific
+* features if the order is changed.
+*/
if (of_device_is_compatible(np, "qcom,adreno-smmu"))
return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl);
+ if (of_match_node(qcom_smmu_impl_of_match, np))
+ return qcom_smmu_create(smmu, &qcom_smmu_impl);
+
return smmu;
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier
On 2021-04-19 20:08, Bjorn Andersson wrote: On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote: Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU both implement "arm,mmu-500" in some QTI SoCs and to run through adreno smmu specific implementation such as enabling split pagetables support, we need to match the "qcom,adreno-smmu" compatible first before apss smmu or else we will be running apps smmu implementation for adreno smmu and the additional features for adreno smmu is never set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps and adreno smmu implementing "arm,mmu-500", so the adreno smmu implementation is never reached because the current sequence checks for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that specific impl and we never reach adreno smmu specific implementation. Suggested-by: Akhil P Oommen Signed-off-by: Sai Prakash Ranjan Sorry for taking my time thinking about this. Reviewed-by: Bjorn Andersson No worries, thanks Bjorn. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] pci: Rename pci_dev->untrusted to pci_dev->external
On Mon, Apr 19, 2021 at 05:30:49PM -0700, Rajat Jain wrote: > The current flag name "untrusted" is not correct as it is populated > using the firmware property "external-facing" for the parent ports. In > other words, the firmware only says which ports are external facing, so > the field really identifies the devices as external (vs internal). > > Only field renaming. No functional change intended. I don't think this is a good idea. First the field should have been added to the generic struct device as requested multiple times before. Right now this requires horrible hacks in the IOMMU code to get at the pci_dev, and also doesn't scale to various other potential users. Second the untrusted is objectively a better name. Because untrusted is how we treat the device, which is what mattes. External is just how we come to that conclusion. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
