Re: [PATCH v5 09/15] KVM: arm64: Handle HDBSS faults

[Tian Zheng]

On 11/2/2026 10:34 PM, Leonardo Bras wrote:
> On Tue, Sep 29, 2026 at 06:36:49PM +0800, Tian Zheng wrote:
> > From: Eillon <yezhenyu2@xxxxxxxxxx>
> >
> > Once the buffer is full, hardware stops promoting writable-clean
> > descriptors and raises a stage-2 Permission fault with
> > ESR_EL2.ISS2.HDBSSF instead. Failed HDBSS accesses, such as external
> > aborts and granule protection faults, are reported the same way.
> >
> > Dispatch from kvm_handle_guest_abort() via the new esr_iss2_is_hdbssf()
> > helper. FSC == OK means the exit path already flushed, so resume the
> > guest; any other FSC is an error - clear it and report -EFAULT.
> >
> > Signed-off-by: Eillon <yezhenyu2@xxxxxxxxxx>
> > Signed-off-by: Tian Zheng <zhengtian10@xxxxxxxxxx>
> > ---
> >   arch/arm64/include/asm/esr.h           |  5 +++++
> >   arch/arm64/include/asm/kvm_dirty_bit.h |  1 +
> >   arch/arm64/kvm/dirty_bit.c             | 29 ++++++++++++++++++++++++++
> >   arch/arm64/kvm/mmu.c                   |  4 ++++
> >   4 files changed, 39 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> > index f816f5d77f1a..4b3ccd407faa 100644
> > --- a/arch/arm64/include/asm/esr.h
> > +++ b/arch/arm64/include/asm/esr.h
> > @@ -437,6 +437,11 @@
> >   #ifndef __ASSEMBLER__
> >   #include <asm/types.h>
> >
> > +static inline bool esr_iss2_is_hdbssf(unsigned long esr)
> > +{
> > +	return !!(ESR_ELx_ISS2(esr) & ESR_ELx_HDBSSF);
> > +}
> > +
> >   static inline unsigned long esr_brk_comment(unsigned long esr)
> >   {
> >   	return esr & ESR_ELx_BRK64_ISS_COMMENT_MASK;
> > diff --git a/arch/arm64/include/asm/kvm_dirty_bit.h b/arch/arm64/include/asm/kvm_dirty_bit.h
> > index d828e6b43fe9..eb2039820777 100644
> > --- a/arch/arm64/include/asm/kvm_dirty_bit.h
> > +++ b/arch/arm64/include/asm/kvm_dirty_bit.h
> > @@ -39,5 +39,6 @@ static inline bool vcpu_hdbss_enabled(struct kvm_vcpu *vcpu)
> >   int kvm_arm_vcpu_alloc_hdbss(struct kvm_vcpu *vcpu);
> >   void kvm_arm_vcpu_free_hdbss(struct kvm_vcpu *vcpu);
> >   void kvm_flush_hdbss_buffer(struct kvm_vcpu *vcpu);
> > +int kvm_handle_hdbss_fault(struct kvm_vcpu *vcpu);
> >
> >   #endif /* __ARM64_KVM_DIRTY_BIT_H__ */
> > diff --git a/arch/arm64/kvm/dirty_bit.c b/arch/arm64/kvm/dirty_bit.c
> > index be0d12555c84..893a8c4248bc 100644
> > --- a/arch/arm64/kvm/dirty_bit.c
> > +++ b/arch/arm64/kvm/dirty_bit.c
> > @@ -98,3 +98,32 @@ void kvm_flush_hdbss_buffer(struct kvm_vcpu *vcpu)
> >   	vcpu->arch.hdbss.hdbssprod_el2 = prod;
> >   	isb();
> >   }
> > +
> > +int kvm_handle_hdbss_fault(struct kvm_vcpu *vcpu)
> > +{
> > +	u64 prod;
> > +	u64 fsc;
> > +
> > +	if (WARN_ON_ONCE(!system_supports_hdbss()))
> > +		return -EFAULT;
> > +
> > +	if (WARN_ON_ONCE(!vcpu_hdbss_enabled(vcpu)))
> > +		return -EFAULT;
> > +
> > +	prod = read_sysreg_s(SYS_HDBSSPROD_EL2);
> > +	fsc = FIELD_GET(HDBSSPROD_EL2_FSC_MASK, prod);
> > +
> > +	if (fsc == HDBSSPROD_EL2_FSC_OK)
> > +		/* Buffer full: the exit path drained it before handle_exit. */
> > +		return 1;
> > +
> > +	if (fsc != HDBSSPROD_EL2_FSC_ExternalAbort &&
> > +	    fsc != HDBSSPROD_EL2_FSC_GPF)
> > +		WARN_ONCE(1,
> > +			  "Unexpected HDBSS fault type, FSC: 0x%llx (prod=0x%llx, vcpu=%d)\n",
> > +			  fsc, prod, vcpu->vcpu_id);
>
> Any reason for using if() before warn_once, given the first parameter is a
> condition? I mean, why not use, instead:
>
> +	WARN_ONCE(fsc != HDBSSPROD_EL2_FSC_ExternalAbort &&
> +		  fsc != HDBSSPROD_EL2_FSC_GPF,
> +		  "Unexpected HDBSS fault type, FSC: 0x%llx (prod=0x%llx, vcpu=%d)\n",
> +		  fsc, prod, vcpu->vcpu_id);

Right, this is a syntax style closer to the Linux kernel. I will modify
it.

> > +
> > +	/* Clear FSC so hardware dirty state updates can resume. */
> > +	write_sysreg_s(prod & ~HDBSSPROD_EL2_FSC_MASK, SYS_HDBSSPROD_EL2);
> > +	return -EFAULT;
> > +}
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 85a98d2c23a9..7bf82d65041c 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -15,6 +15,7 @@
> >   #include <asm/pgalloc.h>
> >   #include <asm/cacheflush.h>
> >   #include <asm/kvm_arm.h>
> > +#include <asm/kvm_dirty_bit.h>
> >   #include <asm/kvm_mmu.h>
> >   #include <asm/kvm_pgtable.h>
> >   #include <asm/kvm_pkvm.h>
> > @@ -2315,6 +2316,9 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
> >
> >   	is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
> >
> > +	if (esr_iss2_is_hdbssf(esr))
> > +		return kvm_handle_hdbss_fault(vcpu);
> > +
> >   	if (esr_fsc_is_translation_fault(esr)) {
> >   		/* Beyond sanitised PARange (which is the IPA limit) */
> >   		if (fault_ipa >= BIT_ULL(get_kvm_ipa_limit())) {
>
> I did not get the criteria used to positioning it here:
> Why placed before is_abt, if not using the result before returning?
> What is the rationale to putting it here?
>
> Thanks!
> Leo

Ah, I'm sorry, there's no particular reason. I'll move it so it's placed
before is_abt.

Thanks!
Tian