linux-kernel - Re: [PATCH 10/13] openrisc: add simple_smp dts and defconfig for simulators

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]

Message-ID: <20170831130510.GC2609@lianli.shorne-pla.net>
Date:   Thu, 31 Aug 2017 22:05:10 +0900
From:   Stafford Horne <shorne@...il.com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Openrisc <openrisc@...ts.librecores.org>,
        Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>,
        Rob Herring <robh+dt@...nel.org>,
        Jonas Bonn <jonas@...thpole.se>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        devicetree@...r.kernel.org
Subject: Re: [PATCH 10/13] openrisc: add simple_smp dts and defconfig for
 simulators

On Thu, Aug 31, 2017 at 11:41:10AM +0100, Mark Rutland wrote:
> On Thu, Aug 31, 2017 at 07:03:11AM +0900, Stafford Horne wrote:
> > From: Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>
> > 
> > Simple enough to be compatible with simulation environments,
> > such as verilated systems, QEMU and other targets supporting OpenRISC
> > SMP.  This also supports our base FPGA SoC's if the cpu frequency is
> > upped to 50Mhz.
> > 
> > Signed-off-by: Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>
> > [shorne@...il.com: Added defconfig]
> > Signed-off-by: Stafford Horne <shorne@...il.com>
> > ---
> >  arch/openrisc/boot/dts/simple_smp.dts      | 58 ++++++++++++++++++++++++++
> >  arch/openrisc/configs/simple_smp_defconfig | 66 ++++++++++++++++++++++++++++++
> >  2 files changed, 124 insertions(+)
> >  create mode 100644 arch/openrisc/boot/dts/simple_smp.dts
> >  create mode 100644 arch/openrisc/configs/simple_smp_defconfig
> > 
> > diff --git a/arch/openrisc/boot/dts/simple_smp.dts b/arch/openrisc/boot/dts/simple_smp.dts
> > new file mode 100644
> > index 000000000000..47c54101baae
> > --- /dev/null
> > +++ b/arch/openrisc/boot/dts/simple_smp.dts
> > @@ -0,0 +1,58 @@
> > +/dts-v1/;
> > +/ {
> > +	compatible = "opencores,or1ksim";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	interrupt-parent = <&pic>;
> > +
> > +	chosen {
> > +		bootargs = "console=uart,mmio,0x90000000,115200";
> > +	};
> 
> Any reason this isn't using stdout-path?

I didn't know about stdout-path.  I will add it.

> > +
> > +	memory@0 {
> > +		device_type = "memory";
> > +		reg = <0x00000000 0x02000000>;
> > +	};
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		cpu@0 {
> > +			compatible = "opencores,or1200-rtlsvn481";
> > +			reg = <0>;
> > +			clock-frequency = <20000000>;
> > +		};
> > +		cpu@1 {
> > +			compatible = "opencores,or1200-rtlsvn481";
> > +			reg = <1>;
> > +			clock-frequency = <20000000>;
> > +		};
> > +	};
> 
> No enable-method or similar?
> 
> Is your SMP bringup/teardown architected?

There is no configurable enable-method yet.  The current SMP bringup method
is described in the architecture manual (SMP still under review).  If you
have any pointers that would be great.  See section 10.4

  https://github.com/stffrdhrn/doc/raw/arch-1.2-proposal/openrisc-arch-1.2-rev0.pdf

In brief, Currently for SMP bringup both CPU's are expected to reset
directly to the startup vectors.  The main CPU (coreid == 0) will start
initialization where the secondary cpu's either spin or sleep until
signalled by the main CPU.

Original patch had cpu's spinning, but after [8/13] secondary cpus will go
into Doze mode.

> 
> > +
> > +	ompic: ompic {
> > +		compatible = "ompic";
> 
> This needs a vendor prefix.

This has also been brought up on [5/13], the question of vendor was
anticipated and I discussed with Stefan before sending the patch.
OpenRISC is not part of opencores any longer and this ompic was definitely
not implemented with an existing vendor in mind.

Perhaps we can just call it openrisc,ompic.  I will comment more on the
other thread.

Thanks for the review.

-Stafford