[PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid zero on i.MX95

[PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid zero on i.MX95

[Peng Fan (OSS)]

i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent memory to
memory operations. However TBU is in the path between eDMA3 and ACP,
need to bypass the default SID 0 to make eDMA3 work properly.

I was also thinking to introduce "bypass-sids = <0xA 0xB 0xC ...>" to
make this reusable for others, but not sure. I could switch to
"bypass-sids" if you prefer.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (2):
      dt-bindings: iommu: arm,smmu-v3: introduce nxp,imx95-bypass-sid-zero
      iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

 .../devicetree/bindings/iommu/arm,smmu-v3.yaml        |  4 ++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c           | 19 ++++++++++++++++---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h           |  1 +
 3 files changed, 21 insertions(+), 3 deletions(-)
---
base-commit: d61a00525464bfc5fe92c6ad713350988e492b88
change-id: 20241014-smmuv3-120b24bc4659

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH RFC 1/2] dt-bindings: iommu: arm,smmu-v3: introduce nxp,imx95-bypass-sid-zero

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent memory to
memory operations. However TBU is in the path between eDMA3 and ACP,
need to bypass the default SID 0 to make eDMA3 work properly.

Introduce the property "nxp,imx95-bypass-sid-zero" for bypassing SID 0.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
index 75fcf4cb52d9f6449238578f20288966af35cab3..88ab908154e31aabf98f3bbe4df348956f49d5e1 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
@@ -69,6 +69,10 @@ properties:
       register access with page 0 offsets. Set for Cavium ThunderX2 silicon that
       doesn't support SMMU page1 register space.
 
+  nxp,imx95-bypass-sid-zero:
+    type: boolean
+    description: StreamID 0 that needs transaction set as bypass mode.
+
 required:
   - compatible
   - reg

-- 
2.37.1

[PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent memory to
memory operations. However TBU is in the path between eDMA3 and ACP,
need to bypass the default SID 0 to make eDMA3 work properly.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 19 ++++++++++++++++---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
 2 files changed, 17 insertions(+), 3 deletions(-)

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 737c5b88235510e3ddb91a28cecbdcdc14854b32..3db7b3e2ac94e16130fc0356f7954ffa1a9dfb33 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -80,6 +80,7 @@ DEFINE_MUTEX(arm_smmu_asid_lock);
 static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
 	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium,cn9900-broken-page1-regspace"},
+	{ ARM_SMMU_OPT_IMX95_BYPASS_SID0, "nxp,imx95-bypass-sid-zero"},
 	{ 0, NULL},
 };
 
@@ -4465,7 +4466,7 @@ static void __iomem *arm_smmu_ioremap(struct device *dev, resource_size_t start,
 	return devm_ioremap_resource(dev, &res);
 }
 
-static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
+static void arm_smmu_install_bypass_ste(struct arm_smmu_device *smmu)
 {
 	struct list_head rmr_list;
 	struct iommu_resv_region *e;
@@ -4496,6 +4497,18 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
 	}
 
 	iort_put_rmr_sids(dev_fwnode(smmu->dev), &rmr_list);
+
+	if (smmu->options & ARM_SMMU_OPT_IMX95_BYPASS_SID0) {
+		int ret = arm_smmu_init_sid_strtab(smmu, 0);
+
+		if (ret) {
+			dev_err(smmu->dev, "i.MX95 SID0 bypass failed\n");
+			return;
+		}
+
+		arm_smmu_make_bypass_ste(smmu,
+					 arm_smmu_get_step_for_sid(smmu, 0));
+	}
 }
 
 static void arm_smmu_impl_remove(void *data)
@@ -4614,8 +4627,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	/* Record our private device structure */
 	platform_set_drvdata(pdev, smmu);
 
-	/* Check for RMRs and install bypass STEs if any */
-	arm_smmu_rmr_install_bypass_ste(smmu);
+	/* Install bypass STEs if any */
+	arm_smmu_install_bypass_ste(smmu);
 
 	/* Reset the device */
 	ret = arm_smmu_device_reset(smmu);
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 1e9952ca989f87957197f4d4b400f9d74bb685ac..06481b923284776e7dc4f3301e5cbe8ab7869a9c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -733,6 +733,7 @@ struct arm_smmu_device {
 #define ARM_SMMU_OPT_MSIPOLL		(1 << 2)
 #define ARM_SMMU_OPT_CMDQ_FORCE_SYNC	(1 << 3)
 #define ARM_SMMU_OPT_TEGRA241_CMDQV	(1 << 4)
+#define ARM_SMMU_OPT_IMX95_BYPASS_SID0	(1 << 5)
 	u32				options;
 
 	struct arm_smmu_cmdq		cmdq;

-- 
2.37.1

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Pranjal Shrivastava]

On Tue, Oct 15, 2024 at 11:14:43AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent memory to
> memory operations. However TBU is in the path between eDMA3 and ACP,
> need to bypass the default SID 0 to make eDMA3 work properly.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 19 ++++++++++++++++---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> 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 737c5b88235510e3ddb91a28cecbdcdc14854b32..3db7b3e2ac94e16130fc0356f7954ffa1a9dfb33 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -80,6 +80,7 @@ DEFINE_MUTEX(arm_smmu_asid_lock);
>  static struct arm_smmu_option_prop arm_smmu_options[] = {
>  	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
>  	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium,cn9900-broken-page1-regspace"},
> +	{ ARM_SMMU_OPT_IMX95_BYPASS_SID0, "nxp,imx95-bypass-sid-zero"},
>  	{ 0, NULL},
>  };

Aghh, let's not put HW-specific bypass under `arm_smmu_options`.
Otherwise, this might become a huge list of SoCs wanting to bypass SIDs.

>  
> @@ -4465,7 +4466,7 @@ static void __iomem *arm_smmu_ioremap(struct device *dev, resource_size_t start,
>  	return devm_ioremap_resource(dev, &res);
>  }
>  
> -static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
> +static void arm_smmu_install_bypass_ste(struct arm_smmu_device *smmu)
>  {
>  	struct list_head rmr_list;
>  	struct iommu_resv_region *e;
> @@ -4496,6 +4497,18 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
>  	}
>  
>  	iort_put_rmr_sids(dev_fwnode(smmu->dev), &rmr_list);
> +
> +	if (smmu->options & ARM_SMMU_OPT_IMX95_BYPASS_SID0) {
> +		int ret = arm_smmu_init_sid_strtab(smmu, 0);
> +
> +		if (ret) {
> +			dev_err(smmu->dev, "i.MX95 SID0 bypass failed\n");
> +			return;
> +		}
> +
> +		arm_smmu_make_bypass_ste(smmu,
> +					 arm_smmu_get_step_for_sid(smmu, 0));
> +	}
>  }

Umm.. this was specific for rmr not a generic thing. I'd suggest to
avoid meddling with the STEs directly for acheiving bypass. Playing
with the iommu domain type could be neater. Perhaps, modify the
ops->def_domain_type to return an appropriate domain?

In general, adding a property/config for bypassing SIDs/devices could
potentially cause security concerns if *somehow* a device can spoof an
SID. For example, if a PCIe device is bypassed it would be easy for
another PCIe endpoint to spoof it's ID. Similarly, certain HW
implementations may provide the option to programmable SIDs, for
example, a HW register to set SIDs, which if compromised can spoof other
SIDs. Although, I'd like to see what the others think about this.

>  
>  static void arm_smmu_impl_remove(void *data)
> @@ -4614,8 +4627,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	/* Record our private device structure */
>  	platform_set_drvdata(pdev, smmu);
>  
> -	/* Check for RMRs and install bypass STEs if any */
> -	arm_smmu_rmr_install_bypass_ste(smmu);
> +	/* Install bypass STEs if any */
> +	arm_smmu_install_bypass_ste(smmu);
>  
>  	/* Reset the device */
>  	ret = arm_smmu_device_reset(smmu);
> 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 1e9952ca989f87957197f4d4b400f9d74bb685ac..06481b923284776e7dc4f3301e5cbe8ab7869a9c 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -733,6 +733,7 @@ struct arm_smmu_device {
>  #define ARM_SMMU_OPT_MSIPOLL		(1 << 2)
>  #define ARM_SMMU_OPT_CMDQ_FORCE_SYNC	(1 << 3)
>  #define ARM_SMMU_OPT_TEGRA241_CMDQV	(1 << 4)
> +#define ARM_SMMU_OPT_IMX95_BYPASS_SID0	(1 << 5)
>  	u32				options;
>  
>  	struct arm_smmu_cmdq		cmdq;
>
> -- 
> 2.37.1
> 
>

Thanks,
Pranjal

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Jason Gunthorpe]

On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:

> Umm.. this was specific for rmr not a generic thing. I'd suggest to
> avoid meddling with the STEs directly for acheiving bypass. Playing
> with the iommu domain type could be neater. Perhaps, modify the
> ops->def_domain_type to return an appropriate domain?

Yeah, that is the expected way, to force the def_domain_type to
IDENTITY and refuse to attach a PAGING/BLOCKED domain.

If this is a common thing we could have the core code take on more of
the job.

Jason

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Pranjal Shrivastava]

On Tue, Oct 15, 2024 at 09:47:23AM -0300, Jason Gunthorpe wrote:
> On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:
> 
> > Umm.. this was specific for rmr not a generic thing. I'd suggest to
> > avoid meddling with the STEs directly for acheiving bypass. Playing
> > with the iommu domain type could be neater. Perhaps, modify the
> > ops->def_domain_type to return an appropriate domain?
> 
> Yeah, that is the expected way, to force the def_domain_type to
> IDENTITY and refuse to attach a PAGING/BLOCKED domain.
> 
> If this is a common thing we could have the core code take on more of
> the job.

Yes! I've seen the IOMMU being bypassed at multiple places, primarily
for performance, people like bypassing the iommu for "trusted" devices.
A few examples that are publically accessible: Qcomm SoCs [1], [2].
Seems like Qualcomm have a DT property `qcomm-s1-bypass` to achieve
something similar.

In fact, *blast from the past*, I tried to do something similar sometime
ago with [3]. Although, perhaps that wasn't the best way (and I was a
kernel newbie :))

A little off-topic, but I think there has been some interest to bypass
the default substream as well while still maintaining PASID isolation.[4]

Although, as far as arm-smmu-v3 is concerned, IIRC, I think there was a
way to tell that the region is reserved and don't map it.

> 
> Jason

Thanks,
Pranjal

[1]
https://github.com/realme-kernel-opensource/realme5-kernel-source/blob/master/arch/arm64/boot/dts/qcom/sa8155-vm-qupv3.dtsi#L22

[2] 
https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/Documentation/devicetree/bindings/platform/msm/ipa.txt#28

[3]
https://lore.kernel.org/all/20230707104857.348353-1-praan@google.com/

[4]
https://lore.kernel.org/all/CAGfWUPziSWNMc_px4E-i+_V_Jxdb_WSwOLXHZ+PANz2Tv5pFPA@mail.gmail.com/

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Pranjal Shrivastava]

On Tue, Oct 15, 2024 at 03:00:10PM +0000, Pranjal Shrivastava wrote:
> On Tue, Oct 15, 2024 at 09:47:23AM -0300, Jason Gunthorpe wrote:
> > On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:
> > 
> > > Umm.. this was specific for rmr not a generic thing. I'd suggest to
> > > avoid meddling with the STEs directly for acheiving bypass. Playing
> > > with the iommu domain type could be neater. Perhaps, modify the
> > > ops->def_domain_type to return an appropriate domain?
> > 
> > Yeah, that is the expected way, to force the def_domain_type to
> > IDENTITY and refuse to attach a PAGING/BLOCKED domain.
> > 
> > If this is a common thing we could have the core code take on more of
> > the job.
> 
> Yes! I've seen the IOMMU being bypassed at multiple places, primarily
> for performance, people like bypassing the iommu for "trusted" devices.
> A few examples that are publically accessible: Qcomm SoCs [1], [2].
> Seems like Qualcomm have a DT property `qcomm-s1-bypass` to achieve
> something similar.
> 
> In fact, *blast from the past*, I tried to do something similar sometime
> ago with [3]. Although, perhaps that wasn't the best way (and I was a
> kernel newbie :))
> 
> A little off-topic, but I think there has been some interest to bypass
> the default substream as well while still maintaining PASID isolation.[4]
> 

Agh, apologies, ignore the following part about rmr,
it won't solve the problem here.
> Although, as far as arm-smmu-v3 is concerned, IIRC, I think there was a
> way to tell that the region is reserved and don't map it.

