| Message ID | 1540074145-31285-7-git-send-email-karahmed@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM/X86: Introduce a new guest mapping interface | expand |
On Sat, Oct 20, 2018 at 3:22 PM, KarimAllah Ahmed <karahmed@amazon.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@amazon.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? > 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 >
On Mon, 2018-10-22 at 14:42 -0700, Jim Mattson wrote: > On Sat, Oct 20, 2018 at 3:22 PM, KarimAllah Ahmed <karahmed@amazon.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@amazon.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
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)) 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; }
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@amazon.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(-)