[RFC 00/12] KVM/X86: Introduce a new guest mapping API

[RFC 00/12] KVM/X86: Introduce a new guest mapping API

[KarimAllah Ahmed]

Guest memory can either be directly managed by the kernel (i.e. have a "struct
page") or they can simply live outside kernel control (i.e. do not have a
"struct page"). KVM mostly support these two modes, except in a few places
where the code seems to assume that guest memory must have a "struct page".

This patchset introduces a new mapping function to map guest memory into host
kernel memory. Ideally I should also get rid of all guest mapping functions
that end up converting a guest address to a page, but I decided to get feedback
on this first and see if this is an acceptable API.

Most of the offending code paths that has been updated are in the nested code
base. Mostly because I stumbled upon this code while looking at the nested MSR
bitmap handling for the IBRS patches. There are also offending code paths in
SVM code, but I will do that once the interface is accepted.

KarimAllah Ahmed (12):
  KVM: Introduce helper functions to map/unmap guest memory
  KVM/VMX: Use the new host mapping API for apic_access_page
  KVM/VMX: Use the new host mapping API for virtual_apic_page
  KVM/VMX: Use the new host mapping API for pi_desc_page
  KVM/VMX: Use the new host mapping API for mapping nested vmptr
  KVM/VMX: Use the new host mapping API for handle_vmptrld
  KVM/VMX: Use the new host mapping API for mapping L12 MSR bitmap
  KVM/VMX: Use the new host mapping API for mapping nested PML
  KVM/VMX: Use the new host mapping API for cmpxchg_emulated
  KVM/VMX: Use the new host mapping API for synic_clear_sint_msg_pending
  KVM/VMX: Use the new host mapping API for synic_deliver_msg
  KVM/VMX: Remove kvm_vcpu_gpa_to_page as it is now unused

 arch/x86/kvm/hyperv.c    |  24 +++----
 arch/x86/kvm/vmx.c       | 159 ++++++++++++++++++++---------------------------
 arch/x86/kvm/x86.c       |  12 ++--
 include/linux/kvm_host.h |  18 +++++-
 virt/kvm/kvm_main.c      |  62 ++++++++++++++++++
 5 files changed, 161 insertions(+), 114 deletions(-)

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

-- 
2.7.4

[RFC 01/12] KVM: Introduce helper functions to map/unmap guest memory

[KarimAllah Ahmed]

Introduce helper functions to map and unmap guest memory into host kernel
memory. These helper functions support mapping guest memory both that are
managed by the host kernel (i.e. have a "struct page") and the ones that
are not managed by the kernel (i.e. does not have a "struct page").

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 include/linux/kvm_host.h | 18 ++++++++++++++
 virt/kvm/kvm_main.c      | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 6bdd4b9..45d2854 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -204,6 +204,12 @@ enum {
 	READING_SHADOW_PAGE_TABLES,
 };
 
+struct kvm_host_mapping {
+	struct page *page;
+	void *kaddr;
+	kvm_pfn_t pfn;
+};
+
 /*
  * Sometimes a large or cross-page mmio needs to be broken up into separate
  * exits for userspace servicing.
@@ -700,6 +706,10 @@ struct kvm_memslots *kvm_vcpu_memslots(struct kvm_vcpu *vcpu);
 struct kvm_memory_slot *kvm_vcpu_gfn_to_memslot(struct kvm_vcpu *vcpu, gfn_t gfn);
 kvm_pfn_t kvm_vcpu_gfn_to_pfn_atomic(struct kvm_vcpu *vcpu, gfn_t gfn);
 kvm_pfn_t kvm_vcpu_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn);
+bool kvm_vcpu_gfn_to_host_mapping(struct kvm_vcpu *vcpu, gfn_t gfn,
+				  struct kvm_host_mapping *mapping,
+				  bool kernel_access);
+void kvm_release_host_mapping(struct kvm_host_mapping *mapping, bool dirty);
 struct page *kvm_vcpu_gfn_to_page(struct kvm_vcpu *vcpu, gfn_t gfn);
 unsigned long kvm_vcpu_gfn_to_hva(struct kvm_vcpu *vcpu, gfn_t gfn);
 unsigned long kvm_vcpu_gfn_to_hva_prot(struct kvm_vcpu *vcpu, gfn_t gfn, bool *writable);
@@ -990,6 +1000,14 @@ static inline hpa_t pfn_to_hpa(kvm_pfn_t pfn)
 	return (hpa_t)pfn << PAGE_SHIFT;
 }
 
+static inline bool kvm_vcpu_gpa_to_host_mapping(struct kvm_vcpu *vcpu, gpa_t gpa,
+						struct kvm_host_mapping *mapping,
+						bool kernel_access)
+{
+	return kvm_vcpu_gfn_to_host_mapping(vcpu, gpa_to_gfn(gpa), mapping,
+					    kernel_access);
+}
+
 static inline struct page *kvm_vcpu_gpa_to_page(struct kvm_vcpu *vcpu,
 						gpa_t gpa)
 {
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 210bf82..2b9f93d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1653,6 +1653,68 @@ struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
 }
 EXPORT_SYMBOL_GPL(gfn_to_page);
 
+bool kvm_vcpu_gfn_to_host_mapping(struct kvm_vcpu *vcpu, gfn_t gfn,
+				  struct kvm_host_mapping *mapping,
+				  bool kernel_access)
+{
+	void *kaddr = NULL;
+	kvm_pfn_t pfn;
+	struct page *page = NULL;
+
+	pfn = kvm_vcpu_gfn_to_pfn(vcpu, gfn);
+
+	if (pfn_valid(pfn)) {
+		page = kvm_pfn_to_page(pfn);
+		if (is_error_page(page))
+			return false;
+
+		if (kernel_access) {
+			kaddr = kmap(page);
+			if (!kaddr)
+				return false;
+		}
+
+		mapping->kaddr = kaddr;
+		mapping->page = page;
+		mapping->pfn = pfn;
+		return true;
+	}
+
+	kaddr = memremap(pfn << PAGE_SHIFT, PAGE_SIZE, MEMREMAP_WB);
+	if (!kaddr)
+		return false;
+
+	mapping->page = NULL;
+	mapping->kaddr = kaddr;
+	mapping->pfn = pfn;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_gfn_to_kaddr);
+
+void kvm_release_host_mapping(struct kvm_host_mapping *mapping, bool dirty)
+{
+	if (mapping->page) {
+		if (mapping->kaddr)
+			kunmap(mapping->page);
+
+		if (dirty)
+			kvm_release_page_dirty(mapping->page);
+		else
+			kvm_release_page_clean(mapping->page);
+	} else {
+		if (mapping->kaddr)
+			memunmap(mapping->kaddr);
+
+		if (dirty)
+			kvm_release_pfn_dirty(mapping->pfn);
+		else
+			kvm_release_pfn_clean(mapping->pfn);
+	}
+
+	memset(mapping, 0, sizeof(*mapping));
+}
+
 struct page *kvm_vcpu_gfn_to_page(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
 	kvm_pfn_t pfn;
-- 
2.7.4

[RFC 02/12] KVM/VMX: Use the new host mapping API for apic_access_page

[KarimAllah Ahmed]

For nested guests the apic_access_page was mapped to the host kernel using
kvm_vcpu_gpa_to_page which assumes that all guest memory is backed by a
"struct page". This breaks guests that have their memory outside the kernel
control.

Switch to the new host mapping API which takes care of this use-case as
well.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/vmx.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 5d8a6a91..b76ab06 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -451,7 +451,7 @@ struct nested_vmx {
 	 * Guest pages referred to in the vmcs02 with host-physical
 	 * pointers, so we must keep them pinned while L2 runs.
 	 */
