From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-f201.google.com (mail-yb1-f201.google.com [209.85.219.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C4B9513ADF for ; Fri, 4 Aug 2023 15:23:56 +0000 (UTC) Received: by mail-yb1-f201.google.com with SMTP id 3f1490d57ef6-d07cb52a768so2403521276.1 for ; Fri, 04 Aug 2023 08:23:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691162635; x=1691767435; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ZFxOXnJrCnLwyosaEoJ5XKTdBgr+iJ5sfCPaX0Y1m90=; b=MqIxKVHf1VXgzNTFjVSItHPlUtA6jlDZ6cMA5loyVnm34+XGPfPfHi7i4Usm4AU7ac W0Mjx6U/igEf4P0ZYjtcGxo5foByhdtowNPxbDsL5QxNZpFdCplK5J4FG0yIuvCkXLS4 a95QuhnNLAnW9bcMzcWNHzyxuBF+wGZZJ0d8TQTo8T4W2a9FWZUJU7HA7KupGKxbn1tI TXlz0Eyq/7GTDtCZ7PQmDvEX3+sJslw8Le4sjx5G9NNo4nB0y3Wd39Afmq319x33Xfyp hRct9w6s7Sl6w6sbkVIYwI3iqZVNtwB5GBxsHDPKWFaosSyjgDGsQ4el6xElhc4kvD9r 626Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691162635; x=1691767435; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ZFxOXnJrCnLwyosaEoJ5XKTdBgr+iJ5sfCPaX0Y1m90=; b=T+m9EItp3Sh51WfH6POpzshbfRYGQNO1e/RXJZaPWn85DubyP4xwz/NLncqWBvZQfw 6GIQPb+TcZ2vwK3qY5oJcX39v7hHUrywKvWxjJiaSRnV3E6YHjGeXXWiukDk9yee/F8H 6SqxL7wCcDtK7C3RmWUT2OxafpWPiT7s++SDPwGOzvnPlhJoe0QPG1PVGQJPPZsdB/kF ZYcTBmg2JvYmXqniA0hJgc9AoNWINFa6E5YTtO6aSrEeO5fDSBDdXoZa4Q9aSNH9zAda oImpnQGupcSEmuVuTAjASQ7Oxq0MklWoC21jR4TqXhZiP6WLyNYvdSKcVE046bHlV3Bq iisg== X-Gm-Message-State: AOJu0YydcR5y8rZwbKRadU7JPw3Sva6pIdrYKctRSzaK2vpBIP59B0qf J043N3OmIgNUQsbxABk61FbMnboorvs= X-Google-Smtp-Source: AGHT+IGE/OCLG15icz/3cvmPraYl9lMENeybPBclQw1v/gSBLG6RHMQjrkxyHk1GNtrEatT/w5sEvyR2tcs= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:b316:0:b0:d0f:bcfe:bc74 with SMTP id l22-20020a25b316000000b00d0fbcfebc74mr11768ybj.9.1691162635611; Fri, 04 Aug 2023 08:23:55 -0700 (PDT) Date: Fri, 4 Aug 2023 08:23:54 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20230804004226.1984505-1-seanjc@google.com> <20230804004226.1984505-4-seanjc@google.com> Message-ID: Subject: Re: [PATCH 3/4] KVM: selftests: Use asserting kvm_ioctl() macros when getting ARM page sizes From: Sean Christopherson To: Michal Luczaj Cc: Marc Zyngier , Oliver Upton , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" On Fri, Aug 04, 2023, Michal Luczaj wrote: > On 8/4/23 02:42, Sean Christopherson wrote: > > Use kvm_ioctl() instead of open coding equivalent ioctl()+TEST_ASSERT() > > calls when getting the support page sizes on ARM. The macro usage is a > > little funky since the "kvm_fd" parameter implies an actual /dev/kvm fd, > > but on the other hand the code is invoking KVM ioctl()s. > > > > Alternatively, the core utilities could expose a vm_open()+vm_close() > > pair so that the ARM code could create a dummy, on-stack VM+vCPU pair and > > use {vm,vcpu}_ioctl() as appropriate. But the odds of something breaking > > due to oddball, partial usage of kvm_vm and kvm_vcpu structures is much > > higher than realizing meaningful benefit from using {vm,vcpu}_ioctl(). > > Since you're doing the cleanup, does mmio_warning_test qualify for the > same (funky usage ahead)? Hmm, I'm heavily leaning towards deleting that test entirely. It's almost literally a copy+paste of the most basic syzkaller test, e.g. spawn a vCPU with no backing memory and watch it die a horrible death. Unless I'm missing something, the test is complete overkill too, e.g. I highly doubt the original KVM bug required userspace to fork() and whatnot, but syzkaller spawns threads for everything and so that got copied into the selftest. And this stuff is just silly: TEST_REQUIRE(host_cpu_is_intel); TEST_REQUIRE(!vm_is_unrestricted_guest(NULL)); because crashing the VM doesn't require Intel, nor does it require !URG, those just happened to be the conditions for the bug. As much as I like having explicit testcases, adding a new selftest for every bug that syzkaller finds is neither realistic nor productive. In other words, I think we should treat syzkaller as being part of KVM's test infrastructure. I'll send a patch to nuke the test. > - kvm = open("/dev/kvm", O_RDWR); > - TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); > - kvmvm = __kvm_ioctl(kvm, KVM_CREATE_VM, NULL); > - TEST_ASSERT(kvmvm > 0, KVM_IOCTL_ERROR(KVM_CREATE_VM, kvmvm)); > - kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); > - TEST_ASSERT(kvmcpu != -1, KVM_IOCTL_ERROR(KVM_CREATE_VCPU, kvmcpu)); > + kvm = open_path_or_exit(KVM_DEV_PATH, O_RDWR); > + kvmvm = kvm_fd_ioctl(kvm, KVM_CREATE_VM, NULL); > + kvmcpu = kvm_fd_ioctl(kvmvm, KVM_CREATE_VCPU, NULL); > > Side note, just in case this wasn't your intention: no kvm@ in cc. Wasn't intentional, I was moving too fast at the end of the day and missed that KVM wasn't Cc'd. Grr. 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E89E8C04A6A for ; Fri, 4 Aug 2023 15:24:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=SGSPAk9nUlqpRwvAnF7gpis1B3pY5Hrf71dpHv4ZgAU=; b=Z8EzX7Hk37U1oDGVTAgH7m0krw g8l4Y85oxnLR7z+AcoY2anFEO94t0v4HDcCtcFhwOVN/j/FxYJPVN2v1EV+vB58VW+nE1CHR1l7sg REzWsArr7zJfesBix4vW+hW7NZUjIUPhQf3CP2Lzf4P0TP+4eoWFYuBrWvPyRE38pJd30zkcFWkNj uvvN0XV3iQrAlDtlY5hUm03dalBOGBwPUhOlNlMhYRPAeyApJubXP2kSjvYbp58LapfDN/tGDOEg8 MFnn2dL8aT/ZuAkUsQ21knsreaYMBEBYNZN/j/JFtBV48xCqagx3OQEJ7WkxFw3gd6sJK6cakkRx8 XdN/nhYA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qRwen-00ChGx-0M; Fri, 04 Aug 2023 15:24:01 +0000 Received: from mail-yw1-x1149.google.com ([2607:f8b0:4864:20::1149]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qRwek-00ChE4-2w for linux-arm-kernel@lists.infradead.org; Fri, 04 Aug 2023 15:24:00 +0000 Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-583da2ac09fso23352077b3.1 for ; Fri, 04 Aug 2023 08:23:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691162635; x=1691767435; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ZFxOXnJrCnLwyosaEoJ5XKTdBgr+iJ5sfCPaX0Y1m90=; b=MqIxKVHf1VXgzNTFjVSItHPlUtA6jlDZ6cMA5loyVnm34+XGPfPfHi7i4Usm4AU7ac W0Mjx6U/igEf4P0ZYjtcGxo5foByhdtowNPxbDsL5QxNZpFdCplK5J4FG0yIuvCkXLS4 a95QuhnNLAnW9bcMzcWNHzyxuBF+wGZZJ0d8TQTo8T4W2a9FWZUJU7HA7KupGKxbn1tI TXlz0Eyq/7GTDtCZ7PQmDvEX3+sJslw8Le4sjx5G9NNo4nB0y3Wd39Afmq319x33Xfyp hRct9w6s7Sl6w6sbkVIYwI3iqZVNtwB5GBxsHDPKWFaosSyjgDGsQ4el6xElhc4kvD9r 626Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691162635; x=1691767435; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ZFxOXnJrCnLwyosaEoJ5XKTdBgr+iJ5sfCPaX0Y1m90=; b=dJoYdodATY1gBQAV/m4IPQrpkc1GP8utu64D1UG/8vpDQy0KEqu5zTO9wIRc8ZxnwI d/siScz2tWWe6/TfOkGD0f4zvWGx1PwKvSiXyJzOz5LMgCWr51TS6F+cFEqCrKiyO3/d xVWvzhTh4kxgRMz3mW0uySlvUnwQmevGoJ0BOKMp8EJwNg8tx67aPjTgnY/BZOj7QnO/ Sgzg2EctLbQy7KWL4Dkb0M50IJVUKNW7qzYjKrXS0jH9wADE88pQoxjzDEpFNqxLw/pU llu8HnE4QjQDkwlr9CxJJ4zyXR3/EEjWvNx1GsUfhFg2zVlJSCLcPutNAhAjvqpKhZhr Uw8w== X-Gm-Message-State: AOJu0YyNRhXgoZbnNCWDtf5cJdJsCKY0m3ZwyrxxJSfqqz8yNC9GCc2W IjUcHpLgXXYL6GaGllPy0McWrUk8YjA= X-Google-Smtp-Source: AGHT+IGE/OCLG15icz/3cvmPraYl9lMENeybPBclQw1v/gSBLG6RHMQjrkxyHk1GNtrEatT/w5sEvyR2tcs= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:b316:0:b0:d0f:bcfe:bc74 with SMTP id l22-20020a25b316000000b00d0fbcfebc74mr11768ybj.9.1691162635611; Fri, 04 Aug 2023 08:23:55 -0700 (PDT) Date: Fri, 4 Aug 2023 08:23:54 -0700 In-Reply-To: Mime-Version: 1.0 References: <20230804004226.1984505-1-seanjc@google.com> <20230804004226.1984505-4-seanjc@google.com> Message-ID: Subject: Re: [PATCH 3/4] KVM: selftests: Use asserting kvm_ioctl() macros when getting ARM page sizes From: Sean Christopherson To: Michal Luczaj Cc: Marc Zyngier , Oliver Upton , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230804_082358_948027_7AA11A86 X-CRM114-Status: GOOD ( 19.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Aug 04, 2023, Michal Luczaj wrote: > On 8/4/23 02:42, Sean Christopherson wrote: > > Use kvm_ioctl() instead of open coding equivalent ioctl()+TEST_ASSERT() > > calls when getting the support page sizes on ARM. The macro usage is a > > little funky since the "kvm_fd" parameter implies an actual /dev/kvm fd, > > but on the other hand the code is invoking KVM ioctl()s. > > > > Alternatively, the core utilities could expose a vm_open()+vm_close() > > pair so that the ARM code could create a dummy, on-stack VM+vCPU pair and > > use {vm,vcpu}_ioctl() as appropriate. But the odds of something breaking > > due to oddball, partial usage of kvm_vm and kvm_vcpu structures is much > > higher than realizing meaningful benefit from using {vm,vcpu}_ioctl(). > > Since you're doing the cleanup, does mmio_warning_test qualify for the > same (funky usage ahead)? Hmm, I'm heavily leaning towards deleting that test entirely. It's almost literally a copy+paste of the most basic syzkaller test, e.g. spawn a vCPU with no backing memory and watch it die a horrible death. Unless I'm missing something, the test is complete overkill too, e.g. I highly doubt the original KVM bug required userspace to fork() and whatnot, but syzkaller spawns threads for everything and so that got copied into the selftest. And this stuff is just silly: TEST_REQUIRE(host_cpu_is_intel); TEST_REQUIRE(!vm_is_unrestricted_guest(NULL)); because crashing the VM doesn't require Intel, nor does it require !URG, those just happened to be the conditions for the bug. As much as I like having explicit testcases, adding a new selftest for every bug that syzkaller finds is neither realistic nor productive. In other words, I think we should treat syzkaller as being part of KVM's test infrastructure. I'll send a patch to nuke the test. > - kvm = open("/dev/kvm", O_RDWR); > - TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); > - kvmvm = __kvm_ioctl(kvm, KVM_CREATE_VM, NULL); > - TEST_ASSERT(kvmvm > 0, KVM_IOCTL_ERROR(KVM_CREATE_VM, kvmvm)); > - kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); > - TEST_ASSERT(kvmcpu != -1, KVM_IOCTL_ERROR(KVM_CREATE_VCPU, kvmcpu)); > + kvm = open_path_or_exit(KVM_DEV_PATH, O_RDWR); > + kvmvm = kvm_fd_ioctl(kvm, KVM_CREATE_VM, NULL); > + kvmcpu = kvm_fd_ioctl(kvmvm, KVM_CREATE_VCPU, NULL); > > Side note, just in case this wasn't your intention: no kvm@ in cc. Wasn't intentional, I was moving too fast at the end of the day and missed that KVM wasn't Cc'd. Grr. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel