linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM
@ 2018-10-13 14:53 lantianyu1986
  2018-10-13 14:53 ` [PATCH V4 1/15] KVM: Add tlb_remote_flush_with_range callback in kvm_x86_ops lantianyu1986
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: lantianyu1986 @ 2018-10-13 14:53 UTC (permalink / raw)
  Cc: linux-mips, linux, kvm, rkrcmar, will.deacon, linux-kernel, hpa,
	kys, kvmarm, sthemmin, x86, michael.h.kelley, mingo,
	catalin.marinas, jhogan, Lan Tianyu, marc.zyngier, haiyangz,
	kvm-ppc, pbonzini, tglx, linux-arm-kernel, christoffer.dall,
	ralf, paul.burton, devel, vkuznets, linuxppc-dev

From: Lan Tianyu <Tianyu.Lan@microsoft.com>

For nested memory virtualization, Hyper-v doesn't set write-protect
L1 hypervisor EPT page directory and page table node to track changes 
while it relies on guest to tell it changes via HvFlushGuestAddressLlist
hypercall. HvFlushGuestAddressLlist hypercall provides a way to flush
EPT page table with ranges which are specified by L1 hypervisor.

If L1 hypervisor uses INVEPT or HvFlushGuestAddressSpace hypercall to
flush EPT tlb, Hyper-V will invalidate associated EPT shadow page table
and sync L1's EPT table when next EPT page fault is triggered.
HvFlushGuestAddressLlist hypercall helps to avoid such redundant EPT
page fault and synchronization of shadow page table.


Change since v3:
	1) Remove code of updating "tlbs_dirty" in kvm_flush_remote_tlbs_with_range()
	2) Remove directly tlb flush in the kvm_handle_hva_range()
	3) Move tlb flush in kvm_set_pte_rmapp() to kvm_mmu_notifier_change_pte()
	4) Combine Vitaly's "don't pass EPT configuration info to
vmx_hv_remote_flush_tlb()" fix

Change since v2:
       1) Fix comment in the kvm_flush_remote_tlbs_with_range()
       2) Move HV_MAX_FLUSH_PAGES and HV_MAX_FLUSH_REP_COUNT to
	hyperv-tlfs.h.
       3) Calculate HV_MAX_FLUSH_REP_COUNT in the macro definition
       4) Use HV_MAX_FLUSH_REP_COUNT to define length of gpa_list in
	struct hv_guest_mapping_flush_list.

Change since v1:
       1) Convert "end_gfn" of struct kvm_tlb_range to "pages" in order
          to avoid confusion as to whether "end_gfn" is inclusive or exlusive.
       2) Add hyperv tlb range struct and replace kvm tlb range struct
          with new struct in order to avoid using kvm struct in the hyperv
	  code directly.



Lan Tianyu (15):
  KVM: Add tlb_remote_flush_with_range callback in kvm_x86_ops
  KVM/MMU: Add tlb flush with range helper function
  KVM: Replace old tlb flush function with new one to flush a specified
    range.
  KVM: Make kvm_set_spte_hva() return int
  KVM/MMU: Move tlb flush in kvm_set_pte_rmapp() to
    kvm_mmu_notifier_change_pte()
  KVM/MMU: Flush tlb directly in the kvm_set_pte_rmapp()
  KVM/MMU: Flush tlb directly in the kvm_zap_gfn_range()
  KVM/MMU: Flush tlb directly in kvm_mmu_zap_collapsible_spte()
  KVM: Add flush_link and parent_pte in the struct kvm_mmu_page
  KVM: Add spte's point in the struct kvm_mmu_page
  KVM/MMU: Replace tlb flush function with range list flush function
  x86/hyper-v: Add HvFlushGuestAddressList hypercall support
  x86/Hyper-v: Add trace in the
    hyperv_nested_flush_guest_mapping_range()
  KVM/VMX: Change hv flush logic when ept tables are mismatched.
  KVM/VMX: Add hv tlb range flush support

 arch/arm/include/asm/kvm_host.h     |   2 +-
 arch/arm64/include/asm/kvm_host.h   |   2 +-
 arch/mips/include/asm/kvm_host.h    |   2 +-
 arch/mips/kvm/mmu.c                 |   3 +-
 arch/powerpc/include/asm/kvm_host.h |   2 +-
 arch/powerpc/kvm/book3s.c           |   3 +-
 arch/powerpc/kvm/e500_mmu_host.c    |   3 +-
 arch/x86/hyperv/nested.c            |  85 ++++++++++++++++++++++
 arch/x86/include/asm/hyperv-tlfs.h  |  32 +++++++++
 arch/x86/include/asm/kvm_host.h     |  12 +++-
 arch/x86/include/asm/mshyperv.h     |  16 +++++
 arch/x86/include/asm/trace/hyperv.h |  14 ++++
 arch/x86/kvm/mmu.c                  | 138 ++++++++++++++++++++++++++++++------
 arch/x86/kvm/paging_tmpl.h          |  10 ++-
 arch/x86/kvm/vmx.c                  |  70 +++++++++++++++---
 virt/kvm/arm/mmu.c                  |   6 +-
 virt/kvm/kvm_main.c                 |   5 +-
 17 files changed, 360 insertions(+), 45 deletions(-)

-- 
2.14.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V4 1/15] KVM: Add tlb_remote_flush_with_range callback in kvm_x86_ops
  2018-10-13 14:53 [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM lantianyu1986
@ 2018-10-13 14:53 ` lantianyu1986
  2018-10-13 14:53 ` [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function lantianyu1986
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: lantianyu1986 @ 2018-10-13 14:53 UTC (permalink / raw)
  Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
	linux-kernel, hpa, kys, kvmarm, sthemmin, x86, michael.h.kelley,
	mingo, jhogan, Lan Tianyu, marc.zyngier, haiyangz, kvm-ppc,
	devel, tglx, linux-arm-kernel, christoffer.dall, ralf,
	paul.burton, pbonzini, vkuznets, linuxppc-dev

From: Lan Tianyu <Tianyu.Lan@microsoft.com>

Add flush range call back in the kvm_x86_ops and platform can use it
to register its associated function. The parameter "kvm_tlb_range"
accepts a single range and flush list which contains a list of ranges.

Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
---
Change since v1:
       Change "end_gfn" to "pages" to aviod confusion as to whether
"end_gfn" is inclusive or exlusive.
---
 arch/x86/include/asm/kvm_host.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4b09d4aa9bf4..fea95aa77319 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -439,6 +439,12 @@ struct kvm_mmu {
 	u64 pdptrs[4]; /* pae */
 };
 
+struct kvm_tlb_range {
+	u64 start_gfn;
+	u64 pages;
+	struct list_head *flush_list;
+};
+
 enum pmc_type {
 	KVM_PMC_GP = 0,
 	KVM_PMC_FIXED,
@@ -1039,6 +1045,8 @@ struct kvm_x86_ops {
 
 	void (*tlb_flush)(struct kvm_vcpu *vcpu, bool invalidate_gpa);
 	int  (*tlb_remote_flush)(struct kvm *kvm);
+	int  (*tlb_remote_flush_with_range)(struct kvm *kvm,
+			struct kvm_tlb_range *range);
 
 	/*
 	 * Flush any TLB entries associated with the given GVA.
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-13 14:53 [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM lantianyu1986
  2018-10-13 14:53 ` [PATCH V4 1/15] KVM: Add tlb_remote_flush_with_range callback in kvm_x86_ops lantianyu1986
