Re: [PATCH v7 09/45] kvm: arm64: Expose debug HW register numbers for Realm

[Gavin Shan <gshan@xxxxxxxxxx>]

On 2/14/25 2:13 AM, Steven Price wrote:
> From: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
>
> Expose VM specific Debug HW register numbers.
>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> Signed-off-by: Steven Price <steven.price@xxxxxxx>
> ---
>   arch/arm64/kvm/arm.c | 24 +++++++++++++++++++++---
>   1 file changed, 21 insertions(+), 3 deletions(-)

Documentation/virt/kvm/api.rst needs to be updated accordingly.

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index b8fa82be251c..df6eb5e9ca96 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -78,6 +78,22 @@ bool is_kvm_arm_initialised(void)
>   	return kvm_arm_initialised;
>   }
>   
> +static u32 kvm_arm_get_num_brps(struct kvm *kvm)
> +{
> +	if (!kvm_is_realm(kvm))
> +		return get_num_brps();
> +	/* Realm guest is not debuggable. */
> +	return 0;
> +}
> +
> +static u32 kvm_arm_get_num_wrps(struct kvm *kvm)
> +{
> +	if (!kvm_is_realm(kvm))
> +		return get_num_wrps();
> +	/* Realm guest is not debuggable. */
> +	return 0;
> +}
> +

The above two comments "Realm guest is not debuggable." can be dropped since
the code is self-explanatory, and those two functions are unnecessary to be
kept in that way, for example:

	case KVM_CAP_GUEST_DEBUG_HW_BPS:
		return kvm_is_realm(kvm) ? 0 : get_num_brps();
	case KVM_CAP_GUEST_DEBUG_HW_WRPS:
		return kvm_is_realm(kvm) ? 0 : get_num_wrps();


>   int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>   {
>   	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
> @@ -323,7 +339,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>   	case KVM_CAP_ARM_IRQ_LINE_LAYOUT_2:
>   	case KVM_CAP_ARM_NISV_TO_USER:
>   	case KVM_CAP_ARM_INJECT_EXT_DABT:
> -	case KVM_CAP_SET_GUEST_DEBUG:
>   	case KVM_CAP_VCPU_ATTRIBUTES:
>   	case KVM_CAP_PTP_KVM:
>   	case KVM_CAP_ARM_SYSTEM_SUSPEND:
> @@ -331,6 +346,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>   	case KVM_CAP_COUNTER_OFFSET:
>   		r = 1;
>   		break;
> +	case KVM_CAP_SET_GUEST_DEBUG:
> +		r = !kvm_is_realm(kvm);
> +		break;
>   	case KVM_CAP_SET_GUEST_DEBUG2:
>   		return KVM_GUESTDBG_VALID_MASK;
>   	case KVM_CAP_ARM_SET_DEVICE_ADDR:
> @@ -376,10 +394,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>   		r = cpus_have_final_cap(ARM64_HAS_32BIT_EL1);
>   		break;
>   	case KVM_CAP_GUEST_DEBUG_HW_BPS:
> -		r = get_num_brps();
> +		r = kvm_arm_get_num_brps(kvm);
>   		break;
>   	case KVM_CAP_GUEST_DEBUG_HW_WPS:
> -		r = get_num_wrps();
> +		r = kvm_arm_get_num_wrps(kvm);
>   		break;
>   	case KVM_CAP_ARM_PMU_V3:
>   		r = kvm_arm_support_pmu_v3();

Thanks,
Gavin