Re: [PATCH v2 11/15] svm: Temporary deactivate AVIC during ExtINT handling

[Suthikulpanit, Suravee]

Alex,

On 8/28/19 2:37 PM, Graf (AWS), Alexander wrote:
>>>> @@ -5522,9 +5558,6 @@ static void enable_irq_window(struct kvm_vcpu
>>>> *vcpu)
>>>>    {
>>>>        struct vcpu_svm *svm = to_svm(vcpu);
>>>> -    if (kvm_vcpu_apicv_active(vcpu))
>>>> -        return;
>>>> -
>>>>        /*
>>>>         * In case GIF=0 we can't rely on the CPU to tell us when GIF
>>>> becomes
>>>>         * 1, because that's a separate STGI/VMRUN intercept.  The next
>>>> time we
>>>> @@ -5534,6 +5567,14 @@ static void enable_irq_window(struct kvm_vcpu
>>>> *vcpu)
>>>>         * window under the assumption that the hardware will set the GIF.
>>>>         */
>>>>        if ((vgif_enabled(svm) || gif_set(svm)) && nested_svm_intr(svm)) {
>>>> +        /*
>>>> +         * IRQ window is not needed when AVIC is enabled,
>>>> +         * unless we have pending ExtINT since it cannot be injected
>>>> +         * via AVIC. In such case, we need to temporarily disable AVIC,
>>>> +         * and fallback to injecting IRQ via V_IRQ.
>>>> +         */
>>>> +        if (kvm_vcpu_apicv_active(vcpu))
>>>> +            svm_request_deactivate_avic(&svm->vcpu);
>>> Did you test AVIC with nesting? Did you actually run across this issue
>>> there?
>> Currently, we have not claimed that AVIC is supported w/ nested VM.
>> That's why we have not enabled AVIC by default yet. We will be looking
>> more into that next.
> If it's not supported, please suspend it when we enter a nested guest then?

Ok, this makes sense. I'll update this in V3.

> In that case, the above change is also unnecessary, as it only affects nested guests, no?

Actually, the function name nested_svm_intr() is misleading. Here it 
returns true when _NOT_ in guest mode:

/* This function returns true if it is save to enable the irq window */
   static inline bool nested_svm_intr(struct vcpu_svm *svm)
   {
           if (!is_guest_mode(&svm->vcpu))
                   return true;
   ....

So, the logic above does what we want when AVIC is enabled.

Thanks,
Suravee