From: Marc Zyngier <maz@kernel.org>
To: Chen Zhongjin <chenzhongjin@huawei.com>
Cc: linux-arch@vger.kernel.org, mark.rutland@arm.com,
daniel.thompson@linaro.org, michal.lkml@markovi.net,
pasha.tatashin@soleen.com, will@kernel.org,
linux-kbuild@vger.kernel.org, peterz@infradead.org,
catalin.marinas@arm.com, masahiroy@kernel.org,
ndesaulniers@google.com, linux-kernel@vger.kernel.org,
madvenka@linux.microsoft.com, rmk+kernel@armlinux.org.uk,
broonie@kernel.org, live-patching@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, jpoimboe@kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 32/33] arm64: irq-gic: Replace unreachable() with -EINVAL
Date: Thu, 23 Jun 2022 09:13:08 +0100 [thread overview]
Message-ID: <7d26e36686495866e0752e12c38f170e@kernel.org> (raw)
In-Reply-To: <20220623014917.199563-33-chenzhongjin@huawei.com>
On 2022-06-23 02:49, Chen Zhongjin wrote:
> Using unreachable() at default of switch generates an extra branch at
> end of the function, and compiler won't generate a ret to close this
> branch because it knows it's unreachable.
>
> If there's no instruction in this branch, compiler will generate a NOP,
> And it will confuse objtool to warn this NOP as a fall through branch.
>
> In fact these branches are actually unreachable, so we can replace
> unreachable() with returning a -EINVAL value.
>
> Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
> ---
> arch/arm64/kvm/hyp/vgic-v3-sr.c | 7 +++----
> drivers/irqchip/irq-gic-v3.c | 2 +-
> 2 files changed, 4 insertions(+), 5 deletions(-)
Basic courtesy would have been to Cc the maintainers of this code.
>
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index 4fb419f7b8b6..f3cee92c3038 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -6,7 +6,6 @@
>
> #include <hyp/adjust_pc.h>
>
> -#include <linux/compiler.h>
> #include <linux/irqchip/arm-gic-v3.h>
> #include <linux/kvm_host.h>
>
> @@ -55,7 +54,7 @@ static u64 __gic_v3_get_lr(unsigned int lr)
> return read_gicreg(ICH_LR15_EL2);
> }
>
> - unreachable();
> + return -EINVAL;
NAK. That's absolutely *wrong*, and will hide future bugs.
Nothing checks for -EINVAL, and we *never* expect to
reach this, hence the perfectly valid annotation.
If something needs fixing, it probably is your tooling.
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2022-06-23 8:14 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-23 1:48 [PATCH v5 00/33] objtool: add base support for arm64 Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 01/33] tools: arm64: Make aarch64 instruction decoder available to tools Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 02/33] objtool: arm64: Add base definition for arm64 backend Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 03/33] objtool: arm64: Decode add/sub instructions Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 04/33] objtool: arm64: Decode jump and call related instructions Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 05/33] objtool: arm64: Decode other system instructions Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 06/33] objtool: arm64: Decode load/store instructions Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 07/33] objtool: arm64: Decode LDR instructions Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 08/33] objtool: arm64: Accept non-instruction data in code sections Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 09/33] objtool: check: Support data in text section Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 10/33] objtool: arm64: Handle supported relocations in alternatives Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 11/33] objtool: arm64: Ignore replacement section for alternative callback Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 12/33] objtool: arm64: Enable stack validation for arm64 Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 13/33] objtool: arm64: Enable ORC " Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 14/33] objtool: arm64: Add annotate_reachable() for objtools Chen Zhongjin
2022-06-23 1:48 ` [PATCH v6 15/33] arm64: bug: Add reachable annotation to warning macros Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 16/33] arm64: kgdb: Add reachable annotation after kgdb brk Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 17/33] objtool: arm64: Add unwind_hint support Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 18/33] arm64: Change symbol type annotations Chen Zhongjin
2022-06-29 17:47 ` Mark Brown
2022-06-30 2:41 ` Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 19/33] arm64: Annotate unwind_hint for symbols with empty stack Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 20/33] arm64: entry: Annotate unwind_hint for entry Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 21/33] arm64: kvm: Annotate unwind_hint for hyp entry Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 22/33] arm64: efi-header: Mark efi header as data Chen Zhongjin
2022-06-29 18:03 ` Mark Brown
2022-06-23 1:49 ` [PATCH v6 23/33] arm64: head: Mark constants " Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 24/33] arm64: proc: Mark constant " Chen Zhongjin
2022-06-29 18:06 ` Mark Brown
2022-06-23 1:49 ` [PATCH v6 25/33] arm64: crypto: " Chen Zhongjin
2022-06-29 18:20 ` Mark Brown
2022-06-23 1:49 ` [PATCH v6 26/33] arm64: crypto: Remove unnecessary stackframe Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 27/33] arm64: Set intra-function call annotations Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 28/33] arm64: sleep: Properly set frame pointer before call Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 29/33] arm64: compat: Move VDSO code to .rodata section Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 30/33] arm64: entry: Align stack size for alternative Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 31/33] arm64: kernel: Skip validation of proton-pack.c Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 32/33] arm64: irq-gic: Replace unreachable() with -EINVAL Chen Zhongjin
2022-06-23 8:13 ` Marc Zyngier [this message]
2022-06-24 1:24 ` Chen Zhongjin
2022-06-23 1:49 ` [PATCH v6 33/33] objtool: revert c_file fallthrough detection for arm64 Chen Zhongjin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7d26e36686495866e0752e12c38f170e@kernel.org \
--to=maz@kernel.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=chenzhongjin@huawei.com \
--cc=daniel.thompson@linaro.org \
--cc=jpoimboe@kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=live-patching@vger.kernel.org \
--cc=madvenka@linux.microsoft.com \
--cc=mark.rutland@arm.com \
--cc=masahiroy@kernel.org \
--cc=michal.lkml@markovi.net \
--cc=ndesaulniers@google.com \
--cc=pasha.tatashin@soleen.com \
--cc=peterz@infradead.org \
--cc=rmk+kernel@armlinux.org.uk \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).