[PATCH 0/3] KVM: arm64: nv: Fixes for Nested Virtualization issues

[Ganapatrao Kulkarni]

On 11-01-2023 02:16 pm, Ganapatrao Kulkarni wrote:
> 
> 
> On 11-01-2023 03:24 am, Marc Zyngier wrote:
>> On Tue, 10 Jan 2023 12:17:20 +0000,
>> Ganapatrao Kulkarni <gankulkarni at os.amperecomputing.com> wrote:
>>>
>>> I am currently working around this with "nohlt" kernel param to
>>> NestedVM. Any suggestions to handle/fix this case/issue and avoid the
>>> slowness of booting of NestedVM with more cores?
>>>
>>> Note: Guest-Hypervisor and NestedVM are using default kernel installed
>>> using Fedora 36 iso.
>>
>> Despite what I said earlier, I have a vague idea here, thanks to the
>> interesting call traces that you provided (this is really awesome work
>> BTW, given how hard it is to trace things across 3 different kernels).
>>
>> We can slightly limit the impact of the prepare/finish sequence if the
>> guest hypervisor only accesses the active registers for SGIs/PPIs on
>> the vcpu that owns them, forbidding any cross-CPU-to-redistributor
>> access.
>>
>> Something along these lines, which is only boot-tested. Let me know
>> how this fares for you.
>>
>> Thanks,
>>
>>     M.
>>
>> diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c 
>> b/arch/arm64/kvm/vgic/vgic-mmio.c
>> index b32d434c1d4a..1cca45be5335 100644
>> --- a/arch/arm64/kvm/vgic/vgic-mmio.c
>> +++ b/arch/arm64/kvm/vgic/vgic-mmio.c
>> @@ -473,9 +473,10 @@ int vgic_uaccess_write_cpending(struct kvm_vcpu 
>> *vcpu,
>>    * active state can be overwritten when the VCPU's state is synced 
>> coming back
>>    * from the guest.
>>    *
>> - * For shared interrupts as well as GICv3 private interrupts, we have to
>> - * stop all the VCPUs because interrupts can be migrated while we 
>> don't hold
>> - * the IRQ locks and we don't want to be chasing moving targets.
>> + * For shared interrupts as well as GICv3 private interrupts accessed 
>> from the
>> + * non-owning CPU, we have to stop all the VCPUs because interrupts 
>> can be
>> + * migrated while we don't hold the IRQ locks and we don't want to be 
>> chasing
>> + * moving targets.
>>    *
>>    * For GICv2 private interrupts we don't have to do anything because
>>    * userspace accesses to the VGIC state already require all VCPUs to be
>> @@ -484,7 +485,8 @@ int vgic_uaccess_write_cpending(struct kvm_vcpu 
>> *vcpu,
>>    */
>>   static void vgic_access_active_prepare(struct kvm_vcpu *vcpu, u32 
>> intid)
>>   {
>> -    if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3 ||
>> +    if ((vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3 &&
>> +         vcpu == kvm_get_running_vcpu()) ||
> 
> Thanks Marc for the patch!
> 
> I think, you mean not equal to?
Sorry, I did not see your follow up email.

> +           vcpu != kvm_get_running_vcpu()) ||
> 
> With the change to not-equal, the issue is fixed and I could see the 
> NestedVM booting is pretty fast with higher number of cores as well.
> 
>>           intid >= VGIC_NR_PRIVATE_IRQS)
>>           kvm_arm_halt_guest(vcpu->kvm);
>>   }
>> @@ -492,7 +494,8 @@ static void vgic_access_active_prepare(struct 
>> kvm_vcpu *vcpu, u32 intid)
>>   /* See vgic_access_active_prepare */
>>   static void vgic_access_active_finish(struct kvm_vcpu *vcpu, u32 intid)
>>   {
>> -    if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3 ||
>> +    if ((vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3 &&
>> +         vcpu == kvm_get_running_vcpu()) ||
> 
> Same, not equal to.
>>           intid >= VGIC_NR_PRIVATE_IRQS)
>>           kvm_arm_resume_guest(vcpu->kvm);
>>   }
>>
> 
> 
> Thanks,
> Ganapat