Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <nick.hawkins@hpe.com> <20220216183704.42084-1-nick.hawkins@hpe.com>
 <CAK8P3a2Ryjr738XgLsGQngvc7j=Kk620LMjWpu-_HiwdK_fX4Q@mail.gmail.com> <9FE3F625-39FC-4ED7-A2CC-567EF0181886@hpe.com>
In-Reply-To: <9FE3F625-39FC-4ED7-A2CC-567EF0181886@hpe.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Tue, 15 Mar 2022 08:34:10 +0100
Message-ID: <CAK8P3a3sR98Ujk5AFA=F7aTWWEx93-Jjan1yXqaqaw-6x85NwQ@mail.gmail.com>
Subject: Re: [PATCH] [v1] arch: arm: configs: gxp_defconfig
To:     "Verdun, Jean-Marie" <verdun@hpe.com>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        "Hawkins, Nick" <nick.hawkins@hpe.com>,
        Russell King <linux@armlinux.org.uk>,
        Joel Stanley <joel@jms.id.au>,
        Andrew Jeffery <andrew@aj.id.au>,
        Olof Johansson <olof@lixom.net>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Mon, Mar 14, 2022 at 11:47 PM Verdun, Jean-Marie <verdun@hpe.com> wrote:
> ﻿On 2/16/22, 1:58 PM, "Arnd Bergmann" <arnd@arndb.de> wrote:
>
>
> >    One bit of information that I would like to see in the defconfig patch
> >    is an explanation about why you need a custom defconfig in the
> >    first place, rather than using multi_v7_defconfig. Please also add
> >    a patch to enable your platform in the multi_v7_defconfig, along with
> >    the drivers you need (as loadable modules).
>
> I took some time to look at the defconfig "challenge". Nick has updated the multi_v7_defconfig with our GXP in a new series of patches, but this won't execute on our ASIC (compilation is ok). The challenge is that we are missing a few features which are enabled by default, and I was wondering if the community would accept to disable them by default.
>
> This is this
>
> CONFIG_PERF_EVENTS=y
> CONFIG_ARCH_VIRT=y
>
> Both of them generate unknown instruction on our platform which lead to kernel crash.

If you get unknown instruction exceptions, that is clearly a bug that has to be
fixed somewhere. Turning the options off should not be necessary, but we have
to figure out why these crash, and make sure we have correct runtime detection
in place that ensures that any driver code runs only on platforms that have the
corresponding hardware.

Do you have any more information about how and why these crash? My first
guess would be that there is something in your DT that describes hardware
that is not actually there. With a correct DTB file, the two options should
not cause any code to run that wouldn't otherwise.

> With these options disabled, we can use the defconfig and add only
>
> CONFIG_ARCH_HPE=y
> CONFIG_ARCH_HPE_GXP=y

These are obviously ok

> CONFIG_GXP_WATCHDOG=y
> CONFIG_ATAGS=y
>
> To it, as to get everything setup and get our new platform booting
> without any issues, assuming the associated code is present. The
> ATAGS is not mandatory it removed some warning messages during
> kernel boot.

Ok, good. I assume you need the watchdog driver to be built-in because
the watchdog timer is active before we enter the kernel? In this case it
is ok, otherwise it should be =m, like any other drivers for your hardware.

I don't think we should enable CONFIG_ATAGS here, the multi_v7_defconfig
intentionally only supports DTB based booting. What is the warning you see
without it?

> I know this is removing some standard feature, but, I probably can't easily fix
>  the missing instructions. I can dig a little bit if needed without any issue.
> If we want to have a working defconfig on HPE GXP platform, then we need
> to either take this modification, or change the code from perf_events and
> arch_virt to properly work if the required underlying hardware is unable to
> support these features (could be probably a dummy test to identify the asic
> at compilation time), or create a specific defconfig.

ARCH_VIRT doesn't do anything itself but only enables a couple of other drivers:

config ARCH_VIRT
        bool "Dummy Virtual Machine"
        depends on ARCH_MULTIPLATFORM
        select ARM_AMBA
        select ARM_GIC
        select ARM_GIC_V2M if PCI
        select ARM_GIC_V3
        select ARM_GIC_V3_ITS if PCI
        select ARM_PSCI
        select HAVE_ARM_ARCH_TIMER
        select ARCH_SUPPORTS_BIG_ENDIAN

Aside from the big-endian option, these all just enable the compilation
of drivers that in turn check the device tree before running any code.

Invalid instructions point to either PSCI or ARCH_TIMER, so try
disabling those first to narrow it down to one option causing the
problem, and make sure you actually run with the DTB that you
submitted, not a DTB that may contain incorrect nodes.

        Arnd