Re: [PATCH v4 09/18] KVM: arm64: Implement HVC handling for protected guests at EL2

[Ryan Roberts <ryan.roberts@arm.com>]

On 11/11/2026 16:45, Fuad Tabba wrote:
> Hi Ryan,
> 
> On Wed, 11 Nov 2026 13:59:17 +0000, Ryan Roberts <ryan.roberts@arm.com> wrote:
> [...]
>>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> [...]
>>> @@ -2622,6 +2622,10 @@ static void kvm_hyp_init_symbols(void)
>>>          kvm_nvhe_sym(__icache_flags) = __icache_flags;
>>>          kvm_nvhe_sym(kvm_arm_vmid_bits) = kvm_arm_vmid_bits;
>>>
>>> +        kvm_nvhe_sym(spectre_v2_state) = arm64_get_spectre_v2_state();
>>> +        kvm_nvhe_sym(spectre_v4_state) = arm64_get_spectre_v4_state();
>>> +        kvm_nvhe_sym(spectre_bhb_state) = arm64_get_spectre_bhb_state();
>>
>> Is it really correct to snapshot this state then use it at runtime assuming its
>> still accurate? See below...
> 
> These can't change after this point: update_mitigation_state() doesn't
> update a state once system_capabilities_finalized() is true, and
> kvm_arm_init() runs after that.
> 
> [...]
>>> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> [...]
>>> @@ -1181,8 +1187,70 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code)
> [...]
>>> +        case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
>>> +                /* The workaround calls themselves are handled on hyp entry. */
>>> +                val[0] = SMCCC_RET_NOT_SUPPORTED;
> [...]
>>> +                case ARM_SMCCC_ARCH_WORKAROUND_2:
>>> +                        switch (spectre_v4_state) {
>>> +                        case SPECTRE_VULNERABLE:
>>> +                                break;
>>> +                        case SPECTRE_MITIGATED:
>>> +                                /* With SSBS advertised the guest's default is safe. */
>>> +                                if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, SSBS, IMP))
>>> +                                        break;
>>
>> For this case (and the break immediately above) you return val[0] =
>> SMCCC_RET_INVALID_PARAMETER (from init at top of this function). But the
>> kernel's kvm_smccc_call_handler() returns val[0] = SMCCC_RET_NOT_SUPPORTED in
>> these cases (which seems more likely to be correct). Does this violate the ABI?
> 
> Both return NOT_SUPPORTED. val[0] is set to it at the top of the
> ARM_SMCCC_ARCH_FEATURES_FUNC_ID case.

Ahh missed that oops!

> 
>>
>>> +                                fallthrough;
>>> +                        case SPECTRE_UNAFFECTED:
>>> +                                val[0] = SMCCC_RET_NOT_REQUIRED;
>>
>> Is it possible that the (compromised) kernel could have disabled the mitigation
>> after the state snapshot was taken? I.e. spectre_v4_state == SPECTRE_MITIGATED
>> and the VM doesn't advertise SSBS so it returns SMCCC_RET_NOT_REQUIRED here. But
>> the kernel has since turned off the mitigation?
> 
> Yes, where the mitigation is the firmware one. The host can disable it
> with an ARM_SMCCC_ARCH_WORKAROUND_2 call, which EL2 forwards, and EL2
> doesn't enable it again before running the guest. With TF-A that's
> only on a Cortex-A76 without SSBS, the only CPU it implements the
> dynamic workaround for.

If the mitigation is via SSBS but SSBS is not exposed to the guest, I'm guessing
this is safe because the guest is expected to set SSBS=0 in this case?

Thanks,
Ryan

> 
> I'll send out a fix. Thanks for spotting this!
> 
> Cheers,
> /fuad