@ 2018-10-13 14:53 ` lantianyu1986
  2018-10-14  7:26   ` Liran Alon
  2018-10-13 14:53 ` [PATCH V4 3/15] KVM: Replace old tlb flush function with new one to flush a specified range lantianyu1986
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: lantianyu1986 @ 2018-10-13 14:53 UTC (permalink / raw)
  Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
	linux-kernel, hpa, kys, kvmarm, sthemmin, x86, michael.h.kelley,
	mingo, jhogan, Lan Tianyu, marc.zyngier, haiyangz, kvm-ppc,
	devel, tglx, linux-arm-kernel, christoffer.dall, ralf,
	paul.burton, pbonzini, vkuznets, linuxppc-dev

From: Lan Tianyu <Tianyu.Lan@microsoft.com>

This patch is to add wrapper functions for tlb_remote_flush_with_range
callback.

Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
---
Change sicne V3:
       Remove code of updating "tlbs_dirty"
Change since V2:
       Fix comment in the kvm_flush_remote_tlbs_with_range()
---
 arch/x86/kvm/mmu.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index c73d9f650de7..ff656d85903a 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
 static union kvm_mmu_page_role
 kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
 
+
+static inline bool kvm_available_flush_tlb_with_range(void)
+{
+	return kvm_x86_ops->tlb_remote_flush_with_range;
+}
+
+static void kvm_flush_remote_tlbs_with_range(struct kvm *kvm,
+		struct kvm_tlb_range *range)
+{
+	int ret = -ENOTSUPP;
+
+	if (range && kvm_x86_ops->tlb_remote_flush_with_range)
+		ret = kvm_x86_ops->tlb_remote_flush_with_range(kvm, range);
+
+	if (ret)
+		kvm_flush_remote_tlbs(kvm);
+}
+
+static void kvm_flush_remote_tlbs_with_list(struct kvm *kvm,
+		struct list_head *flush_list)
+{
+	struct kvm_tlb_range range;
+
+	range.flush_list = flush_list;
+
+	kvm_flush_remote_tlbs_with_range(kvm, &range);
+}
+
+static void kvm_flush_remote_tlbs_with_address(struct kvm *kvm,
+		u64 start_gfn, u64 pages)
+{
+	struct kvm_tlb_range range;
+
+	range.start_gfn = start_gfn;
+	range.pages = pages;
+	range.flush_list = NULL;
+
+	kvm_flush_remote_tlbs_with_range(kvm, &range);
+}
+
 void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask, u64 mmio_value)
 {
 	BUG_ON((mmio_mask & mmio_value) != mmio_value);
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V4 3/15] KVM: Replace old tlb flush function with new one to flush a specified range.
  2018-10-13 14:53 [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM lantianyu1986
  2018-10-13 14:53 ` [PATCH V4 1/15] KVM: Add tlb_remote_flush_with_range callback in kvm_x86_ops lantianyu1986
  2018-10-13 14:53 ` [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function lantianyu1986
@ 2018-10-13 14:53 ` lantianyu1986
  2018-10-13 14:53 ` [PATCH V4 4/15] KVM: Make kvm_set_spte_hva() return int lantianyu1986
  2018-10-15 12:04 ` [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM Paolo Bonzini
  4 siblings, 0 replies; 15+ messages in thread
From: lantianyu1986 @ 2018-10-13 14:53 UTC (permalink / raw)
  Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
	linux-kernel, hpa, kys, kvmarm, sthemmin, x86, michael.h.kelley,
	mingo, jhogan, Lan Tianyu, marc.zyngier, haiyangz, kvm-ppc,
	devel, tglx, linux-arm-kernel, christoffer.dall, ralf,
	paul.burton, pbonzini, vkuznets, linuxppc-dev

From: Lan Tianyu <Tianyu.Lan@microsoft.com>

This patch is to replace kvm_flush_remote_tlbs() with kvm_flush_
remote_tlbs_with_address() in some functions without logic change.

Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
---
 arch/x86/kvm/mmu.c         | 31 +++++++++++++++++++++----------
 arch/x86/kvm/paging_tmpl.h |  3 ++-
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ff656d85903a..9b9db36df103 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1490,8 +1490,12 @@ static bool __drop_large_spte(struct kvm *kvm, u64 *sptep)
 
 static void drop_large_spte(struct kvm_vcpu *vcpu, u64 *sptep)
 {
-	if (__drop_large_spte(vcpu->kvm, sptep))
-		kvm_flush_remote_tlbs(vcpu->kvm);
+	if (__drop_large_spte(vcpu->kvm, sptep)) {
+		struct kvm_mmu_page *sp = page_header(__pa(sptep));
+
+		kvm_flush_remote_tlbs_with_address(vcpu->kvm, sp->gfn,
+			KVM_PAGES_PER_HPAGE(sp->role.level));
+	}
 }
 
 /*
@@ -1959,7 +1963,8 @@ static void rmap_recycle(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 	rmap_head = gfn_to_rmap(vcpu->kvm, gfn, sp);
 
 	kvm_unmap_rmapp(vcpu->kvm, rmap_head, NULL, gfn, sp->role.level, 0);
-	kvm_flush_remote_tlbs(vcpu->kvm);
+	kvm_flush_remote_tlbs_with_address(vcpu->kvm, sp->gfn,
+			KVM_PAGES_PER_HPAGE(sp->role.level));
 }
 
 int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end)
@@ -2475,7 +2480,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		account_shadowed(vcpu->kvm, sp);
 		if (level == PT_PAGE_TABLE_LEVEL &&
 		      rmap_write_protect(vcpu, gfn))
-			kvm_flush_remote_tlbs(vcpu->kvm);
+			kvm_flush_remote_tlbs_with_address(vcpu->kvm, gfn, 1);
 
 		if (level > PT_PAGE_TABLE_LEVEL && need_sync)
 			flush |= kvm_sync_pages(vcpu, gfn, &invalid_list);
@@ -2595,7 +2600,7 @@ static void validate_direct_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 			return;
 
 		drop_parent_pte(child, sptep);
-		kvm_flush_remote_tlbs(vcpu->kvm);
+		kvm_flush_remote_tlbs_with_address(vcpu->kvm, child->gfn, 1);
 	}
 }
 
@@ -3019,8 +3024,10 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep, unsigned pte_access,
 			ret = RET_PF_EMULATE;
 		kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
 	}
+
 	if (set_spte_ret & SET_SPTE_NEED_REMOTE_TLB_FLUSH || flush)
-		kvm_flush_remote_tlbs(vcpu->kvm);
+		kvm_flush_remote_tlbs_with_address(vcpu->kvm, gfn,
+				KVM_PAGES_PER_HPAGE(level));
 
 	if (unlikely(is_mmio_spte(*sptep)))
 		ret = RET_PF_EMULATE;
@@ -5695,7 +5702,8 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm,
 	 * on PT_WRITABLE_MASK anymore.
 	 */
 	if (flush)
-		kvm_flush_remote_tlbs(kvm);
+		kvm_flush_remote_tlbs_with_address(kvm, memslot->base_gfn,
+			memslot->npages);
 }
 
 static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
