From: Oliver Upton <oupton@kernel.org>
To: Leonardo Bras <leo.bras@arm.com>
Cc: kvmarm@lists.linux.dev, Marc Zyngier <maz@kernel.org>,
Joey Gouly <joey.gouly@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Zenghui Yu <yuzenghui@huawei.com>,
Wei-Lin Chang <weilin.chang@arm.com>,
Steffen Eiden <seiden@linux.ibm.com>,
Fuad Tabba <fuad.tabba@linux.dev>,
stable@vger.kernel.org
Subject: Re: [PATCH 8/8] KVM: arm64: Stop leaning on __kvm_inject_sea() for abort injection
Date: Thu, 12 Nov 2026 14:22:25 -0800 [thread overview]
Message-ID: <avY8IUOoxScjsisA@kernel.org> (raw)
In-Reply-To: <avXXSLhMCQVOvv4G@LeoBrasDK>
On Thu, Nov 12, 2026 at 03:12:08PM +0000, Leonardo Bras wrote:
> > -static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long addr)
> > +static void __inject_abt64(struct kvm_vcpu *vcpu, bool iabt, u32 iss, unsigned long addr)
> > {
> > - unsigned long cpsr = *vcpu_cpsr(vcpu);
> > - bool is_aarch32 = vcpu_mode_is_32bit(vcpu);
> > - u64 esr = 0, fsc;
> > - int level;
> > + u64 esr;
> > + u8 ec;
> > +
> > + if (vcpu_mode_priv(vcpu))
>
> Maybe I am getting this wrong, but this line was previously:
> - if (is_aarch32 || (cpsr & PSR_MODE_MASK) == PSR_MODE_EL0t)
>
> but vcpu_mode_priv() does not test exactly the same thing, it would test:
> vcpu_mode_is_32bit(), tested in is_aarch32, &&
> cpsr_mode > PSR_AA32_MODE_USR, not tested above, ||
> *vcpu_cpsr(vcpu) & PSR_MODE_MASK != PSR_MODE_EL0t
>
> This last line is testing the exact opposite of what was here.
> Is that another fix?
No, it's equivalent. What we had here before was inscrutable. If we're
taking an exception from 32-bit into a 64-bit EL1, the only way we get
there is from a 32-bit EL0, so this short-circuited it to xABT_LOW.
If there's no exception level change, xABT_CUR. Otherwise, xABT_LOW.
Thanks,
Oliver
> > + ec = iabt ? ESR_ELx_EC_IABT_CUR : ESR_ELx_EC_DABT_CUR;
> > + else
> > + ec = iabt ? ESR_ELx_EC_IABT_LOW : ESR_ELx_EC_DABT_LOW;
> > +
> > + esr = FIELD_PREP(ESR_ELx_EC_MASK, ec) | FIELD_PREP(ESR_ELx_ISS_MASK, iss) |
> > + ESR_ELx_IL;
> > +
> > + vcpu_write_sys_reg(vcpu, addr, exception_far_elx(vcpu));
> > + vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
> > +}
> > +
> > +static void inject_abt64(struct kvm_vcpu *vcpu, bool iabt, u32 iss, u64 addr)
> > +{
> > + pend_sync_exception(vcpu);
> > + __inject_abt64(vcpu, iabt, iss, addr);
> > +}
> > +
> > +static void inject_sea64(struct kvm_vcpu *vcpu, bool iabt, u64 addr)
> > +{
> > + u8 fsc;
> >
> > /*
> > * If injecting an abort from a failed S1PTW, rewalk the S1 PTs to
> > @@ -116,7 +136,7 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long addr
> > */
> > if (kvm_vcpu_abt_iss1tw(vcpu)) {
> > u64 hpfar = kvm_vcpu_get_fault_ipa(vcpu);
> > - int ret;
> > + int ret, level;
> >
> > if (hpfar == INVALID_GPA)
> > return;
> > @@ -137,28 +157,7 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long addr
> > else
> > pend_sync_exception(vcpu);
> >
> > - /*
> > - * Build an {i,d}abort, depending on the level.
> > - * Report an external synchronous abort.
> > - */
> > - esr |= ESR_ELx_IL;
> > -
> > - /*
> > - * Here, the guest runs in AArch64 mode when in EL1. If we get
> > - * an AArch32 fault, it means we managed to trap an EL0 fault.
> > - */
> > - if (is_aarch32 || (cpsr & PSR_MODE_MASK) == PSR_MODE_EL0t)
> > - esr |= (ESR_ELx_EC_IABT_LOW << ESR_ELx_EC_SHIFT);
> > - else
> > - esr |= (ESR_ELx_EC_IABT_CUR << ESR_ELx_EC_SHIFT);
> > -
> > - if (!is_iabt)
> > - esr |= ESR_ELx_EC_DABT_LOW << ESR_ELx_EC_SHIFT;
> > -
> > - esr |= fsc;
> > -
> > - vcpu_write_sys_reg(vcpu, addr, exception_far_elx(vcpu));
> > - vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
> > + __inject_abt64(vcpu, iabt, fsc, addr);
> > }
> >
> > void kvm_inject_sync(struct kvm_vcpu *vcpu, u64 esr)
> > @@ -174,31 +173,41 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
> > kvm_inject_sync(vcpu, esr);
> > }
> >
> > -#define DFSR_FSC_EXTABT_LPAE 0x10
> > -#define DFSR_FSC_EXTABT_nLPAE 0x08
> > -#define DFSR_LPAE BIT(9)
> > -#define TTBCR_EAE BIT(31)
> > +#define DFSR_FS_4 BIT(10)
> > +#define DFSR_FS_3_0 GENMASK(3, 0)
> > +#define DFSR_FS_EXTABT_nLPAE 0x08
> > +#define DFSR_FS_EXCL_ATOMIC_nLPAE 0x15
>
> Maybe I am reading the wrong place, but in M.d, I see:
> DFSR.FS = 0b10101
> IMPLEMENTATION DEFINED fault (Unsupported Exclusive access fault).
>
> > +#define DFSR_FSC GENMASK(5, 0)
> > +#define DFSR_FSC_EXTABT_LPAE 0x10
>
> Wouldn't this be STATUS bits instead of FSC?
> (Maybe FSC got renamed STATUS in the meantime)
>
>
> > +#define DFSR_LPAE BIT(9)
> > +#define TTBCR_EAE BIT(31)
> >
> > static void inject_undef32(struct kvm_vcpu *vcpu)
> > {
> > kvm_pend_exception(vcpu, EXCEPT_AA32_UND);
> > }
> >
> > +static bool vcpu_trans_32bit_non_lpae(struct kvm_vcpu *vcpu)
> > +{
> > + return vcpu_el1_is_32bit(vcpu) && !(vcpu_read_sys_reg(vcpu, TCR_EL1) & TTBCR_EAE);
> > +}
> > +
> > /*
> > * Modelled after TakeDataAbortException() and TakePrefetchAbortException
> > * pseudocode.
> > */
> > -static void inject_abt32(struct kvm_vcpu *vcpu, bool is_pabt, u32 addr)
> > +static void inject_abt32(struct kvm_vcpu *vcpu, bool is_pabt, u8 status, u32 addr)
> > {
> > u64 far;
> > u32 fsr;
> >
> > - /* Give the guest an IMPLEMENTATION DEFINED exception */
> > - if (vcpu_read_sys_reg(vcpu, TCR_EL1) & TTBCR_EAE) {
> > - fsr = DFSR_LPAE | DFSR_FSC_EXTABT_LPAE;
> > + if (vcpu_trans_32bit_non_lpae(vcpu)) {
> > + /* Ensure status actually fits in DFSR.FS */
> > + KVM_BUG_ON(status > 0x1F, vcpu->kvm);
> > + fsr = FIELD_PREP(DFSR_FS_4, FIELD_GET(BIT(4), status)) |
> > + FIELD_PREP(DFSR_FS_3_0, FIELD_GET(GENMASK(3, 0), status));
> > } else {
> > - /* no need to shuffle FS[4] into DFSR[10] as it's 0 */
> > - fsr = DFSR_FSC_EXTABT_nLPAE;
> > + fsr = FIELD_PREP(DFSR_FSC, status) | DFSR_LPAE;
> > }
> >
> > far = vcpu_read_sys_reg(vcpu, FAR_EL1);
> > @@ -218,12 +227,28 @@ static void inject_abt32(struct kvm_vcpu *vcpu, bool is_pabt, u32 addr)
> > vcpu_write_sys_reg(vcpu, far, FAR_EL1);
> > }
> >
> > +static void inject_sea32(struct kvm_vcpu *vcpu, bool is_pabt, u32 addr)
> > +{
> > + if (vcpu_trans_32bit_non_lpae(vcpu))
> > + inject_abt32(vcpu, is_pabt, DFSR_FS_EXTABT_nLPAE, addr);
> > + else
> > + inject_abt32(vcpu, is_pabt, DFSR_FSC_EXTABT_LPAE, addr);
> > +}
> > +
> > static void __kvm_inject_sea(struct kvm_vcpu *vcpu, bool iabt, u64 addr)
> > {
> > if (vcpu_el1_is_32bit(vcpu))
> > - inject_abt32(vcpu, iabt, addr);
> > + inject_sea32(vcpu, iabt, addr);
> > else
> > - inject_abt64(vcpu, iabt, addr);
> > + inject_sea64(vcpu, iabt, addr);
> > +}
> > +
> > +static void __kvm_inject_abort(struct kvm_vcpu *vcpu, bool iabt, u8 fsc, u64 addr)
> > +{
> > + if (vcpu_el1_is_32bit(vcpu))
> > + inject_abt32(vcpu, iabt, fsc, addr);
> > + else
> > + inject_abt64(vcpu, iabt, fsc, addr);
> > }
> >
> > static bool kvm_sea_target_is_el2(struct kvm_vcpu *vcpu)
> > @@ -270,28 +295,25 @@ static int kvm_inject_nested_excl_atomic(struct kvm_vcpu *vcpu, u64 addr)
> > */
> > int kvm_inject_dabt_excl_atomic(struct kvm_vcpu *vcpu, u64 addr)
> > {
> > - u64 esr;
> > -
> > if (is_nested_ctxt(vcpu) && (vcpu_read_sys_reg(vcpu, HCR_EL2) & HCR_EL2_VM))
> > return kvm_inject_nested_excl_atomic(vcpu, addr);
> >
> > - __kvm_inject_sea(vcpu, false, addr);
> > - esr = vcpu_read_sys_reg(vcpu, exception_esr_elx(vcpu));
> > - esr &= ~ESR_ELx_FSC;
> > - esr |= ESR_ELx_FSC_EXCL_ATOMIC;
> > - vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
> > + if (vcpu_trans_32bit_non_lpae(vcpu))
> > + __kvm_inject_abort(vcpu, false, DFSR_FS_EXCL_ATOMIC_nLPAE, addr);
> > + else
> > + __kvm_inject_abort(vcpu, false, ESR_ELx_FSC_EXCL_ATOMIC, addr);
> > +
> > return 1;
> > }
> >
> > void kvm_inject_size_fault(struct kvm_vcpu *vcpu)
> > {
> > - unsigned long addr, esr;
> > + bool iabt = kvm_vcpu_trap_is_iabt(vcpu);
> > + unsigned long addr;
> >
> > addr = kvm_vcpu_get_fault_ipa(vcpu);
> > addr |= FAR_TO_FIPA_OFFSET(kvm_vcpu_get_hfar(vcpu));
> >
> > - __kvm_inject_sea(vcpu, kvm_vcpu_trap_is_iabt(vcpu), addr);
> > -
> > /*
> > * If AArch64 or LPAE, set FSC to 0 to indicate an Address
> > * Size Fault at level 0, as if exceeding PARange.
> > @@ -299,13 +321,10 @@ void kvm_inject_size_fault(struct kvm_vcpu *vcpu)
> > * Non-LPAE guests will only get the external abort, as there
> > * is no way to describe the ASF.
> > */
> > - if (vcpu_el1_is_32bit(vcpu) &&
> > - !(vcpu_read_sys_reg(vcpu, TCR_EL1) & TTBCR_EAE))
> > - return;
> > -
> > - esr = vcpu_read_sys_reg(vcpu, exception_esr_elx(vcpu));
> > - esr &= ~GENMASK_ULL(5, 0);
> > - vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
> > + if (vcpu_trans_32bit_non_lpae(vcpu))
> > + __kvm_inject_sea(vcpu, iabt, addr);
> > + else
> > + __kvm_inject_abort(vcpu, iabt, ESR_ELx_FSC_ADDRSZ, addr);
>
> The rest seems correct, and make easier to comprehend. :)
>
> Thanks!
> Leo
prev parent reply other threads:[~2026-11-12 22:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20261104192124.3932743-1-oupton@kernel.org>
2026-11-04 19:21 ` [PATCH 1/8] KVM: arm64: nv: Disallow KVM_CAP_ARM_MTE + FEAT_EL2 Oliver Upton
2026-11-04 19:21 ` [PATCH 7/8] KVM: arm64: nv: Enforce sysreg visibility before evaulating guest traps Oliver Upton
2026-11-11 12:16 ` Leonardo Bras
2026-11-04 19:21 ` [PATCH 8/8] KVM: arm64: Stop leaning on __kvm_inject_sea() for abort injection Oliver Upton
2026-11-12 15:12 ` Leonardo Bras
2026-11-12 22:22 ` Oliver Upton [this message]
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=avY8IUOoxScjsisA@kernel.org \
--to=oupton@kernel.org \
--cc=fuad.tabba@linux.dev \
--cc=joey.gouly@arm.com \
--cc=kvmarm@lists.linux.dev \
--cc=leo.bras@arm.com \
--cc=maz@kernel.org \
--cc=seiden@linux.ibm.com \
--cc=stable@vger.kernel.org \
--cc=suzuki.poulose@arm.com \
--cc=weilin.chang@arm.com \
--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