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

[Marc Zyngier]

On 2023-01-11 12:46, Ganapatrao Kulkarni wrote:
> 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.

3/3 should already be in the branch, merged with the shadow
S2 fault handling.

         M.
-- 
Jazz is not dead. It just smells funny...