diff mbox series

arm64: dts: rockchip: disable IOMMU when running rk3588 in PCIe endpoint mode

Message ID 20250207143900.2047949-2-cassel@kernel.org (mailing list archive)
State New
Headers show
Series arm64: dts: rockchip: disable IOMMU when running rk3588 in PCIe endpoint mode | expand

Commit Message

Niklas Cassel Feb. 7, 2025, 2:39 p.m. UTC
Commit da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU
on the rk3588 SoC") enabled the mmu600_pcie IOMMU, both in the normal case
(when all PCIe controllers are running in Root Complex mode) and in the
case when running the pcie3x4 PCIe controller in Endpoint mode.

There have been no issues detected when running the PCIe controllers in
Root Complex mode. During PCI probe time, we will add a SID to the IOMMU
for each PCI device enumerated on the bus, including the root port itself.

However, when running the pcie3x4 PCIe controller in Endpoint mode, we
will only add a single SID to the IOMMU (the SID specified in the iommus
DT property).

The enablement of IOMMU in endpoint mode was verified on setup with two
Rock 5b:s, where the BDF of the Root Complex has BDF (00:00.0).

A Root Complex sending a TLP to the Endpoint will have Requester ID set
to the BDF of the initiator. On the EP side, the Requester ID will then
be used as the SID. This works fine if the Root Complex has a BDF that
matches the iommus DT property, however, if the Root Complex has any other
BDF, we will see something like:
arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x1600 ssid: 0x0
on the endpoint side.

For PCIe controllers running in endpoint mode that always uses the
incoming Requester ID as the SID, the iommus DT property simply isn't
a viable solution. (Neither is iommu-map a viable solution, as there is
no enumeration done on the endpoint side.)

