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

[Ganapatrao Kulkarni]

On 11-01-2023 05:09 pm, Marc Zyngier wrote:
> On 2023-01-11 08:46, 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?
>> +           vcpu != kvm_get_running_vcpu()) ||
> 
> Yeah, exactly. I woke up this morning realising this patch was
> *almost* right. Don't write patches like this after a long day
> at work...
> 
>> 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.
> 
> Good, thanks for testing it. I'll roll up an actual patch for that
> and stick it in the monster queue.

Thanks, Please pull patch 3/3 also to nv-6.2 tree along with this patch. 
I will move my setup to nv-6.2 once these patches are in.

> 
> Cheers,
> 
>         M.


Thanks,
Ganapat