| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ce534365-0110-4aba-b8b5-0a46c5ea81d0@arm.com>
Date: Fri, 27 Sep 2024 18:54:38 +0100
From: Robin Murphy <robin.murphy@....com>
To: Abdellatif El Khlifi <abdellatif.elkhlifi@....com>,
mathieu.poirier@...aro.org
Cc: Adam.Johnston@....com, Hugues.KambaMpiana@....com, Drew.Reed@....com,
andersson@...nel.org, conor+dt@...nel.org, devicetree@...r.kernel.org,
krzysztof.kozlowski+dt@...aro.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-remoteproc@...r.kernel.org,
liviu.dudau@....com, lpieralisi@...nel.org, robh@...nel.org,
sudeep.holla@....com
Subject: Re: [PATCH v2 1/5] dt-bindings: remoteproc: sse710: Add the External
Systems remote processors
On 22/08/2024 6:09 pm, Abdellatif El Khlifi wrote:
> Add devicetree binding schema for the External Systems remote processors
>
> The External Systems remote processors are provided on the Corstone-1000
> IoT Reference Design Platform via the SSE-710 subsystem.
>
> For more details about the External Systems, please see Corstone SSE-710
> subsystem features [1].
>
> [1]: https://developer.arm.com/documentation/102360/0000/Overview-of-Corstone-1000/Corstone-SSE-710-subsystem-features
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@....com>
> ---
> .../remoteproc/arm,sse710-extsys.yaml | 90 +++++++++++++++++++
> 1 file changed, 90 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml
> new file mode 100644
> index 000000000000..827ba8d962f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/arm,sse710-extsys.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SSE-710 External System Remote Processor
Thing is, this is not describing SSE-710. As far as I can work out, it
is describing the firmware and hardware that a particular example
implementation of the Corstone-1000 kit has chosen to put in the
"external system" hole in the SSE-710 within that kit.
If I license SSE-710 alone or even the Corstone-1000 kit, I can put
whatever I want in *my* implementation of those subsystems, so there
clearly cannot possibly be a common binding for that.
For instance what if I decide to combine a Cortex-M core plus a radio
and some other glue as my external subsystem? Do we have dozens of
remoteproc bindings and drivers for weird fixed-function remoteprocs
whose "firmware-name" implies a Bluetooth protocol stack? No, we treat
them as Bluetooth controller devices. Look at
devicetree/bindings/sound/fsl,rpmsg.yaml - it's even unashamedly an
rpmsg client, but it's still not abusing the remoteproc subsystem
because its function to the host OS is as an audio controller, not an
arbitrarily configurable processor.
As I said before, all SSE-710 actually implements is a reset mechanism,
so it only seems logical to model it as a reset controller, e.g.
something like:
hbsys: syscon@xyz {
compatible = "arm,sse710-host-base-sysctrl", "syscon";
reg = <xyz>;
#reset-cells = <1>;
};
something {
...
resets = <&hbsys 0>;
};
something-else {
...
resets = <&hbsys 1>;
};
Then if there is actually any meaningful functionality in the default
extsys0 firmware preloaded on the FPGA setup then define a binding for
"arm,corstone1000-an550-extsys0" to describe whatever that actually
does. If a user chooses to create and load their own different firmware,
they're going to need their own binding and driver for whatever *that*
firmware does.
FWIW, driver-wise the mapping to the reset API seems straightforward -
.assert hits RST_REQ, .deassert clears CPUWAIT (.status is possibly a
combination of CPUWAIT and RST_ACK?)
Thanks,
Robin.
> +
> +maintainers:
> + - Abdellatif El Khlifi <abdellatif.elkhlifi@....com>
> + - Hugues Kamba Mpiana <hugues.kambampiana@....com>
> +
> +description: |
> + SSE-710 is an heterogeneous subsystem supporting up to two remote
> + processors aka the External Systems.
> +
> +properties:
> + compatible:
> + enum:
> + - arm,sse710-extsys
> +
> + firmware-name:
> + description:
> + The default name of the firmware to load to the remote processor.
> +
> + '#extsys-id':
> + description:
> + The External System ID.
> + enum: [0, 1]
> +
> + mbox-names:
> + items:
> + - const: txes0
> + - const: rxes0
> +
> + mboxes:
> + description:
> + The list of Message Handling Unit (MHU) channels used for bidirectional
> + communication. This property is only required if the virtio-based Rpmsg
> + messaging bus is used. For more details see the Arm MHUv2 Mailbox
> + Controller at devicetree/bindings/mailbox/arm,mhuv2.yaml
> +
> + minItems: 2
> + maxItems: 2
> +
> + memory-region:
> + description:
> + If present, a phandle for a reserved memory area that used for vdev
> + buffer, resource table, vring region and others used by the remote
> + processor.
> + minItems: 2
> + maxItems: 32
> +
> +required:
> + - compatible
> + - firmware-name
> + - '#extsys-id'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + extsys0_vring0: vdev0vring0@...01000 {
> + reg = <0 0x82001000 0 0x8000>;
> + no-map;
> + };
> +
> + extsys0_vring1: vdev0vring1@...09000 {
> + reg = <0 0x82009000 0 0x8000>;
> + no-map;
> + };
> + };
> +
> + syscon@...10000 {
> + compatible = "arm,sse710-host-base-sysctrl", "simple-mfd", "syscon";
> + reg = <0x1a010000 0x1000>;
> +
> + extsys0 {
> + compatible = "arm,sse710-extsys";
> + #extsys-id = <0>;
> + firmware-name = "es_flashfw.elf";
> + mbox-names = "txes0", "rxes0";
> + mboxes = <&mhu0_hes0 0 1>, <&mhu0_es0h 0 1>;
> + memory-region = <&extsys0_vring0>, <&extsys0_vring1>;
> + };
> + };
Powered by blists - more mailing lists