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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Sep 2019 15:46:54 +0000
From:   "Suthikulpanit, Suravee" <Suravee.Suthikulpanit@....com>
To:     "Graf (AWS), Alexander" <graf@...zon.de>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "rkrcmar@...hat.com" <rkrcmar@...hat.com>,
        "joro@...tes.org" <joro@...tes.org>,
        "Schoenherr, Jan H." <jschoenh@...zon.de>,
        "Raslan, KarimAllah" <karahmed@...zon.de>,
        "Lukaszewicz, Rimas" <rimasluk@...zon.com>,
        "Grimm, Jon" <Jon.Grimm@....com>
Subject: Re: [PATCH v2 11/15] svm: Temporary deactivate AVIC during ExtINT
 handling

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