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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 6 Jun 2018 14:31:07 -0700
From:   Frank Rowand <frowand.list@...il.com>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     Michel Pollet <michel.pollet@...renesas.com>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>,
        Simon Horman <horms@...ge.net.au>,
        Michel Pollet <buserror+upstream@...il.com>,
        Mark Rutland <mark.rutland@....com>,
        Phil Edworthy <phil.edworthy@...esas.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Stefan Wahren <stefan.wahren@...e.com>,
        Magnus Damm <magnus.damm@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Douglas Anderson <dianders@...omium.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Carlo Caione <carlo@...lessm.com>,
        Andreas Färber <afaerber@...e.de>,
        Frank Rowand <frank.rowand@...y.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler
 driver

On 06/06/18 12:35, Rob Herring wrote:
> On Wed, Jun 6, 2018 at 2:30 PM, Frank Rowand <frowand.list@...il.com> wrote:
>> Hi Michel,
>>
>> On 06/05/18 23:36, Michel Pollet wrote:
>>> Hi Frank,
>>>
>>> On 05 June 2018 18:34, Frank wrote:
>>>> On 06/05/18 04:28, Michel Pollet wrote:
>>>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>>>> it requires a special enable method to get it started.
>>>>>
>>>>> Signed-off-by: Michel Pollet <michel.pollet@...renesas.com>
>>>>> ---
>>>>>  arch/arm/mach-shmobile/Makefile        |  1 +
>>>>>  arch/arm/mach-shmobile/smp-r9a06g032.c | 79
>>>>> ++++++++++++++++++++++++++++++++++
>>>>>  2 files changed, 80 insertions(+)
>>>>>  create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>>
>>>>> diff --git a/arch/arm/mach-shmobile/Makefile
>>>>> b/arch/arm/mach-shmobile/Makefile index 1939f52..d7fc98f 100644
>>>>> --- a/arch/arm/mach-shmobile/Makefile
>>>>> +++ b/arch/arm/mach-shmobile/Makefile
>>>>> @@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)+= smp-sh73a0.o
>>>> headsmp-scu.o platsmp-scu.o
>>>>>  smp-$(CONFIG_ARCH_R8A7779)+= smp-r8a7779.o headsmp-scu.o
>>>> platsmp-scu.o
>>>>>  smp-$(CONFIG_ARCH_R8A7790)+= smp-r8a7790.o
>>>>>  smp-$(CONFIG_ARCH_R8A7791)+= smp-r8a7791.o
>>>>> +smp-$(CONFIG_ARCH_R9A06G032)+= smp-r9a06g032.o
>>>>>  smp-$(CONFIG_ARCH_EMEV2)+= smp-emev2.o headsmp-scu.o
>>>> platsmp-scu.o
>>>>>
>>>>>  # PM objects
>>>>> diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>> b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>> new file mode 100644
>>>>> index 0000000..cd40e6e
>>>>> --- /dev/null
>>>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>> @@ -0,0 +1,79 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * R9A06G032 Second CA7 enabler.
>>>>> + *
>>>>> + * Copyright (C) 2018 Renesas Electronics Europe Limited
>>>>> + *
>>>>> + * Michel Pollet <michel.pollet@...renesas.com>,
>>>> <buserror@...il.com>
>>>>> + * Derived from action,s500-smp
>>>>> + */
>>>>> +
>>>>> +#include <linux/io.h>
>>>>> +#include <linux/of.h>
>>>>> +#include <linux/of_address.h>
>>>>> +#include <linux/smp.h>
>>>>> +
>>>>> +/*
>>>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>>>> +it after
>>>>> + * writing an address into the BOOTADDR register of sysctrl.
>>>>> + *
>>>>> + * So the default value of the "cpu-release-addr" corresponds to
>>>> BOOTADDR...
>>>>> + *
>>>>> + * *However* the BOOTADDR register is not available when the kernel
>>>>> + * starts in NONSEC mode.
>>>>> + *
>>>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>>>> +pen
>>>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>>>> +SRAM address,
>>>>> + * which is not restricted.
>>>>
>>>> The binding document for cpu-release-addr does not have a definition for 32
>>>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>>>> for 32 bit arm to patch 1.
>>>
>>> Hmmm I do find a definition in
>>> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
>>> added my 'enable-method' -- And it is already used as 32 bits in at least
>>> arch/arm/boot/dts/stih407-family.dtsi.
>>
>> From cpus.txt:
>>
>>         - cpu-release-addr
>>                 Usage: required for systems that have an "enable-method"
>>                        property value of "spin-table".
>>                 Value type: <prop-encoded-array>
>>                 Definition:
>>                         # On ARM v8 64-bit systems must be a two cell
>>                           property identifying a 64-bit zero-initialised
>>                           memory location.
>>
>> The definition specifies a two cell property for 64-bit systems.
>>
>> Please add to the definition that cpu-release-addr is a one cell property
>> for 32-bit systems.
> 
> Actually, this is all already documented in the DT spec and it is
> always 2 cells[1]. We should perhaps just remove whatever is
> duplicated from the spec.

Thanks for noting that.  I jumped to the (incorrect) conclusion that the
property should be one cell based on the code and the .dtsi.

There are about 4 more emails following this in the thread that discuss
what size cpu-release-addr should be.  Whatever the end result is (one
cell or two or based on some #XXX-calls value), it needs to be documented
consistently in the binding and in the DT spec (or preferably only in the
DT spec), and if it is a two cell property for this system then
smp-r9a06g032.c and r9a06g032.dtsi need to be adjusted to reflect that.

-Frank

> 
> Rob
> 
> [1]
>    ``cpu-release-addr``   | SD  | ``<u64>``        The
> cpu-release-addr property is required for
>                                                    cpu nodes that have
> an enable-method property
>                                                    value of
> ``"spin-table"``. The value specifies the
>                                                    physical address of
> a spin table entry that
>                                                    releases a
> secondary CPU from its spin loop.
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