Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <43859417-ae76-ed1f-eb4f-8a84a35998fc@amazon.com>
References: <1517271028-15916-1-git-send-email-karahmed@amazon.de>
 <1517271028-15916-5-git-send-email-karahmed@amazon.de> <CALMp9eSmjkJR466C6QuudsFNjEvmyn1hBoK8zUh9EGRNSoMu9Q@mail.gmail.com>
 <43859417-ae76-ed1f-eb4f-8a84a35998fc@amazon.com>
From:   Jim Mattson <jmattson@google.com>
Date:   Tue, 30 Jan 2018 14:49:33 -0800
Message-ID: <CALMp9eQsHzuMx042E50Q3c5H1twZ6-d4ojQ=EE664BGX9GGn8Q@mail.gmail.com>
Subject: Re: [PATCH v3 4/4] KVM: VMX: Allow direct access to MSR_IA32_SPEC_CTRL
To:     KarimAllah Ahmed <karahmed@amazon.com>
Cc:     KarimAllah Ahmed <karahmed@amazon.de>,
        kvm list <kvm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        Asit Mallick <asit.k.mallick@intel.com>,
        Arjan Van De Ven <arjan.van.de.ven@intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Dan Williams <dan.j.williams@intel.com>,
        Jun Nakajima <jun.nakajima@intel.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        David Woodhouse <dwmw@amazon.co.uk>,
        Greg KH <gregkh@linuxfoundation.org>,
        Andy Lutomirski <luto@kernel.org>,
        Ashok Raj <ashok.raj@intel.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jan 30, 2018 at 1:00 PM, KarimAllah Ahmed <karahmed@amazon.com> wrote:
> Ooops! I did not think at all about nested :)
>
> This should be addressed now, I hope:
>
> http://git.infradead.org/linux-retpoline.git/commitdiff/f7f0cbba3e0cffcee050a8a5a9597a162d57e572

+               if (cpu_has_vmx_msr_bitmap() && data &&
+                   !vmx->save_spec_ctrl_on_exit) {
+                       vmx->save_spec_ctrl_on_exit = true;
+
+                       msr_bitmap = is_guest_mode(vcpu) ?
vmx->nested.vmcs02.msr_bitmap :
+
vmx->vmcs01.msr_bitmap;
+                       vmx_disable_intercept_for_msr(msr_bitmap,
+                                                     MSR_IA32_SPEC_CTRL,
+                                                     MSR_TYPE_RW);
+               }

There are two ways to get to this point in vmx_set_msr while
is_guest_mode(vcpu) is true:
1) L0 is processing vmcs12's VM-entry MSR load list on emulated
VM-entry (see enter_vmx_non_root_mode).
2) L2 tried to execute WRMSR, writes to the MSR are intercepted in
vmcs02's MSR permission bitmap, and writes to the MSR are not
intercepted in vmcs12's MSR permission bitmap.

In the first case, disabling the intercepts for the MSR in
vmx->nested.vmcs02.msr_bitmap is incorrect, because we haven't yet
determined that the intercepts are clear in vmcs12's MSR permission
bitmap.
In the second case, disabling *both* of the intercepts for the MSR in
vmx->nested.vmcs02.msr_bitmap is incorrect, because we don't know that
the read intercept is clear in vmcs12's MSR permission bitmap.
Furthermore, disabling the write intercept for the MSR in
vmx->nested.vmcs02.msr_bitmap is somewhat fruitless, because
nested_vmx_merge_msr_bitmap is just going to undo that change on the
next emulated VM-entry.