@@ -5759,7 +5767,8 @@ void kvm_mmu_slot_leaf_clear_dirty(struct kvm *kvm,
 	 * dirty_bitmap.
 	 */
 	if (flush)
-		kvm_flush_remote_tlbs(kvm);
+		kvm_flush_remote_tlbs_with_address(kvm, memslot->base_gfn,
+				memslot->npages);
 }
 EXPORT_SYMBOL_GPL(kvm_mmu_slot_leaf_clear_dirty);
 
@@ -5777,7 +5786,8 @@ void kvm_mmu_slot_largepage_remove_write_access(struct kvm *kvm,
 	lockdep_assert_held(&kvm->slots_lock);
 
 	if (flush)
-		kvm_flush_remote_tlbs(kvm);
+		kvm_flush_remote_tlbs_with_address(kvm, memslot->base_gfn,
+				memslot->npages);
 }
 EXPORT_SYMBOL_GPL(kvm_mmu_slot_largepage_remove_write_access);
 
@@ -5794,7 +5804,8 @@ void kvm_mmu_slot_set_dirty(struct kvm *kvm,
 
 	/* see kvm_mmu_slot_leaf_clear_dirty */
 	if (flush)
-		kvm_flush_remote_tlbs(kvm);
+		kvm_flush_remote_tlbs_with_address(kvm, memslot->base_gfn,
+				memslot->npages);
 }
 EXPORT_SYMBOL_GPL(kvm_mmu_slot_set_dirty);
 
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 7cf2185b7eb5..6bdca39829bc 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -894,7 +894,8 @@ static void FNAME(invlpg)(struct kvm_vcpu *vcpu, gva_t gva, hpa_t root_hpa)
 			pte_gpa += (sptep - sp->spt) * sizeof(pt_element_t);
 
 			if (mmu_page_zap_pte(vcpu->kvm, sp, sptep))
-				kvm_flush_remote_tlbs(vcpu->kvm);
+				kvm_flush_remote_tlbs_with_address(vcpu->kvm,
+					sp->gfn, KVM_PAGES_PER_HPAGE(sp->role.level));
 
 			if (!rmap_can_add(vcpu))
 				break;
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V4 4/15] KVM: Make kvm_set_spte_hva() return int
  2018-10-13 14:53 [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM lantianyu1986
                   ` (2 preceding siblings ...)
  2018-10-13 14:53 ` [PATCH V4 3/15] KVM: Replace old tlb flush function with new one to flush a specified range lantianyu1986
@ 2018-10-13 14:53 ` lantianyu1986
  2018-10-15 12:04 ` [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM Paolo Bonzini
  4 siblings, 0 replies; 15+ messages in thread
From: lantianyu1986 @ 2018-10-13 14:53 UTC (permalink / raw)
  Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
	linux-kernel, hpa, kys, kvmarm, sthemmin, x86, michael.h.kelley,
	mingo, jhogan, Lan Tianyu, marc.zyngier, haiyangz, kvm-ppc,
	devel, tglx, linux-arm-kernel, christoffer.dall, ralf,
	paul.burton, pbonzini, vkuznets, linuxppc-dev

From: Lan Tianyu <Tianyu.Lan@microsoft.com>

The patch is to make kvm_set_spte_hva() return int and caller can
check return value to determine flush tlb or not.

Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
---
 arch/arm/include/asm/kvm_host.h     | 2 +-
 arch/arm64/include/asm/kvm_host.h   | 2 +-
 arch/mips/include/asm/kvm_host.h    | 2 +-
 arch/mips/kvm/mmu.c                 | 3 ++-
 arch/powerpc/include/asm/kvm_host.h | 2 +-
 arch/powerpc/kvm/book3s.c           | 3 ++-
 arch/powerpc/kvm/e500_mmu_host.c    | 3 ++-
 arch/x86/include/asm/kvm_host.h     | 2 +-
 arch/x86/kvm/mmu.c                  | 3 ++-
 virt/kvm/arm/mmu.c                  | 6 ++++--
 10 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 3ad482d2f1eb..efb820bdad2c 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -225,7 +225,7 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
 #define KVM_ARCH_WANT_MMU_NOTIFIER
 int kvm_unmap_hva_range(struct kvm *kvm,
 			unsigned long start, unsigned long end);
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 
 unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
 int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 3d6d7336f871..2e506c0b3eb7 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -358,7 +358,7 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
 #define KVM_ARCH_WANT_MMU_NOTIFIER
 int kvm_unmap_hva_range(struct kvm *kvm,
 			unsigned long start, unsigned long end);
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end);
 int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
 
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 2c1c53d12179..71c3f21d80d5 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -933,7 +933,7 @@ enum kvm_mips_fault_result kvm_trap_emul_gva_fault(struct kvm_vcpu *vcpu,
 #define KVM_ARCH_WANT_MMU_NOTIFIER
 int kvm_unmap_hva_range(struct kvm *kvm,
 			unsigned long start, unsigned long end);
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end);
 int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
 
diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c
index d8dcdb350405..97e538a8c1be 100644
--- a/arch/mips/kvm/mmu.c
+++ b/arch/mips/kvm/mmu.c
@@ -551,7 +551,7 @@ static int kvm_set_spte_handler(struct kvm *kvm, gfn_t gfn, gfn_t gfn_end,
 	       (pte_dirty(old_pte) && !pte_dirty(hva_pte));
 }
 
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
 {
 	unsigned long end = hva + PAGE_SIZE;
 	int ret;
@@ -559,6 +559,7 @@ void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
 	ret = handle_hva_to_gpa(kvm, hva, end, &kvm_set_spte_handler, &pte);
 	if (ret)
 		kvm_mips_callbacks->flush_shadow_all(kvm);
+	return 0;
 }
 
 static int kvm_age_hva_handler(struct kvm *kvm, gfn_t gfn, gfn_t gfn_end,
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index fac6f631ed29..ab23379c53a9 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -72,7 +72,7 @@ extern int kvm_unmap_hva_range(struct kvm *kvm,
 			       unsigned long start, unsigned long end);
 extern int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end);
 extern int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
-extern void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
+extern int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 
 #define HPTEG_CACHE_NUM			(1 << 15)
 #define HPTEG_HASH_BITS_PTE		13
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index fd9893bc7aa1..437613bb609a 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -850,9 +850,10 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)
 	return kvm->arch.kvm_ops->test_age_hva(kvm, hva);
 }
 
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
 {
 	kvm->arch.kvm_ops->set_spte_hva(kvm, hva, pte);
+	return 0;
 }
 
 void kvmppc_mmu_destroy(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 8f2985e46f6f..c3f312b2bcb3 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -757,10 +757,11 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)
 	return 0;
 }
 
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
 {
 	/* The page will get remapped properly on its next fault */
 	kvm_unmap_hva(kvm, hva);
+	return 0;
 }
 
 /*****************************************/
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index fea95aa77319..19985c602ed6 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1504,7 +1504,7 @@ asmlinkage void kvm_spurious_fault(void);
 int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end);
 int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end);
 int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v);
 int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu);
 int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 9b9db36df103..fd24a4dc45e9 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1918,9 +1918,10 @@ int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end)
 	return kvm_handle_hva_range(kvm, start, end, 0, kvm_unmap_rmapp);
 }
 
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
 {
 	kvm_handle_hva(kvm, hva, (unsigned long)&pte, kvm_set_pte_rmapp);
+	return 0;
 }
 
 static int kvm_age_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index ed162a6c57c5..89a9c5fa9fd7 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1845,14 +1845,14 @@ static int kvm_set_spte_handler(struct kvm *kvm, gpa_t gpa, u64 size, void *data
 }
 
 