> 
> > 
> > Jason
> 
> Thanks,
> Pranjal
> 
> [1]
> https://github.com/realme-kernel-opensource/realme5-kernel-source/blob/master/arch/arm64/boot/dts/qcom/sa8155-vm-qupv3.dtsi#L22
> 
> [2] 
> https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/Documentation/devicetree/bindings/platform/msm/ipa.txt#28
> 
> [3]
> https://lore.kernel.org/all/20230707104857.348353-1-praan@google.com/
> 
> [4]
> https://lore.kernel.org/all/CAGfWUPziSWNMc_px4E-i+_V_Jxdb_WSwOLXHZ+PANz2Tv5pFPA@mail.gmail.com/

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Jason Gunthorpe]

On Tue, Oct 15, 2024 at 03:00:10PM +0000, Pranjal Shrivastava wrote:
> On Tue, Oct 15, 2024 at 09:47:23AM -0300, Jason Gunthorpe wrote:
> > On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:
> > 
> > > Umm.. this was specific for rmr not a generic thing. I'd suggest to
> > > avoid meddling with the STEs directly for acheiving bypass. Playing
> > > with the iommu domain type could be neater. Perhaps, modify the
> > > ops->def_domain_type to return an appropriate domain?
> > 
> > Yeah, that is the expected way, to force the def_domain_type to
> > IDENTITY and refuse to attach a PAGING/BLOCKED domain.
> > 
> > If this is a common thing we could have the core code take on more of
> > the job.
> 
> Yes! I've seen the IOMMU being bypassed at multiple places, primarily
> for performance, people like bypassing the iommu for "trusted" devices.
> A few examples that are publically accessible: Qcomm SoCs [1], [2].
> Seems like Qualcomm have a DT property `qcomm-s1-bypass` to achieve
> something similar.

It is not good to encode policy in the kernel in this way. If the
device works then it should be permitted to be non-identity, even if
it is slow.

I suppose things are done this way because the policy can't be changed
once the drivers are bound, so this has to be decided early boot and
so it is easiest path...

But it does suggest to me that the DT encoding is more an indication
"device is really slow with translation".

Once I was looking at the idea of pulling all the identity stuff out
of the drivers since alot of it is mostly device specific
quirks/etc. It could then be marked as mandatory/performance and that
could help understanding alot.

Jason

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Robin Murphy]

On 2024-10-15 1:47 pm, Jason Gunthorpe wrote:
> On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:
> 
>> Umm.. this was specific for rmr not a generic thing. I'd suggest to
>> avoid meddling with the STEs directly for acheiving bypass. Playing
>> with the iommu domain type could be neater. Perhaps, modify the
>> ops->def_domain_type to return an appropriate domain?
> 
> Yeah, that is the expected way, to force the def_domain_type to
> IDENTITY and refuse to attach a PAGING/BLOCKED domain.

There is no domain, this is bypassing an arbitrary StreamID not 
associated with any device. Which incidentally is something an IORT RMR 
can quite happily achieve already (I think the DT reserved-memory 
binding does need a proper device node to relate to, though).

Thanks,
Robin.

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Pranjal Shrivastava]

On Tue, Oct 15, 2024 at 04:13:13PM +0100, Robin Murphy wrote:
> On 2024-10-15 1:47 pm, Jason Gunthorpe wrote:
> > On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:
> > 
> > > Umm.. this was specific for rmr not a generic thing. I'd suggest to
> > > avoid meddling with the STEs directly for acheiving bypass. Playing
> > > with the iommu domain type could be neater. Perhaps, modify the
> > > ops->def_domain_type to return an appropriate domain?
> > 
> > Yeah, that is the expected way, to force the def_domain_type to
> > IDENTITY and refuse to attach a PAGING/BLOCKED domain.
> 
> There is no domain, this is bypassing an arbitrary StreamID not associated
> with any device. Which incidentally is something an IORT RMR can quite
> happily achieve already (I think the DT reserved-memory binding does need a
> proper device node to relate to, though).

+1. I assumed that the use-case was to first attach the streamID to a
device and then intall a bypass for that specific streamID. If that's
not the case, I'm not sure why are we trying to achieve that.

I thought about rmr too, but it looks like the "device" is a DMA and may
want to write to more than a fixed region of memory.

> 
> Thanks,
> Robin.

Thanks,
Pranjal

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Jason Gunthorpe]

