Re: [PATCH] Describe "fail" status for /cpus/cpu* nodes

Re: [PATCH] Describe "fail" status for /cpus/cpu* nodes

[Matthias Schiffer]

On Thu, 2021-09-16 at 16:10 +0200, Matthias Schiffer wrote:
> There are situations where it is desirable to use the same base Device
> Tree for devices with a different number of CPUs: There may be CPU
> variants with different numbers of cores that can be used interchangably
> on the same mainboard, or there are multiple CPU sockets. Not needing to
> explicitly build a device tree for each such variant can make
> maintenance significantly easier.
> 
> For this to work, a system firmware / bootloader needs to adjust the
> Device Tree by removing or disabling the excess CPU nodes. However, this
> is currently not easily possible due to the special meaning of the
> "disabled" status for CPU nodes:
> 
> - A "disabled" CPU node is interpreted as inactive, but existent. The
>   Linux kernel will attempt to enable such CPUs on boot, which will
>   obviously fail for non-existent CPUs
> - Removing the CPU node altogether from a Device Tree is much more
>   complex than setting a single property, as it may leave dangling
>   phandle references, often requiring specific knowledge of other nodes'
>   structure to deal with them.
> 
> In the discussion [1] it was suggested to introduce a new status value
> for CPUs that should really not be used at all. Rob proposed to use the
> value "fail", which already exists in the generic definitions of the
> status property.
> 
> [1] https://www.lkml.org/lkml/2020/8/26/1237

Hi,
I haven't received any feedback regarding this spec update yet. Should
I also send a kernel patch that actually implements this behaviour?

Do properties described in the spec also need to be documented in the
kernel's Documentation/devicetree/bindings? It seems that there is no
generic "cpu" binding documentation at the moment, only arch-specific
variants.



> 
> Suggested-by: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> ---
>  source/chapter3-devicenodes.rst | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/source/chapter3-devicenodes.rst b/source/chapter3-devicenodes.rst
> index 1cea148..9d8ef6e 100644
> --- a/source/chapter3-devicenodes.rst
> +++ b/source/chapter3-devicenodes.rst
> @@ -614,8 +614,8 @@ standard properties with specific applicable detail.
>                                                     CPU. This property shall be present for nodes
>                                                     representing CPUs in a symmetric
>                                                     multiprocessing (SMP) configuration. For a CPU
> -                                                   node the meaning of the ``"okay"`` and
> -                                                   ``"disabled"`` values are as follows:
> +                                                   node the meaning of the ``"okay"``, ``"disabled"``
> +                                                   and ``"fail"`` values are as follows:
>  
>                                                     ``"okay"`` :
>                                                        The CPU is running.
> @@ -623,6 +623,9 @@ standard properties with specific applicable detail.
>                                                     ``"disabled"`` :
>                                                        The CPU is in a quiescent state.
>  
> +                                                   ``"fail"`` :
> +                                                      The CPU is not operational or does not exist.
> +
>                                                     A quiescent CPU is in a state where it cannot
>                                                     interfere with the normal operation of other
>                                                     CPUs, nor can its state be affected by the
> @@ -639,6 +642,11 @@ standard properties with specific applicable detail.
>                                                     loop, held in reset, and electrically isolated
>                                                     from the system bus or in another
>                                                     implementation dependent state.
> +
> +                                                   A CPU with ``"fail"`` status does not affect the
> +                                                   system in any way.
> +                                                   The status is assigned to nodes for which no
> +                                                   corresponding CPU exists.
>     ``enable-method``      | SD  | ``<stringlist>`` Describes the method by which a CPU in a
>                                                     disabled state is enabled. This property is
>                                                     required for CPUs with a status property with

Re: [PATCH] Describe "fail" status for /cpus/cpu* nodes

[Rob Herring]

On Fri, Oct 8, 2021 at 5:31 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:
>
> On Thu, 2021-09-16 at 16:10 +0200, Matthias Schiffer wrote:
> > There are situations where it is desirable to use the same base Device
> > Tree for devices with a different number of CPUs: There may be CPU
> > variants with different numbers of cores that can be used interchangably
> > on the same mainboard, or there are multiple CPU sockets. Not needing to
> > explicitly build a device tree for each such variant can make
> > maintenance significantly easier.
> >
> > For this to work, a system firmware / bootloader needs to adjust the
> > Device Tree by removing or disabling the excess CPU nodes. However, this
> > is currently not easily possible due to the special meaning of the
> > "disabled" status for CPU nodes:
> >
> > - A "disabled" CPU node is interpreted as inactive, but existent. The
> >   Linux kernel will attempt to enable such CPUs on boot, which will
> >   obviously fail for non-existent CPUs
> > - Removing the CPU node altogether from a Device Tree is much more
> >   complex than setting a single property, as it may leave dangling
> >   phandle references, often requiring specific knowledge of other nodes'
> >   structure to deal with them.
> >
> > In the discussion [1] it was suggested to introduce a new status value
> > for CPUs that should really not be used at all. Rob proposed to use the
> > value "fail", which already exists in the generic definitions of the
> > status property.
> >
> > [1] https://www.lkml.org/lkml/2020/8/26/1237
>
> Hi,
> I haven't received any feedback regarding this spec update yet. Should
> I also send a kernel patch that actually implements this behaviour?

Looks fine to me, just hadn't gotten around to applying.

> Do properties described in the spec also need to be documented in the
> kernel's Documentation/devicetree/bindings? It seems that there is no
> generic "cpu" binding documentation at the moment, only arch-specific
> variants.

https://github.com/devicetree-org/dt-schema/blob/main/schemas/cpus.yaml

Rob