Thus, partly revert commit da92d3dfc871 ("arm64: dts: rockchip: enable the
mmu600_pcie IOMMU on the rk3588 SoC") by disabling the PCI IOMMU when
running the pcie3x4 PCIe controller in Endpoint mode.

Since the PCI IOMMU is working as expected in the normal case, keep it
enabled when running all PCIe controllers in Root Complex mode.

Fixes: da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU on the rk3588 SoC")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi           | 1 -
 arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso | 4 ++++
 2 files changed, 4 insertions(+), 1 deletion(-)

Comments

Frank Li Feb. 7, 2025, 4:04 p.m. UTC | #1
On Fri, Feb 07, 2025 at 03:39:01PM +0100, Niklas Cassel wrote:
> Commit da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU
> on the rk3588 SoC") enabled the mmu600_pcie IOMMU, both in the normal case
> (when all PCIe controllers are running in Root Complex mode) and in the
> case when running the pcie3x4 PCIe controller in Endpoint mode.
>
> There have been no issues detected when running the PCIe controllers in
> Root Complex mode. During PCI probe time, we will add a SID to the IOMMU
> for each PCI device enumerated on the bus, including the root port itself.
>
> However, when running the pcie3x4 PCIe controller in Endpoint mode, we
> will only add a single SID to the IOMMU (the SID specified in the iommus
> DT property).
>
> The enablement of IOMMU in endpoint mode was verified on setup with two
> Rock 5b:s, where the BDF of the Root Complex has BDF (00:00.0).
>
> A Root Complex sending a TLP to the Endpoint will have Requester ID set
> to the BDF of the initiator. On the EP side, the Requester ID will then
> be used as the SID. This works fine if the Root Complex has a BDF that
> matches the iommus DT property, however, if the Root Complex has any other
> BDF, we will see something like:
> arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x1600 ssid: 0x0
> on the endpoint side.
>
> For PCIe controllers running in endpoint mode that always uses the
> incoming Requester ID as the SID, the iommus DT property simply isn't
> a viable solution. (Neither is iommu-map a viable solution, as there is
> no enumeration done on the endpoint side.)
>
> Thus, partly revert commit da92d3dfc871 ("arm64: dts: rockchip: enable the
> mmu600_pcie IOMMU on the rk3588 SoC") by disabling the PCI IOMMU when
> running the pcie3x4 PCIe controller in Endpoint mode.
>
> Since the PCI IOMMU is working as expected in the normal case, keep it
> enabled when running all PCIe controllers in Root Complex mode.
>
> Fixes: da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU on the rk3588 SoC")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

I notify this problem when try to enable msi doorbell problem. Some system
(may most sysystem simple use PCIe Requester ID (BDF) as stream ID. BDF is
variances depend on host system's configuration.

Some platform supports mask BD. The key EP don't known request ID before
recieve a request from PCI RC.

May we need discuss at PCI mail list to figure out how to resolve this
problem.

Frank

> ---
>  arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi           | 1 -
>  arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso | 4 ++++
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> index 42b57e27ee4c..b5feefda9d2d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> @@ -215,7 +215,6 @@ pcie3x4_ep: pcie-ep@fe150000 {
>  		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
>  				  "dma0", "dma1", "dma2", "dma3";
>  		max-link-speed = <3>;
> -		iommus = <&mmu600_pcie 0x0000>;
>  		num-lanes = <4>;
>  		phys = <&pcie30phy>;
>  		phy-names = "pcie-phy";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
> index 672d748fcc67..f229cb49da68 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
> @@ -23,3 +23,7 @@ &pcie3x4_ep {
>  	vpcie3v3-supply = <&vcc3v3_pcie30>;
>  	status = "okay";
>  };
> +
> +&mmu600_pcie {
> +	status = "disabled";
> +};
> --
> 2.48.1
>
Robin Murphy Feb. 11, 2025, 5:49 p.m. UTC | #2
On 2025-02-07 2:39 pm, Niklas Cassel wrote:
> Commit da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU
> on the rk3588 SoC") enabled the mmu600_pcie IOMMU, both in the normal case
> (when all PCIe controllers are running in Root Complex mode) and in the
> case when running the pcie3x4 PCIe controller in Endpoint mode.
> 
> There have been no issues detected when running the PCIe controllers in
> Root Complex mode. During PCI probe time, we will add a SID to the IOMMU
> for each PCI device enumerated on the bus, including the root port itself.
> 
> However, when running the pcie3x4 PCIe controller in Endpoint mode, we
> will only add a single SID to the IOMMU (the SID specified in the iommus
> DT property).
> 
> The enablement of IOMMU in endpoint mode was verified on setup with two
> Rock 5b:s, where the BDF of the Root Complex has BDF (00:00.0).
> 
> A Root Complex sending a TLP to the Endpoint will have Requester ID set
> to the BDF of the initiator. On the EP side, the Requester ID will then
> be used as the SID. This works fine if the Root Complex has a BDF that
> matches the iommus DT property, however, if the Root Complex has any other
> BDF, we will see something like:
> arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x1600 ssid: 0x0
> on the endpoint side.
> 
> For PCIe controllers running in endpoint mode that always uses the
> incoming Requester ID as the SID, the iommus DT property simply isn't
> a viable solution. (Neither is iommu-map a viable solution, as there is
> no enumeration done on the endpoint side.)

Well, strictly the controller should own all the StreamIDs it's capable 
of emitting - if that's just one per possible bus number (as the iATU 
FUNC_NUM/FUNC_BYPASS stuff appears to suggest?) then 256 "iommus" 
entries isn't *entirely* unmanageable. If it were to support being 
arbitrary (or multiple) devices/functions, though, then we probably 
would want to look for a different solution, because nobody wants a 
512KB DT property... :)

Thanks,
Robin.

> Thus, partly revert commit da92d3dfc871 ("arm64: dts: rockchip: enable the
> mmu600_pcie IOMMU on the rk3588 SoC") by disabling the PCI IOMMU when
> running the pcie3x4 PCIe controller in Endpoint mode.
> 
> Since the PCI IOMMU is working as expected in the normal case, keep it
> enabled when running all PCIe controllers in Root Complex mode.
> 
> Fixes: da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU on the rk3588 SoC")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>   arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi           | 1 -
>   arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso | 4 ++++
>   2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> index 42b57e27ee4c..b5feefda9d2d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> @@ -215,7 +215,6 @@ pcie3x4_ep: pcie-ep@fe150000 {
>   		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
>   				  "dma0", "dma1", "dma2", "dma3";
>   		max-link-speed = <3>;
> -		iommus = <&mmu600_pcie 0x0000>;
>   		num-lanes = <4>;
>   		phys = <&pcie30phy>;
>   		phy-names = "pcie-phy";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
> index 672d748fcc67..f229cb49da68 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
> @@ -23,3 +23,7 @@ &pcie3x4_ep {
>   	vpcie3v3-supply = <&vcc3v3_pcie30>;
>   	status = "okay";
>   };
> +
> +&mmu600_pcie {
> +	status = "disabled";
> +};
Niklas Cassel Feb. 11, 2025, 7:44 p.m. UTC | #3
Hello Robin,

On Tue, Feb 11, 2025 at 05:49:29PM +0000, Robin Murphy wrote:
> On 2025-02-07 2:39 pm, Niklas Cassel wrote:
> > Commit da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU
> > on the rk3588 SoC") enabled the mmu600_pcie IOMMU, both in the normal case
> > (when all PCIe controllers are running in Root Complex mode) and in the
> > case when running the pcie3x4 PCIe controller in Endpoint mode.
> > 
> > There have been no issues detected when running the PCIe controllers in
> > Root Complex mode. During PCI probe time, we will add a SID to the IOMMU
> > for each PCI device enumerated on the bus, including the root port itself.
> > 
> > However, when running the pcie3x4 PCIe controller in Endpoint mode, we
> > will only add a single SID to the IOMMU (the SID specified in the iommus
> > DT property).
> > 
> > The enablement of IOMMU in endpoint mode was verified on setup with two
> > Rock 5b:s, where the BDF of the Root Complex has BDF (00:00.0).
> > 
> > A Root Complex sending a TLP to the Endpoint will have Requester ID set
> > to the BDF of the initiator. On the EP side, the Requester ID will then
> > be used as the SID. This works fine if the Root Complex has a BDF that
> > matches the iommus DT property, however, if the Root Complex has any other
> > BDF, we will see something like:
> > arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x1600 ssid: 0x0
> > on the endpoint side.
> > 
> > For PCIe controllers running in endpoint mode that always uses the
> > incoming Requester ID as the SID, the iommus DT property simply isn't
> > a viable solution. (Neither is iommu-map a viable solution, as there is
> > no enumeration done on the endpoint side.)
> 
> Well, strictly the controller should own all the StreamIDs it's capable of
> emitting - if that's just one per possible bus number (as the iATU
> FUNC_NUM/FUNC_BYPASS stuff appears to suggest?) then 256 "iommus" entries
> isn't *entirely* unmanageable. If it were to support being arbitrary (or
> multiple) devices/functions, though, then we probably would want to look for
> a different solution, because nobody wants a 512KB DT property... :)

Well, FUNC_BYPASS and FUNC_NUM is in the IATU_REGION_CTRL_1_OFF_OUTBOUND_i
register, so it is for outbound PCI TLPs, not inbound PCI TLPs.
(Only inbound PCI TLPs will get sent to the IOMMU after passing the iATU).

There is FUNC_NUM in IATU_REGION_CTRL_1_OFF_INBOUND_i register, but it has
a different meaning. (An inbound PCI TLP will only get translated if the
FUNC_NUM matches the value in this register).
(This check is only performed if the "Function Number Match Enable" bit
of the "iATU Region Control 2 Register" is set.)


The SMMU on the rock5b, when running the PCIe controller in endpoint mode,
has printed the following:
arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x1600 ssid: 0x0
but also:
arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x20 ssid: 0x0

depending on which host system we had connected to it.

So I'm a bit worried that 256 "iommus" entries will not be enough.

I don't have any idea on how to solve this, but right now I don't see any
other option but to disable the IOMMU when running the PCIe controller in
endpoint mode.

(We have no issues when running with the iommu enabled when running the PCIe
controller(s) in Root Complex mode.)


Kind regards,
Niklas
Robin Murphy Feb. 11, 2025, 8:03 p.m. UTC | #4
On 2025-02-11 7:44 pm, Niklas Cassel wrote:
> Hello Robin,
> 
> On Tue, Feb 11, 2025 at 05:49:29PM +0000, Robin Murphy wrote:
>> On 2025-02-07 2:39 pm, Niklas Cassel wrote:
>>> Commit da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU
>>> on the rk3588 SoC") enabled the mmu600_pcie IOMMU, both in the normal case
>>> (when all PCIe controllers are running in Root Complex mode) and in the
>>> case when running the pcie3x4 PCIe controller in Endpoint mode.
>>>
>>> There have been no issues detected when running the PCIe controllers in
>>> Root Complex mode. During PCI probe time, we will add a SID to the IOMMU
>>> for each PCI device enumerated on the bus, including the root port itself.
>>>
>>> However, when running the pcie3x4 PCIe controller in Endpoint mode, we
>>> will only add a single SID to the IOMMU (the SID specified in the iommus
>>> DT property).
>>>
>>> The enablement of IOMMU in endpoint mode was verified on setup with two
>>> Rock 5b:s, where the BDF of the Root Complex has BDF (00:00.0).
>>>
>>> A Root Complex sending a TLP to the Endpoint will have Requester ID set
>>> to the BDF of the initiator. On the EP side, the Requester ID will then
>>> be used as the SID. This works fine if the Root Complex has a BDF that
>>> matches the iommus DT property, however, if the Root Complex has any other
>>> BDF, we will see something like:
>>> arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x1600 ssid: 0x0
>>> on the endpoint side.
>>>
>>> For PCIe controllers running in endpoint mode that always uses the
>>> incoming Requester ID as the SID, the iommus DT property simply isn't
>>> a viable solution. (Neither is iommu-map a viable solution, as there is
>>> no enumeration done on the endpoint side.)
>>
>> Well, strictly the controller should own all the StreamIDs it's capable of
>> emitting - if that's just one per possible bus number (as the iATU
>> FUNC_NUM/FUNC_BYPASS stuff appears to suggest?) then 256 "iommus" entries
>> isn't *entirely* unmanageable. If it were to support being arbitrary (or
>> multiple) devices/functions, though, then we probably would want to look for
>> a different solution, because nobody wants a 512KB DT property... :)
> 
> Well, FUNC_BYPASS and FUNC_NUM is in the IATU_REGION_CTRL_1_OFF_OUTBOUND_i
> register, so it is for outbound PCI TLPs, not inbound PCI TLPs.
> (Only inbound PCI TLPs will get sent to the IOMMU after passing the iATU).
> 
> There is FUNC_NUM in IATU_REGION_CTRL_1_OFF_INBOUND_i register, but it has
> a different meaning. (An inbound PCI TLP will only get translated if the
> FUNC_NUM matches the value in this register).
> (This check is only performed if the "Function Number Match Enable" bit
> of the "iATU Region Control 2 Register" is set.)