On Tue, Oct 15, 2024 at 04:13:13PM +0100, Robin Murphy wrote:
> On 2024-10-15 1:47 pm, Jason Gunthorpe wrote:
> > On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:
> > 
> > > Umm.. this was specific for rmr not a generic thing. I'd suggest to
> > > avoid meddling with the STEs directly for acheiving bypass. Playing
> > > with the iommu domain type could be neater. Perhaps, modify the
> > > ops->def_domain_type to return an appropriate domain?
> > 
> > Yeah, that is the expected way, to force the def_domain_type to
> > IDENTITY and refuse to attach a PAGING/BLOCKED domain.
> 
> There is no domain, this is bypassing an arbitrary StreamID not associated
> with any device.

If the stream ID is going to flow traffic shouldn't it have a DT node
for it? Something must be driving the DMA on this SID, and the kernel
does need to know what that is in some way, even for basic security
things like making sure VFIO doesn't get a hold of it :\

Jason

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Robin Murphy]

On 2024-10-15 4:31 pm, Jason Gunthorpe wrote:
> On Tue, Oct 15, 2024 at 04:13:13PM +0100, Robin Murphy wrote:
>> On 2024-10-15 1:47 pm, Jason Gunthorpe wrote:
>>> On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:
>>>
>>>> Umm.. this was specific for rmr not a generic thing. I'd suggest to
>>>> avoid meddling with the STEs directly for acheiving bypass. Playing
>>>> with the iommu domain type could be neater. Perhaps, modify the
>>>> ops->def_domain_type to return an appropriate domain?
>>>
>>> Yeah, that is the expected way, to force the def_domain_type to
>>> IDENTITY and refuse to attach a PAGING/BLOCKED domain.
>>
>> There is no domain, this is bypassing an arbitrary StreamID not associated
>> with any device.
> 
> If the stream ID is going to flow traffic shouldn't it have a DT node
> for it? Something must be driving the DMA on this SID, and the kernel
> does need to know what that is in some way, even for basic security
> things like making sure VFIO doesn't get a hold of it :\

Exactly, hence this RFC is definitely not the right approach.

Thanks,
Robin.

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Pranjal Shrivastava]

On Tue, Oct 15, 2024 at 04:37:25PM +0100, Robin Murphy wrote:
> On 2024-10-15 4:31 pm, Jason Gunthorpe wrote:
> > On Tue, Oct 15, 2024 at 04:13:13PM +0100, Robin Murphy wrote:
> > > On 2024-10-15 1:47 pm, Jason Gunthorpe wrote:
> > > > On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava wrote:
> > > > 
> > > > > Umm.. this was specific for rmr not a generic thing. I'd suggest to
> > > > > avoid meddling with the STEs directly for acheiving bypass. Playing
> > > > > with the iommu domain type could be neater. Perhaps, modify the
> > > > > ops->def_domain_type to return an appropriate domain?
> > > > 
> > > > Yeah, that is the expected way, to force the def_domain_type to
> > > > IDENTITY and refuse to attach a PAGING/BLOCKED domain.
> > > 
> > > There is no domain, this is bypassing an arbitrary StreamID not associated
> > > with any device.
> > 
> > If the stream ID is going to flow traffic shouldn't it have a DT node
> > for it? Something must be driving the DMA on this SID, and the kernel
> > does need to know what that is in some way, even for basic security
> > things like making sure VFIO doesn't get a hold of it :\
> 
> Exactly, hence this RFC is definitely not the right approach.

Agreed. I assumed the bypass was needed for a registered SID.

> 
> Thanks,
> Robin.

Thanks,
Pranjal

RE: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Peng Fan]

All,

> Subject: Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for
> NXP i.MX95

Thanks for the discussion on this topic to show much information
that I not foresee.

> 
> On Tue, Oct 15, 2024 at 04:37:25PM +0100, Robin Murphy wrote:
> > On 2024-10-15 4:31 pm, Jason Gunthorpe wrote:
> > > On Tue, Oct 15, 2024 at 04:13:13PM +0100, Robin Murphy wrote:
> > > > On 2024-10-15 1:47 pm, Jason Gunthorpe wrote:
> > > > > On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava
> wrote:
> > > > >
> > > > > > Umm.. this was specific for rmr not a generic thing. I'd
> > > > > > suggest to avoid meddling with the STEs directly for acheiving
> > > > > > bypass. Playing with the iommu domain type could be neater.
> > > > > > Perhaps, modify the
> > > > > > ops->def_domain_type to return an appropriate domain?
> > > > >
> > > > > Yeah, that is the expected way, to force the def_domain_type to
> > > > > IDENTITY and refuse to attach a PAGING/BLOCKED domain.
> > > >
> > > > There is no domain, this is bypassing an arbitrary StreamID not
> > > > associated with any device.
> > >
> > > If the stream ID is going to flow traffic shouldn't it have a DT
> > > node for it? Something must be driving the DMA on this SID, and
> the
> > > kernel does need to know what that is in some way, even for basic
> > > security things like making sure VFIO doesn't get a hold of it :\
> >
> > Exactly, hence this RFC is definitely not the right approach.
> 
> Agreed. I assumed the bypass was needed for a registered SID.

