Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-1047-g9e4af4ada4-fm-20221005.001-g9e4af4ad
Mime-Version: 1.0
Message-Id: <506fe4e5-a203-48e6-84a6-f70133be15dd@app.fastmail.com>
In-Reply-To: <20221016133418.2122777-1-chenhuacai@loongson.cn>
References: <20221016133418.2122777-1-chenhuacai@loongson.cn>
Date:   Mon, 17 Oct 2022 09:12:02 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Huacai Chen" <chenhuacai@loongson.cn>,
        "Huacai Chen" <chenhuacai@kernel.org>
Cc:     loongarch@lists.linux.dev, "Xuefeng Li" <lixuefeng@loongson.cn>,
        "Tiezhu Yang" <yangtiezhu@loongson.cn>, guoren <guoren@kernel.org>,
        "WANG Xuerui" <kernel@xen0n.name>,
        "Jiaxun Yang" <jiaxun.yang@flygoat.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] LoongArch: Add unaligned access support
Content-Type: text/plain
Precedence: bulk

On Sun, Oct 16, 2022, at 3:34 PM, Huacai Chen wrote:
> Loongson-2 series (Loongson-2K500, Loongson-2K1000) don't support
> unaligned access in hardware, while Loongson-3 series (Loongson-3A5000,
> Loongson-3C5000) are configurable whether support unaligned access in
> hardware. This patch add unaligned access emulation for those LoongArch
> processors without hardware support.
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>

What does the Loongarch ELF ABI say about this? On most architectures,
C compilers are not allowed to produce unaligned accesses for standard
compliant source code, the only way you'd get this is when casting
a an unaligned (e.g. char*) pointer to another type with higher alignment
requirement.

> +/* sysctl hooks */
> +int unaligned_enabled __read_mostly = 1;	/* Enabled by default */
> +int no_unaligned_warning __read_mostly = 1;	/* Only 1 warning by default */

The comment says 'sysctl', the implementation has a debugfs interface.

> +#ifdef CONFIG_DEBUG_FS
> +static int __init debugfs_unaligned(void)
> +{
> +	struct dentry *d;
> +
> +	d = debugfs_create_dir("loongarch", NULL);
> +	if (!d)
> +		return -ENOMEM;
> +
> +	debugfs_create_u32("unaligned_instructions_user",
> +				S_IRUGO, d, &unaligned_instructions_user);
> +	debugfs_create_u32("unaligned_instructions_kernel",
> +				S_IRUGO, d, &unaligned_instructions_kernel);
> +
> +	return 0;
> +}
> +arch_initcall(debugfs_unaligned);
> +#endif

The debugfs interface does not sound like a good way to do this.
Overall, my feeling is that for a new architecture we should not
introduce this at all but instead provide a way to diagnose and
fix user space, since we do not have to keep compatibility with
broken binaries that worked in the past.

If the ELF ABI actually allows compilers to produce unaligned
accesses for correct code, there should at least be a more generic
way of enabling this that follows what other architectures do.
We are already somewhat inconsistent there between architectures,
but I don't think anything else uses debugfs here.

     Arnd