From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
kvm@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
Ajay Garg <ajaygargnsit@gmail.com>,
Paolo Bonzini <pbonzini@redhat.com>,
"K. Y. Srinivasan" <kys@microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Stephen Hemminger <sthemmin@microsoft.com>,
Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH v2 2/8] KVM: x86: Get the number of Hyper-V sparse banks from the VARHEAD field
Date: Mon, 01 Nov 2021 10:52:07 +0100 [thread overview]
Message-ID: <87a6iokxtk.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <20211030000800.3065132-3-seanjc@google.com>
Sean Christopherson <seanjc@google.com> writes:
> Get the number of sparse banks from the VARHEAD field, which the guest is
> required to provide as "The size of a variable header, in QWORDS.", where
> the variable header is:
>
> Variable Header Bytes = {Total Header Bytes - sizeof(Fixed Header)}
> rounded up to nearest multiple of 8
> Variable HeaderSize = Variable Header Bytes / 8
>
> In other words, the VARHEAD should match the number of sparse banks.
> Keep the manual count as a sanity check, but otherwise rely on the field
> so as to more closely align with the logic defined in the TLFS and to
> allow for future cleanups.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/hyperv.c | 35 ++++++++++++++++++-------------
> arch/x86/kvm/trace.h | 14 +++++++------
> include/asm-generic/hyperv-tlfs.h | 1 +
> 3 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 814d1a1f2cb8..cf18aa1712bf 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1742,6 +1742,7 @@ struct kvm_hv_hcall {
> u64 ingpa;
> u64 outgpa;
> u16 code;
> + u16 var_cnt;
> u16 rep_cnt;
> u16 rep_idx;
> bool fast;
> @@ -1761,7 +1762,6 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
> unsigned long *vcpu_mask;
> u64 valid_bank_mask;
> u64 sparse_banks[64];
> - int sparse_banks_len;
> bool all_cpus;
>
> if (!ex) {
> @@ -1811,24 +1811,28 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
> all_cpus = flush_ex.hv_vp_set.format !=
> HV_GENERIC_SET_SPARSE_4K;
>
> - sparse_banks_len = bitmap_weight((unsigned long *)&valid_bank_mask, 64);
> + if (hc->var_cnt != bitmap_weight((unsigned long *)&valid_bank_mask, 64))
> + return HV_STATUS_INVALID_HYPERCALL_INPUT;
Let's hope Windows doesn't break this ruls when vp_set.format != HV_GENERIC_SET_SPARSE_4K
>
> - if (!sparse_banks_len && !all_cpus)
> + if (!hc->var_cnt && !all_cpus)
> goto ret_success;
>
> if (!all_cpus) {
> if (hc->fast) {
> - if (sparse_banks_len > HV_HYPERCALL_MAX_XMM_REGISTERS - 1)
> + if (hc->var_cnt > HV_HYPERCALL_MAX_XMM_REGISTERS - 1)
> return HV_STATUS_INVALID_HYPERCALL_INPUT;
> - for (i = 0; i < sparse_banks_len; i += 2) {
> + 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]);
> }
> } else {
> + if (hc->var_cnt > 64)
> + return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +
> gpa = hc->ingpa + offsetof(struct hv_tlb_flush_ex,
> hv_vp_set.bank_contents);
> if (unlikely(kvm_read_guest(kvm, gpa, sparse_banks,
> - sparse_banks_len *
> + hc->var_cnt *
> sizeof(sparse_banks[0]))))
> return HV_STATUS_INVALID_HYPERCALL_INPUT;
> }
> @@ -1884,7 +1888,6 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
> unsigned long *vcpu_mask;
> unsigned long valid_bank_mask;
> u64 sparse_banks[64];
> - int sparse_banks_len;
> u32 vector;
> bool all_cpus;
>
> @@ -1917,22 +1920,25 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>
> 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]);
> -
> all_cpus = send_ipi_ex.vp_set.format == HV_GENERIC_SET_ALL;
>
> + if (hc->var_cnt != bitmap_weight(&valid_bank_mask, 64))
> + return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +
> if (all_cpus)
> goto check_and_send_ipi;
>
> - if (!sparse_banks_len)
> + if (!hc->var_cnt)
> goto ret_success;
>
> + if (hc->var_cnt > 64)
> + return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +
> if (kvm_read_guest(kvm,
> hc->ingpa + offsetof(struct hv_send_ipi_ex,
> vp_set.bank_contents),
> sparse_banks,
> - sparse_banks_len))
> + hc->var_cnt * sizeof(sparse_banks[0])))
> return HV_STATUS_INVALID_HYPERCALL_INPUT;
> }
>
> @@ -2190,13 +2196,14 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
> }
>
> hc.code = hc.param & 0xffff;
> + hc.var_cnt = (hc.param & HV_HYPERCALL_VARHEAD_MASK) >> HV_HYPERCALL_VARHEAD_OFFSET;
> hc.fast = !!(hc.param & HV_HYPERCALL_FAST_BIT);
> hc.rep_cnt = (hc.param >> HV_HYPERCALL_REP_COMP_OFFSET) & 0xfff;
> hc.rep_idx = (hc.param >> HV_HYPERCALL_REP_START_OFFSET) & 0xfff;
> hc.rep = !!(hc.rep_cnt || hc.rep_idx);
>
> - trace_kvm_hv_hypercall(hc.code, hc.fast, hc.rep_cnt, hc.rep_idx,
> - hc.ingpa, hc.outgpa);
> + trace_kvm_hv_hypercall(hc.code, hc.fast, hc.var_cnt, hc.rep_cnt,
> + hc.rep_idx, hc.ingpa, hc.outgpa);
>
> if (unlikely(!hv_check_hypercall_access(hv_vcpu, hc.code))) {
> ret = HV_STATUS_ACCESS_DENIED;
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index 953b0fcb21ee..f6625cfb686c 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -64,9 +64,9 @@ TRACE_EVENT(kvm_hypercall,
> * Tracepoint for hypercall.
> */
> TRACE_EVENT(kvm_hv_hypercall,
> - TP_PROTO(__u16 code, bool fast, __u16 rep_cnt, __u16 rep_idx,
> - __u64 ingpa, __u64 outgpa),
> - TP_ARGS(code, fast, rep_cnt, rep_idx, ingpa, outgpa),
> + TP_PROTO(__u16 code, bool fast, __u16 var_cnt, __u16 rep_cnt,
> + __u16 rep_idx, __u64 ingpa, __u64 outgpa),
> + TP_ARGS(code, fast, var_cnt, rep_cnt, rep_idx, ingpa, outgpa),
>
> TP_STRUCT__entry(
> __field( __u16, rep_cnt )
> @@ -74,6 +74,7 @@ TRACE_EVENT(kvm_hv_hypercall,
> __field( __u64, ingpa )
> __field( __u64, outgpa )
> __field( __u16, code )
> + __field( __u16, var_cnt )
> __field( bool, fast )
> ),
>
> @@ -83,13 +84,14 @@ TRACE_EVENT(kvm_hv_hypercall,
> __entry->ingpa = ingpa;
> __entry->outgpa = outgpa;
> __entry->code = code;
> + __entry->var_cnt = var_cnt;
> __entry->fast = fast;
> ),
>
> - TP_printk("code 0x%x %s cnt 0x%x idx 0x%x in 0x%llx out 0x%llx",
> + TP_printk("code 0x%x %s var_cnt 0x%x cnt 0x%x idx 0x%x in 0x%llx out 0x%llx",
Nit: 'cnt' is (and was) a bit ambiguous, I'd suggest to explicitly say
'rep_cnt' (and probably 'rep_idx') instead.
> __entry->code, __entry->fast ? "fast" : "slow",
> - __entry->rep_cnt, __entry->rep_idx, __entry->ingpa,
> - __entry->outgpa)
> + __entry->var_cnt, __entry->rep_cnt, __entry->rep_idx,
> + __entry->ingpa, __entry->outgpa)
> );
>
> TRACE_EVENT(kvm_hv_hypercall_done,
> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> index 56348a541c50..1ba8e6da4427 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -182,6 +182,7 @@ enum HV_GENERIC_SET_FORMAT {
> #define HV_HYPERCALL_RESULT_MASK GENMASK_ULL(15, 0)
> #define HV_HYPERCALL_FAST_BIT BIT(16)
> #define HV_HYPERCALL_VARHEAD_OFFSET 17
> +#define HV_HYPERCALL_VARHEAD_MASK GENMASK_ULL(26, 17)
> #define HV_HYPERCALL_REP_COMP_OFFSET 32
> #define HV_HYPERCALL_REP_COMP_1 BIT_ULL(32)
> #define HV_HYPERCALL_REP_COMP_MASK GENMASK_ULL(43, 32)
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
next prev parent reply other threads:[~2021-11-01 9:54 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-30 0:07 [PATCH v2 0/8] KVM: x86: Hyper-V hypercall fix and cleanups Sean Christopherson
2021-10-30 0:07 ` [PATCH v2 1/8] KVM: x86: Ignore sparse banks size for an "all CPUs", non-sparse IPI req Sean Christopherson
2021-11-01 9:05 ` Vitaly Kuznetsov
2021-10-30 0:07 ` [PATCH v2 2/8] KVM: x86: Get the number of Hyper-V sparse banks from the VARHEAD field Sean Christopherson
2021-11-01 9:52 ` Vitaly Kuznetsov [this message]
2021-10-30 0:07 ` [PATCH v2 3/8] KVM: x86: Refactor kvm_hv_flush_tlb() to reduce indentation Sean Christopherson
2021-11-01 10:00 ` Vitaly Kuznetsov
2021-12-03 23:45 ` Sean Christopherson
2021-10-30 0:07 ` [PATCH v2 4/8] KVM: x86: Add a helper to get the sparse VP_SET for IPIs and TLB flushes Sean Christopherson
2021-11-01 10:06 ` Vitaly Kuznetsov
2021-10-30 0:07 ` [PATCH v2 5/8] KVM: x86: Don't bother reading sparse banks that end up being ignored Sean Christopherson
2021-11-01 9:46 ` Vitaly Kuznetsov
2021-10-30 0:07 ` [PATCH v2 6/8] KVM: x86: Shove vp_bitmap handling down into sparse_set_to_vcpu_mask() Sean Christopherson
2021-11-01 10:12 ` Vitaly Kuznetsov
2021-10-30 0:07 ` [PATCH v2 7/8] KVM: x86: Reject fixeds-size Hyper-V hypercalls with non-zero "var_cnt" Sean Christopherson
2021-11-01 10:27 ` Vitaly Kuznetsov
2021-12-03 23:48 ` Sean Christopherson
2021-10-30 0:08 ` [PATCH v2 8/8] KVM: x86: Add checks for reserved-to-zero Hyper-V hypercall fields Sean Christopherson
2021-11-01 10:33 ` Vitaly Kuznetsov
2021-12-02 2:13 ` Sean Christopherson
2021-12-02 15:16 ` Michael Kelley (LINUX)
2021-12-03 14:09 ` ** POTENTIAL FRAUD ALERT - RED HAT ** " Vitaly Kuznetsov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a6iokxtk.fsf@vitty.brq.redhat.com \
--to=vkuznets@redhat.com \
--cc=ajaygargnsit@gmail.com \
--cc=arnd@arndb.de \
--cc=decui@microsoft.com \
--cc=haiyangz@microsoft.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=kys@microsoft.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=sthemmin@microsoft.com \
--cc=wanpengli@tencent.com \
--cc=wei.liu@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.