linux-kernel - Re: [PATCH v3 06/13] KVM/nVMX: Use kvm_vcpu

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] [day] [month] [year] [list]

Message-ID: <1540244996.11839.46.camel@amazon.de>
Date:   Mon, 22 Oct 2018 21:49:56 +0000
From:   "Raslan, KarimAllah" <karahmed@...zon.de>
To:     "jmattson@...gle.com" <jmattson@...gle.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "rkrcmar@...hat.com" <rkrcmar@...hat.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>
Subject: Re: [PATCH v3 06/13] KVM/nVMX: Use kvm_vcpu_map when mapping the L1
 MSR bitmap

On Mon, 2018-10-22 at 14:42 -0700, Jim Mattson wrote:
> On Sat, Oct 20, 2018 at 3:22 PM, KarimAllah Ahmed <karahmed@...zon.de> wrote:
> > 
> > Use kvm_vcpu_map when mapping the L1 MSR bitmap since using
> > kvm_vcpu_gpa_to_page() and kmap() will only work for guest memory that has
> > a "struct page".
> > 
> > Signed-off-by: KarimAllah Ahmed <karahmed@...zon.de>
> > ---
> > v1 -> v2:
> > - Do not change the lifecycle of the mapping (pbonzini)
> > ---
> >  arch/x86/kvm/vmx.c | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index d857401..5b15ca2 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -847,6 +847,9 @@ struct nested_vmx {
> >         struct page *apic_access_page;
> >         struct page *virtual_apic_page;
> >         struct page *pi_desc_page;
> > +
> > +       struct kvm_host_map msr_bitmap_map;
> > +
> >         struct pi_desc *pi_desc;
> >         bool pi_pending;
> >         u16 posted_intr_nv;
> > @@ -11546,9 +11549,10 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
> >                                                  struct vmcs12 *vmcs12)
> >  {
> >         int msr;
> > -       struct page *page;
> >         unsigned long *msr_bitmap_l1;
> >         unsigned long *msr_bitmap_l0 = to_vmx(vcpu)->nested.vmcs02.msr_bitmap;
> > +       struct kvm_host_map *map = &to_vmx(vcpu)->nested.msr_bitmap_map;
> > +
> >         /*
> >          * pred_cmd & spec_ctrl are trying to verify two things:
> >          *
> > @@ -11574,11 +11578,10 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
> >             !pred_cmd && !spec_ctrl)
> >                 return false;
> > 
> > -       page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->msr_bitmap);
> > -       if (is_error_page(page))
> > +       if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmcs12->msr_bitmap), map))
> 
> Isn't this the sort of high frequency operation that should not use the new API?

With the current implementation of the API, yes. The performance will be 
horrible. This does not affect the current users though (i.e. when guest memory 
is backed by "struct page").

I have a few patches that implements a pfn_cache on top of this as suggested by 
Paolo. This would allow this API to be used for this type of high-frequency 
mappings.

For example with this pfn_cache, booting an Ubuntu was 10x faster (from ~ 2 
minutes to 13 seconds).

> 
> > 
> >                 return false;
> > 
> > -       msr_bitmap_l1 = (unsigned long *)kmap(page);
> > +       msr_bitmap_l1 = (unsigned long *)map->hva;
> >         if (nested_cpu_has_apic_reg_virt(vmcs12)) {
> >                 /*
> >                  * L0 need not intercept reads for MSRs between 0x800 and 0x8ff, it
> > @@ -11626,8 +11629,7 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
> >                                         MSR_IA32_PRED_CMD,
> >                                         MSR_TYPE_W);
> > 
> > -       kunmap(page);
> > -       kvm_release_page_clean(page);
> > +       kvm_vcpu_unmap(&to_vmx(vcpu)->nested.msr_bitmap_map);
> > 
> >         return true;
> >  }
> > --
> > 2.7.4
> > 
> 
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B