Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Atish Patra <Atish.Patra@wdc.com>
To:     Palmer Dabbelt <palmer@sifive.com>
CC:     Marc Zyngier <marc.zyngier@arm.com>,
        "david.abdurachmanov@gmail.com" <david.abdurachmanov@gmail.com>,
        Christoph Hellwig <hch@infradead.org>,
        Damien Le Moal <Damien.LeMoal@wdc.com>,
        "aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>,
        "jason@lakedaemon.net" <jason@lakedaemon.net>,
        "alankao@andestech.com" <alankao@andestech.com>,
        "dmitriy@oss-tech.org" <dmitriy@oss-tech.org>,
        "anup@brainfault.org" <anup@brainfault.org>,
        "daniel.lezcano@linaro.org" <daniel.lezcano@linaro.org>,
        "me@packi.ch" <me@packi.ch>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        "schwab@suse.de" <schwab@suse.de>,
        "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "zongbox@gmail.com" <zongbox@gmail.com>
Subject: Re: [v3 PATCH 8/8] RISC-V: Assign hwcap only according to boot cpu.
Thread-Topic: [v3 PATCH 8/8] RISC-V: Assign hwcap only according to boot cpu.
Thread-Index: AQHUv1DZTg7Di6/hKki5WijoQT5U96XVnfCAgADoRACAAFpRgIAEGYUAgAARCwCAACRNgIAAAsGAgAARSAA=
Date:   Mon, 11 Feb 2019 23:25:09 +0000
Message-ID: <5353E827-644D-4BA1-B8E4-6075793C6E8A@wdc.com>
References: <mhng-99c5091e-32b8-4609-a92d-d3ff2a239887@palmer-si-x1c4>
In-Reply-To: <mhng-99c5091e-32b8-4609-a92d-d3ff2a239887@palmer-si-x1c4>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-ID: <7E83E73C75EA444D8D3714704FC6B0BC@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 0b64d534-e2c4-4c87-f77b-08d690782a0f
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Feb 2019 23:25:09.4517
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB4726
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Feb 11, 2019, at 2:23 PM, Palmer Dabbelt <palmer@sifive.com> wrote:
>=20
> On Mon, 11 Feb 2019 14:13:25 PST (-0800), marc.zyngier@arm.com wrote:
>> On Mon, 11 Feb 2019 12:03:30 -0800
>> Atish Patra <atish.patra@wdc.com> wrote:
>>=20
>>> On 2/11/19 11:02 AM, Palmer Dabbelt wrote:
>>> > On Fri, 08 Feb 2019 20:26:07 PST (-0800), david.abdurachmanov@gmail.c=
om wrote:
>>> >> On Sat, Feb 9, 2019 at 12:03 AM Atish Patra <atish.patra@wdc.com> wr=
ote:
>>> >>>
>>> >>> On 2/8/19 1:11 AM, Christoph Hellwig wrote:
>>> >>>>> +     * We don't support running Linux on hertergenous ISA system=
s.
>>> >>>>> +     * But first "okay" processor might not be the boot cpu.
>>> >>>>> +     * Check the ISA of boot cpu.
>>> >>>>
>>> >>>> Please use up your available 80 characters per line in comments.
>>> >>>>
>>> >>> I will fix it.
>>> >>>
>>> >>>>> +            /*
>>> >>>>> +             * All "okay" hart should have same isa. We don't kn=
ow how to
>>> >>>>> +             * handle if they don't. Throw a warning for now.
>>> >>>>> +             */
>>> >>>>> +            if (elf_hwcap && temp_hwcap !=3D elf_hwcap)
>>> >>>>> +                    pr_warn("isa mismatch: 0x%lx !=3D 0x%lx\n",
>>> >>>>> +                            elf_hwcap, temp_hwcap);
>>> >>>>> +
>>> >>>>> +            if (hartid =3D=3D boot_cpu_hartid)
>>> >>>>> +                    boot_hwcap =3D temp_hwcap;
>>> >>>>> +            elf_hwcap =3D temp_hwcap;
>>> >>>>
>>> >>>> So we always set elf_hwcap to the capabilities of the previous cpu=
.
>>> >>>>
>>> >>>>> +            temp_hwcap =3D 0;
>>> >>>>
>>> >>>> I think tmp_hwcap should be declared and initialized inside the ou=
ter loop
>>> >>>> instead having to manually reset it like this.
>>> >>>>
>>> >>>>> +    }
>>> >>>>>
>>> >>>>> +    elf_hwcap =3D boot_hwcap;
>>> >>>>
>>> >>>> And then reset it here to the boot cpu.
>>> >>>>
>>> >>>> Shoudn't we only report the features supported by all cores?  Othe=
rwise
>>> >>>> we'll still have problems if the boot cpu supports a feature, but =
not
>>> >>>> others.
>>> >>>>
>>> >>>
>>> >>> Hmm. The other side of the argument is boot cpu does have a feature=
 that
>>> >>> is not supported by other hart that didn't even boot.
>>> >>> The user space may execute something based on boot cpu capability b=
ut
>>> >>> that won't be enabled.
>>> >>>
>>> >>> At least, in this way we know that we are compatible completely wit=
h
>>> >>> boot cpu capabilities. Thoughts ?
>>> >>
>>> >> There is one example on the market, e.g., Samsung Exynos 9810.
>>> >>
>>> >> Mongoose 3 (big cores) only support ARMv8.0, while Cortex-A55
>>> >> (little ones) support ARMv8.2 (and that brings atomics support).
>>> >> I think, it's the only ARM SOC that supports different ISA extension=
s
>>> >> between cores on the same package.
>>> >>
>>> >> Kernel scheduler doesn't know that big cores are missing atomics
>>> >> support or that applications needs it and moves the thread
>>> >> resulting in illegal instruction.
>>> >>
>>> >> E.g., see Golang issue: https://github.com/golang/go/issues/28431
>>> >>
>>> >> I also recall Jon Masters (Computer Architect at Red Hat) advocating
>>> >> against having cores with mismatched capabilities on the server mark=
et.
>>> >>
>>> >> It just causes more problems down the line.
>>> > > IMO the best bet is to only put extensions in HWCAP that are suppor=
ted by all
>>> > the harts that userspace will be scheduled on.
>>> > Fair enough. Instead of setting HWCAP in setup_arch() once, we can se=
t it only for boot cpu. It will be updated after every cpu comes up online.
>>>=20
>>> Thus, HWCAP will consists all extensions supported by all cpus that are=
 online currently.
>>=20
>> You must thus prevent CPUs that have a different set of capabilities
>> from coming up late (once userspace has started).
>=20
> and we have no way to do that.  I'd prefer if we just looked through the =
entire device tree and only showed userspace the features that are on every=
 possible CPU from the start.  Otherwise the HWCAP will shift around during=
 a userspace run, which seems odd.

ok. I will do this for now. Once we have cpu hotplug enabled, we can revisi=
t this.

Regards,
Atish=