Sigh, the "i" in iATU doesn't stand for "inbound", does it... major 
brain fart there.

> The SMMU on the rock5b, when running the PCIe controller in endpoint mode,
> has printed the following:
> arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x1600 ssid: 0x0
> but also:
> arm-smmu-v3 fc900000.iommu: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x20 ssid: 0x0

Yeah, that one pretty much settles it - we can certainly expect host 
root ports with nonzero device numbers, so that's at least 13 bits of 
the StreamID space to cover, which isn't going to fly.

> depending on which host system we had connected to it.
> 
> So I'm a bit worried that 256 "iommus" entries will not be enough.
> 
> I don't have any idea on how to solve this, but right now I don't see any
> other option but to disable the IOMMU when running the PCIe controller in
> endpoint mode.

Agreed; FWIW, for the patch as it is:

Acked-by: Robin Murphy <robin.murphy@arm.com>

> (We have no issues when running with the iommu enabled when running the PCIe
> controller(s) in Root Complex mode.)
> 
> 
> Kind regards,
> Niklas
Heiko Stübner Feb. 11, 2025, 8:35 p.m. UTC | #5
On Fri, 07 Feb 2025 15:39:01 +0100, Niklas Cassel wrote:
> Commit da92d3dfc871 ("arm64: dts: rockchip: enable the mmu600_pcie IOMMU
> on the rk3588 SoC") enabled the mmu600_pcie IOMMU, both in the normal case
> (when all PCIe controllers are running in Root Complex mode) and in the
> case when running the pcie3x4 PCIe controller in Endpoint mode.
> 
> There have been no issues detected when running the PCIe controllers in
> Root Complex mode. During PCI probe time, we will add a SID to the IOMMU
> for each PCI device enumerated on the bus, including the root port itself.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: rockchip: disable IOMMU when running rk3588 in PCIe endpoint mode
      commit: 7d1163fc08936fcb5cf5d9daf366c322c3b4e882

Best regards,
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
index 42b57e27ee4c..b5feefda9d2d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
@@ -215,7 +215,6 @@  pcie3x4_ep: pcie-ep@fe150000 {
 		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
 				  "dma0", "dma1", "dma2", "dma3";
 		max-link-speed = <3>;
-		iommus = <&mmu600_pcie 0x0000>;
 		num-lanes = <4>;
 		phys = <&pcie30phy>;
 		phy-names = "pcie-phy";
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
index 672d748fcc67..f229cb49da68 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtso
@@ -23,3 +23,7 @@  &pcie3x4_ep {
 	vpcie3v3-supply = <&vcc3v3_pcie30>;
 	status = "okay";
 };
+
+&mmu600_pcie {
+	status = "disabled";
+};