linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes
@ 2022-02-22 15:46 Vitaly Kuznetsov
  2022-02-22 15:46 ` [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi() Vitaly Kuznetsov
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-22 15:46 UTC (permalink / raw)
  To: kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

While working on some Hyper-V TLB flush improvements and Direct TLB flush
feature for Hyper-V on KVM I experienced Windows Server 2019 crashes on
boot when XMM fast hypercall input feature is advertised. Turns out,
HVCALL_SEND_IPI_EX is also an XMM fast hypercall and returning an error
kills the guest. This is fixed in PATCH4. PATCH3 fixes erroneous capping
of sparse CPU banks for XMM fast TLB flush hypercalls. The problem should
be reproducible with >360 vCPUs.

Vitaly Kuznetsov (4):
  KVM: x86: hyper-v: Drop redundant 'ex' parameter from
    kvm_hv_send_ipi()
  KVM: x86: hyper-v: Drop redundant 'ex' parameter from
    kvm_hv_flush_tlb()
  KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast
    TLB flush hypercalls
  KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall

 arch/x86/kvm/hyperv.c | 84 +++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 39 deletions(-)

-- 
2.35.1


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

* [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi()
  2022-02-22 15:46 [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Vitaly Kuznetsov
@ 2022-02-22 15:46 ` Vitaly Kuznetsov
  2022-02-25 18:21   ` Maxim Levitsky
  2022-02-28 10:45   ` Siddharth Chandrasekaran
  2022-02-22 15:46 ` [PATCH 2/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_flush_tlb() Vitaly Kuznetsov
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-22 15:46 UTC (permalink / raw)
  To: kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

'struct kvm_hv_hcall' has all the required information already,
there's no need to pass 'ex' additionally.

No functional change intended.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/hyperv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 6e38a7d22e97..15b6a7bd2346 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1875,7 +1875,7 @@ static void kvm_send_ipi_to_many(struct kvm *kvm, u32 vector,
 	}
 }
 
-static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
+static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 {
 	struct kvm *kvm = vcpu->kvm;
 	struct hv_send_ipi_ex send_ipi_ex;
@@ -1889,7 +1889,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
 	u32 vector;
 	bool all_cpus;
 
-	if (!ex) {
+	if (hc->code == HVCALL_SEND_IPI) {
 		if (!hc->fast) {
 			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi,
 						    sizeof(send_ipi))))
@@ -2279,14 +2279,14 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
 			break;
 		}
-		ret = kvm_hv_send_ipi(vcpu, &hc, false);
+		ret = kvm_hv_send_ipi(vcpu, &hc);
 		break;
 	case HVCALL_SEND_IPI_EX:
 		if (unlikely(hc.fast || hc.rep)) {
 			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
 			break;
 		}
-		ret = kvm_hv_send_ipi(vcpu, &hc, true);
+		ret = kvm_hv_send_ipi(vcpu, &hc);
 		break;
 	case HVCALL_POST_DEBUG_DATA:
 	case HVCALL_RETRIEVE_DEBUG_DATA:
-- 
2.35.1


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

* [PATCH 2/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_flush_tlb()
  2022-02-22 15:46 [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Vitaly Kuznetsov
  2022-02-22 15:46 ` [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi() Vitaly Kuznetsov
@ 2022-02-22 15:46 ` Vitaly Kuznetsov
  2022-02-25 18:22   ` Maxim Levitsky
  2022-02-28 10:46   ` Siddharth Chandrasekaran
  2022-02-22 15:46 ` [PATCH 3/4] KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast TLB flush hypercalls Vitaly Kuznetsov
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-22 15:46 UTC (permalink / raw)
  To: kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

'struct kvm_hv_hcall' has all the required information already,
there's no need to pass 'ex' additionally.

No functional change intended.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/hyperv.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 15b6a7bd2346..714af3b94f31 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1750,7 +1750,7 @@ struct kvm_hv_hcall {
 	sse128_t xmm[HV_HYPERCALL_MAX_XMM_REGISTERS];
 };
 
-static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
+static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 {
 	int i;
 	gpa_t gpa;
@@ -1765,7 +1765,8 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
 	int sparse_banks_len;
 	bool all_cpus;
 
-	if (!ex) {
+	if (hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST ||
+	    hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE) {
 		if (hc->fast) {
 			flush.address_space = hc->ingpa;
 			flush.flags = hc->outgpa;
@@ -2247,32 +2248,20 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 				kvm_hv_hypercall_complete_userspace;
 		return 0;
 	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
-		if (unlikely(!hc.rep_cnt || hc.rep_idx)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_flush_tlb(vcpu, &hc, false);
-		break;
-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
-		if (unlikely(hc.rep)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_flush_tlb(vcpu, &hc, false);
-		break;
 	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
 		if (unlikely(!hc.rep_cnt || hc.rep_idx)) {
 			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
 			break;
 		}
-		ret = kvm_hv_flush_tlb(vcpu, &hc, true);
+		ret = kvm_hv_flush_tlb(vcpu, &hc);
 		break;
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
 	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
 		if (unlikely(hc.rep)) {
 			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
 			break;
 		}
-		ret = kvm_hv_flush_tlb(vcpu, &hc, true);
+		ret = kvm_hv_flush_tlb(vcpu, &hc);
 		break;
 	case HVCALL_SEND_IPI:
 		if (unlikely(hc.rep)) {
-- 
2.35.1


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

* [PATCH 3/4] KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast TLB flush hypercalls
  2022-02-22 15:46 [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Vitaly Kuznetsov
  2022-02-22 15:46 ` [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi() Vitaly Kuznetsov
  2022-02-22 15:46 ` [PATCH 2/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_flush_tlb() Vitaly Kuznetsov
@ 2022-02-22 15:46 ` Vitaly Kuznetsov
  2022-02-28 10:47   ` Siddharth Chandrasekaran
  2022-02-22 15:46 ` [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall Vitaly Kuznetsov
  2022-02-25 11:55 ` [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Paolo Bonzini
  4 siblings, 1 reply; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-22 15:46 UTC (permalink / raw)
  To: kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

When TLB flush hypercalls (HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE}_EX are
issued in 'XMM fast' mode, the maximum number of allowed sparse_banks is
not 'HV_HYPERCALL_MAX_XMM_REGISTERS - 1' (5) but twice as many (10) as each
XMM register is 128 bit long and can hold two 64 bit long banks.

Fixes: 5974565bc26d ("KVM: x86: kvm_hv_flush_tlb use inputs from XMM registers")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/hyperv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 714af3b94f31..6dda93bf98ae 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1820,7 +1820,8 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 
 		if (!all_cpus) {
 			if (hc->fast) {
-				if (sparse_banks_len > HV_HYPERCALL_MAX_XMM_REGISTERS - 1)
+				/* XMM0 is already consumed, each XMM holds two sparse banks. */
+				if (sparse_banks_len > 2 * (HV_HYPERCALL_MAX_XMM_REGISTERS - 1))
 					return HV_STATUS_INVALID_HYPERCALL_INPUT;
 				for (i = 0; i < sparse_banks_len; i += 2) {
 					sparse_banks[i] = sse128_lo(hc->xmm[i / 2 + 1]);
-- 
2.35.1


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

* [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall
  2022-02-22 15:46 [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Vitaly Kuznetsov
                   ` (2 preceding siblings ...)
  2022-02-22 15:46 ` [PATCH 3/4] KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast TLB flush hypercalls Vitaly Kuznetsov
@ 2022-02-22 15:46 ` Vitaly Kuznetsov
  2022-02-25 18:33   ` Maxim Levitsky
  2022-02-28 10:48   ` Siddharth Chandrasekaran
  2022-02-25 11:55 ` [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Paolo Bonzini
  4 siblings, 2 replies; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-22 15:46 UTC (permalink / raw)
  To: kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

It has been proven on practice that at least Windows Server 2019 tries
using HVCALL_SEND_IPI_EX in 'XMM fast' mode when it has more than 64 vCPUs
and it needs to send an IPI to a vCPU > 63. Similarly to other XMM Fast
hypercalls (HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE}{,_EX}), this
information is missing in TLFS as of 6.0b. Currently, KVM returns an error
(HV_STATUS_INVALID_HYPERCALL_INPUT) and Windows crashes.

Note, HVCALL_SEND_IPI is a 'standard' fast hypercall (not 'XMM fast') as
all its parameters fit into RDX:R8 and this is handled by KVM correctly.

Fixes: d8f5537a8816 ("KVM: hyper-v: Advertise support for fast XMM hypercalls")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/hyperv.c | 52 ++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 6dda93bf98ae..3060057bdfd4 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1890,6 +1890,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 	int sparse_banks_len;
 	u32 vector;
 	bool all_cpus;
+	int i;
 
 	if (hc->code == HVCALL_SEND_IPI) {
 		if (!hc->fast) {
@@ -1910,9 +1911,15 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 
 		trace_kvm_hv_send_ipi(vector, sparse_banks[0]);
 	} else {
-		if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
-					    sizeof(send_ipi_ex))))
-			return HV_STATUS_INVALID_HYPERCALL_INPUT;
+		if (!hc->fast) {
+			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
+						    sizeof(send_ipi_ex))))
+				return HV_STATUS_INVALID_HYPERCALL_INPUT;
+		} else {
+			send_ipi_ex.vector = (u32)hc->ingpa;
+			send_ipi_ex.vp_set.format = hc->outgpa;
+			send_ipi_ex.vp_set.valid_bank_mask = sse128_lo(hc->xmm[0]);
+		}
 
 		trace_kvm_hv_send_ipi_ex(send_ipi_ex.vector,
 					 send_ipi_ex.vp_set.format,
@@ -1920,8 +1927,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 
 		vector = send_ipi_ex.vector;
 		valid_bank_mask = send_ipi_ex.vp_set.valid_bank_mask;
-		sparse_banks_len = bitmap_weight(&valid_bank_mask, 64) *
-			sizeof(sparse_banks[0]);
+		sparse_banks_len = bitmap_weight(&valid_bank_mask, 64);
 
 		all_cpus = send_ipi_ex.vp_set.format == HV_GENERIC_SET_ALL;
 
@@ -1931,12 +1937,27 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 		if (!sparse_banks_len)
 			goto ret_success;
 
-		if (kvm_read_guest(kvm,
-				   hc->ingpa + offsetof(struct hv_send_ipi_ex,
-							vp_set.bank_contents),
-				   sparse_banks,
-				   sparse_banks_len))
-			return HV_STATUS_INVALID_HYPERCALL_INPUT;
+		if (!hc->fast) {
+			if (kvm_read_guest(kvm,
+					   hc->ingpa + offsetof(struct hv_send_ipi_ex,
+								vp_set.bank_contents),
+					   sparse_banks,
+					   sparse_banks_len * sizeof(sparse_banks[0])))
+				return HV_STATUS_INVALID_HYPERCALL_INPUT;
+		} else {
+			/*
+			 * The lower half of XMM0 is already consumed, each XMM holds
+			 * two sparse banks.
+			 */
+			if (sparse_banks_len > (2 * HV_HYPERCALL_MAX_XMM_REGISTERS - 1))
+				return HV_STATUS_INVALID_HYPERCALL_INPUT;
+			for (i = 0; i < sparse_banks_len; i++) {
+				if (i % 2)
+					sparse_banks[i] = sse128_lo(hc->xmm[(i + 1) / 2]);
+				else
+					sparse_banks[i] = sse128_hi(hc->xmm[i / 2]);
+			}
+		}
 	}
 
 check_and_send_ipi:
@@ -2098,6 +2119,7 @@ static bool is_xmm_fast_hypercall(struct kvm_hv_hcall *hc)
 	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
 	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
 	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
+	case HVCALL_SEND_IPI_EX:
 		return true;
 	}
 
@@ -2265,14 +2287,8 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 		ret = kvm_hv_flush_tlb(vcpu, &hc);
 		break;
 	case HVCALL_SEND_IPI:
-		if (unlikely(hc.rep)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_send_ipi(vcpu, &hc);
-		break;
 	case HVCALL_SEND_IPI_EX:
-		if (unlikely(hc.fast || hc.rep)) {
+		if (unlikely(hc.rep)) {
 			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
 			break;
 		}
-- 
2.35.1


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

* Re: [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes
  2022-02-22 15:46 [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Vitaly Kuznetsov
                   ` (3 preceding siblings ...)
  2022-02-22 15:46 ` [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall Vitaly Kuznetsov
@ 2022-02-25 11:55 ` Paolo Bonzini
  2022-02-25 13:13   ` Vitaly Kuznetsov
  4 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2022-02-25 11:55 UTC (permalink / raw)
  To: Vitaly Kuznetsov, kvm
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

On 2/22/22 16:46, Vitaly Kuznetsov wrote:
> While working on some Hyper-V TLB flush improvements and Direct TLB flush
> feature for Hyper-V on KVM I experienced Windows Server 2019 crashes on
> boot when XMM fast hypercall input feature is advertised. Turns out,
> HVCALL_SEND_IPI_EX is also an XMM fast hypercall and returning an error
> kills the guest. This is fixed in PATCH4. PATCH3 fixes erroneous capping
> of sparse CPU banks for XMM fast TLB flush hypercalls. The problem should
> be reproducible with >360 vCPUs.
> 
> Vitaly Kuznetsov (4):
>    KVM: x86: hyper-v: Drop redundant 'ex' parameter from
>      kvm_hv_send_ipi()
>    KVM: x86: hyper-v: Drop redundant 'ex' parameter from
>      kvm_hv_flush_tlb()
>    KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast
>      TLB flush hypercalls
>    KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall
> 
>   arch/x86/kvm/hyperv.c | 84 +++++++++++++++++++++++--------------------
>   1 file changed, 45 insertions(+), 39 deletions(-)
> 

Merging this in 5.18 is a bit messy.  Please check that the below
patch against kvm/next makes sense:

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 653e08c993c4..98fb998c31ce 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1770,9 +1770,11 @@ struct kvm_hv_hcall {
  };
  
  static u64 kvm_get_sparse_vp_set(struct kvm *kvm, struct kvm_hv_hcall *hc,
+				 int consumed_xmm_halves,
  				 u64 *sparse_banks, gpa_t offset)
  {
  	u16 var_cnt;
+	int i;
  
  	if (hc->var_cnt > 64)
  		return -EINVAL;
@@ -1780,13 +1782,29 @@ static u64 kvm_get_sparse_vp_set(struct kvm *kvm, struct kvm_hv_hcall *hc,
  	/* Ignore banks that cannot possibly contain a legal VP index. */
  	var_cnt = min_t(u16, hc->var_cnt, KVM_HV_MAX_SPARSE_VCPU_SET_BITS);
  
+	if (hc->fast) {
+		/*
+		 * Each XMM holds two sparse banks, but do not count halves that
+		 * have already been consumed for hypercall parameters.
+		 */
+		if (hc->var_cnt > 2 * HV_HYPERCALL_MAX_XMM_REGISTERS - consumed_xmm_halves)
+			return HV_STATUS_INVALID_HYPERCALL_INPUT;
+		for (i = 0; i < var_cnt; i++) {
+			int j = i + consumed_xmm_halves;
+			if (j % 2)
+				sparse_banks[i] = sse128_lo(hc->xmm[j / 2]);
+			else
+				sparse_banks[i] = sse128_hi(hc->xmm[j / 2]);
+		}
+		return 0;
+	}
+
  	return kvm_read_guest(kvm, hc->ingpa + offset, sparse_banks,
  			      var_cnt * sizeof(*sparse_banks));
  }
  
-static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
+static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
  {
-	int i;
  	struct kvm *kvm = vcpu->kvm;
  	struct hv_tlb_flush_ex flush_ex;
  	struct hv_tlb_flush flush;
@@ -1803,7 +1821,8 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
  	 */
  	BUILD_BUG_ON(KVM_HV_MAX_SPARSE_VCPU_SET_BITS > 64);
  
-	if (!ex) {
+	if (hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST ||
+	    hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE) {
  		if (hc->fast) {
  			flush.address_space = hc->ingpa;
  			flush.flags = hc->outgpa;
@@ -1859,17 +1878,7 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
  		if (!hc->var_cnt)
  			goto ret_success;
  
-		if (hc->fast) {
-			if (hc->var_cnt > HV_HYPERCALL_MAX_XMM_REGISTERS - 1)
-				return HV_STATUS_INVALID_HYPERCALL_INPUT;
-			for (i = 0; i < hc->var_cnt; i += 2) {
-				sparse_banks[i] = sse128_lo(hc->xmm[i / 2 + 1]);
-				sparse_banks[i + 1] = sse128_hi(hc->xmm[i / 2 + 1]);
-			}
-			goto do_flush;
-		}
-
-		if (kvm_get_sparse_vp_set(kvm, hc, sparse_banks,
+		if (kvm_get_sparse_vp_set(kvm, hc, 2, sparse_banks,
  					  offsetof(struct hv_tlb_flush_ex,
  						   hv_vp_set.bank_contents)))
  			return HV_STATUS_INVALID_HYPERCALL_INPUT;
@@ -1913,7 +1922,7 @@ static void kvm_send_ipi_to_many(struct kvm *kvm, u32 vector,
  	}
  }
  
-static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
+static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
  {
  	struct kvm *kvm = vcpu->kvm;
  	struct hv_send_ipi_ex send_ipi_ex;
@@ -1924,7 +1933,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
  	u32 vector;
  	bool all_cpus;
  
-	if (!ex) {
+	if (hc->code == HVCALL_SEND_IPI) {
  		if (!hc->fast) {
  			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi,
  						    sizeof(send_ipi))))
@@ -1943,9 +1952,15 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
  
  		trace_kvm_hv_send_ipi(vector, sparse_banks[0]);
  	} else {
-		if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
-					    sizeof(send_ipi_ex))))
-			return HV_STATUS_INVALID_HYPERCALL_INPUT;
+		if (!hc->fast) {
+			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
+						    sizeof(send_ipi_ex))))
+				return HV_STATUS_INVALID_HYPERCALL_INPUT;
+		} else {
+			send_ipi_ex.vector = (u32)hc->ingpa;
+			send_ipi_ex.vp_set.format = hc->outgpa;
+			send_ipi_ex.vp_set.valid_bank_mask = sse128_lo(hc->xmm[0]);
+		}
  
  		trace_kvm_hv_send_ipi_ex(send_ipi_ex.vector,
  					 send_ipi_ex.vp_set.format,
@@ -1964,7 +1979,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
  		if (!hc->var_cnt)
  			goto ret_success;
  
-		if (kvm_get_sparse_vp_set(kvm, hc, sparse_banks,
+		if (kvm_get_sparse_vp_set(kvm, hc, 1, sparse_banks,
  					  offsetof(struct hv_send_ipi_ex,
  						   vp_set.bank_contents)))
  			return HV_STATUS_INVALID_HYPERCALL_INPUT;
@@ -2126,6 +2141,7 @@ static bool is_xmm_fast_hypercall(struct kvm_hv_hcall *hc)
  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
+	case HVCALL_SEND_IPI_EX:
  		return true;
  	}
  
@@ -2283,46 +2299,43 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
  				kvm_hv_hypercall_complete_userspace;
  		return 0;
  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
-		if (unlikely(!hc.rep_cnt || hc.rep_idx || hc.var_cnt)) {
+		if (unlikely(hc.var_cnt)) {
  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
  			break;
  		}
-		ret = kvm_hv_flush_tlb(vcpu, &hc, false);
-		break;
-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
-		if (unlikely(hc.rep || hc.var_cnt)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_flush_tlb(vcpu, &hc, false);
-		break;
+		fallthrough;
  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
  		if (unlikely(!hc.rep_cnt || hc.rep_idx)) {
  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
  			break;
  		}
-		ret = kvm_hv_flush_tlb(vcpu, &hc, true);
+		ret = kvm_hv_flush_tlb(vcpu, &hc);
  		break;
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
+		if (unlikely(hc.var_cnt)) {
+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+			break;
+		}
+		fallthrough;
  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
  		if (unlikely(hc.rep)) {
  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
  			break;
  		}
-		ret = kvm_hv_flush_tlb(vcpu, &hc, true);
+		ret = kvm_hv_flush_tlb(vcpu, &hc);
  		break;
  	case HVCALL_SEND_IPI:
-		if (unlikely(hc.rep || hc.var_cnt)) {
+		if (unlikely(hc.var_cnt)) {
  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
  			break;
  		}
-		ret = kvm_hv_send_ipi(vcpu, &hc, false);
-		break;
+		fallthrough;
  	case HVCALL_SEND_IPI_EX:
-		if (unlikely(hc.fast || hc.rep)) {
+		if (unlikely(hc.rep)) {
  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
  			break;
  		}
-		ret = kvm_hv_send_ipi(vcpu, &hc, true);
+		ret = kvm_hv_send_ipi(vcpu, &hc);
  		break;
  	case HVCALL_POST_DEBUG_DATA:
  	case HVCALL_RETRIEVE_DEBUG_DATA:


The resulting merge commit is already in kvm/queue shortly (which should
become the next kvm/next as soon as tests complete).

Paolo


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

* Re: [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes
  2022-02-25 11:55 ` [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Paolo Bonzini
@ 2022-02-25 13:13   ` Vitaly Kuznetsov
  2022-02-25 13:17     ` Paolo Bonzini
  0 siblings, 1 reply; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-25 13:13 UTC (permalink / raw)
  To: Paolo Bonzini, kvm
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 2/22/22 16:46, Vitaly Kuznetsov wrote:
>> While working on some Hyper-V TLB flush improvements and Direct TLB flush
>> feature for Hyper-V on KVM I experienced Windows Server 2019 crashes on
>> boot when XMM fast hypercall input feature is advertised. Turns out,
>> HVCALL_SEND_IPI_EX is also an XMM fast hypercall and returning an error
>> kills the guest. This is fixed in PATCH4. PATCH3 fixes erroneous capping
>> of sparse CPU banks for XMM fast TLB flush hypercalls. The problem should
>> be reproducible with >360 vCPUs.
>> 
>> Vitaly Kuznetsov (4):
>>    KVM: x86: hyper-v: Drop redundant 'ex' parameter from
>>      kvm_hv_send_ipi()
>>    KVM: x86: hyper-v: Drop redundant 'ex' parameter from
>>      kvm_hv_flush_tlb()
>>    KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast
>>      TLB flush hypercalls
>>    KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall
>> 
>>   arch/x86/kvm/hyperv.c | 84 +++++++++++++++++++++++--------------------
>>   1 file changed, 45 insertions(+), 39 deletions(-)
>> 
>
> Merging this in 5.18 is a bit messy.  Please check that the below
> patch against kvm/next makes sense:

Something is wrong with the diff as it doesn't apply :-(

>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 653e08c993c4..98fb998c31ce 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1770,9 +1770,11 @@ struct kvm_hv_hcall {
>   };
>   
>   static u64 kvm_get_sparse_vp_set(struct kvm *kvm, struct kvm_hv_hcall *hc,
> +				 int consumed_xmm_halves,
>   				 u64 *sparse_banks, gpa_t offset)
>   {
>   	u16 var_cnt;
> +	int i;
>   
>   	if (hc->var_cnt > 64)
>   		return -EINVAL;
> @@ -1780,13 +1782,29 @@ static u64 kvm_get_sparse_vp_set(struct kvm *kvm, struct kvm_hv_hcall *hc,
>   	/* Ignore banks that cannot possibly contain a legal VP index. */
>   	var_cnt = min_t(u16, hc->var_cnt, KVM_HV_MAX_SPARSE_VCPU_SET_BITS);
>   
> +	if (hc->fast) {
> +		/*
> +		 * Each XMM holds two sparse banks, but do not count halves that
> +		 * have already been consumed for hypercall parameters.
> +		 */
> +		if (hc->var_cnt > 2 * HV_HYPERCALL_MAX_XMM_REGISTERS - consumed_xmm_halves)
> +			return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +		for (i = 0; i < var_cnt; i++) {
> +			int j = i + consumed_xmm_halves;
> +			if (j % 2)
> +				sparse_banks[i] = sse128_lo(hc->xmm[j / 2]);
> +			else
> +				sparse_banks[i] = sse128_hi(hc->xmm[j / 2]);

Let's say we have 1 half of XMM0 consumed. Now:

 i = 0;
 j = 1;
 if (1) 
     sparse_banks[0] = sse128_lo(hc->xmm[0]); 

 This doesn't look right as we need to get the upper half of XMM0.

 I guess it should be reversed, 

     if (j % 2)
         sparse_banks[i] = sse128_hi(hc->xmm[j / 2]);
     else
         sparse_banks[i] = sse128_lo(hc->xmm[j / 2]);

> +		}
> +		return 0;
> +	}
> +
>   	return kvm_read_guest(kvm, hc->ingpa + offset, sparse_banks,
>   			      var_cnt * sizeof(*sparse_banks));
>   }
>   
> -static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
> +static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>   {
> -	int i;
>   	struct kvm *kvm = vcpu->kvm;
>   	struct hv_tlb_flush_ex flush_ex;
>   	struct hv_tlb_flush flush;
> @@ -1803,7 +1821,8 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>   	 */
>   	BUILD_BUG_ON(KVM_HV_MAX_SPARSE_VCPU_SET_BITS > 64);
>   
> -	if (!ex) {
> +	if (hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST ||
> +	    hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE) {

In case you're trying to come up with a smaller patch for 5.18, we can
certainly drop these 'ex'/'non-ex' changes as these are merely
cosmetic.

>   		if (hc->fast) {
>   			flush.address_space = hc->ingpa;
>   			flush.flags = hc->outgpa;
> @@ -1859,17 +1878,7 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>   		if (!hc->var_cnt)
>   			goto ret_success;
>   
> -		if (hc->fast) {
> -			if (hc->var_cnt > HV_HYPERCALL_MAX_XMM_REGISTERS - 1)
> -				return HV_STATUS_INVALID_HYPERCALL_INPUT;
> -			for (i = 0; i < hc->var_cnt; i += 2) {
> -				sparse_banks[i] = sse128_lo(hc->xmm[i / 2 + 1]);
> -				sparse_banks[i + 1] = sse128_hi(hc->xmm[i / 2 + 1]);
> -			}
> -			goto do_flush;
> -		}
> -
> -		if (kvm_get_sparse_vp_set(kvm, hc, sparse_banks,
> +		if (kvm_get_sparse_vp_set(kvm, hc, 2, sparse_banks,
>   					  offsetof(struct hv_tlb_flush_ex,
>   						   hv_vp_set.bank_contents)))

I like your idea to put 'consumed_xmm_halves' into
kvm_get_sparse_vp_set() as kvm_hv_flush_tlb is getting too big.

>   			return HV_STATUS_INVALID_HYPERCALL_INPUT;
> @@ -1913,7 +1922,7 @@ static void kvm_send_ipi_to_many(struct kvm *kvm, u32 vector,
>   	}
>   }
>   
> -static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
> +static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>   {
>   	struct kvm *kvm = vcpu->kvm;
>   	struct hv_send_ipi_ex send_ipi_ex;
> @@ -1924,7 +1933,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>   	u32 vector;
>   	bool all_cpus;
>   
> -	if (!ex) {
> +	if (hc->code == HVCALL_SEND_IPI) {
>   		if (!hc->fast) {
>   			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi,
>   						    sizeof(send_ipi))))
> @@ -1943,9 +1952,15 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>   
>   		trace_kvm_hv_send_ipi(vector, sparse_banks[0]);
>   	} else {
> -		if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
> -					    sizeof(send_ipi_ex))))
> -			return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +		if (!hc->fast) {
> +			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
> +						    sizeof(send_ipi_ex))))
> +				return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +		} else {
> +			send_ipi_ex.vector = (u32)hc->ingpa;
> +			send_ipi_ex.vp_set.format = hc->outgpa;
> +			send_ipi_ex.vp_set.valid_bank_mask = sse128_lo(hc->xmm[0]);
> +		}
>   
>   		trace_kvm_hv_send_ipi_ex(send_ipi_ex.vector,
>   					 send_ipi_ex.vp_set.format,
> @@ -1964,7 +1979,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>   		if (!hc->var_cnt)
>   			goto ret_success;
>   
> -		if (kvm_get_sparse_vp_set(kvm, hc, sparse_banks,
> +		if (kvm_get_sparse_vp_set(kvm, hc, 1, sparse_banks,
>   					  offsetof(struct hv_send_ipi_ex,
>   						   vp_set.bank_contents)))
>   			return HV_STATUS_INVALID_HYPERCALL_INPUT;
> @@ -2126,6 +2141,7 @@ static bool is_xmm_fast_hypercall(struct kvm_hv_hcall *hc)
>   	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
>   	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
>   	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
> +	case HVCALL_SEND_IPI_EX:
>   		return true;
>   	}
>   
> @@ -2283,46 +2299,43 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>   				kvm_hv_hypercall_complete_userspace;
>   		return 0;
>   	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
> -		if (unlikely(!hc.rep_cnt || hc.rep_idx || hc.var_cnt)) {
> +		if (unlikely(hc.var_cnt)) {
>   			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>   			break;
>   		}
> -		ret = kvm_hv_flush_tlb(vcpu, &hc, false);
> -		break;
> -	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
> -		if (unlikely(hc.rep || hc.var_cnt)) {
> -			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
> -			break;
> -		}
> -		ret = kvm_hv_flush_tlb(vcpu, &hc, false);
> -		break;
> +		fallthrough;
>   	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
>   		if (unlikely(!hc.rep_cnt || hc.rep_idx)) {
>   			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>   			break;
>   		}
> -		ret = kvm_hv_flush_tlb(vcpu, &hc, true);
> +		ret = kvm_hv_flush_tlb(vcpu, &hc);
>   		break;
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
> +		if (unlikely(hc.var_cnt)) {
> +			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
> +			break;
> +		}
> +		fallthrough;
>   	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
>   		if (unlikely(hc.rep)) {
>   			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>   			break;
>   		}
> -		ret = kvm_hv_flush_tlb(vcpu, &hc, true);
> +		ret = kvm_hv_flush_tlb(vcpu, &hc);
>   		break;
>   	case HVCALL_SEND_IPI:
> -		if (unlikely(hc.rep || hc.var_cnt)) {
> +		if (unlikely(hc.var_cnt)) {
>   			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>   			break;
>   		}
> -		ret = kvm_hv_send_ipi(vcpu, &hc, false);
> -		break;
> +		fallthrough;
>   	case HVCALL_SEND_IPI_EX:
> -		if (unlikely(hc.fast || hc.rep)) {
> +		if (unlikely(hc.rep)) {
>   			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>   			break;
>   		}
> -		ret = kvm_hv_send_ipi(vcpu, &hc, true);
> +		ret = kvm_hv_send_ipi(vcpu, &hc);
>   		break;
>   	case HVCALL_POST_DEBUG_DATA:
>   	case HVCALL_RETRIEVE_DEBUG_DATA:

I've smoke tested this (with the change I've mentioned above) and WS2019
booted with 65 vCPUs. This is a good sign)

>
>
> The resulting merge commit is already in kvm/queue shortly (which should
> become the next kvm/next as soon as tests complete).
>

I see, please swap sse128_lo()/sse128_hi() there too :-)

-- 
Vitaly


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

* Re: [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes
  2022-02-25 13:13   ` Vitaly Kuznetsov
@ 2022-02-25 13:17     ` Paolo Bonzini
  2022-02-28 10:52       ` Siddharth Chandrasekaran
  0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2022-02-25 13:17 UTC (permalink / raw)
  To: Vitaly Kuznetsov, kvm
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

On 2/25/22 14:13, Vitaly Kuznetsov wrote:
> Let's say we have 1 half of XMM0 consumed. Now:
> 
>   i = 0;
>   j = 1;
>   if (1)
>       sparse_banks[0] = sse128_lo(hc->xmm[0]);
> 
>   This doesn't look right as we need to get the upper half of XMM0.
> 
>   I guess it should be reversed,
> 
>       if (j % 2)
>           sparse_banks[i] = sse128_hi(hc->xmm[j / 2]);
>       else
>           sparse_banks[i] = sse128_lo(hc->xmm[j / 2]);

Yeah, of course.

Thanks!

Paolo


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

* Re: [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi()
  2022-02-22 15:46 ` [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi() Vitaly Kuznetsov
@ 2022-02-25 18:21   ` Maxim Levitsky
  2022-02-28  9:58     ` Vitaly Kuznetsov
  2022-02-28 10:45   ` Siddharth Chandrasekaran
  1 sibling, 1 reply; 20+ messages in thread
From: Maxim Levitsky @ 2022-02-25 18:21 UTC (permalink / raw)
  To: Vitaly Kuznetsov, kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

On Tue, 2022-02-22 at 16:46 +0100, Vitaly Kuznetsov wrote:
> 'struct kvm_hv_hcall' has all the required information already,
> there's no need to pass 'ex' additionally.
> 
> No functional change intended.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  arch/x86/kvm/hyperv.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 6e38a7d22e97..15b6a7bd2346 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1875,7 +1875,7 @@ static void kvm_send_ipi_to_many(struct kvm *kvm, u32 vector,
>  	}
>  }
>  
> -static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
> +static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  {
>  	struct kvm *kvm = vcpu->kvm;
>  	struct hv_send_ipi_ex send_ipi_ex;
> @@ -1889,7 +1889,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>  	u32 vector;
>  	bool all_cpus;
>  
> -	if (!ex) {
> +	if (hc->code == HVCALL_SEND_IPI) {

I am thinking, if we already touch this code,
why not to use switch here instead on the hc->code,
so that we can catch this function being called with something else than
HVCALL_SEND_IPI_EX

>  		if (!hc->fast) {
>  			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi,
>  						    sizeof(send_ipi))))
> @@ -2279,14 +2279,14 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>  			break;
>  		}
> -		ret = kvm_hv_send_ipi(vcpu, &hc, false);
> +		ret = kvm_hv_send_ipi(vcpu, &hc);
>  		break;
>  	case HVCALL_SEND_IPI_EX:
>  		if (unlikely(hc.fast || hc.rep)) {
>  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>  			break;
>  		}
> -		ret = kvm_hv_send_ipi(vcpu, &hc, true);
> +		ret = kvm_hv_send_ipi(vcpu, &hc);
>  		break;
>  	case HVCALL_POST_DEBUG_DATA:
>  	case HVCALL_RETRIEVE_DEBUG_DATA:



Other than this minor nitpick:

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>


Best regards,
	Maxim Levitsky


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

* Re: [PATCH 2/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_flush_tlb()
  2022-02-22 15:46 ` [PATCH 2/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_flush_tlb() Vitaly Kuznetsov
@ 2022-02-25 18:22   ` Maxim Levitsky
  2022-02-28 10:46   ` Siddharth Chandrasekaran
  1 sibling, 0 replies; 20+ messages in thread
From: Maxim Levitsky @ 2022-02-25 18:22 UTC (permalink / raw)
  To: Vitaly Kuznetsov, kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

On Tue, 2022-02-22 at 16:46 +0100, Vitaly Kuznetsov wrote:
> 'struct kvm_hv_hcall' has all the required information already,
> there's no need to pass 'ex' additionally.
> 
> No functional change intended.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  arch/x86/kvm/hyperv.c | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 15b6a7bd2346..714af3b94f31 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1750,7 +1750,7 @@ struct kvm_hv_hcall {
>  	sse128_t xmm[HV_HYPERCALL_MAX_XMM_REGISTERS];
>  };
>  
> -static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
> +static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  {
>  	int i;
>  	gpa_t gpa;
> @@ -1765,7 +1765,8 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>  	int sparse_banks_len;
>  	bool all_cpus;
>  
> -	if (!ex) {
> +	if (hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST ||
> +	    hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE) {
>  		if (hc->fast) {
>  			flush.address_space = hc->ingpa;
>  			flush.flags = hc->outgpa;
> @@ -2247,32 +2248,20 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  				kvm_hv_hypercall_complete_userspace;
>  		return 0;
>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
> -		if (unlikely(!hc.rep_cnt || hc.rep_idx)) {
> -			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
> -			break;
> -		}
> -		ret = kvm_hv_flush_tlb(vcpu, &hc, false);
> -		break;
> -	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
> -		if (unlikely(hc.rep)) {
> -			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
> -			break;
> -		}
> -		ret = kvm_hv_flush_tlb(vcpu, &hc, false);
> -		break;
>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
>  		if (unlikely(!hc.rep_cnt || hc.rep_idx)) {
>  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>  			break;
>  		}
> -		ret = kvm_hv_flush_tlb(vcpu, &hc, true);
> +		ret = kvm_hv_flush_tlb(vcpu, &hc);
>  		break;
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
>  		if (unlikely(hc.rep)) {
>  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>  			break;
>  		}
> -		ret = kvm_hv_flush_tlb(vcpu, &hc, true);
> +		ret = kvm_hv_flush_tlb(vcpu, &hc);
>  		break;
>  	case HVCALL_SEND_IPI:
>  		if (unlikely(hc.rep)) {


Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky


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

* Re: [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall
  2022-02-22 15:46 ` [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall Vitaly Kuznetsov
@ 2022-02-25 18:33   ` Maxim Levitsky
  2022-02-28 10:02     ` Vitaly Kuznetsov
  2022-02-28 10:48   ` Siddharth Chandrasekaran
  1 sibling, 1 reply; 20+ messages in thread
From: Maxim Levitsky @ 2022-02-25 18:33 UTC (permalink / raw)
  To: Vitaly Kuznetsov, kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel

On Tue, 2022-02-22 at 16:46 +0100, Vitaly Kuznetsov wrote:
> It has been proven on practice that at least Windows Server 2019 tries
> using HVCALL_SEND_IPI_EX in 'XMM fast' mode when it has more than 64 vCPUs
> and it needs to send an IPI to a vCPU > 63. Similarly to other XMM Fast
> hypercalls (HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE}{,_EX}), this
> information is missing in TLFS as of 6.0b. Currently, KVM returns an error
> (HV_STATUS_INVALID_HYPERCALL_INPUT) and Windows crashes.
> 
> Note, HVCALL_SEND_IPI is a 'standard' fast hypercall (not 'XMM fast') as
> all its parameters fit into RDX:R8 and this is handled by KVM correctly.
> 
> Fixes: d8f5537a8816 ("KVM: hyper-v: Advertise support for fast XMM hypercalls")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  arch/x86/kvm/hyperv.c | 52 ++++++++++++++++++++++++++++---------------
>  1 file changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 6dda93bf98ae..3060057bdfd4 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1890,6 +1890,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  	int sparse_banks_len;
>  	u32 vector;
>  	bool all_cpus;
> +	int i;
>  
>  	if (hc->code == HVCALL_SEND_IPI) {
>  		if (!hc->fast) {
> @@ -1910,9 +1911,15 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  
>  		trace_kvm_hv_send_ipi(vector, sparse_banks[0]);
>  	} else {
> -		if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
> -					    sizeof(send_ipi_ex))))
> -			return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +		if (!hc->fast) {
> +			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
> +						    sizeof(send_ipi_ex))))
> +				return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +		} else {
> +			send_ipi_ex.vector = (u32)hc->ingpa;
> +			send_ipi_ex.vp_set.format = hc->outgpa;
> +			send_ipi_ex.vp_set.valid_bank_mask = sse128_lo(hc->xmm[0]);
> +		}
>  
>  		trace_kvm_hv_send_ipi_ex(send_ipi_ex.vector,
>  					 send_ipi_ex.vp_set.format,
> @@ -1920,8 +1927,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  
>  		vector = send_ipi_ex.vector;
>  		valid_bank_mask = send_ipi_ex.vp_set.valid_bank_mask;
> -		sparse_banks_len = bitmap_weight(&valid_bank_mask, 64) *
> -			sizeof(sparse_banks[0]);
> +		sparse_banks_len = bitmap_weight(&valid_bank_mask, 64);
Is this change intentional? 

I haven't fully reviewed this, because kvm/queue seem to have a bit different
version of this, and I didn't fully follow on all of this.

Best regards,
	Maxim Levitsky

>  
>  		all_cpus = send_ipi_ex.vp_set.format == HV_GENERIC_SET_ALL;
>  
> @@ -1931,12 +1937,27 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  		if (!sparse_banks_len)
>  			goto ret_success;
>  
> -		if (kvm_read_guest(kvm,
> -				   hc->ingpa + offsetof(struct hv_send_ipi_ex,
> -							vp_set.bank_contents),
> -				   sparse_banks,
> -				   sparse_banks_len))
> -			return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +		if (!hc->fast) {
> +			if (kvm_read_guest(kvm,
> +					   hc->ingpa + offsetof(struct hv_send_ipi_ex,
> +								vp_set.bank_contents),
> +					   sparse_banks,
> +					   sparse_banks_len * sizeof(sparse_banks[0])))
> +				return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +		} else {
> +			/*
> +			 * The lower half of XMM0 is already consumed, each XMM holds
> +			 * two sparse banks.
> +			 */
> +			if (sparse_banks_len > (2 * HV_HYPERCALL_MAX_XMM_REGISTERS - 1))
> +				return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +			for (i = 0; i < sparse_banks_len; i++) {
> +				if (i % 2)
> +					sparse_banks[i] = sse128_lo(hc->xmm[(i + 1) / 2]);
> +				else
> +					sparse_banks[i] = sse128_hi(hc->xmm[i / 2]);
> +			}
> +		}
>  	}
>  
>  check_and_send_ipi:
> @@ -2098,6 +2119,7 @@ static bool is_xmm_fast_hypercall(struct kvm_hv_hcall *hc)
>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
> +	case HVCALL_SEND_IPI_EX:
>  		return true;
>  	}
>  
> @@ -2265,14 +2287,8 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  		ret = kvm_hv_flush_tlb(vcpu, &hc);
>  		break;
>  	case HVCALL_SEND_IPI:
> -		if (unlikely(hc.rep)) {
> -			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
> -			break;
> -		}
> -		ret = kvm_hv_send_ipi(vcpu, &hc);
> -		break;
>  	case HVCALL_SEND_IPI_EX:
> -		if (unlikely(hc.fast || hc.rep)) {
> +		if (unlikely(hc.rep)) {
>  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>  			break;
>  		}



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

* Re: [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi()
  2022-02-25 18:21   ` Maxim Levitsky
@ 2022-02-28  9:58     ` Vitaly Kuznetsov
  0 siblings, 0 replies; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-28  9:58 UTC (permalink / raw)
  To: Maxim Levitsky, kvm
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel, Paolo Bonzini

Maxim Levitsky <mlevitsk@redhat.com> writes:

> On Tue, 2022-02-22 at 16:46 +0100, Vitaly Kuznetsov wrote:
>> 'struct kvm_hv_hcall' has all the required information already,
>> there's no need to pass 'ex' additionally.
>> 
>> No functional change intended.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  arch/x86/kvm/hyperv.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index 6e38a7d22e97..15b6a7bd2346 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -1875,7 +1875,7 @@ static void kvm_send_ipi_to_many(struct kvm *kvm, u32 vector,
>>  	}
>>  }
>>  
>> -static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
>> +static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>>  {
>>  	struct kvm *kvm = vcpu->kvm;
>>  	struct hv_send_ipi_ex send_ipi_ex;
>> @@ -1889,7 +1889,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>>  	u32 vector;
>>  	bool all_cpus;
>>  
>> -	if (!ex) {
>> +	if (hc->code == HVCALL_SEND_IPI) {
>
> I am thinking, if we already touch this code,
> why not to use switch here instead on the hc->code,
> so that we can catch this function being called with something else than
> HVCALL_SEND_IPI_EX

I'm not against this second line of defense but kvm_hv_send_ipi() is
only called explicitly from kvm_hv_hypercall()'s switch so something is
really screwed up if we end up seeing something different from
HVCALL_SEND_IPI_EX/HVCALL_SEND_IPI here.

I'm now working on a bigger series for TLB flush improvements, will use
your suggestion there, thanks!

>
>>  		if (!hc->fast) {
>>  			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi,
>>  						    sizeof(send_ipi))))
>> @@ -2279,14 +2279,14 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>>  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>>  			break;
>>  		}
>> -		ret = kvm_hv_send_ipi(vcpu, &hc, false);
>> +		ret = kvm_hv_send_ipi(vcpu, &hc);
>>  		break;
>>  	case HVCALL_SEND_IPI_EX:
>>  		if (unlikely(hc.fast || hc.rep)) {
>>  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>>  			break;
>>  		}
>> -		ret = kvm_hv_send_ipi(vcpu, &hc, true);
>> +		ret = kvm_hv_send_ipi(vcpu, &hc);
>>  		break;
>>  	case HVCALL_POST_DEBUG_DATA:
>>  	case HVCALL_RETRIEVE_DEBUG_DATA:
>
>
>
> Other than this minor nitpick:
>
> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
>
>
> Best regards,
> 	Maxim Levitsky
>

-- 
Vitaly


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

* Re: [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall
  2022-02-25 18:33   ` Maxim Levitsky
@ 2022-02-28 10:02     ` Vitaly Kuznetsov
  0 siblings, 0 replies; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-28 10:02 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson,
	Siddharth Chandrasekaran, linux-kernel, kvm, Paolo Bonzini

Maxim Levitsky <mlevitsk@redhat.com> writes:

> On Tue, 2022-02-22 at 16:46 +0100, Vitaly Kuznetsov wrote:
>> It has been proven on practice that at least Windows Server 2019 tries
>> using HVCALL_SEND_IPI_EX in 'XMM fast' mode when it has more than 64 vCPUs
>> and it needs to send an IPI to a vCPU > 63. Similarly to other XMM Fast
>> hypercalls (HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE}{,_EX}), this
>> information is missing in TLFS as of 6.0b. Currently, KVM returns an error
>> (HV_STATUS_INVALID_HYPERCALL_INPUT) and Windows crashes.
>> 
>> Note, HVCALL_SEND_IPI is a 'standard' fast hypercall (not 'XMM fast') as
>> all its parameters fit into RDX:R8 and this is handled by KVM correctly.
>> 
>> Fixes: d8f5537a8816 ("KVM: hyper-v: Advertise support for fast XMM hypercalls")
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  arch/x86/kvm/hyperv.c | 52 ++++++++++++++++++++++++++++---------------
>>  1 file changed, 34 insertions(+), 18 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index 6dda93bf98ae..3060057bdfd4 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -1890,6 +1890,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>>  	int sparse_banks_len;
>>  	u32 vector;
>>  	bool all_cpus;
>> +	int i;
>>  
>>  	if (hc->code == HVCALL_SEND_IPI) {
>>  		if (!hc->fast) {
>> @@ -1910,9 +1911,15 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>>  
>>  		trace_kvm_hv_send_ipi(vector, sparse_banks[0]);
>>  	} else {
>> -		if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
>> -					    sizeof(send_ipi_ex))))
>> -			return HV_STATUS_INVALID_HYPERCALL_INPUT;
>> +		if (!hc->fast) {
>> +			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi_ex,
>> +						    sizeof(send_ipi_ex))))
>> +				return HV_STATUS_INVALID_HYPERCALL_INPUT;
>> +		} else {
>> +			send_ipi_ex.vector = (u32)hc->ingpa;
>> +			send_ipi_ex.vp_set.format = hc->outgpa;
>> +			send_ipi_ex.vp_set.valid_bank_mask = sse128_lo(hc->xmm[0]);
>> +		}
>>  
>>  		trace_kvm_hv_send_ipi_ex(send_ipi_ex.vector,
>>  					 send_ipi_ex.vp_set.format,
>> @@ -1920,8 +1927,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>>  
>>  		vector = send_ipi_ex.vector;
>>  		valid_bank_mask = send_ipi_ex.vp_set.valid_bank_mask;
>> -		sparse_banks_len = bitmap_weight(&valid_bank_mask, 64) *
>> -			sizeof(sparse_banks[0]);
>> +		sparse_banks_len = bitmap_weight(&valid_bank_mask, 64);
> Is this change intentional? 
>

Yes it is. Previously, 'sparse_banks_len' was the number of bytes to
read, now it's in u64-s.

(see below)

> I haven't fully reviewed this, because kvm/queue seem to have a bit different
> version of this, and I didn't fully follow on all of this.
>
>>  
>>  		all_cpus = send_ipi_ex.vp_set.format == HV_GENERIC_SET_ALL;
>>  
>> @@ -1931,12 +1937,27 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>>  		if (!sparse_banks_len)
>>  			goto ret_success;
>>  
>> -		if (kvm_read_guest(kvm,
>> -				   hc->ingpa + offsetof(struct hv_send_ipi_ex,
>> -							vp_set.bank_contents),
>> -				   sparse_banks,
>> -				   sparse_banks_len))
>> -			return HV_STATUS_INVALID_HYPERCALL_INPUT;
>> +		if (!hc->fast) {
>> +			if (kvm_read_guest(kvm,
>> +					   hc->ingpa + offsetof(struct hv_send_ipi_ex,
>> +								vp_set.bank_contents),
>> +					   sparse_banks,
>> +					   sparse_banks_len * sizeof(sparse_banks[0])))

^^^ here ^^^

>> +				return HV_STATUS_INVALID_HYPERCALL_INPUT;
>> +		} else {
>> +			/*
>> +			 * The lower half of XMM0 is already consumed, each XMM holds
>> +			 * two sparse banks.
>> +			 */
>> +			if (sparse_banks_len > (2 * HV_HYPERCALL_MAX_XMM_REGISTERS - 1))
>> +				return HV_STATUS_INVALID_HYPERCALL_INPUT;

And here. This is the reason for change: it's more convenient to count
it 'xmm halves' than in bytes.

>> +			for (i = 0; i < sparse_banks_len; i++) {
>> +				if (i % 2)
>> +					sparse_banks[i] = sse128_lo(hc->xmm[(i + 1) / 2]);
>> +				else
>> +					sparse_banks[i] = sse128_hi(hc->xmm[i / 2]);
>> +			}
>> +		}
>>  	}
>>  
>>  check_and_send_ipi:
>> @@ -2098,6 +2119,7 @@ static bool is_xmm_fast_hypercall(struct kvm_hv_hcall *hc)
>>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
>>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
>>  	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
>> +	case HVCALL_SEND_IPI_EX:
>>  		return true;
>>  	}
>>  
>> @@ -2265,14 +2287,8 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>>  		ret = kvm_hv_flush_tlb(vcpu, &hc);
>>  		break;
>>  	case HVCALL_SEND_IPI:
>> -		if (unlikely(hc.rep)) {
>> -			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>> -			break;
>> -		}
>> -		ret = kvm_hv_send_ipi(vcpu, &hc);
>> -		break;
>>  	case HVCALL_SEND_IPI_EX:
>> -		if (unlikely(hc.fast || hc.rep)) {
>> +		if (unlikely(hc.rep)) {
>>  			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>>  			break;
>>  		}
>
>

-- 
Vitaly


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

* Re: [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi()
  2022-02-22 15:46 ` [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi() Vitaly Kuznetsov
  2022-02-25 18:21   ` Maxim Levitsky
@ 2022-02-28 10:45   ` Siddharth Chandrasekaran
  1 sibling, 0 replies; 20+ messages in thread
From: Siddharth Chandrasekaran @ 2022-02-28 10:45 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: kvm, Paolo Bonzini, Sean Christopherson, Wanpeng Li, Jim Mattson,
	linux-kernel

On Tue, Feb 22, 2022 at 04:46:39PM +0100, Vitaly Kuznetsov wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> 'struct kvm_hv_hcall' has all the required information already,
> there's no need to pass 'ex' additionally.
> 
> No functional change intended.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Reviewed-by: Siddharth Chandrasekaran <sidcha@amazon.de>



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

* Re: [PATCH 2/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_flush_tlb()
  2022-02-22 15:46 ` [PATCH 2/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_flush_tlb() Vitaly Kuznetsov
  2022-02-25 18:22   ` Maxim Levitsky
@ 2022-02-28 10:46   ` Siddharth Chandrasekaran
  1 sibling, 0 replies; 20+ messages in thread
From: Siddharth Chandrasekaran @ 2022-02-28 10:46 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: kvm, Paolo Bonzini, Sean Christopherson, Wanpeng Li, Jim Mattson,
	linux-kernel

On Tue, Feb 22, 2022 at 04:46:40PM +0100, Vitaly Kuznetsov wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> 'struct kvm_hv_hcall' has all the required information already,
> there's no need to pass 'ex' additionally.
> 
> No functional change intended.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Reviewed-by: Siddharth Chandrasekaran <sidcha@amazon.de>



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

* Re: [PATCH 3/4] KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast TLB flush hypercalls
  2022-02-22 15:46 ` [PATCH 3/4] KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast TLB flush hypercalls Vitaly Kuznetsov
@ 2022-02-28 10:47   ` Siddharth Chandrasekaran
  0 siblings, 0 replies; 20+ messages in thread
From: Siddharth Chandrasekaran @ 2022-02-28 10:47 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: kvm, Paolo Bonzini, Sean Christopherson, Wanpeng Li, Jim Mattson,
	linux-kernel

On Tue, Feb 22, 2022 at 04:46:41PM +0100, Vitaly Kuznetsov wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> When TLB flush hypercalls (HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE}_EX are
> issued in 'XMM fast' mode, the maximum number of allowed sparse_banks is
> not 'HV_HYPERCALL_MAX_XMM_REGISTERS - 1' (5) but twice as many (10) as each
> XMM register is 128 bit long and can hold two 64 bit long banks.
> 
> Fixes: 5974565bc26d ("KVM: x86: kvm_hv_flush_tlb use inputs from XMM registers")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Indeed :)

Reviewed-by: Siddharth Chandrasekaran <sidcha@amazon.de>



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

* Re: [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall
  2022-02-22 15:46 ` [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall Vitaly Kuznetsov
  2022-02-25 18:33   ` Maxim Levitsky
@ 2022-02-28 10:48   ` Siddharth Chandrasekaran
  1 sibling, 0 replies; 20+ messages in thread
From: Siddharth Chandrasekaran @ 2022-02-28 10:48 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: kvm, Paolo Bonzini, Sean Christopherson, Wanpeng Li, Jim Mattson,
	linux-kernel

On Tue, Feb 22, 2022 at 04:46:42PM +0100, Vitaly Kuznetsov wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> It has been proven on practice that at least Windows Server 2019 tries
> using HVCALL_SEND_IPI_EX in 'XMM fast' mode when it has more than 64 vCPUs
> and it needs to send an IPI to a vCPU > 63. Similarly to other XMM Fast
> hypercalls (HVCALL_FLUSH_VIRTUAL_ADDRESS_{LIST,SPACE}{,_EX}), this
> information is missing in TLFS as of 6.0b. Currently, KVM returns an error
> (HV_STATUS_INVALID_HYPERCALL_INPUT) and Windows crashes.
> 
> Note, HVCALL_SEND_IPI is a 'standard' fast hypercall (not 'XMM fast') as
> all its parameters fit into RDX:R8 and this is handled by KVM correctly.
> 
> Fixes: d8f5537a8816 ("KVM: hyper-v: Advertise support for fast XMM hypercalls")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Reviewed-by: Siddharth Chandrasekaran <sidcha@amazon.de>



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

* Re: [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes
  2022-02-25 13:17     ` Paolo Bonzini
@ 2022-02-28 10:52       ` Siddharth Chandrasekaran
  2022-02-28 11:09         ` Vitaly Kuznetsov
  0 siblings, 1 reply; 20+ messages in thread
From: Siddharth Chandrasekaran @ 2022-02-28 10:52 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Vitaly Kuznetsov, kvm, Sean Christopherson, Wanpeng Li,
	Jim Mattson, linux-kernel

On Fri, Feb 25, 2022 at 02:17:04PM +0100, Paolo Bonzini wrote:
> On 2/25/22 14:13, Vitaly Kuznetsov wrote:
> > Let's say we have 1 half of XMM0 consumed. Now:
> > 
> >   i = 0;
> >   j = 1;
> >   if (1)
> >       sparse_banks[0] = sse128_lo(hc->xmm[0]);
> > 
> >   This doesn't look right as we need to get the upper half of XMM0.
> > 
> >   I guess it should be reversed,
> > 
> >       if (j % 2)
> >           sparse_banks[i] = sse128_hi(hc->xmm[j / 2]);
> >       else
> >           sparse_banks[i] = sse128_lo(hc->xmm[j / 2]);

Maybe I am missing parts of this series.. I dont see this change in any
of the 4 patches Vitaly sent. Yes, they look swapped to me too.

~ Sid.



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

* Re: [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes
  2022-02-28 10:52       ` Siddharth Chandrasekaran
@ 2022-02-28 11:09         ` Vitaly Kuznetsov
  2022-02-28 16:18           ` Siddharth Chandrasekaran
  0 siblings, 1 reply; 20+ messages in thread
From: Vitaly Kuznetsov @ 2022-02-28 11:09 UTC (permalink / raw)
  To: Siddharth Chandrasekaran
  Cc: kvm, Sean Christopherson, Wanpeng Li, Jim Mattson, linux-kernel,
	Paolo Bonzini

Siddharth Chandrasekaran <sidcha@amazon.de> writes:

> On Fri, Feb 25, 2022 at 02:17:04PM +0100, Paolo Bonzini wrote:
>> On 2/25/22 14:13, Vitaly Kuznetsov wrote:
>> > Let's say we have 1 half of XMM0 consumed. Now:
>> > 
>> >   i = 0;
>> >   j = 1;
>> >   if (1)
>> >       sparse_banks[0] = sse128_lo(hc->xmm[0]);
>> > 
>> >   This doesn't look right as we need to get the upper half of XMM0.
>> > 
>> >   I guess it should be reversed,
>> > 
>> >       if (j % 2)
>> >           sparse_banks[i] = sse128_hi(hc->xmm[j / 2]);
>> >       else
>> >           sparse_banks[i] = sse128_lo(hc->xmm[j / 2]);
>
> Maybe I am missing parts of this series.. I dont see this change in any
> of the 4 patches Vitaly sent. Yes, they look swapped to me too.
>

There was a conflict with a patch series from Sean:
https://lore.kernel.org/kvm/20211207220926.718794-1-seanjc@google.com/

and this is a part of the resolution:

commit c0f1eaeb9e628bf86bf50f11cb4a2b671528391e
Merge: 4dfc4ec2b7f5 47d3e5cdfe60
Author: Paolo Bonzini <pbonzini@redhat.com>
Date:   Fri Feb 25 06:28:10 2022 -0500

    Merge branch 'kvm-hv-xmm-hypercall-fixes' into HEAD

-- 
Vitaly


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

* Re: [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes
  2022-02-28 11:09         ` Vitaly Kuznetsov
@ 2022-02-28 16:18           ` Siddharth Chandrasekaran
  0 siblings, 0 replies; 20+ messages in thread
From: Siddharth Chandrasekaran @ 2022-02-28 16:18 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: kvm, Sean Christopherson, Wanpeng Li, Jim Mattson, linux-kernel,
	Paolo Bonzini

On Mon, Feb 28, 2022 at 12:09:14PM +0100, Vitaly Kuznetsov wrote:
> Siddharth Chandrasekaran <sidcha@amazon.de> writes:
> > On Fri, Feb 25, 2022 at 02:17:04PM +0100, Paolo Bonzini wrote:
> >> On 2/25/22 14:13, Vitaly Kuznetsov wrote:
> >> > Let's say we have 1 half of XMM0 consumed. Now:
> >> >
> >> >   i = 0;
> >> >   j = 1;
> >> >   if (1)
> >> >       sparse_banks[0] = sse128_lo(hc->xmm[0]);
> >> >
> >> >   This doesn't look right as we need to get the upper half of XMM0.
> >> >
> >> >   I guess it should be reversed,
> >> >
> >> >       if (j % 2)
> >> >           sparse_banks[i] = sse128_hi(hc->xmm[j / 2]);
> >> >       else
> >> >           sparse_banks[i] = sse128_lo(hc->xmm[j / 2]);
> >
> > Maybe I am missing parts of this series.. I dont see this change in any
> > of the 4 patches Vitaly sent. Yes, they look swapped to me too.
> >
> 
> There was a conflict with a patch series from Sean:
> https://lore.kernel.org/kvm/20211207220926.718794-1-seanjc@google.com/
> 
> and this is a part of the resolution:
> 
> commit c0f1eaeb9e628bf86bf50f11cb4a2b671528391e
> Merge: 4dfc4ec2b7f5 47d3e5cdfe60
> Author: Paolo Bonzini <pbonzini@redhat.com>
> Date:   Fri Feb 25 06:28:10 2022 -0500
> 
>     Merge branch 'kvm-hv-xmm-hypercall-fixes' into HEAD

Got it, thank you!

~ Sid.



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

end of thread, other threads:[~2022-02-28 16:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-22 15:46 [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Vitaly Kuznetsov
2022-02-22 15:46 ` [PATCH 1/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_send_ipi() Vitaly Kuznetsov
2022-02-25 18:21   ` Maxim Levitsky
2022-02-28  9:58     ` Vitaly Kuznetsov
2022-02-28 10:45   ` Siddharth Chandrasekaran
2022-02-22 15:46 ` [PATCH 2/4] KVM: x86: hyper-v: Drop redundant 'ex' parameter from kvm_hv_flush_tlb() Vitaly Kuznetsov
2022-02-25 18:22   ` Maxim Levitsky
2022-02-28 10:46   ` Siddharth Chandrasekaran
2022-02-22 15:46 ` [PATCH 3/4] KVM: x86: hyper-v: Fix the maximum number of sparse banks for XMM fast TLB flush hypercalls Vitaly Kuznetsov
2022-02-28 10:47   ` Siddharth Chandrasekaran
2022-02-22 15:46 ` [PATCH 4/4] KVM: x86: hyper-v: HVCALL_SEND_IPI_EX is an XMM fast hypercall Vitaly Kuznetsov
2022-02-25 18:33   ` Maxim Levitsky
2022-02-28 10:02     ` Vitaly Kuznetsov
2022-02-28 10:48   ` Siddharth Chandrasekaran
2022-02-25 11:55 ` [PATCH 0/4] KVM: x86: hyper-v: XMM fast hypercalls fixes Paolo Bonzini
2022-02-25 13:13   ` Vitaly Kuznetsov
2022-02-25 13:17     ` Paolo Bonzini
2022-02-28 10:52       ` Siddharth Chandrasekaran
2022-02-28 11:09         ` Vitaly Kuznetsov
2022-02-28 16:18           ` Siddharth Chandrasekaran

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).