Enable Service layer driver for Agilex5
Subsystems (2)
v3 cover Enable Service layer driver for Agilex5
This patch series introduces support for the Agilex5 service layer driver (SVC) in the Linux kernel. The changes span across device tree bindings, platform DTS files, and the Stratix10 SVC firmware driver.
These changes are necessary to enable firmware communication on Agilex5 SoCs via the SVC interface, similar to existing support for Stratix10 and Agilex platforms.
Agilex5 introduces the ability for the TBU to operate in non-secure mode, making it accessible to Linux through the IOMMU framework. This key difference enables improved memory management in non-secure environments. As a result, this series extends the SVC driver and device tree bindings to support IOMMU integration for Agilex5.
---
Notes:
This patch series is applied on socfpga maintainer's tree
https://git.kernel.org/pub/scm/linux/kernel/git/dinguyen/linux.git/log/?h=socfpga_dts_for_v6.19
Changes in v3:
- Remove driver changes in driver as it is fully compatible
- Add iommu entry and rewrite the git commit message to describe why the changes is required.
Changes in v2:
- Add driver changes for Agilex5-svc compatible
---
Khairul Anuar Romli (2):
dt-bindings: firmware: svc: Add IOMMU support for Agilex5
arm64: dts: intel: Add Agilex5 SVC node with memory region
.../firmware/intel,stratix10-svc.yaml | 20 +++++++++++++++---- .../arm64/boot/dts/intel/socfpga_agilex5.dtsi | 9 +++++++++ 2 files changed, 25 insertions(+), 4 deletions(-)
Signature · 1 line
2.43.7
PATCH v3 1/2 dt-bindings: firmware: svc: Add IOMMU support for Agilex5
In Agilex5, the TBU (Translation Buffer Unit) can now operate in non-secure mode, enabling Linux to utilize it through the IOMMU framework. This allows improved memory management capabilities in non-secure environments. With Agilex5 lifting this restriction, we are now extending the device tree bindings to support IOMMU for the Agilex5 SVC.
---
Changes in v3:
- Add iommu property.
- Add logical check where only Agilex5 required iommus
- Rewrite the commit message to explain why this changes is needed.
Changes in v2:
- Reprase commit message to exclude iommu
--- .../firmware/intel,stratix10-svc.yaml | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +16 -4
@@ -34,6 +34,7 @@ properties: enum: - intel,stratix10-svc - intel,agilex-svc + - intel,agilex5-svc method: description: |
@@ -54,14 +55,25 @@ properties: reserved memory region for the service layer driver to communicate with the secure device manager. + iommus: + maxItems: 1 + fpga-mgr: $ref: /schemas/fpga/intel,stratix10-soc-fpga-mgr.yaml description: Optional child node for fpga manager to perform fabric configuration. -required: - - compatible - - method - - memory-region +allOf: + - required: + - compatible + - method + - memory-region + - if: + properties: + compatible: + const: intel,agilex5-svc + then: + required: + - iommus additionalProperties: false
Signature · 1 line
2.43.7
Re: [PATCH v3 1/2] dt-bindings: firmware: svc: Add IOMMU support for Agilex5
Quoted 9 lines · Khairul Anuar Romli
In Agilex5, the TBU (Translation Buffer Unit) can now operate in non-secure mode, enabling Linux to utilize it through the IOMMU framework. This allows improved memory management capabilities in non-secure environments. With Agilex5 lifting this restriction, we are now extending the device tree bindings to support IOMMU for the Agilex5 SVC.
Signed-off-by Khairul Anuar Romli <[email protected]>Reviewed-by Adrian Ng Ho Yin <[email protected]>Reviewed-by Niravkumar L Rabara <[email protected]>
Two reviews but...
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml properties: · line 69
Quoted 10 lines
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +5
-4-required: - - compatible - - method - - memory-region +allOf: + - required: + - compatible + - method + - memory-region
... none told you this is not the correct syntax / style?
Were these reviews really happening? What exactly was reviewed here?
Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: firmware: svc: Add IOMMU support for Agilex5
Quoted 26 lines · Krzysztof Kozlowski
Quoted 9 lines · Khairul Anuar Romli
In Agilex5, the TBU (Translation Buffer Unit) can now operate in non-secure mode, enabling Linux to utilize it through the IOMMU framework. This allows improved memory management capabilities in non-secure environments. With Agilex5 lifting this restriction, we are now extending the device tree bindings to support IOMMU for the Agilex5 SVC.
Signed-off-by Khairul Anuar Romli <[email protected]>Reviewed-by Adrian Ng Ho Yin <[email protected]>Reviewed-by Niravkumar L Rabara <[email protected]>Two reviews but...
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml properties: · line 69
Quoted 10 lines
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +5
-4-required: - - compatible - - method - - memory-region +allOf: + - required: + - compatible + - method + - memory-region... none told you this is not the correct syntax / style?
Quoted 5 lines
Were these reviews really happening? What exactly was reviewed here?
Best regards,
Krzysztof
The peer review was on the property and compatible added in this patch. I was at fault for not emphasize to check on the syntax / style for the logical changes.
I refers to Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml and the changes could be as follows:
required:
- compatible
- method
- memory-region
+allOf:
+ - if:
+ properties:
+ compatible:
+ const: intel,agilex5-svc
+ then:
+ required:
+ - iommus
Do I need to re-specify the rest of the compatible property other than intel,agilex5-svc and their required properties under the allOf block?
My objective is to specify that iommus only for intel,agilex5-svc but not the rest of devices.
Thanks.
Best Regards,
Khairul
Re: [PATCH v3 1/2] dt-bindings: firmware: svc: Add IOMMU support for Agilex5
Quoted 29 lines · Khairul Anuar
Quoted 28 lines · Krzysztof Kozlowski
Quoted 9 lines · Khairul Anuar Romli
In Agilex5, the TBU (Translation Buffer Unit) can now operate in non-secure mode, enabling Linux to utilize it through the IOMMU framework. This allows improved memory management capabilities in non-secure environments. With Agilex5 lifting this restriction, we are now extending the device tree bindings to support IOMMU for the Agilex5 SVC.
Signed-off-by Khairul Anuar Romli <[email protected]>Reviewed-by Adrian Ng Ho Yin <[email protected]>Reviewed-by Niravkumar L Rabara <[email protected]>Two reviews but...
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml properties: · line 69
Quoted 10 lines
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +5
-4-required: - - compatible - - method - - memory-region +allOf: + - required: + - compatible + - method + - memory-region... none told you this is not the correct syntax / style?
Were these reviews really happening? What exactly was reviewed here?
Quoted 7 lines
Quoted 3 lines
Best regards,
KrzysztofThe peer review was on the property and compatible added in this patch. I was at fault for not emphasize to check on the syntax / style for the logical changes.
Rather the reviewers gave you just simple acks without doing their job. That's a 10 liner patch...
Feels like just to tick gerrit or satisfy internal procedures.
If internal reviewers give you such "okay" marks, without adhering to reviewers statement of oversight, you can freely ignore them and instead educate them to provide full review on the mailing list.
Quoted 18 lines
I refers to
Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yamland the changes could be as follows:required:
- compatible
- method
- memory-regionDocumentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +8
-0+allOf: + - if: + properties: + compatible: + const: intel,agilex5-svc + then: + required: + - iommus
Quoted 5 lines
Do I need to re-specify the rest of the compatible property other than intel,agilex5-svc and their required properties under the allOf block?
My objective is to specify that iommus only for intel,agilex5-svc but not the rest of devices.
It all depends whether other devices also use IOMMU.
Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: firmware: svc: Add IOMMU support for Agilex5
In reply to Khairul Anuar RomliQuoted 47 lines · Khairul Anuar
In Agilex5, the TBU (Translation Buffer Unit) can now operate in non-secure mode, enabling Linux to utilize it through the IOMMU framework. This allows improved memory management capabilities in non-secure environments. With Agilex5 lifting this restriction, we are now extending the device tree bindings to support IOMMU for the Agilex5 SVC.
Signed-off-by Khairul Anuar Romli <[email protected]>Reviewed-by Adrian Ng Ho Yin <[email protected]>Reviewed-by Niravkumar L Rabara <[email protected]>---
Changes in v3:
- Add iommu property.
- Add logical check where only Agilex5 required iommus
- Rewrite the commit message to explain why this changes is needed.
Changes in v2:
- Reprase commit message to exclude iommu--- .../firmware/intel,stratix10-svc.yaml | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +4
-3@@ -34,6 +34,7 @@ properties: enum: - intel,stratix10-svc - intel,agilex-svc + - intel,agilex5-svc method: description: |@@ -54,14 +55,25 @@ properties: reserved memory region for the service layer driver to communicate with the secure device manager. + iommus: + maxItems: 1 + fpga-mgr: $ref: /schemas/fpga/intel,stratix10-soc-fpga-mgr.yaml description: Optional child node for fpga manager to perform fabric configuration. -required: - - compatible - - method
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml properties: · line 73
Quoted 10 lines
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +9
-1- - memory-region +allOf: + - required: + - compatible + - method + - memory-region + - if: + properties: + compatible: + const: intel,agilex5-svc
I am sending the next revision to use contains: enum: instead of const.
Quoted 6 lines
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +3
-0+ then: + required: + - iommus additionalProperties: false
PATCH v3 2/2 arm64: dts: intel: Add Agilex5 SVC node with memory region
In reply to Khairul Anuar RomliIntroduce the Stratix10 SoC Service Layer (SVC) node for Agilex5 SoCs. This node includes the compatible string "intel,agilex5-svc" and references a reserved memory region used for communication with the Secure Device Manager (SDM).
Agilex5 introduces changes in how reserved memory is mapped and accessed compared to previous SoC generations, particularly with the addition of IOMMU support. Unlike earlier platforms, Agilex5 enables the use of the Translation Buffer Unit (TBU) in non-secure mode, allowing Linux to access it through the IOMMU framework. This commit updates the device tree structure to support Agilex5-specific handling of the SVC interface, including the necessary bindings for IOMMU integration.
---
Changes in v3:
- include iommu property in svc node.
- Rephrase git commit message to describe iommu presence in Agilex5
Changes in v2:
- Rephrase commit message to exclude mentioning iommu
--- arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+)
arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi +9 -0
@@ -841,5 +841,14 @@ queue7 { }; }; }; + + firmware { + svc { + compatible = "intel,agilex5-svc"; + method = "smc"; + memory-region = <&service_reserved>; + iommus = <&smmu 10>; + }; + }; }; };
Signature · 1 line
2.43.7