On Mon, Jun 05, 2023, Michal Luczaj wrote: > On 6/2/23 18:56, Sean Christopherson wrote: > > On Fri, Jun 02, 2023, Michal Luczaj wrote: > >> I guess this makes the !! in kvm_vm_ioctl_create_vcpu() unnecessary: > >> > >> KVM_BUG_ON(!!xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0)... > > > > Ya, I saw that, which in addition to Wei's ping, is what reminded me that the > > KVM_BUG_ON() fix hadn't been merged. > > > >> Is it worth a patch (perhaps along with chopping off !! in > >> kvm_msr_allowed() and few other places)? > > > > Yes, I think so. > > OK, so xa_store() aside[*], I see some bool-to-bools: > > arch/x86/kvm/x86.c: > kvm_msr_allowed():allowed = !!test_bit(index - start, bitmap); > arch/x86/kvm/hyperv.c: > kvm_hv_hypercall():hc.rep = !!(hc.rep_cnt || hc.rep_idx); > arch/x86/kvm/mmu/mmu.c: > update_pkru_bitmask(): > pkey_bits = !!check_pkey; > pkey_bits |= (!!check_write) << 1; > arch/x86/kvm/svm/svm.c: > msr_write_intercepted():return !!test_bit(bit_write, &tmp); > svm_vcpu_after_set_cpuid(): > 2x set_msr_interception... > tools/testing/selftests/kvm/x86_64/vmx_exception_with_invalid_guest_state.c: > set_or_clear_invalid_guest_state():sregs.tr.unusable = !!set; > > But perhaps this is a matter of style and those were meant to be this kind-of > explicit? I doubt it, I'm guessing most cases are due to the author being overzealous for one reason or another, e.g. I suspect the test_bit() ones are due to the original author incorrectly assuming test_bit() returned an unsigned long, i.e. the bit, as opposed to the bool. If you want to clean these up, I'd say "fix" the test_bit() cases, but leave the others alone. The test_bit() ones are clearly redundant, and IMO can be actively due to implying test_bit() returns something other than a bool. > [*] https://lore.kernel.org/kvm/20230605114852.288964-1-mhal@xxxxxxx/
