Received-SPF: pass (google.com: domain of linux-kernel+bounces-183784-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Subject: Re: [PATCH] arm64: asm-bug: Add .align 2 to the end of __BUG_ENTRY
To: Li Zetao <lizetao1@huawei.com>, Mark Rutland <mark.rutland@arm.com>
References: <1715955208-17109-1-git-send-email-xiaojiangfeng@huawei.com>
 <ZksnARrLkKHcX4C_@J2N7QTR9R3>
 <20d70835-9411-9a08-c567-56d7040e01dd@huawei.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>
From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
Message-ID: <876b3571-b7f5-03f1-5da2-07a2ec47ad08@huawei.com>
Date: Mon, 20 May 2024 21:21:39 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.1
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <20d70835-9411-9a08-c567-56d7040e01dd@huawei.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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.