-	struct page *apic_access_page;
+	struct kvm_host_mapping apic_access_mapping;
 	struct page *virtual_apic_page;
 	struct page *pi_desc_page;
 	struct pi_desc *pi_desc;
@@ -7500,10 +7500,8 @@ static void free_nested(struct vcpu_vmx *vmx)
 	}
 	kfree(vmx->nested.cached_vmcs12);
 	/* Unpin physical memory we referred to in the vmcs02 */
-	if (vmx->nested.apic_access_page) {
-		kvm_release_page_dirty(vmx->nested.apic_access_page);
-		vmx->nested.apic_access_page = NULL;
-	}
+	if (vmx->nested.apic_access_mapping.pfn)
+		kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
 	if (vmx->nested.virtual_apic_page) {
 		kvm_release_page_dirty(vmx->nested.virtual_apic_page);
 		vmx->nested.virtual_apic_page = NULL;
@@ -10056,25 +10054,22 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
 		 * physical address remains valid. We keep a reference
 		 * to it so we can release it later.
 		 */
-		if (vmx->nested.apic_access_page) { /* shouldn't happen */
-			kvm_release_page_dirty(vmx->nested.apic_access_page);
-			vmx->nested.apic_access_page = NULL;
-		}
-		page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->apic_access_addr);
+		if (vmx->nested.apic_access_mapping.pfn) /* shouldn't happen */
+			kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
+
 		/*
 		 * If translation failed, no matter: This feature asks
 		 * to exit when accessing the given address, and if it
 		 * can never be accessed, this feature won't do
 		 * anything anyway.
 		 */
-		if (!is_error_page(page)) {
-			vmx->nested.apic_access_page = page;
-			hpa = page_to_phys(vmx->nested.apic_access_page);
-			vmcs_write64(APIC_ACCESS_ADDR, hpa);
-		} else {
+		if (kvm_vcpu_gpa_to_host_mapping(vcpu, vmcs12->apic_access_addr,
+						 &vmx->nested.apic_access_mapping, false))
+			vmcs_write64(APIC_ACCESS_ADDR,
+				     vmx->nested.apic_access_mapping.pfn << PAGE_SHIFT);
+		else
 			vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
 					SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES);
