On Fri, Aug 09, 2019 at 07:00:10PM +0300, Adalbert Lazăr wrote: > +int kvmi_arch_cmd_control_msr(struct kvm_vcpu *vcpu, > + const struct kvmi_control_msr *req) > +{ > + int err; > + > + if (req->padding1 || req->padding2) > + return -KVM_EINVAL; > + > + err = msr_control(vcpu, req->msr, req->enable); > + > + if (!err && req->enable) This needs a comment explaining that it intentionally calls into arch code only for the enable case so as to avoid having to deal with tracking whether or not it's safe to disable interception. At first (and second) glance it look like KVM is silently ignoring the @enable=false case. > + kvm_arch_msr_intercept(vcpu, req->msr, req->enable); Renaming to kvm_arch_enable_msr_intercept() would also help communicate that KVMI can't be used to disable msr interception. The function can always be renamed if someone takes on the task of enhancing the arch code to handling disabling interception. > + > + return err; > +} _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization