Re: [PATCH v17 04/20] KVM: arm64: Refactor the vcpu_load to allow for VM specific callbacks

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

On 10/09/2026 05:00, Gavin Shan wrote:
> Hi Suzuki,
> 
> On 9/9/26 2:22 AM, Suzuki K Poulose wrote:
>> Based on a work by Marc Zyngier
>>
>> To keep the VCPU load/put handling cleaner with the different kinds of 
>> VM types,
>> we are about to introduce VM specific callbacks to do just the right 
>> thing.
>> In preparation for that, make some refactoring to add the change easier.
>>
>> No functional changes intended.
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   arch/arm64/kvm/arm.c | 46 +++++++++++++++++++++++++++-----------------
>>   1 file changed, 28 insertions(+), 18 deletions(-)
>>
> 
> Nitpicks below, none of them are introduced by this patch itself. In 
> either way:
> 
> Reviewed-by: Gavin Shan <gshan@redhat.com>

Thank you !

> 
>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>> index 67f1ff9bc4fbe..51fc651267157 100644
>> --- a/arch/arm64/kvm/arm.c
>> +++ b/arch/arm64/kvm/arm.c
>> @@ -683,14 +683,11 @@ static bool kvm_vcpu_should_clear_twe(struct 
>> kvm_vcpu *vcpu)
>>       return single_task_running();
>>   }
>> -void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>> +static void vcpu_prepare_mmu(struct kvm_vcpu *vcpu)
>>   {
>>       struct kvm_s2_mmu *mmu;
>>       int *last_ran;
>> -    if (is_protected_kvm_enabled())
>> -        goto nommu;
>> -
>>       if (vcpu_has_nv(vcpu))
>>           kvm_vcpu_load_hw_mmu(vcpu);
>> @@ -720,10 +717,33 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, 
>> int cpu)
>>           kvm_call_hyp(__kvm_flush_cpu_context, mmu);
>>           *last_ran = vcpu->vcpu_idx;
>>       }
>> +}
>> +
>> +static void vcpu_set_wfx_traps(struct kvm_vcpu *vcpu)
>> +{
>> +    if (kvm_vcpu_should_clear_twe(vcpu))
>> +        vcpu->arch.hcr_el2 &= ~HCR_TWE;
>> +    else
>> +        vcpu->arch.hcr_el2 |= HCR_TWE;
>> +
>> +    if (kvm_vcpu_should_clear_twi(vcpu))
>> +        vcpu->arch.hcr_el2 &= ~HCR_TWI;
>> +    else
>> +        vcpu->arch.hcr_el2 |= HCR_TWI;
>> +}
>> +
> 
> It seems FIELD_MODIFY() can be applied here if no visible constraints 
> exist:
> 
>      FIELD_MODIFY(HCR_TWE, vcpu->arch.hcr_el2, ! 
> kvm_vcpu_should_clear_twe(vcpu));
>      FIELD_MODIFY(HCR_TWI, vcpu->arch.hcr_el2, ! 
> kvm_vcpu_should_clear_twi(vcpu));

I would leave this out for now. Can be a separate patch

> 
>> +static void vcpu_load_pvtime(struct kvm_vcpu *vcpu)
>> +{
>> +    if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
>> +        kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
>> +}
>> +
>> +void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>> +{
>> +    if (!is_protected_kvm_enabled())
>> +        vcpu_prepare_mmu(vcpu);
>>
> 
> I think we probably try to depend on kvm->arch.vm_flavor instead
> of !is_protected_kvm_enabled().
> 
>      struct kvm *kvm = vcpu->kvm;
> 
>      if (kvm_vm_is_protected(kvm) || kvm_vm_flavor_eq(kvm, VM_PKVM))
>          vcpu_prepare_mmu(vcpu);

This will be removed as part of the call back in the next patch.

Cheers
Suzuki