From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C0005C433F5 for ; Fri, 13 May 2022 01:03:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1376260AbiEMBDc (ORCPT ); Thu, 12 May 2022 21:03:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44392 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1359861AbiEMBD3 (ORCPT ); Thu, 12 May 2022 21:03:29 -0400 Received: from mail-pg1-x532.google.com (mail-pg1-x532.google.com [IPv6:2607:f8b0:4864:20::532]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3B2BF40A1A for ; Thu, 12 May 2022 18:03:28 -0700 (PDT) Received: by mail-pg1-x532.google.com with SMTP id a191so6105715pge.2 for ; Thu, 12 May 2022 18:03:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=yhdX1EL/sWCz5vJoug+D2WrNzRnzdgksXTuD2mvaEPE=; b=VDI9xOkNKnEokzk8xUh4JyQX+Yo006IxdyOCXuWDTE0qkS6LZ/bo2zcSHasiEdI1U2 c7qHNu2EXwJkXha5TxAgbRCpy7iFg0MSX9FUACYYgIm4l+d5zkSECCApXjn1Q4Xks/ME hekKe9IZ/YzFZUnKHkSLZfqeVzia4h/B3HHlw9LyOYgRceHjA0QTFuok0EAiyievqFry GQV63bihxwfmUxM88kHArPmj6FIjbVk1S6BEiadN6tyiasy5SQP2yhqi8Ln4VECCPuto Wap2+ixLdbE07JfNNqGouV7cQNosdDRuTM1Vu1vMSkJicY9+brKkqSIkxDFlPWlyijAB J4sg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=yhdX1EL/sWCz5vJoug+D2WrNzRnzdgksXTuD2mvaEPE=; b=0J7rtZ9JoLIVI0LGldhNww6FeFIM9EZzMk8U32zMyH14zmt2JHQQZSOhUKddk0vdR/ jLdnYejl8ACboAoRewDJDDY8mrQkWpgHxUMQLZLJl2ocm9EKNm3+9fycAtRQI5qUSrBg wo5YNZoTVYaWyx/MbegAYtLPjl3q+8HP8w3QI0rRnxOMS/hjOjUNysWalV7vl7sGO6gD 28iCVOK2Wc0IuN9X5eYp+suBgCJHnRCLz7Xfbd8yh41g13JSY/P1/5XcA8rAhrYhhVON X3iVJOxv1HDLfE4oWjSHRPgSOd7rpU6MAriJo8B70rqLrb7H+ura89rBIgI/UA+9zGju obbg== X-Gm-Message-State: AOAM530EwYvT7KFKV18KnN+xXRY0ZZ2SQAUdVSf71b/tgyzWOoheTXCU Ky2Vo4O+WQYlO2tFMR9xldY1GA== X-Google-Smtp-Source: ABdhPJzP8QtZ6ZkiVz6FrHZg1aCTdDrF4Bg9uDZaDvtm2YaM/rA3cvXwo4MwPuEgNfYvb3txDVILbA== X-Received: by 2002:aa7:84d1:0:b0:510:8796:4f38 with SMTP id x17-20020aa784d1000000b0051087964f38mr2017729pfn.8.1652403807512; Thu, 12 May 2022 18:03:27 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id b2-20020a170902650200b0015e8d4eb264sm491423plk.174.2022.05.12.18.03.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 May 2022 18:03:27 -0700 (PDT) Date: Fri, 13 May 2022 01:03:23 +0000 From: Sean Christopherson To: Zeng Guang Cc: Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , kvm@vger.kernel.org, Dave Hansen , Tony Luck , Kan Liang , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , Kim Phillips , Jarkko Sakkinen , Jethro Beekman , Kai Huang , x86@kernel.org, linux-kernel@vger.kernel.org, Robert Hu , Gao Chao Subject: Re: [PATCH v2] kvm: selftests: Add KVM_CAP_MAX_VCPU_ID cap test Message-ID: References: <20220503064037.10822-1-guang.zeng@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220503064037.10822-1-guang.zeng@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 03, 2022, Zeng Guang wrote: > +int main(int argc, char *argv[]) > +{ > + struct kvm_vm *vm; > + struct kvm_enable_cap cap = { 0 }; > + int ret; > + > + vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); > + > + /* Get KVM_CAP_MAX_VCPU_ID cap supported in KVM */ > + ret = vm_check_cap(vm, KVM_CAP_MAX_VCPU_ID); Because it's trivial to do so, and will avoid a hardcoded "max", what about looping over all possible values? And then some arbitrary number of the max? max_nr_vcpus = vm_check_cap(vm, KVM_CAP_MAX_VCPU_ID); TEST_ASSERT(max_nr_vcpus > ???, ...) for (i = 0; i < max_nr_vcpus; i++) vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); vm_set_max_nr_vcpus(vm, 0); vm_create_invalid_vcpu(vm, 0); vm_set_max_nr_vcpus(vm, i); vm_set_max_nr_vcpus(vm, i); vm_create_invalid_vcpu(vm, i); vm_create_invalid_vcpu(vm, i + 1); vm_set_invalid_max_nr_vcpus(vm, 0); vm_set_invalid_max_nr_vcpus(vm, i + 1); vm_set_invalid_max_nr_vcpus(vm, i - 1); vm_set_invalid_max_nr_vcpus(vm, max_nr_vcpus); close(vm->fd); } for ( ; i < max_nr_vcpus + 100; i++) { vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); vm_set_invalid_max_nr_vcpus(vm, i); close(vm->fd); } > + > + /* Check failure if set KVM_CAP_MAX_VCPU_ID beyond KVM cap */ > + cap.cap = KVM_CAP_MAX_VCPU_ID; > + cap.args[0] = ret + 1; > + ret = ioctl(vm->fd, KVM_ENABLE_CAP, &cap); A helper or two to set the cap would be, uh, helpful :-) See above for ideas. > + TEST_ASSERT(ret < 0, > + "Unexpected success to enable KVM_CAP_MAX_VCPU_ID" > + "beyond KVM cap!\n"); Please don't wrap quoted strings. Shorten the string and/or let the line run long. For the string/message, prioritize information that the user _can't_ get from looking at the code, and info that is highly relevant to the expectations. E.g. print the the return value, the errno, and the allegedly bad value. It's definitely helpful to provide context too (KVM Unit Tests drive me bonkers for their terse messages), but for cases like this, it's redundant. "Unexpected success" is redundant because the "ret < 0" conveys that failure was expected, and hopefully most people will intuit that the test was trying "to enable" KVM_CAP_MAX_VCPU_ID. If not, a quick glance at the code (file and line provided) will give them that info. E.g. assuming this ends up in a helper, something like TEST_ASSERT(ret == -1 && errno == EINVAL, "KVM_CAP_MAX_VCPU_ID bug, max ID = %d, ret = %d, errno = %d (%s), max_id, errno, strerror(errno)); IMO it's worth checking errno to reduce the probability of false pass, e.g. if the ioctl() is rejected for some other reason due to a test bug. > + > + /* Check success if set KVM_CAP_MAX_VCPU_ID */ > + cap.args[0] = MAX_VCPU_ID; > + ret = ioctl(vm->fd, KVM_ENABLE_CAP, &cap); > + TEST_ASSERT(ret == 0, > + "Unexpected failure to enable KVM_CAP_MAX_VCPU_ID!\n");