I just reply here, not reply each thread.

The SID is not a registered SID.

Considering the security things, except "iommus = <&smmu 0>"
being added, is there other method for this issue?

Thanks,
Peng.

> 
> >
> > Thanks,
> > Robin.
> 
> Thanks,
> Pranjal

Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95

[Pranjal Shrivastava]

On Wed, Oct 16, 2024 at 09:02:39AM +0000, Peng Fan wrote:
> All,
> 
> > Subject: Re: [PATCH RFC 2/2] iommu/arm-smmu-v3: Bypass SID0 for
> > NXP i.MX95
> 
> Thanks for the discussion on this topic to show much information
> that I not foresee.
> 
> > 
> > On Tue, Oct 15, 2024 at 04:37:25PM +0100, Robin Murphy wrote:
> > > On 2024-10-15 4:31 pm, Jason Gunthorpe wrote:
> > > > On Tue, Oct 15, 2024 at 04:13:13PM +0100, Robin Murphy wrote:
> > > > > On 2024-10-15 1:47 pm, Jason Gunthorpe wrote:
> > > > > > On Tue, Oct 15, 2024 at 08:13:28AM +0000, Pranjal Shrivastava
> > wrote:
> > > > > >
> > > > > > > Umm.. this was specific for rmr not a generic thing. I'd
> > > > > > > suggest to avoid meddling with the STEs directly for acheiving
> > > > > > > bypass. Playing with the iommu domain type could be neater.
> > > > > > > Perhaps, modify the
> > > > > > > ops->def_domain_type to return an appropriate domain?
> > > > > >
> > > > > > Yeah, that is the expected way, to force the def_domain_type to
> > > > > > IDENTITY and refuse to attach a PAGING/BLOCKED domain.
> > > > >
> > > > > There is no domain, this is bypassing an arbitrary StreamID not
> > > > > associated with any device.
> > > >
> > > > If the stream ID is going to flow traffic shouldn't it have a DT
> > > > node for it? Something must be driving the DMA on this SID, and
> > the
> > > > kernel does need to know what that is in some way, even for basic
> > > > security things like making sure VFIO doesn't get a hold of it :\
> > >
> > > Exactly, hence this RFC is definitely not the right approach.
> > 
> > Agreed. I assumed the bypass was needed for a registered SID.
> 
> I just reply here, not reply each thread.

Apologies, I responded to the other thread before looking at this one

> 
> The SID is not a registered SID.
> 
> Considering the security things, except "iommus = <&smmu 0>"
> being added, is there other method for this issue?

I can only think of RMRs if you can ensure/restrict eDMA3 to access a
fixed region of memory. Something like a DMA zone if feasible.

> 
> Thanks,
> Peng.
> 
> > 
> > >
> > > Thanks,
> > > Robin.
> > 
Thanks,
Pranjal

Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid zero on i.MX95

[Pranjal Shrivastava]

Hi Peng,

On Tue, Oct 15, 2024 at 11:14:41AM +0800, Peng Fan (OSS) wrote:
> i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent memory to
> memory operations. However TBU is in the path between eDMA3 and ACP,
> need to bypass the default SID 0 to make eDMA3 work properly.
> 
> I was also thinking to introduce "bypass-sids = <0xA 0xB 0xC ...>" to
> make this reusable for others, but not sure. I could switch to
> "bypass-sids" if you prefer.

Any reason why you can't use the sysfs to change the iommu domain type?
i.e. by using the /sys/kernel/iommu_groups/<iommu_group_number>/type
AFAIK, the arm-smmu-v3 driver allocates one iommu_group per device.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> Peng Fan (2):
>       dt-bindings: iommu: arm,smmu-v3: introduce nxp,imx95-bypass-sid-zero
>       iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95
> 
>  .../devicetree/bindings/iommu/arm,smmu-v3.yaml        |  4 ++++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c           | 19 ++++++++++++++++---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h           |  1 +
>  3 files changed, 21 insertions(+), 3 deletions(-)
> ---
> base-commit: d61a00525464bfc5fe92c6ad713350988e492b88
> change-id: 20241014-smmuv3-120b24bc4659
> 
> Best regards,
> -- 
> Peng Fan <peng.fan@nxp.com>
> 
> 

Thanks,
Pranjal

Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid zero on i.MX95

[Robin Murphy]

On 2024-10-15 4:14 am, Peng Fan (OSS) wrote:
> i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent memory to
> memory operations. However TBU is in the path between eDMA3 and ACP,
> need to bypass the default SID 0 to make eDMA3 work properly.

I'm confused, why not just describe that the device owns this StreamID 
in the DT the normal way, i.e, "iommus = <&smmu 0>;"?

Thanks,
Robin.

> I was also thinking to introduce "bypass-sids = <0xA 0xB 0xC ...>" to
> make this reusable for others, but not sure. I could switch to
> "bypass-sids" if you prefer.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> Peng Fan (2):
>        dt-bindings: iommu: arm,smmu-v3: introduce nxp,imx95-bypass-sid-zero
>        iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95
> 
>   .../devicetree/bindings/iommu/arm,smmu-v3.yaml        |  4 ++++
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c           | 19 ++++++++++++++++---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h           |  1 +
>   3 files changed, 21 insertions(+), 3 deletions(-)
> ---
> base-commit: d61a00525464bfc5fe92c6ad713350988e492b88
> change-id: 20241014-smmuv3-120b24bc4659
> 
> Best regards,

RE: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid zero on i.MX95

[Peng Fan]

Hi Robin,

> Subject: Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid
> zero on i.MX95
> 
> On 2024-10-15 4:14 am, Peng Fan (OSS) wrote:
> > i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent
> memory to
> > memory operations. However TBU is in the path between eDMA3
> and ACP,
> > need to bypass the default SID 0 to make eDMA3 work properly.
> 
> I'm confused, why not just describe that the device owns this StreamID
> in the DT the normal way, i.e, "iommus = <&smmu 0>;"?

Current SMMU-v3 driver not support streamID sharing, but we have
limited streamIDs(i.MX95 max supports 64 SIDs). However the eDMA
supports 64 channels, each channel may have a SID, but not supported
by current smmuv3 driver.

We see smmu would degrade the performance in somecases, so
we wanna to give user an option to bypass SMMU for eDMA.

Thanks,
Peng.

> 
> Thanks,
> Robin.
> 
> > I was also thinking to introduce "bypass-sids = <0xA 0xB 0xC ...>" to
> > make this reusable for others, but not sure. I could switch to
> > "bypass-sids" if you prefer.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > Peng Fan (2):
> >        dt-bindings: iommu: arm,smmu-v3: introduce nxp,imx95-bypass-
> sid-zero
> >        iommu/arm-smmu-v3: Bypass SID0 for NXP i.MX95
> >
> >   .../devicetree/bindings/iommu/arm,smmu-v3.yaml        |  4 ++++
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c           | 19
> ++++++++++++++++---
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h           |  1 +
> >   3 files changed, 21 insertions(+), 3 deletions(-)
> > ---
> > base-commit: d61a00525464bfc5fe92c6ad713350988e492b88
> > change-id: 20241014-smmuv3-120b24bc4659
> >
> > Best regards,

Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid zero on i.MX95

[Nicolin Chen]

On Wed, Oct 16, 2024 at 12:56:54AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid
> > zero on i.MX95
> >
> > On 2024-10-15 4:14 am, Peng Fan (OSS) wrote:
> > > i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent
> > memory to
> > > memory operations. However TBU is in the path between eDMA3
> > and ACP,
> > > need to bypass the default SID 0 to make eDMA3 work properly.
> >
> > I'm confused, why not just describe that the device owns this StreamID
> > in the DT the normal way, i.e, "iommus = <&smmu 0>;"?
> 
> Current SMMU-v3 driver not support streamID sharing, but we have
> limited streamIDs(i.MX95 max supports 64 SIDs). However the eDMA
> supports 64 channels, each channel may have a SID, but not supported
> by current smmuv3 driver.
> 
> We see smmu would degrade the performance in somecases, so
> we wanna to give user an option to bypass SMMU for eDMA.

If it's a system-wise global need, adding "iommu.passthrough=y"
to the bootargs string should work for all trusted devices. And
particular group can be changed to the IDENTITY domain too upon
cmdline setting, e.g.
echo identity | sudo tee /sys/class/pci_bus/0009\:01/device/iommu_group/type

Thanks
Nicolin

RE: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid zero on i.MX95

[Peng Fan]

> Subject: Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid
> zero on i.MX95
> 
> On Wed, Oct 16, 2024 at 12:56:54AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass
> streamid zero
> > > on i.MX95
> > >
> > > On 2024-10-15 4:14 am, Peng Fan (OSS) wrote:
> > > > i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent
> > > memory to
> > > > memory operations. However TBU is in the path between eDMA3
> > > and ACP,
> > > > need to bypass the default SID 0 to make eDMA3 work properly.
> > >
> > > I'm confused, why not just describe that the device owns this
> > > StreamID in the DT the normal way, i.e, "iommus = <&smmu 0>;"?
> >
> > Current SMMU-v3 driver not support streamID sharing, but we have
> > limited streamIDs(i.MX95 max supports 64 SIDs). However the eDMA
> > supports 64 channels, each channel may have a SID, but not
> supported
> > by current smmuv3 driver.
> >
> > We see smmu would degrade the performance in somecases, so we
> wanna to
> > give user an option to bypass SMMU for eDMA.
> 
> If it's a system-wise global need, adding "iommu.passthrough=y"
> to the bootargs string should work for all trusted devices. And

No. not system wide.

> particular group can be changed to the IDENTITY domain too upon
> cmdline setting, e.g.
> echo identity | sudo tee
> /sys/class/pci_bus/0009\:01/device/iommu_group/type

This means I still need to put iommus = <&smmu 0> under
the device tree node.

Thanks,
Peng.

> 
> Thanks
> Nicolin

Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid zero on i.MX95

[Pranjal Shrivastava]

On Wed, Oct 16, 2024 at 08:53:20AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass streamid
> > zero on i.MX95
> > 
> > On Wed, Oct 16, 2024 at 12:56:54AM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH RFC 0/2] iommu/arm-smmu-v3: bypass
> > streamid zero
> > > > on i.MX95
> > > >
> > > > On 2024-10-15 4:14 am, Peng Fan (OSS) wrote:
> > > > > i.MX95 eDMA3 connects to DSU ACP, supporting dma coherent
> > > > memory to
> > > > > memory operations. However TBU is in the path between eDMA3
> > > > and ACP,
> > > > > need to bypass the default SID 0 to make eDMA3 work properly.
> > > >
> > > > I'm confused, why not just describe that the device owns this
> > > > StreamID in the DT the normal way, i.e, "iommus = <&smmu 0>;"?
> > >
> > > Current SMMU-v3 driver not support streamID sharing, but we have
> > > limited streamIDs(i.MX95 max supports 64 SIDs). However the eDMA
> > > supports 64 channels, each channel may have a SID, but not
> > supported
> > > by current smmuv3 driver.
> > >
> > > We see smmu would degrade the performance in somecases, so we
> > wanna to
> > > give user an option to bypass SMMU for eDMA.
> > 
> > If it's a system-wise global need, adding "iommu.passthrough=y"
> > to the bootargs string should work for all trusted devices. And
> 
> No. not system wide.
> 
> > particular group can be changed to the IDENTITY domain too upon
> > cmdline setting, e.g.
> > echo identity | sudo tee
> > /sys/class/pci_bus/0009\:01/device/iommu_group/type
> 
> This means I still need to put iommus = <&smmu 0> under
> the device tree node.
> 

Yes, otherwise the bypassing an unassigned SID isn't something that the
community wouldn't prefer as it smashes a hole in security for everyone.

Does your hardware have a way all the devices can configure/program
SIDs or does the hardware have multiple devices that generate SID 0?
If it's programmable, I'd suggest reserving streamID 0 for bypass and
let the system work with the other 63.

Apart from that, if you can guarantee that the `eDMA3` would always
access a fixed region of memory, then maybe use RMRs to install bypass.

> Thanks,
> Peng.
> 
> > 
> > Thanks
> > Nicolin
> 

Thanks,
Pranjal