Message-ID: <5947F498.2040505@arm.com>
Date: Mon, 19 Jun 2017 16:58:16 +0100
From: James Morse <james.morse@arm.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0
MIME-Version: 1.0
To: Yury Norov <ynorov@caviumnetworks.com>
CC: Catalin Marinas <catalin.marinas@arm.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
        Andrew Pinski <pinskia@gmail.com>,
        Andrew Pinski <Andrew.Pinski@caviumnetworks.com>,
        Adam Borowski <kilobyte@angband.pl>,
        Chris Metcalf <cmetcalf@ezchip.com>,
        Steve Ellcey <sellcey@caviumnetworks.com>,
        Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>,
        Ramana Radhakrishnan <ramana.gcc@googlemail.com>,
        Florian Weimer <fweimer@redhat.com>,
        Bamvor Zhangjian <bamvor.zhangjian@huawei.com>,
        Andreas Schwab <schwab@suse.de>, Chris Metcalf <cmetcalf@mellanox.com>,
        Heiko Carstens <heiko.carstens@de.ibm.com>, schwidefsky@de.ibm.com,
        broonie@kernel.org, Joseph Myers <joseph@codesourcery.com>,
        christoph.muellner@theobroma-systems.com, szabolcs.nagy@arm.com,
        klimov.linux@gmail.com, Nathan_Lynch@mentor.com, agraf@suse.de,
        Prasun.Kapoor@caviumnetworks.com, geert@linux-m68k.org,
        philipp.tomsich@theobroma-systems.com, manuel.montezelo@gmail.com,
        linyongting@huawei.com, davem@davemloft.net, zhouchengming1@huawei.com,
        Andrew Pinski <apinski@cavium.com>,
        Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
Subject: Re: [PATCH 05/20] arm64: rename COMPAT to AARCH32_EL0 in Kconfig
References: <20170604120009.342-1-ynorov@caviumnetworks.com> <20170604120009.342-6-ynorov@caviumnetworks.com>
In-Reply-To: <20170604120009.342-6-ynorov@caviumnetworks.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1497
Lines: 45

Hi Yury,

On 04/06/17 12:59, Yury Norov wrote:
> From: Andrew Pinski <apinski@cavium.com>
> 
> In this patchset  ILP32 ABI support is added. Additionally to AARCH32,
> which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.
> 
> From now, AARCH32_EL0 (former COMPAT) config option means the support of
> AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches),
> and COMPAT indicates that one of them, or both, is enabled.
> 
> Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead

Nit: You have 'COMPAT' around compat_hwcap_str's definition, but its only user
is wrapped in 'AARCH32_EL0'.


After this patch
arch/arm64/kernel/perf_callchain.c::perf_callchain_user() still has:
>	if (!compat_user_mode(regs)) {
>		/* AARCH64 mode */
...
>	} else {
> #ifdef CONFIG_COMPAT
> 		/* AARCH32 compat mode */
...
> #endif
>	}

I think this one should become CONFIG_AARCH32_EL0. compat to this code means the
fp is 'compat_fp' in x11, and it should read a 32bit call chain from user-space.

This is confusing as 'is_compat_task()' matches one of aarch32 or ilp32, but
compat_user_mode(regs) only matches aarch32 as it checks the saved spsr. I can't
see any problem caused by this today, but its going to bite someone in the
future. Can this be renamed aarch32_user_mode()? (turns out 'a32' is the name of
just one of aarch32's instruction sets[0].)


Thanks,

James

[0] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka16137.html