From: "Pierre-Clément Tosi" <ptosi@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
kvm@vger.kernel.org, James Morse <james.morse@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Oliver Upton <oliver.upton@linux.dev>,
Zenghui Yu <yuzenghui@huawei.com>, Will Deacon <will@kernel.org>,
Quentin Perret <qperret@google.com>,
Vincent Donnefort <vdonnefort@google.com>
Subject: Re: [PATCH 09/10] KVM: arm64: nVHE: Support CONFIG_CFI_CLANG at EL2
Date: Wed, 10 Apr 2024 15:58:28 +0100 [thread overview]
Message-ID: <5pgvud47ohozxjnbzs2ei5jc5imoabcc7x2c6dbcvrmtopgn2g@dglnupfwwaec> (raw)
In-Reply-To: <867ci10zv6.wl-maz@kernel.org>
Hi Marc,
On Sun, Mar 17, 2024 at 01:09:01PM +0000, Marc Zyngier wrote:
> On Thu, 14 Mar 2024 20:25:43 +0000,
> Pierre-Clément Tosi <ptosi@google.com> wrote:
> >
> > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> > index b0c23e7d6595..281e352a4c94 100644
> > --- a/arch/arm64/include/asm/esr.h
> > +++ b/arch/arm64/include/asm/esr.h
> > @@ -397,6 +397,12 @@ static inline bool esr_is_data_abort(unsigned long esr)
> > return ec == ESR_ELx_EC_DABT_LOW || ec == ESR_ELx_EC_DABT_CUR;
> > }
> >
> > +static inline bool esr_is_cfi_brk(unsigned long esr)
> > +{
> > + return ESR_ELx_EC(esr) == ESR_ELx_EC_BRK64 &&
> > + (esr_comment(esr) & ~CFI_BRK_IMM_MASK) == CFI_BRK_IMM_BASE;
> > +}
> > +
>
> nit: since there is a single user, please place this helper in handle_exit.c.
I've placed this here as I'm introducing a second user in a following patch of
this series (in the VHE code) and wanted to avoid adding code then immediately
moving it around.
I've therefore kept this part unchanged in v2 but let me know if you prefer the
commits to add-then-move and I'll update that for v3.
> > static inline bool esr_fsc_is_translation_fault(unsigned long esr)
> > {
> > return (esr & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_FAULT;
> > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > index ffa67ac6656c..9b6574e50b13 100644
> > --- a/arch/arm64/kvm/handle_exit.c
> > +++ b/arch/arm64/kvm/handle_exit.c
> > @@ -383,6 +383,15 @@ void handle_exit_early(struct kvm_vcpu *vcpu, int exception_index)
> > kvm_handle_guest_serror(vcpu, kvm_vcpu_get_esr(vcpu));
> > }
> >
> > +static void kvm_nvhe_report_cfi_failure(u64 panic_addr)
> > +{
> > + kvm_err("nVHE hyp CFI failure at: [<%016llx>] %pB!\n", panic_addr,
> > + (void *)(panic_addr + kaslr_offset()));
> > +
> > + if (IS_ENABLED(CONFIG_CFI_PERMISSIVE))
> > + kvm_err(" (CONFIG_CFI_PERMISSIVE ignored for hyp failures)\n");
> > +}
> > +
> > void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr,
> > u64 elr_virt, u64 elr_phys,
> > u64 par, uintptr_t vcpu,
> > @@ -413,6 +422,8 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr,
> > else
> > kvm_err("nVHE hyp BUG at: [<%016llx>] %pB!\n", panic_addr,
> > (void *)(panic_addr + kaslr_offset()));
> > + } else if (IS_ENABLED(CONFIG_CFI_CLANG) && esr_is_cfi_brk(esr)) {
>
> It would seem logical to move the IS_ENABLED() into the ESR check helper.
I suppose it makes sense for a static function but, given that I've kept the
helper in a shared header and as it essentially is a straightforward
shift-mask-compare (like the existing helpers in <asm/esr.h>), wouldn't it be
confusing for its result to depend on a Kconfig flag?
Anyway, same as above; left unchanged in v2 but happy to update this in v3.
--
Pierre
next prev parent reply other threads:[~2024-04-10 14:58 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-14 20:23 [PATCH 00/10] KVM: arm64: Add support for hypervisor kCFI Pierre-Clément Tosi
2024-03-14 20:23 ` [PATCH 01/10] KVM: arm64: Fix clobbered ELR in sync abort Pierre-Clément Tosi
2024-03-17 12:26 ` Marc Zyngier
2024-03-14 20:23 ` [PATCH 02/10] KVM: arm64: Fix __pkvm_init_switch_pgd C signature Pierre-Clément Tosi
2024-03-14 20:23 ` [PATCH 03/10] KVM: arm64: Pass pointer to __pkvm_init_switch_pgd Pierre-Clément Tosi
2024-03-14 20:24 ` [PATCH 04/10] KVM: arm64: nVHE: Simplify __guest_exit_panic path Pierre-Clément Tosi
2024-03-14 20:24 ` [PATCH 05/10] KVM: arm64: nVHE: Add EL2 sync exception handler Pierre-Clément Tosi
2024-03-17 11:42 ` Marc Zyngier
2024-04-10 14:44 ` Pierre-Clément Tosi
2024-03-14 20:24 ` [PATCH 06/10] KVM: arm64: nVHE: gen-hyprel: Skip R_AARCH64_ABS32 Pierre-Clément Tosi
2024-03-14 20:25 ` [PATCH 07/10] KVM: arm64: VHE: Mark __hyp_call_panic __noreturn Pierre-Clément Tosi
2024-03-14 20:25 ` [PATCH 08/10] arm64: Move esr_comment() to <asm/esr.h> Pierre-Clément Tosi
2024-03-17 12:50 ` Marc Zyngier
2024-03-14 20:25 ` [PATCH 09/10] KVM: arm64: nVHE: Support CONFIG_CFI_CLANG at EL2 Pierre-Clément Tosi
2024-03-17 13:09 ` Marc Zyngier
2024-04-10 14:58 ` Pierre-Clément Tosi [this message]
2024-03-14 20:26 ` [PATCH 10/10] KVM: arm64: Improve CONFIG_CFI_CLANG error message Pierre-Clément Tosi
2024-03-14 22:40 ` [PATCH 00/10] KVM: arm64: Add support for hypervisor kCFI Marc Zyngier
2024-03-15 10:22 ` Pierre-Clément Tosi
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=5pgvud47ohozxjnbzs2ei5jc5imoabcc7x2c6dbcvrmtopgn2g@dglnupfwwaec \
--to=ptosi@google.com \
--cc=james.morse@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=qperret@google.com \
--cc=suzuki.poulose@arm.com \
--cc=vdonnefort@google.com \
--cc=will@kernel.org \
--cc=yuzenghui@huawei.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).