Re: [PATCH] arm64: asm-bug: Add .align 2 to the end of __BUG_ENTRY

From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
To: Li Zetao <lizetao1@huawei.com>, Mark Rutland <mark.rutland@arm.com>
Cc: <catalin.marinas@arm.com>, <will@kernel.org>,
	<Dave.Martin@arm.com>, <xieyuanbin1@huawei.com>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <nixiaoming@huawei.com>,
	<wangbing6@huawei.com>, <douzhaolei@huawei.com>,
	<liaohua4@huawei.com>, <lijiahuan5@huawei.com>,
	<wangfangpeng1@huawei.com>,
	"zhangjianhua (E)" <chris.zjh@huawei.com>
Subject: Re: [PATCH] arm64: asm-bug: Add .align 2 to the end of __BUG_ENTRY
Date: Mon, 20 May 2024 21:21:39 +0800	[thread overview]
Message-ID: <876b3571-b7f5-03f1-5da2-07a2ec47ad08@huawei.com> (raw)
In-Reply-To: <20d70835-9411-9a08-c567-56d7040e01dd@huawei.com>

On 2024/5/20 20:05, Li Zetao wrote:

>>> diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
>>> index c762038..6e73809 100644
>>> --- a/arch/arm64/include/asm/asm-bug.h
>>> +++ b/arch/arm64/include/asm/asm-bug.h
>>> @@ -28,6 +28,7 @@
>>>       14470:    .long 14471f - .;            \
>>>   _BUGVERBOSE_LOCATION(__FILE__, __LINE__)        \
>>>           .short flags;                 \
>>> +        .align 2;                \
> The use of .align 2 here is based on the assumption that struct bug_entry is 4-byte aligned. Currently, there is no problem with this assumption, but for compatibility reasons, refer to the riscv architecture and refactor the implementation of __BUG_FLAGS:
> 
> #define __BUG_FLAGS(flags)                    \
> do {                                \
>     __asm__ __volatile__ (                    \
>         "1:\n\t"                    \
>             "ebreak\n"                \
>             ".pushsection __bug_table,\"aw\"\n\t"    \
>         "2:\n\t"                    \
>             __BUG_ENTRY "\n\t"            \
>             ".org 2b + %3\n\t"                      \
>             ".popsection"                \
>         :                        \
>         : "i" (__FILE__), "i" (__LINE__),        \
>           "i" (flags),                    \
>           "i" (sizeof(struct bug_entry)));              \
> } while (0)
> 
> Align the real size of struct bug_entry through .org. What do you think?

The implementation of risc-v BUG_ENTRY does handle
the `implicit padding` at the end of the struct correctly,
however, it does not handle the `implicit padding` in the middle of
the struct correctly, for example, assume that
the struct bug_entry changes as follows in the future:
struct bug_entry {
	signed int bug_addr_disp; // 4 bytes
	unsigned short flags; // 2 bytes
	< implicit padding > // 6 bytes
	unsigned long long flags2; // 8 bytes
}

Even the implementation of risc-v BUG_ENTRY
can't handle this situation. Referencing risc-v solution
complicates things, but doesn't completely solve the problem.

In the current scenario, we know the contents of struct bug_entry
and generate variables using assembly language.
I don't think it's necessary to complicate things.