Re: [PATCH v19 02/20] KVM: arm64: Disable Steal time accounting for protected guests

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 20/09/2026 22:44, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] arm64: kvm: contradictory SMCCC advertisement for PV_TIME on protected VMs
> --
> 
> commit db2c057e4ceefd58162587d4fd417788a4db3dbc
> Author: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
>      KVM: arm64: Disable Steal time accounting for protected guests
>      
>      PVTIME support is advertised by KVM_CAP_STEAL_TIME, which doesn't take into
>      account the kvm instance. Even with that, a VMM could skip the CAP check
>      and proceed to configure the PVTIME as we don't do further check on the
>      DEVICE_CTRL. Tighten this up by passing the KVM instance around wherever
>      possible and catch things early.
> 
>> diff --git a/arch/arm64/kvm/pvtime.c b/arch/arm64/kvm/pvtime.c
>> index 4ceabaa4c30bd..579e0a4720ad2 100644
>> --- a/arch/arm64/kvm/pvtime.c
>> +++ b/arch/arm64/kvm/pvtime.c
>> @@ -67,9 +67,9 @@ gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu)
>>   	return base;
>>   }
>>   
>> -bool kvm_arm_pvtime_supported(void)
>> +bool kvm_arm_pvtime_supported(struct kvm *kvm)
>>   {
>> -	return !!sched_info_on();
>> +	return !!sched_info_on() && (!kvm || !kvm_vm_is_protected(kvm));
>>   }
> 
> [Severity: Medium]
> Does this create a contradictory SMCCC advertisement for protected VMs?
> 
> A protected guest querying ARM_SMCCC_ARCH_FEATURES_FUNC_ID for
> ARM_SMCCC_HV_PV_TIME_FEATURES in kvm_smccc_call_handler() will receive
> SMCCC_RET_SUCCESS because the KVM_REG_ARM_STD_HYP_BIT_PV_TIME bit is
> still set in std_hyp_bmap:

This looks like an existing issue in the code. I have something
like this to fix this :

diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index b11b8821c9fbc..6fabcd8aaacb9 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -402,6 +402,9 @@ void kvm_arm_init_hypercalls(struct kvm *kvm)

         smccc_feat->std_bmap = KVM_ARM_SMCCC_STD_FEATURES;
         smccc_feat->std_hyp_bmap = KVM_ARM_SMCCC_STD_HYP_FEATURES;
+       /* Clear PV_TIME support for protected VMs */
+       if (kvm_vm_is_protected(kvm))
+               clear_bit(KVM_REG_ARM_STD_HYP_BIT_PV_TIME, 
&smccc_feat->std_hyp_bmap);
         smccc_feat->vendor_hyp_bmap = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;

         mt_init(&kvm->arch.smccc_filter);
@@ -532,6 +535,9 @@ static int kvm_arm_set_fw_reg_bmap(struct kvm_vcpu 
*vcpu, u64 reg_id, u64 val)
         case KVM_REG_ARM_STD_HYP_BMAP:
                 fw_reg_bmap = &smccc_feat->std_hyp_bmap;
                 fw_reg_features = KVM_ARM_SMCCC_STD_HYP_FEATURES;
+               /* PV_TIME is not supported for protected VMs */
+               if (vcpu_is_protected(vcpu))
+                       clear_bit(KVM_REG_ARM_STD_HYP_BIT_PV_TIME, 
&fw_reg_features);
                 break;
         case KVM_REG_ARM_VENDOR_HYP_BMAP:
                 fw_reg_bmap = &smccc_feat->vendor_hyp_bmap;

Suzuki

> 
> arch/arm64/kvm/hypercalls.c:kvm_smccc_call_handler() {
> 	...
> 	case ARM_SMCCC_HV_PV_TIME_FEATURES:
> 		if (test_bit(KVM_REG_ARM_STD_HYP_BIT_PV_TIME,
> 			     &smccc_feat->std_hyp_bmap))
> 			val[0] = SMCCC_RET_SUCCESS;
> 		break;
> 	...
> }
> 
> However, when the guest actually invokes the feature,
> kvm_hypercall_pv_features() returns SMCCC_RET_NOT_SUPPORTED because
> the IPA is unconfigured:
> 
> arch/arm64/kvm/pvtime.c:kvm_hypercall_pv_features() {
> 	...
> 	case ARM_SMCCC_HV_PV_TIME_FEATURES:
> 	case ARM_SMCCC_HV_PV_TIME_ST:
> 		if (vcpu->arch.steal.base != INVALID_GPA)
> 			val = SMCCC_RET_SUCCESS;
> 		break;
> 	...
> }
> 
> This violates the SMCCC ABI by advertising ARM_SMCCC_HV_PV_TIME_FEATURES
> as supported but returning NOT_SUPPORTED when it is invoked. Should the
> KVM_REG_ARM_STD_HYP_BIT_PV_TIME bit be cleared for protected VMs?
>