On 2020/7/31 14:44, Paolo Bonzini wrote: > On 31/07/20 08:39, Zhenyu Ye wrote: >> On 2020/7/31 2:03, Paolo Bonzini wrote: >>> Yes, I think it's not needed. Probably the deassign check can be turned into an assertion? >>> >>> Paolo >>> >> >> I think we can do this in the same function, and turnt he check of >> p->eventfd into assertion in kvm_deassign_ioeventfd_idx(). Just like: >> >> ---8<--- >> static inline struct _ioeventfd * >> get_ioeventfd(struct kvm *kvm, enum kvm_bus bus_idx, >> struct kvm_ioeventfd *args) >> { >> static struct _ioeventfd *_p; >> bool wildcard = !(args->flags & KVM_IOEVENTFD_FLAG_DATAMATCH); >> >> list_for_each_entry(_p, &kvm->ioeventfds, list) >> if (_p->bus_idx == bus_idx && >> _p->addr == args->addr && >> (!_p->length || !args->len || >> (_p->length == args->len && >> (_p->wildcard || wildcard || >> _p->datamatch == args->datamatch)))) >> return _p; >> >> return NULL; >> } >> >> kvm_deassign_ioeventfd_idx() { >> ... >> p = get_ioeventfd(kvm, bus_idx, args); >> if (p) { >> assert(p->eventfd == eventfd); >> ... >> } >> >> ---8<---- >> >> This may be easier to understand (keep the same logic in assign/deassign). > > I think you should also warn if: > > 1) p->length != args->len > > 2) p->wildcard != args->wildcard if p->length > > 3) p->datamatch != args->datamatch if p->length && !p->wildcard > > but yeah it sounds like a plan. > I will try to do this. :) Zhenyu