-void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
+int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
 {
 	unsigned long end = hva + PAGE_SIZE;
 	kvm_pfn_t pfn = pte_pfn(pte);
 	pte_t stage2_pte;
 
 	if (!kvm->arch.pgd)
-		return;
+		return 0;
 
 	trace_kvm_set_spte_hva(hva);
 
@@ -1863,6 +1863,8 @@ void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
 	clean_dcache_guest_page(pfn, PAGE_SIZE);
 	stage2_pte = pfn_pte(pfn, PAGE_S2);
 	handle_hva_to_gpa(kvm, hva, end, &kvm_set_spte_handler, &stage2_pte);
+
+	return 0;
 }
 
 static int kvm_age_hva_handler(struct kvm *kvm, gpa_t gpa, u64 size, void *data)
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-13 14:53 ` [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function lantianyu1986
@ 2018-10-14  7:26   ` Liran Alon
  2018-10-14  8:16     ` Thomas Gleixner
  0 siblings, 1 reply; 15+ messages in thread
From: Liran Alon @ 2018-10-14  7:26 UTC (permalink / raw)
  To: lantianyu1986
  Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
	linux-kernel, hpa, kys, kvmarm, sthemmin, x86, michael.h.kelley,
	mingo, jhogan, Lan Tianyu, marc.zyngier, haiyangz, kvm-ppc,
	devel, tglx, linux-arm-kernel, christoffer.dall, ralf,
	paul.burton, pbonzini, vkuznets, linuxppc-dev



> On 13 Oct 2018, at 17:53, lantianyu1986@gmail.com wrote:
> 
> From: Lan Tianyu <Tianyu.Lan@microsoft.com>
> 
> This patch is to add wrapper functions for tlb_remote_flush_with_range
> callback.
> 
> Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> ---
> Change sicne V3:
>       Remove code of updating "tlbs_dirty"
> Change since V2:
>       Fix comment in the kvm_flush_remote_tlbs_with_range()
> ---
> arch/x86/kvm/mmu.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index c73d9f650de7..ff656d85903a 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> static union kvm_mmu_page_role
> kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> 
> +
> +static inline bool kvm_available_flush_tlb_with_range(void)
> +{
> +	return kvm_x86_ops->tlb_remote_flush_with_range;
> +}

Seems that kvm_available_flush_tlb_with_range() is not used in this patch…

> +
> +static void kvm_flush_remote_tlbs_with_range(struct kvm *kvm,
> +		struct kvm_tlb_range *range)
> +{
> +	int ret = -ENOTSUPP;
> +
> +	if (range && kvm_x86_ops->tlb_remote_flush_with_range)
> +		ret = kvm_x86_ops->tlb_remote_flush_with_range(kvm, range);
> +
> +	if (ret)
> +		kvm_flush_remote_tlbs(kvm);
> +}
> +
> +static void kvm_flush_remote_tlbs_with_list(struct kvm *kvm,
> +		struct list_head *flush_list)
> +{
> +	struct kvm_tlb_range range;
> +
> +	range.flush_list = flush_list;
> +
> +	kvm_flush_remote_tlbs_with_range(kvm, &range);
> +}
> +
> +static void kvm_flush_remote_tlbs_with_address(struct kvm *kvm,
> +		u64 start_gfn, u64 pages)
> +{
> +	struct kvm_tlb_range range;
> +
> +	range.start_gfn = start_gfn;
> +	range.pages = pages;
> +	range.flush_list = NULL;
> +
> +	kvm_flush_remote_tlbs_with_range(kvm, &range);
> +}
> +
> void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask, u64 mmio_value)
> {
> 	BUG_ON((mmio_mask & mmio_value) != mmio_value);
> -- 
> 2.14.4
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-14  7:26   ` Liran Alon
@ 2018-10-14  8:16     ` Thomas Gleixner
  2018-10-14  9:20       ` Liran Alon
                         ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Thomas Gleixner @ 2018-10-14  8:16 UTC (permalink / raw)
  To: Liran Alon
  Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
	linux-kernel, hpa, kys, kvmarm, lantianyu1986, sthemmin, x86,
	michael.h.kelley, mingo, jhogan, Lan Tianyu, marc.zyngier,
	haiyangz, kvm-ppc, devel, linux-arm-kernel, christoffer.dall,
	ralf, paul.burton, pbonzini, vkuznets, linuxppc-dev

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

On Sun, 14 Oct 2018, Liran Alon wrote:
> > On 13 Oct 2018, at 17:53, lantianyu1986@gmail.com wrote:
> > 
> > From: Lan Tianyu <Tianyu.Lan@microsoft.com>
> > 
> > This patch is to add wrapper functions for tlb_remote_flush_with_range
> > callback.
> > 
> > Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> > ---
> > Change sicne V3:
> >       Remove code of updating "tlbs_dirty"
> > Change since V2:
> >       Fix comment in the kvm_flush_remote_tlbs_with_range()
> > ---
> > arch/x86/kvm/mmu.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index c73d9f650de7..ff656d85903a 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> > static union kvm_mmu_page_role
> > kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> > 
> > +
> > +static inline bool kvm_available_flush_tlb_with_range(void)
> > +{
> > +	return kvm_x86_ops->tlb_remote_flush_with_range;
> > +}
> 
> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…

What's wrong with that? 

It provides the implementation and later patches make use of it. It's a
sensible way to split patches into small, self contained entities.

Thanks,

	tglx
	

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-14  8:16     ` Thomas Gleixner
@ 2018-10-14  9:20       ` Liran Alon
  2018-10-14 12:57         ` Tianyu Lan
  2018-10-14  9:27       ` Russell King - ARM Linux
  2018-10-15 12:02       ` Paolo Bonzini
  2 siblings, 1 reply; 15+ messages in thread
From: Liran Alon @ 2018-10-14  9:20 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
	linux-kernel, hpa, kys, kvmarm, lantianyu1986, sthemmin, x86,
	michael.h.kelley, mingo, jhogan, Lan Tianyu, marc.zyngier,
	haiyangz, kvm-ppc, devel, linux-arm-kernel, christoffer.dall,
	ralf, paul.burton, pbonzini, vkuznets, linuxppc-dev



> On 14 Oct 2018, at 11:16, Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> On Sun, 14 Oct 2018, Liran Alon wrote:
>>> On 13 Oct 2018, at 17:53, lantianyu1986@gmail.com wrote:
>>> 
>>> +
>>> +static inline bool kvm_available_flush_tlb_with_range(void)
>>> +{
>>> +	return kvm_x86_ops->tlb_remote_flush_with_range;
>>> +}
>> 
>> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> 
> What's wrong with that? 
> 
> It provides the implementation and later patches make use of it. It's a
> sensible way to split patches into small, self contained entities.
> 
> Thanks,
> 
> 	tglx
> 	

I guess it’s a matter of taste, but I prefer to not add dead-code for patches
in order for each commit to compile nicely without warnings of declared and unused functions.
I would prefer to just add this utility function on the patch that actually use it.

-Liran


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-14  8:16     ` Thomas Gleixner
  2018-10-14  9:20       ` Liran Alon
@ 2018-10-14  9:27       ` Russell King - ARM Linux
  2018-10-14  9:35         ` Russell King - ARM Linux
  2018-10-15 12:02       ` Paolo Bonzini
  2 siblings, 1 reply; 15+ messages in thread
