On Thu, Apr 07, 2022, Sean Christopherson wrote:
> > @@ -1857,12 +1940,13 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
> > struct hv_tlb_flush_ex flush_ex;
> > struct hv_tlb_flush flush;
> > DECLARE_BITMAP(vcpu_mask, KVM_MAX_VCPUS);
> > + u64 entries[KVM_HV_TLB_FLUSH_RING_SIZE - 2];
>
> What's up with the -2? And given the multitude of things going on in this code,
> I'd strongly prefer this be tlbflush_entries.
>
> Actually, if you do:
>
> u64 __tlbflush_entries[KVM_HV_TLB_FLUSH_RING_SIZE - 2];
> u64 *tlbflush_entries;
Looking at future patches, tlb_flush_entries is better for consistency (apply everywhere).
> and drop all_addr, the code to get entries can be
>
> if (hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE ||
> hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX ||
> hc->rep_cnt > ARRAY_SIZE(tlbflush_entries)) {
> tlbfluish_entries = NULL;
> } else {
> if (kvm_hv_get_tlbflush_entries(kvm, hc, __tlbflush_entries,
> consumed_xmm_halves, data_offset))
> return HV_STATUS_INVALID_HYPERCALL_INPUT;
> tlbfluish_entries = __tlbflush_entries;
Heh, fluish, because TLB entries are somewhat fluid?
> }
>
> and the calls to queue flushes becomes
>
> hv_tlb_flush_ring_enqueue(v, tlbflush_entries, hc->rep_cnt);
>
> That way a bug will "just" be a NULL pointer dereference and not consumption of
> uninitialized data (though such a bug might be caught be caught by the compiler).
