Re: [PATCH 1/6] dt-bindings: firmware: Add arm,errata-management

[Marc Zyngier <maz@kernel.org>]

On Mon, 03 Apr 2023 10:15:19 +0100,
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> On 31/03/2023 18:58, James Morse wrote:
> > Hi Krzysztof
> > 
> > On 31/03/2023 09:29, Krzysztof Kozlowski wrote:
> >> On 30/03/2023 18:51, James Morse wrote:
> >>> The Errata Management SMCCC interface allows firmware to advertise whether
> >>> the OS is affected by an erratum, or if a higher exception level has
> >>> mitigated the issue. This allows properties of the device that are not
> >>> discoverable by the OS to be described. e.g. some errata depend on the
> >>> behaviour of the interconnect, which is not visible to the OS.
> >>>
> >>> Deployed devices may find it significantly harder to update EL3
> >>> firmware than the device tree. Erratum workarounds typically have to
> >>> fail safe, and assume the platform is affected putting correctness
> >>> above performance.
> >>>
> >>> Instead of adding a device-tree entry for any CPU errata that is
> >>> relevant (or not) to the platform, allow the device-tree to describe
> >>> firmware's responses for the SMCCC interface. This could be used as
> >>> the data source for the firmware interface, or be parsed by the OS if
> >>> the firmware interface is missing.
> >>>
> >>> Most errata can be detected from CPU id registers. These mechanisms
> >>> are only needed for the rare cases that external knowledge is needed.
> > 
> >>> diff --git a/Documentation/devicetree/bindings/firmware/arm,errata-management.yaml b/Documentation/devicetree/bindings/firmware/arm,errata-management.yaml
> >>> new file mode 100644
> >>> index 000000000000..9baeb3d35213
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/firmware/arm,errata-management.yaml
> >>> @@ -0,0 +1,77 @@
> >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/firmware/arm,errata-management.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>
> >> Except missing testing...
> > 
> > After a couple of hours of testing this, I went blind and missed that it was still
> > complaining about spaces.
> > 
> > 
> >>> +
> >>> +title: Errata Management Firmware Interface
> >>> +
> >>> +maintainers:
> >>> +  - James Morse <james.morse@arm.com>
> >>> +
> >>> +description: |+
> >>
> >> Do not need '|+'.
> >>
> >>> +  The SMC-CC has an erratum discovery interface that allows the OS to discover
> >>> +  whether a particular CPU is affected by a specific erratum when the
> >>> +  configurations affected is only known by firmware. See the specification of
> >>> +  the same title on developer.arm.com, document DEN0100.
> >>> +  Provide the values that should be used by the interface, either to supplement
> >>> +  firmware, or override the values firmware provides.
> >>
> >> Why? If you have the discovery interface, don't add stuff to the DT, but
> >> use that interface.

If you read the cover letter, you'll notice that *nobody* implements
the discovery mechanism, and yet we still need a way to identify
broken systems.

> > 
> > A DT property was explicitly requested by Marc Z on the RFC:
> > https://lore.kernel.org/linux-arm-kernel/86mt5dxxbc.wl-maz@kernel.org/
> > 
> > The concern is that platforms where the CPU is affected, but the issue is masked by the
> > interconnect will never bother with a firmware interface. The kernel can't know this, so
> > has to enable the workaround at the cost of performance.
> 
> It would have to bother DT, so same problem... DT is not optimization
> mechanism for SW decisions.

What does SW has to do with this? This describes the state of the
HW. The HW is broken, SW has no way to discover it otherwise, so DT
*is* the place to put it.

In any case, it is far easier to update a DT that your EL3 firmware.

	M.

-- 
Without deviation from the norm, progress is not possible.