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

[Tian Zheng <zhengtian10@huawei.com>]

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@huawei.com>
>>
>> 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@huawei.com>
>> Signed-off-by: Tian Zheng <zhengtian10@huawei.com>
>> ---
>>   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