From: Russell King - ARM Linux @ 2018-10-14  9:27 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-mips, linux, kvm, rkrcmar, will.deacon, christoffer.dall,
	hpa, kys, kvmarm, lantianyu1986, sthemmin, x86, michael.h.kelley,
	mingo, catalin.marinas, jhogan, Lan Tianyu, marc.zyngier,
	haiyangz, kvm-ppc, Liran Alon, pbonzini, linux-arm-kernel,
	linux-kernel, ralf, paul.burton, devel, vkuznets, linuxppc-dev

On Sun, Oct 14, 2018 at 10:16:56AM +0200, Thomas Gleixner wrote:
> On Sun, 14 Oct 2018, Liran Alon wrote:
> > > On 13 Oct 2018, at 17:53, lantianyu1986@gmail.com wrote:
> > > 
> > > From: Lan Tianyu <Tianyu.Lan@microsoft.com>
> > > 
> > > This patch is to add wrapper functions for tlb_remote_flush_with_range
> > > callback.
> > > 
> > > Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> > > ---
> > > Change sicne V3:
> > >       Remove code of updating "tlbs_dirty"
> > > Change since V2:
> > >       Fix comment in the kvm_flush_remote_tlbs_with_range()
> > > ---
> > > arch/x86/kvm/mmu.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 40 insertions(+)
> > > 
> > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > index c73d9f650de7..ff656d85903a 100644
> > > --- a/arch/x86/kvm/mmu.c
> > > +++ b/arch/x86/kvm/mmu.c
> > > @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> > > static union kvm_mmu_page_role
> > > kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> > > 
> > > +
> > > +static inline bool kvm_available_flush_tlb_with_range(void)
> > > +{
> > > +	return kvm_x86_ops->tlb_remote_flush_with_range;
> > > +}
> > 
> > Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> 
> What's wrong with that? 
> 
> It provides the implementation and later patches make use of it. It's a
> sensible way to split patches into small, self contained entities.

From what I can see of the patches that follow _this_ patch in the
series, this function remains unused.  So, not only is it not used
in this patch, it's not used in this series.

I think the real question that needs asking is - what is the plan
for this function, and when will it be used?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-14  9:27       ` Russell King - ARM Linux
@ 2018-10-14  9:35         ` Russell King - ARM Linux
  2018-10-14 13:21           ` Tianyu Lan
  0 siblings, 1 reply; 15+ messages in thread
From: Russell King - ARM Linux @ 2018-10-14  9:35 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-mips, linux, kvm, rkrcmar, will.deacon, linux-kernel, hpa,
	kys, kvmarm, lantianyu1986, sthemmin, x86, michael.h.kelley,
	mingo, catalin.marinas, jhogan, Lan Tianyu, marc.zyngier,
	haiyangz, kvm-ppc, Liran Alon, devel, linux-arm-kernel,
	christoffer.dall, ralf, paul.burton, pbonzini, vkuznets,
	linuxppc-dev

On Sun, Oct 14, 2018 at 10:27:34AM +0100, Russell King - ARM Linux wrote:
> On Sun, Oct 14, 2018 at 10:16:56AM +0200, Thomas Gleixner wrote:
> > On Sun, 14 Oct 2018, Liran Alon wrote:
> > > > On 13 Oct 2018, at 17:53, lantianyu1986@gmail.com wrote:
> > > > 
> > > > From: Lan Tianyu <Tianyu.Lan@microsoft.com>
> > > > 
> > > > This patch is to add wrapper functions for tlb_remote_flush_with_range
> > > > callback.
> > > > 
> > > > Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> > > > ---
> > > > Change sicne V3:
> > > >       Remove code of updating "tlbs_dirty"
> > > > Change since V2:
> > > >       Fix comment in the kvm_flush_remote_tlbs_with_range()
> > > > ---
> > > > arch/x86/kvm/mmu.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 40 insertions(+)
> > > > 
> > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > > index c73d9f650de7..ff656d85903a 100644
> > > > --- a/arch/x86/kvm/mmu.c
> > > > +++ b/arch/x86/kvm/mmu.c
> > > > @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> > > > static union kvm_mmu_page_role
> > > > kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> > > > 
> > > > +
> > > > +static inline bool kvm_available_flush_tlb_with_range(void)
> > > > +{
> > > > +	return kvm_x86_ops->tlb_remote_flush_with_range;
> > > > +}
> > > 
> > > Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> > 
> > What's wrong with that? 
> > 
> > It provides the implementation and later patches make use of it. It's a
> > sensible way to split patches into small, self contained entities.
> 
> From what I can see of the patches that follow _this_ patch in the
> series, this function remains unused.  So, not only is it not used
> in this patch, it's not used in this series.

Note - I seem to have only received patches 1 through 4, so this is
based on the patches I've received.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-14  9:20       ` Liran Alon
@ 2018-10-14 12:57         ` Tianyu Lan
  0 siblings, 0 replies; 15+ messages in thread
From: Tianyu Lan @ 2018-10-14 12:57 UTC (permalink / raw)
  To: liran.alon
  Cc: linux-mips, linux, kvm, Radim Krcmar, catalin.marinas,
	will.deacon, linux-kernel@vger kernel org, H. Peter Anvin, kys,
	kvmarm, sthemmin, the arch/x86 maintainers, michael.h.kelley,
	Ingo Molnar, jhogan, Tianyu Lan, marc.zyngier, haiyangz, kvm-ppc,
	devel, Thomas Gleixner, linux-arm-kernel, christoffer.dall, ralf,
	paul.burton, Paolo Bonzini, vkuznets, linuxppc-dev

Hi Liran & Thomas:
              Thanks for your review.


On Sun, Oct 14, 2018 at 5:20 PM Liran Alon <liran.alon@oracle.com> wrote:
>
>
>
> > On 14 Oct 2018, at 11:16, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > On Sun, 14 Oct 2018, Liran Alon wrote:
> >>> On 13 Oct 2018, at 17:53, lantianyu1986@gmail.com wrote:
> >>>
> >>> +
> >>> +static inline bool kvm_available_flush_tlb_with_range(void)
> >>> +{
> >>> +   return kvm_x86_ops->tlb_remote_flush_with_range;
> >>> +}
> >>
> >> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> >
> > What's wrong with that?
> >
> > It provides the implementation and later patches make use of it. It's a
> > sensible way to split patches into small, self contained entities.
> >
> > Thanks,
> >
> >       tglx
> >
>
> I guess it’s a matter of taste, but I prefer to not add dead-code for patches
> in order for each commit to compile nicely without warnings of declared and unused functions.
> I would prefer to just add this utility function on the patch that actually use it.
>
> -Liran
>

Normally, I also prefer to put the function definition into the patch
which use it.
But the following patch "KVM: Replace old tlb flush function with new
one to flush a specified range"
and other patches which use new functions will change a lot of places.
It's not friendly for review and
so I split them into pieces.
--
Best regards
Tianyu Lan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-14  9:35         ` Russell King - ARM Linux
@ 2018-10-14 13:21           ` Tianyu Lan
  2018-10-14 13:33             ` Russell King - ARM Linux
  0 siblings, 1 reply; 15+ messages in thread