-		}
 	} else if (!(nested_cpu_has_virt_x2apic_mode(vmcs12)) &&
 		   cpu_need_virtualize_apic_accesses(&vmx->vcpu)) {
 		vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
@@ -11686,10 +11681,8 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 	vmx->host_rsp = 0;
 
 	/* Unpin physical memory we referred to in vmcs02 */
-	if (vmx->nested.apic_access_page) {
-		kvm_release_page_dirty(vmx->nested.apic_access_page);
-		vmx->nested.apic_access_page = NULL;
-	}
+	if (vmx->nested.apic_access_mapping.pfn)
+		kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
 	if (vmx->nested.virtual_apic_page) {
 		kvm_release_page_dirty(vmx->nested.virtual_apic_page);
 		vmx->nested.virtual_apic_page = NULL;
-- 
2.7.4

[RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page

[KarimAllah Ahmed]

For nested guests the virtual_apic_page was mapped to the host kernel using
kvm_vcpu_gpa_to_page which assumes that all guest memory is backed by a
"struct page". This breaks guests that have their memory outside the kernel
control.

Switch to the new host mapping API which takes care of this use-case as
well.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/vmx.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b76ab06..6bd0c45 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -452,7 +452,7 @@ struct nested_vmx {
 	 * pointers, so we must keep them pinned while L2 runs.
 	 */
 	struct kvm_host_mapping apic_access_mapping;
-	struct page *virtual_apic_page;
+	struct kvm_host_mapping virtual_apic_mapping;
 	struct page *pi_desc_page;
 	struct pi_desc *pi_desc;
 	bool pi_pending;
@@ -5264,9 +5264,8 @@ static void vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
 
 	max_irr = find_last_bit((unsigned long *)vmx->nested.pi_desc->pir, 256);
 	if (max_irr != 256) {
-		vapic_page = kmap(vmx->nested.virtual_apic_page);
+		vapic_page = vmx->nested.virtual_apic_mapping.kaddr;
 		__kvm_apic_update_irr(vmx->nested.pi_desc->pir, vapic_page);
-		kunmap(vmx->nested.virtual_apic_page);
 
 		status = vmcs_read16(GUEST_INTR_STATUS);
 		if ((u8)max_irr > ((u8)status & 0xff)) {
@@ -7502,10 +7501,8 @@ static void free_nested(struct vcpu_vmx *vmx)
 	/* Unpin physical memory we referred to in the vmcs02 */
 	if (vmx->nested.apic_access_mapping.pfn)
 		kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
-	if (vmx->nested.virtual_apic_page) {
-		kvm_release_page_dirty(vmx->nested.virtual_apic_page);
-		vmx->nested.virtual_apic_page = NULL;
-	}
+	if (vmx->nested.virtual_apic_mapping.pfn)
+		kvm_release_host_mapping(&vmx->nested.virtual_apic_mapping, true);
 	if (vmx->nested.pi_desc_page) {
 		kunmap(vmx->nested.pi_desc_page);
 		kvm_release_page_dirty(vmx->nested.pi_desc_page);
@@ -10045,7 +10042,6 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	struct page *page;
-	u64 hpa;
 
 	if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
 		/*
@@ -10078,11 +10074,8 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
 	}
 
 	if (nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW)) {
-		if (vmx->nested.virtual_apic_page) { /* shouldn't happen */
-			kvm_release_page_dirty(vmx->nested.virtual_apic_page);
-			vmx->nested.virtual_apic_page = NULL;
-		}
-		page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->virtual_apic_page_addr);
+		if (vmx->nested.virtual_apic_mapping.pfn) /* shouldn't happen */
+			kvm_release_host_mapping(&vmx->nested.virtual_apic_mapping, true);
 
 		/*
 		 * If translation failed, VM entry will fail because
@@ -10097,11 +10090,10 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
 		 * control.  But such a configuration is useless, so
 		 * let's keep the code simple.
 		 */
-		if (!is_error_page(page)) {
-			vmx->nested.virtual_apic_page = page;
-			hpa = page_to_phys(vmx->nested.virtual_apic_page);
-			vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, hpa);
-		}
+		if (kvm_vcpu_gpa_to_host_mapping(vcpu, vmcs12->virtual_apic_page_addr,
+						 &vmx->nested.virtual_apic_mapping, true))
+			vmcs_write64(VIRTUAL_APIC_PAGE_ADDR,
+				     vmx->nested.virtual_apic_mapping.pfn << PAGE_SHIFT);
 	}
 
 	if (nested_cpu_has_posted_intr(vmcs12)) {
@@ -11683,10 +11675,8 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 	/* Unpin physical memory we referred to in vmcs02 */
 	if (vmx->nested.apic_access_mapping.pfn)
 		kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
-	if (vmx->nested.virtual_apic_page) {
-		kvm_release_page_dirty(vmx->nested.virtual_apic_page);
-		vmx->nested.virtual_apic_page = NULL;
-	}
+	if (vmx->nested.virtual_apic_mapping.pfn)
+		kvm_release_host_mapping(&vmx->nested.virtual_apic_mapping, true);
 	if (vmx->nested.pi_desc_page) {
 		kunmap(vmx->nested.pi_desc_page);
 		kvm_release_page_dirty(vmx->nested.pi_desc_page);
-- 
2.7.4

[RFC 04/12] KVM/VMX: Use the new host mapping API for pi_desc_page

[KarimAllah Ahmed]

For nested guests the pi_desc_page was mapped to the host kernel using
kvm_vcpu_gpa_to_page which assumes that all guest memory is backed by a
"struct page". This breaks guests that have their memory outside the kernel
control.

Switch to the new host mapping API which takes care of this use-case as
well.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/vmx.c | 40 ++++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6bd0c45..40d73f4 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -453,7 +453,7 @@ struct nested_vmx {
 	 */
 	struct kvm_host_mapping apic_access_mapping;
 	struct kvm_host_mapping virtual_apic_mapping;
-	struct page *pi_desc_page;
+	struct kvm_host_mapping pi_desc_mapping;
 	struct pi_desc *pi_desc;
 	bool pi_pending;
 	u16 posted_intr_nv;
@@ -7501,12 +7501,11 @@ static void free_nested(struct vcpu_vmx *vmx)
 	/* Unpin physical memory we referred to in the vmcs02 */
 	if (vmx->nested.apic_access_mapping.pfn)
 		kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
+
 	if (vmx->nested.virtual_apic_mapping.pfn)
 		kvm_release_host_mapping(&vmx->nested.virtual_apic_mapping, true);
-	if (vmx->nested.pi_desc_page) {
-		kunmap(vmx->nested.pi_desc_page);
-		kvm_release_page_dirty(vmx->nested.pi_desc_page);
-		vmx->nested.pi_desc_page = NULL;
+	if (vmx->nested.pi_desc_mapping.pfn) {
+		kvm_release_host_mapping(&vmx->nested.pi_desc_mapping, true);
 		vmx->nested.pi_desc = NULL;
 	}
 
@@ -10041,7 +10040,6 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
 					struct vmcs12 *vmcs12)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	struct page *page;
 
 	if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
 		/*
@@ -10097,24 +10095,22 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
 	}
 
 	if (nested_cpu_has_posted_intr(vmcs12)) {
-		if (vmx->nested.pi_desc_page) { /* shouldn't happen */
-			kunmap(vmx->nested.pi_desc_page);
-			kvm_release_page_dirty(vmx->nested.pi_desc_page);
-			vmx->nested.pi_desc_page = NULL;
-		}
-		page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->posted_intr_desc_addr);
-		if (is_error_page(page))
+		if (vmx->nested.pi_desc_mapping.pfn) /* shouldn't happen */
+			kvm_release_host_mapping(&vmx->nested.pi_desc_mapping, true);
+
+		if (!kvm_vcpu_gpa_to_host_mapping(vcpu, vmcs12->posted_intr_desc_addr,
+						  &vmx->nested.pi_desc_mapping, true))
 			return;
-		vmx->nested.pi_desc_page = page;
-		vmx->nested.pi_desc = kmap(vmx->nested.pi_desc_page);
+
+		vmx->nested.pi_desc = vmx->nested.pi_desc_mapping.kaddr;
 		vmx->nested.pi_desc =
 			(struct pi_desc *)((void *)vmx->nested.pi_desc +
 			(unsigned long)(vmcs12->posted_intr_desc_addr &
 			(PAGE_SIZE - 1)));
 		vmcs_write64(POSTED_INTR_DESC_ADDR,
-			page_to_phys(vmx->nested.pi_desc_page) +
-			(unsigned long)(vmcs12->posted_intr_desc_addr &
-			(PAGE_SIZE - 1)));
+			     (vmx->nested.pi_desc_mapping.pfn << PAGE_SHIFT) +
+			     (unsigned long)(vmcs12->posted_intr_desc_addr &
+					     (PAGE_SIZE - 1)));
 	}
 	if (cpu_has_vmx_msr_bitmap() &&
 	    nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS) &&
@@ -11675,12 +11671,12 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 	/* Unpin physical memory we referred to in vmcs02 */
 	if (vmx->nested.apic_access_mapping.pfn)
 		kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
+
 	if (vmx->nested.virtual_apic_mapping.pfn)
 		kvm_release_host_mapping(&vmx->nested.virtual_apic_mapping, true);
-	if (vmx->nested.pi_desc_page) {
-		kunmap(vmx->nested.pi_desc_page);
-		kvm_release_page_dirty(vmx->nested.pi_desc_page);
-		vmx->nested.pi_desc_page = NULL;
+
+	if (vmx->nested.pi_desc_mapping.pfn) {
+		kvm_release_host_mapping(&vmx->nested.pi_desc_mapping, true);
 		vmx->nested.pi_desc = NULL;
 	}
 
-- 
2.7.4

[RFC 05/12] KVM/VMX: Use the new host mapping API for mapping nested vmptr

[KarimAllah Ahmed]

For nested guests the vmptr was mapped to the host kernel using
kvm_vcpu_gpa_to_page which assumes that all guest memory is backed by a
"struct page". This breaks guests that have their memory outside the kernel
control.

Switch to the new host mapping API which takes care of this use-case as
well.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/vmx.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 40d73f4..9e45bd1 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7364,7 +7364,7 @@ static int handle_vmon(struct kvm_vcpu *vcpu)
 {
 	int ret;
 	gpa_t vmptr;
-	struct page *page;
+	struct kvm_host_mapping mapping;
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	const u64 VMXON_NEEDED_FEATURES = FEATURE_CONTROL_LOCKED
 		| FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
@@ -7410,19 +7410,17 @@ static int handle_vmon(struct kvm_vcpu *vcpu)
 		return kvm_skip_emulated_instruction(vcpu);
 	}
 
-	page = kvm_vcpu_gpa_to_page(vcpu, vmptr);
-	if (is_error_page(page)) {
+	if (!kvm_vcpu_gpa_to_host_mapping(vcpu, vmptr, &mapping, true)) {
 		nested_vmx_failInvalid(vcpu);
 		return kvm_skip_emulated_instruction(vcpu);
 	}
-	if (*(u32 *)kmap(page) != VMCS12_REVISION) {
-		kunmap(page);
-		kvm_release_page_clean(page);
+	if (*(u32 *)mapping.kaddr != VMCS12_REVISION) {
+		kvm_release_host_mapping(&mapping, false);
 		nested_vmx_failInvalid(vcpu);
 		return kvm_skip_emulated_instruction(vcpu);
 	}
-	kunmap(page);
-	kvm_release_page_clean(page);
+
+	kvm_release_host_mapping(&mapping, false);
 
 	vmx->nested.vmxon_ptr = vmptr;
 	ret = enter_vmx_operation(vcpu);
-- 
2.7.4

[RFC 06/12] KVM/VMX: Use the new host mapping API for handle_vmptrld

[KarimAllah Ahmed]

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/vmx.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9e45bd1..9544df0 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7865,30 +7865,30 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
 	}
 
 	if (vmx->nested.current_vmptr != vmptr) {
+		struct kvm_host_mapping mapping;
 		struct vmcs12 *new_vmcs12;
-		struct page *page;
-		page = kvm_vcpu_gpa_to_page(vcpu, vmptr);
-		if (is_error_page(page)) {
+
+		if (!kvm_vcpu_gpa_to_host_mapping(vcpu, vmptr, &mapping, true)) {
 			nested_vmx_failInvalid(vcpu);
 			return kvm_skip_emulated_instruction(vcpu);
 		}
-		new_vmcs12 = kmap(page);
+
+		new_vmcs12 = mapping.kaddr;
 		if (new_vmcs12->revision_id != VMCS12_REVISION) {
-			kunmap(page);
-			kvm_release_page_clean(page);
+			kvm_release_host_mapping(&mapping, false);
 			nested_vmx_failValid(vcpu,
 				VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID);
 			return kvm_skip_emulated_instruction(vcpu);
 		}
 
 		nested_release_vmcs12(vmx);
+
 		/*
 		 * Load VMCS12 from guest memory since it is not already
 		 * cached.
 		 */
 		memcpy(vmx->nested.cached_vmcs12, new_vmcs12, VMCS12_SIZE);
-		kunmap(page);
-		kvm_release_page_clean(page);
+		kvm_release_host_mapping(&mapping, false);
 
 		set_current_vmptr(vmx, vmptr);
 	}
-- 
2.7.4

[RFC 07/12] KVM/VMX: Use the new host mapping API for mapping L12 MSR bitmap

[KarimAllah Ahmed]

For nested guests the L12 MSR bitmap was mapped to the host kernel using
kvm_vcpu_gpa_to_page which assumes that all guest memory is backed by a
"struct page". This breaks guests that have their memory outside the kernel
control.

Switch to the new host mapping API which takes care of this use-case as
well.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/vmx.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9544df0..7177176 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10186,7 +10186,7 @@ static inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu,
 					       struct vmcs12 *vmcs12)
 {
 	int msr;
-	struct page *page;
+	struct kvm_host_mapping mapping;
 	unsigned long *msr_bitmap_l1;
 	unsigned long *msr_bitmap_l0 = to_vmx(vcpu)->nested.vmcs02.msr_bitmap;
 	/*
@@ -10209,10 +10209,10 @@ static inline bool nested_vmx_merge_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_gpa_to_host_mapping(vcpu, vmcs12->msr_bitmap, &mapping, true))
 		return false;
-	msr_bitmap_l1 = (unsigned long *)kmap(page);
+
+	msr_bitmap_l1 = (unsigned long *)mapping.kaddr;
 
 	memset(msr_bitmap_l0, 0xff, PAGE_SIZE);
 
@@ -10252,8 +10252,7 @@ static inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu,
 					MSR_IA32_PRED_CMD,
 					MSR_TYPE_W);
 
-	kunmap(page);
-	kvm_release_page_clean(page);
+	kvm_release_host_mapping(&mapping , false);
 
 	return true;
 }
-- 
2.7.4

[RFC 08/12] KVM/VMX: Use the new host mapping API for mapping nested PML

[KarimAllah Ahmed]

For nested guests the PML page was mapped to the host kernel using
kvm_vcpu_gpa_to_page which assumes that all guest memory is backed by a
"struct page". This breaks guests that have their memory outside the kernel
control.

Switch to the new host mapping API which takes care of this use-case as
well.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/vmx.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 7177176..620b4ff 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11866,7 +11866,7 @@ static int vmx_write_pml_buffer(struct kvm_vcpu *vcpu)
 	struct vmcs12 *vmcs12;
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	gpa_t gpa;
-	struct page *page = NULL;
+	struct kvm_host_mapping mapping;
 	u64 *pml_address;
 
 	if (is_guest_mode(vcpu)) {
@@ -11888,14 +11888,13 @@ static int vmx_write_pml_buffer(struct kvm_vcpu *vcpu)
 
 		gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS) & ~0xFFFull;
 
-		page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->pml_address);
-		if (is_error_page(page))
+		if (!kvm_vcpu_gpa_to_host_mapping(vcpu, vmcs12->pml_address, &mapping, true))
 			return 0;
 
-		pml_address = kmap(page);
+		pml_address = mapping.kaddr;
 		pml_address[vmcs12->guest_pml_index--] = gpa;
-		kunmap(page);
-		kvm_release_page_clean(page);
+
+		kvm_release_host_mapping(&mapping, false);
 	}
 
 	return 0;
-- 
2.7.4

[RFC 09/12] KVM/VMX: Use the new host mapping API for cmpxchg_emulated

[KarimAllah Ahmed]

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/x86.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ac38143..db0fd24 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4843,9 +4843,9 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
 				     unsigned int bytes,
 				     struct x86_exception *exception)
 {
+	struct kvm_host_mapping mapping;
 	struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
 	gpa_t gpa;
-	struct page *page;
 	char *kaddr;
 	bool exchanged;
 
@@ -4862,12 +4862,10 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
 	if (((gpa + bytes - 1) & PAGE_MASK) != (gpa & PAGE_MASK))
 		goto emul_write;
 
-	page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
-	if (is_error_page(page))
+	if (!kvm_vcpu_gpa_to_host_mapping(vcpu, gpa, &mapping, true))
 		goto emul_write;
 
-	kaddr = kmap_atomic(page);
-	kaddr += offset_in_page(gpa);
+	kaddr = mapping.kaddr + offset_in_page(gpa);
 	switch (bytes) {
 	case 1:
 		exchanged = CMPXCHG_TYPE(u8, kaddr, old, new);
@@ -4884,8 +4882,8 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
 	default:
 		BUG();
 	}
-	kunmap_atomic(kaddr);
-	kvm_release_page_dirty(page);
+
+	kvm_release_host_mapping(&mapping, true);
 
 	if (!exchanged)
 		return X86EMUL_CMPXCHG_FAILED;
-- 
2.7.4

[RFC 10/12] KVM/VMX: Use the new host mapping API for synic_clear_sint_msg_pending

[KarimAllah Ahmed]

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/hyperv.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index dc97f25..408428a 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -137,25 +137,25 @@ static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
 					u32 sint)
 {
 	struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
-	struct page *page;
+	struct kvm_host_mapping mapping;
 	gpa_t gpa;
 	struct hv_message *msg;
 	struct hv_message_page *msg_page;
 
 	gpa = synic->msg_page & PAGE_MASK;
-	page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
-	if (is_error_page(page)) {
+
+
+	if (!kvm_vcpu_gpa_to_host_mapping(vcpu, gpa, &mapping, true)) {
 		vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
 			 gpa);
 		return;
 	}
-	msg_page = kmap_atomic(page);
+	msg_page = mapping.kaddr;
 
 	msg = &msg_page->sint_message[sint];
 	msg->header.message_flags.msg_pending = 0;
 
-	kunmap_atomic(msg_page);
-	kvm_release_page_dirty(page);
+	kvm_release_host_mapping(&mapping, true);
 	kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
 }
 
-- 
2.7.4

[RFC 11/12] KVM/VMX: Use the new host mapping API for synic_deliver_msg

[KarimAllah Ahmed]

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 arch/x86/kvm/hyperv.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 408428a..be6ce0e 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -563,7 +563,7 @@ static int synic_deliver_msg(struct kvm_vcpu_hv_synic *synic, u32 sint,
 			     struct hv_message *src_msg)
 {
 	struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
-	struct page *page;
+	struct kvm_host_mapping mapping;
 	gpa_t gpa;
 	struct hv_message *dst_msg;
 	int r;
@@ -573,11 +573,11 @@ static int synic_deliver_msg(struct kvm_vcpu_hv_synic *synic, u32 sint,
 		return -ENOENT;
 
 	gpa = synic->msg_page & PAGE_MASK;
-	page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
-	if (is_error_page(page))
+
+	if (!kvm_vcpu_gpa_to_host_mapping(vcpu, gpa, &mapping, true))
 		return -EFAULT;
 
-	msg_page = kmap_atomic(page);
+	msg_page = mapping.kaddr;
 	dst_msg = &msg_page->sint_message[sint];
 	if (sync_cmpxchg(&dst_msg->header.message_type, HVMSG_NONE,
 			 src_msg->header.message_type) != HVMSG_NONE) {
@@ -594,8 +594,8 @@ static int synic_deliver_msg(struct kvm_vcpu_hv_synic *synic, u32 sint,
 		else if (r == 0)
 			r = -EFAULT;
 	}
-	kunmap_atomic(msg_page);
-	kvm_release_page_dirty(page);
+
+	kvm_release_host_mapping(&mapping, true);
 	kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
 	return r;
 }
-- 
2.7.4

[RFC 12/12] KVM/VMX: Remove kvm_vcpu_gpa_to_page as it is now unused

[KarimAllah Ahmed]

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
---
 include/linux/kvm_host.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 45d2854..44db14a 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1008,12 +1008,6 @@ static inline bool kvm_vcpu_gpa_to_host_mapping(struct kvm_vcpu *vcpu, gpa_t gpa
 					    kernel_access);
 }
 
-static inline struct page *kvm_vcpu_gpa_to_page(struct kvm_vcpu *vcpu,
-						gpa_t gpa)
-{
-	return kvm_vcpu_gfn_to_page(vcpu, gpa_to_gfn(gpa));
-}
-
 static inline bool kvm_is_error_gpa(struct kvm *kvm, gpa_t gpa)
 {
 	unsigned long hva = gfn_to_hva(kvm, gpa_to_gfn(gpa));
-- 
2.7.4

Re: [RFC 00/12] KVM/X86: Introduce a new guest mapping API

[Mihai Donțu]

On Mon, 2018-02-05 at 19:47 +0100, KarimAllah Ahmed wrote:
> Guest memory can either be directly managed by the kernel (i.e. have a "struct
> page") or they can simply live outside kernel control (i.e. do not have a
> "struct page"). KVM mostly support these two modes, except in a few places
> where the code seems to assume that guest memory must have a "struct page".

In cases where there is no 'struct page', would it be possible to get
two VM-s to share memory (in the way Xen's grant tables do)?

We are working on a page sharing mechanism and would like to know if
this use case can be accommodated.

Thank you,

> This patchset introduces a new mapping function to map guest memory into host
> kernel memory. Ideally I should also get rid of all guest mapping functions
> that end up converting a guest address to a page, but I decided to get feedback
> on this first and see if this is an acceptable API.
> 
> Most of the offending code paths that has been updated are in the nested code
> base. Mostly because I stumbled upon this code while looking at the nested MSR
> bitmap handling for the IBRS patches. There are also offending code paths in
> SVM code, but I will do that once the interface is accepted.
> 
> KarimAllah Ahmed (12):
>   KVM: Introduce helper functions to map/unmap guest memory
>   KVM/VMX: Use the new host mapping API for apic_access_page
>   KVM/VMX: Use the new host mapping API for virtual_apic_page
>   KVM/VMX: Use the new host mapping API for pi_desc_page
>   KVM/VMX: Use the new host mapping API for mapping nested vmptr
>   KVM/VMX: Use the new host mapping API for handle_vmptrld
>   KVM/VMX: Use the new host mapping API for mapping L12 MSR bitmap
>   KVM/VMX: Use the new host mapping API for mapping nested PML
>   KVM/VMX: Use the new host mapping API for cmpxchg_emulated
>   KVM/VMX: Use the new host mapping API for synic_clear_sint_msg_pending
>   KVM/VMX: Use the new host mapping API for synic_deliver_msg
>   KVM/VMX: Remove kvm_vcpu_gpa_to_page as it is now unused
> 
>  arch/x86/kvm/hyperv.c    |  24 +++----
>  arch/x86/kvm/vmx.c       | 159 ++++++++++++++++++++---------------------------
>  arch/x86/kvm/x86.c       |  12 ++--
>  include/linux/kvm_host.h |  18 +++++-
>  virt/kvm/kvm_main.c      |  62 ++++++++++++++++++
>  5 files changed, 161 insertions(+), 114 deletions(-)
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

-- 
Mihai Donțu

Re: [RFC 02/12] KVM/VMX: Use the new host mapping API for apic_access_page

[Jim Mattson]

Perhaps this is a good time to address the long-standing issues with kvm's
treatment of the "APIC-access address" in the VMCS. This address is simply
a token that the hypervisor puts into the PFN of a 4K EPTE (or PTE if using
shadow paging) that triggers APIC virtualization whenever a page walk
terminates with that PFN. This address has to be a legal address (i.e.
within the physical address with supported by the CPU), but it need not
have WB memory behind it. In fact, it need not have anything at all behind
it. When bit 31 ("activate secondary controls") of the primary
processor-based VM-execution controls is set and bit 0 ("virtualize APIC
accesses") of the secondary processor-based VM-execution controls is set,
the PFN recorded in the VMCS "APIC-access address" field will never be
touched. (Instead, the access triggers APIC virtualization, which may
access the PFN recorded in the "Virtual-APIC address" field of the VMCS.)
Mapping the "APIC-access address" page into the kernel is not necessary.

On Mon, Feb 5, 2018 at 10:50 AM KarimAllah Ahmed <karahmed@amazon.de> wrote:

> For nested guests the apic_access_page was mapped to the host kernel using
> kvm_vcpu_gpa_to_page which assumes that all guest memory is backed by a
> "struct page". This breaks guests that have their memory outside the
> kernel
> control.

> Switch to the new host mapping API which takes care of this use-case as
> well.

> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
> ---
>   arch/x86/kvm/vmx.c | 33 +++++++++++++--------------------
>   1 file changed, 13 insertions(+), 20 deletions(-)

> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5d8a6a91..b76ab06 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -451,7 +451,7 @@ struct nested_vmx {
>           * Guest pages referred to in the vmcs02 with host-physical
>           * pointers, so we must keep them pinned while L2 runs.
>           */
> -       struct page *apic_access_page;
> +       struct kvm_host_mapping apic_access_mapping;
>          struct page *virtual_apic_page;
>          struct page *pi_desc_page;
>          struct pi_desc *pi_desc;
> @@ -7500,10 +7500,8 @@ static void free_nested(struct vcpu_vmx *vmx)
>          }
>          kfree(vmx->nested.cached_vmcs12);
>          /* Unpin physical memory we referred to in the vmcs02 */
> -       if (vmx->nested.apic_access_page) {
> -               kvm_release_page_dirty(vmx->nested.apic_access_page);
> -               vmx->nested.apic_access_page = NULL;
> -       }
> +       if (vmx->nested.apic_access_mapping.pfn)
> +               kvm_release_host_mapping(&vmx->nested.apic_access_mapping,
> true);
>          if (vmx->nested.virtual_apic_page) {
>                  kvm_release_page_dirty(vmx->nested.virtual_apic_page);
>                  vmx->nested.virtual_apic_page = NULL;
> @@ -10056,25 +10054,22 @@ static void nested_get_vmcs12_pages(struct
> kvm_vcpu *vcpu,
>                   * physical address remains valid. We keep a reference
>                   * to it so we can release it later.
>                   */
> -               if (vmx->nested.apic_access_page) { /* shouldn't happen */
> -
>   kvm_release_page_dirty(vmx->nested.apic_access_page);
> -                       vmx->nested.apic_access_page = NULL;
> -               }
> -               page = kvm_vcpu_gpa_to_page(vcpu,
> vmcs12->apic_access_addr);
> +               if (vmx->nested.apic_access_mapping.pfn) /* shouldn't
> happen */
> +
>   kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
> +
>                  /*
>                   * If translation failed, no matter: This feature asks
>                   * to exit when accessing the given address, and if it
>                   * can never be accessed, this feature won't do
>                   * anything anyway.
>                   */
> -               if (!is_error_page(page)) {
> -                       vmx->nested.apic_access_page = page;
> -                       hpa = page_to_phys(vmx->nested.apic_access_page);
> -                       vmcs_write64(APIC_ACCESS_ADDR, hpa);
> -               } else {
> +               if (kvm_vcpu_gpa_to_host_mapping(vcpu,
> vmcs12->apic_access_addr,
> +
> &vmx->nested.apic_access_mapping, false))
> +                       vmcs_write64(APIC_ACCESS_ADDR,
> +                                    vmx->nested.apic_access_mapping.pfn
> << PAGE_SHIFT);
> +               else
>                          vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,

> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES);
> -               }
>          } else if (!(nested_cpu_has_virt_x2apic_mode(vmcs12)) &&
>                     cpu_need_virtualize_apic_accesses(&vmx->vcpu)) {
>                  vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
> @@ -11686,10 +11681,8 @@ static void nested_vmx_vmexit(struct kvm_vcpu
> *vcpu, u32 exit_reason,
>          vmx->host_rsp = 0;

>          /* Unpin physical memory we referred to in vmcs02 */
> -       if (vmx->nested.apic_access_page) {
> -               kvm_release_page_dirty(vmx->nested.apic_access_page);
> -               vmx->nested.apic_access_page = NULL;
> -       }
> +       if (vmx->nested.apic_access_mapping.pfn)
> +               kvm_release_host_mapping(&vmx->nested.apic_access_mapping,
> true);
>          if (vmx->nested.virtual_apic_page) {
>                  kvm_release_page_dirty(vmx->nested.virtual_apic_page);
>                  vmx->nested.virtual_apic_page = NULL;
> --
> 2.7.4

Re: [RFC 05/12] KVM/VMX: Use the new host mapping API for mapping nested vmptr

[Jim Mattson]

On Mon, Feb 5, 2018 at 10:49 AM KarimAllah Ahmed <karahmed@amazon.de> wrote:


> @@ -7410,19 +7410,17 @@ static int handle_vmon(struct kvm_vcpu *vcpu)
>                  return kvm_skip_emulated_instruction(vcpu);
>          }

> -       page = kvm_vcpu_gpa_to_page(vcpu, vmptr);
> -       if (is_error_page(page)) {
> +       if (!kvm_vcpu_gpa_to_host_mapping(vcpu, vmptr, &mapping, true)) {
>                  nested_vmx_failInvalid(vcpu);
>                  return kvm_skip_emulated_instruction(vcpu);
>          }
> -       if (*(u32 *)kmap(page) != VMCS12_REVISION) {
> -               kunmap(page);
> -               kvm_release_page_clean(page);
> +       if (*(u32 *)mapping.kaddr != VMCS12_REVISION) {
> +               kvm_release_host_mapping(&mapping, false);
>                  nested_vmx_failInvalid(vcpu);
>                  return kvm_skip_emulated_instruction(vcpu);
>          }
> -       kunmap(page);
> -       kvm_release_page_clean(page);
> +
> +       kvm_release_host_mapping(&mapping, false);

Why go through this explicit mapping/release dance? Why not just:

uint32_t revision;
...
if (kvm_read_guest(vcpu->kvm, vmptr, &revision, sizeof(revision)) ||
     revision != VMCS12_REVISION) {
         nested_vmx_failInvalid(vcpu);
         return kvm_skip_emulated_instruction(vcpu);
}

Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page

[Jim Mattson]

On Mon, Feb 5, 2018 at 10:48 AM KarimAllah Ahmed <karahmed@amazon.de> wrote:

> @@ -5264,9 +5264,8 @@ static void
vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)

>          max_irr = find_last_bit((unsigned long
*)vmx->nested.pi_desc->pir, 256);
>          if (max_irr != 256) {
> -               vapic_page = kmap(vmx->nested.virtual_apic_page);
> +               vapic_page = vmx->nested.virtual_apic_mapping.kaddr;
>                  __kvm_apic_update_irr(vmx->nested.pi_desc->pir,
vapic_page);
> -               kunmap(vmx->nested.virtual_apic_page);

Your kernel mapping now survives exits to userspace. I didn't think that
was kosher, but I'd be happy to hear that I'm wrong.

> @@ -7502,10 +7501,8 @@ static void free_nested(struct vcpu_vmx *vmx)
>          /* Unpin physical memory we referred to in the vmcs02 */
>          if (vmx->nested.apic_access_mapping.pfn)

kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
> -       if (vmx->nested.virtual_apic_page) {
> -               kvm_release_page_dirty(vmx->nested.virtual_apic_page);
> -               vmx->nested.virtual_apic_page = NULL;
> -       }
> +       if (vmx->nested.virtual_apic_mapping.pfn)
> +
kvm_release_host_mapping(&vmx->nested.virtual_apic_mapping, true);

Is the host PFN guaranteed to be non-zero?

Re: [RFC 05/12] KVM/VMX: Use the new host mapping API for mapping nested vmptr

[KarimAllah Ahmed]

On 02/05/2018 11:15 PM, Jim Mattson wrote:
> On Mon, Feb 5, 2018 at 10:49 AM KarimAllah Ahmed <karahmed@amazon.de> wrote:
> 
> 
>> @@ -7410,19 +7410,17 @@ static int handle_vmon(struct kvm_vcpu *vcpu)
>>                   return kvm_skip_emulated_instruction(vcpu);
>>           }
> 
>> -       page = kvm_vcpu_gpa_to_page(vcpu, vmptr);
>> -       if (is_error_page(page)) {
>> +       if (!kvm_vcpu_gpa_to_host_mapping(vcpu, vmptr, &mapping, true)) {
>>                   nested_vmx_failInvalid(vcpu);
>>                   return kvm_skip_emulated_instruction(vcpu);
>>           }
>> -       if (*(u32 *)kmap(page) != VMCS12_REVISION) {
>> -               kunmap(page);
>> -               kvm_release_page_clean(page);
>> +       if (*(u32 *)mapping.kaddr != VMCS12_REVISION) {
>> +               kvm_release_host_mapping(&mapping, false);
>>                   nested_vmx_failInvalid(vcpu);
>>                   return kvm_skip_emulated_instruction(vcpu);
>>           }
>> -       kunmap(page);
>> -       kvm_release_page_clean(page);
>> +
>> +       kvm_release_host_mapping(&mapping, false);
> 
> Why go through this explicit mapping/release dance? Why not just:
> 
> uint32_t revision;
> ...
> if (kvm_read_guest(vcpu->kvm, vmptr, &revision, sizeof(revision)) ||
>       revision != VMCS12_REVISION) {
>           nested_vmx_failInvalid(vcpu);
>           return kvm_skip_emulated_instruction(vcpu);
> }
> 

Fair enough, I will update.
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

Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page

[KarimAllah Ahmed]

On 02/05/2018 11:26 PM, Jim Mattson wrote:
> On Mon, Feb 5, 2018 at 10:48 AM KarimAllah Ahmed <karahmed@amazon.de> wrote:
> 
>> @@ -5264,9 +5264,8 @@ static void
> vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
> 
>>           max_irr = find_last_bit((unsigned long
> *)vmx->nested.pi_desc->pir, 256);
>>           if (max_irr != 256) {
>> -               vapic_page = kmap(vmx->nested.virtual_apic_page);
>> +               vapic_page = vmx->nested.virtual_apic_mapping.kaddr;
>>                   __kvm_apic_update_irr(vmx->nested.pi_desc->pir,
> vapic_page);
>> -               kunmap(vmx->nested.virtual_apic_page);
> 
> Your kernel mapping now survives exits to userspace. I didn't think that
> was kosher, but I'd be happy to hear that I'm wrong.

Can you elaborate a bit? I do not really understand what is the concern.

>> @@ -7502,10 +7501,8 @@ static void free_nested(struct vcpu_vmx *vmx)
>>           /* Unpin physical memory we referred to in the vmcs02 */
>>           if (vmx->nested.apic_access_mapping.pfn)
> 
> kvm_release_host_mapping(&vmx->nested.apic_access_mapping, true);
>> -       if (vmx->nested.virtual_apic_page) {
>> -               kvm_release_page_dirty(vmx->nested.virtual_apic_page);
>> -               vmx->nested.virtual_apic_page = NULL;
>> -       }
>> +       if (vmx->nested.virtual_apic_mapping.pfn)
>> +
> kvm_release_host_mapping(&vmx->nested.virtual_apic_mapping, true);
> 
> Is the host PFN guaranteed to be non-zero?

Yeah, I should not have assumed that PFN zero is invalid. Will fix.
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

Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page

[Jim Mattson]

On Fri, Feb 9, 2018 at 4:15 AM, KarimAllah Ahmed <karahmed@amazon.com> wrote:

> Can you elaborate a bit? I do not really understand what is the concern.

I can't find the posting that gave me this impression. The only thing
I can find is the following in Documentation/vm/highmem.txt:

 (*) kmap().  This permits a short duration mapping of a single page.  It needs
     global synchronization, but is amortized somewhat.  It is also prone to
     deadlocks when using in a nested fashion, and so it is not recommended for
     new code.

What constitutes a "short duration," and does this new usage qualify?

Re: [RFC 00/12] KVM/X86: Introduce a new guest mapping API

[KarimAllah Ahmed]

On 02/05/2018 08:26 PM, Mihai Donțu wrote:
> On Mon, 2018-02-05 at 19:47 +0100, KarimAllah Ahmed wrote:
>> Guest memory can either be directly managed by the kernel (i.e. have a "struct
>> page") or they can simply live outside kernel control (i.e. do not have a
>> "struct page"). KVM mostly support these two modes, except in a few places
>> where the code seems to assume that guest memory must have a "struct page".
> 
> In cases where there is no 'struct page', would it be possible to get
> two VM-s to share memory (in the way Xen's grant tables do)?
> 
> We are working on a page sharing mechanism and would like to know if
> this use case can be accommodated.

If your code is posted somewhere, I can take a look and let you know it
can be accommodated or not.

> 
> Thank you,
> 
>> This patchset introduces a new mapping function to map guest memory into host
>> kernel memory. Ideally I should also get rid of all guest mapping functions
>> that end up converting a guest address to a page, but I decided to get feedback
>> on this first and see if this is an acceptable API.
>>
>> Most of the offending code paths that has been updated are in the nested code
>> base. Mostly because I stumbled upon this code while looking at the nested MSR
>> bitmap handling for the IBRS patches. There are also offending code paths in
>> SVM code, but I will do that once the interface is accepted.
>>
>> KarimAllah Ahmed (12):
>>    KVM: Introduce helper functions to map/unmap guest memory
>>    KVM/VMX: Use the new host mapping API for apic_access_page
>>    KVM/VMX: Use the new host mapping API for virtual_apic_page
>>    KVM/VMX: Use the new host mapping API for pi_desc_page
>>    KVM/VMX: Use the new host mapping API for mapping nested vmptr
>>    KVM/VMX: Use the new host mapping API for handle_vmptrld
>>    KVM/VMX: Use the new host mapping API for mapping L12 MSR bitmap
>>    KVM/VMX: Use the new host mapping API for mapping nested PML
>>    KVM/VMX: Use the new host mapping API for cmpxchg_emulated
>>    KVM/VMX: Use the new host mapping API for synic_clear_sint_msg_pending
>>    KVM/VMX: Use the new host mapping API for synic_deliver_msg
>>    KVM/VMX: Remove kvm_vcpu_gpa_to_page as it is now unused
>>
>>   arch/x86/kvm/hyperv.c    |  24 +++----
>>   arch/x86/kvm/vmx.c       | 159 ++++++++++++++++++++---------------------------
>>   arch/x86/kvm/x86.c       |  12 ++--
>>   include/linux/kvm_host.h |  18 +++++-
>>   virt/kvm/kvm_main.c      |  62 ++++++++++++++++++
>>   5 files changed, 161 insertions(+), 114 deletions(-)
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: kvm@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
> 
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

Re: [RFC 00/12] KVM/X86: Introduce a new guest mapping API

[Mihai Donțu]

[-- Attachment #1: Type: text/plain, Size: 3365 bytes --]

On Sat, 2018-02-10 at 12:33 +0100, KarimAllah Ahmed wrote:
> On 02/05/2018 08:26 PM, Mihai Donțu wrote:
> > On Mon, 2018-02-05 at 19:47 +0100, KarimAllah Ahmed wrote:
> > > Guest memory can either be directly managed by the kernel (i.e. have a "struct
> > > page") or they can simply live outside kernel control (i.e. do not have a
> > > "struct page"). KVM mostly support these two modes, except in a few places
> > > where the code seems to assume that guest memory must have a "struct page".
> > 
> > In cases where there is no 'struct page', would it be possible to get
> > two VM-s to share memory (in the way Xen's grant tables do)?
> > 
> > We are working on a page sharing mechanism and would like to know if
> > this use case can be accommodated.
> 
> If your code is posted somewhere, I can take a look and let you know it
> can be accommodated or not.

Here is the latest patch: https://patchwork.kernel.org/patch/10121697/

The work is fairly alpha quality, but my colleague Mircea Cirjaliu is
essentially trying to get two VMA-s (one belonging to a guest and one
to the other) to point to the same page (much like KSM does). I'm not
very familiar with the mm subsystem and whether something similar can
be achieved with VMA-s of type VM_PFNMAP.

> > > This patchset introduces a new mapping function to map guest memory into host
> > > kernel memory. Ideally I should also get rid of all guest mapping functions
> > > that end up converting a guest address to a page, but I decided to get feedback
> > > on this first and see if this is an acceptable API.
> > > 
> > > Most of the offending code paths that has been updated are in the nested code
> > > base. Mostly because I stumbled upon this code while looking at the nested MSR
> > > bitmap handling for the IBRS patches. There are also offending code paths in
> > > SVM code, but I will do that once the interface is accepted.
> > > 
> > > KarimAllah Ahmed (12):
> > >    KVM: Introduce helper functions to map/unmap guest memory
> > >    KVM/VMX: Use the new host mapping API for apic_access_page
> > >    KVM/VMX: Use the new host mapping API for virtual_apic_page
> > >    KVM/VMX: Use the new host mapping API for pi_desc_page
> > >    KVM/VMX: Use the new host mapping API for mapping nested vmptr
> > >    KVM/VMX: Use the new host mapping API for handle_vmptrld
> > >    KVM/VMX: Use the new host mapping API for mapping L12 MSR bitmap
> > >    KVM/VMX: Use the new host mapping API for mapping nested PML
> > >    KVM/VMX: Use the new host mapping API for cmpxchg_emulated
> > >    KVM/VMX: Use the new host mapping API for synic_clear_sint_msg_pending
> > >    KVM/VMX: Use the new host mapping API for synic_deliver_msg
> > >    KVM/VMX: Remove kvm_vcpu_gpa_to_page as it is now unused
> > > 
> > >   arch/x86/kvm/hyperv.c    |  24 +++----
> > >   arch/x86/kvm/vmx.c       | 159 ++++++++++++++++++++---------------------------
> > >   arch/x86/kvm/x86.c       |  12 ++--
> > >   include/linux/kvm_host.h |  18 +++++-
> > >   virt/kvm/kvm_main.c      |  62 ++++++++++++++++++
> > >   5 files changed, 161 insertions(+), 114 deletions(-)
> > > 
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > > Cc: kvm@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org

-- 
Mihai Donțu

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]