From: Tianyu Lan @ 2018-10-14 13:21 UTC (permalink / raw)
  To: linux
  Cc: linux-mips, linux, kvm, Radim Krcmar, will.deacon,
	linux-kernel@vger kernel org, H. Peter Anvin, kys, kvmarm,
	sthemmin, the arch/x86 maintainers, michael.h.kelley,
	Ingo Molnar, catalin.marinas, jhogan, Tianyu Lan, marc.zyngier,
	haiyangz, kvm-ppc, liran.alon, devel, Thomas Gleixner,
	linux-arm-kernel, christoffer.dall, ralf, paul.burton,
	Paolo Bonzini, vkuznets, linuxppc-dev

Hi Russell:
              Thanks for your review.

On Sun, Oct 14, 2018 at 5:36 PM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> On Sun, Oct 14, 2018 at 10:27:34AM +0100, Russell King - ARM Linux wrote:
> > On Sun, Oct 14, 2018 at 10:16:56AM +0200, Thomas Gleixner wrote:
> > > On Sun, 14 Oct 2018, Liran Alon wrote:
> > > > > On 13 Oct 2018, at 17:53, lantianyu1986@gmail.com wrote:
> > > > >
> > > > > From: Lan Tianyu <Tianyu.Lan@microsoft.com>
> > > > >
> > > > > This patch is to add wrapper functions for tlb_remote_flush_with_range
> > > > > callback.
> > > > >
> > > > > Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> > > > > ---
> > > > > Change sicne V3:
> > > > >       Remove code of updating "tlbs_dirty"
> > > > > Change since V2:
> > > > >       Fix comment in the kvm_flush_remote_tlbs_with_range()
> > > > > ---
> > > > > arch/x86/kvm/mmu.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 40 insertions(+)
> > > > >
> > > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > > > index c73d9f650de7..ff656d85903a 100644
> > > > > --- a/arch/x86/kvm/mmu.c
> > > > > +++ b/arch/x86/kvm/mmu.c
> > > > > @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> > > > > static union kvm_mmu_page_role
> > > > > kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> > > > >
> > > > > +
> > > > > +static inline bool kvm_available_flush_tlb_with_range(void)
> > > > > +{
> > > > > +       return kvm_x86_ops->tlb_remote_flush_with_range;
> > > > > +}
> > > >
> > > > Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> > >
> > > What's wrong with that?
> > >
> > > It provides the implementation and later patches make use of it. It's a
> > > sensible way to split patches into small, self contained entities.
> >
> > From what I can see of the patches that follow _this_ patch in the
> > series, this function remains unused.  So, not only is it not used
> > in this patch, it's not used in this series.
>
> Note - I seem to have only received patches 1 through 4, so this is
> based on the patches I've received.
>

Sorry to confuse your. I get from CCers from get_maintainer.pl script.
The next patch "[PATCH V4 3/15]KVM: Replace old tlb flush function with
new one to flush a specified range" calls new function.
https://lkml.org/lkml/2018/10/13/254

The patch "[PATCH V4 4/15] KVM: Make kvm_set_spte_hva() return int"
changes under ARM directory. Please have a look. Thanks.
-- 
Best regards
Tianyu Lan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-14 13:21           ` Tianyu Lan
@ 2018-10-14 13:33             ` Russell King - ARM Linux
  0 siblings, 0 replies; 15+ messages in thread
From: Russell King - ARM Linux @ 2018-10-14 13:33 UTC (permalink / raw)
  To: Tianyu Lan
  Cc: linux-mips, kvm, Radim Krcmar, will.deacon,
	linux-kernel@vger kernel org, H. Peter Anvin, kys, kvmarm,
	sthemmin, the arch/x86 maintainers, michael.h.kelley,
	Ingo Molnar, catalin.marinas, jhogan, Tianyu Lan, marc.zyngier,
	haiyangz, kvm-ppc, liran.alon, devel, Thomas Gleixner,
	linux-arm-kernel, christoffer.dall, ralf, paul.burton,
	Paolo Bonzini, vkuznets, linuxppc-dev

On Sun, Oct 14, 2018 at 09:21:23PM +0800, Tianyu Lan wrote:
> Sorry to confuse your. I get from CCers from get_maintainer.pl script.

Unfortunately you seem to have made a mistake.  My email address is
'linux@armlinux.org.uk' not 'linux@armlinux.org'.  There is no
'linux@armlinux.org' in MAINTAINERS, yet your emails appear to be
copied to this address.

Consequently, if it was your intention to copy me with the entire
series, that didn't happen.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
  2018-10-14  8:16     ` Thomas Gleixner
  2018-10-14  9:20       ` Liran Alon
  2018-10-14  9:27       ` Russell King - ARM Linux
@ 2018-10-15 12:02       ` Paolo Bonzini
  2 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-10-15 12:02 UTC (permalink / raw)
  To: Thomas Gleixner, Liran Alon
  Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
	linux-kernel, hpa, kys, kvmarm, lantianyu1986, sthemmin, x86,
	michael.h.kelley, mingo, jhogan, Lan Tianyu, marc.zyngier,
	haiyangz, kvm-ppc, linux-arm-kernel, christoffer.dall, ralf,
	paul.burton, devel, vkuznets, linuxppc-dev

On 14/10/2018 10:16, Thomas Gleixner wrote:
>>> +static inline bool kvm_available_flush_tlb_with_range(void)
>>> +{
>>> +	return kvm_x86_ops->tlb_remote_flush_with_range;
>>> +}
>> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> What's wrong with that? 
> 
> It provides the implementation and later patches make use of it. It's a
> sensible way to split patches into small, self contained entities.

That's true, on the other hand I have indeed a concerns with this patch:
this series is not bisectable at all, because all the new code is dead
until the very last patch.  Uses of the new feature should come _after_
the implementation.

I don't have any big problem with what Liran pointed out (and I can live
with the unused static functions that would warn with -Wunused, too),
but the above should be fixed in v5, basically by moving patches 12-15
at the beginning of the series.

Paolo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM
  2018-10-13 14:53 [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM lantianyu1986
                   ` (3 preceding siblings ...)
  2018-10-13 14:53 ` [PATCH V4 4/15] KVM: Make kvm_set_spte_hva() return int lantianyu1986
@ 2018-10-15 12:04 ` Paolo Bonzini
  4 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-10-15 12:04 UTC (permalink / raw)
  To: lantianyu1986
  Cc: linux-mips, linux, kvm, rkrcmar, will.deacon, linux-kernel, hpa,
	kys, kvmarm, sthemmin, x86, michael.h.kelley, mingo,
	catalin.marinas, jhogan, Lan Tianyu, marc.zyngier, haiyangz,
	kvm-ppc, tglx, linux-arm-kernel, christoffer.dall, ralf,
	paul.burton, devel, vkuznets, linuxppc-dev

On 13/10/2018 16:53, lantianyu1986@gmail.com wrote:
> From: Lan Tianyu <Tianyu.Lan@microsoft.com>
> 
> For nested memory virtualization, Hyper-v doesn't set write-protect
> L1 hypervisor EPT page directory and page table node to track changes 
> while it relies on guest to tell it changes via HvFlushGuestAddressLlist
> hypercall. HvFlushGuestAddressLlist hypercall provides a way to flush
> EPT page table with ranges which are specified by L1 hypervisor.
> 
> If L1 hypervisor uses INVEPT or HvFlushGuestAddressSpace hypercall to
> flush EPT tlb, Hyper-V will invalidate associated EPT shadow page table
> and sync L1's EPT table when next EPT page fault is triggered.
> HvFlushGuestAddressLlist hypercall helps to avoid such redundant EPT
> page fault and synchronization of shadow page table.

So I just told you that the first part is well understood but I must
retract that; after carefully reviewing the whole series, I think one of
us is actually very confused.

I am not afraid to say it can be me, but my understanding is that you're
passing L1 GPAs to the hypercall and instead the spec says it expects L2
GPAs.  (Consider that, because KVM's shadow paging code is shared
between nested EPT and !EPT cases, every time you see gpa/gfn in the
code it is for L1, while nested EPT GPAs are really what the code calls
gva.)

What's going on?

Paolo

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-10-15 12:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-13 14:53 [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM lantianyu1986
2018-10-13 14:53 ` [PATCH V4 1/15] KVM: Add tlb_remote_flush_with_range callback in kvm_x86_ops lantianyu1986
2018-10-13 14:53 ` [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function lantianyu1986
2018-10-14  7:26   ` Liran Alon
2018-10-14  8:16     ` Thomas Gleixner
2018-10-14  9:20       ` Liran Alon
2018-10-14 12:57         ` Tianyu Lan
2018-10-14  9:27       ` Russell King - ARM Linux
2018-10-14  9:35         ` Russell King - ARM Linux
2018-10-14 13:21           ` Tianyu Lan
2018-10-14 13:33             ` Russell King - ARM Linux
2018-10-15 12:02       ` Paolo Bonzini
2018-10-13 14:53 ` [PATCH V4 3/15] KVM: Replace old tlb flush function with new one to flush a specified range lantianyu1986
2018-10-13 14:53 ` [PATCH V4 4/15] KVM: Make kvm_set_spte_hva() return int lantianyu1986
2018-10-15 12:04 ` [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).