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 2B44BC4321E for ; Fri, 14 Oct 2022 09:41:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230180AbiJNJll (ORCPT ); Fri, 14 Oct 2022 05:41:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60186 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230166AbiJNJlg (ORCPT ); Fri, 14 Oct 2022 05:41:36 -0400 Received: from mail-pl1-x629.google.com (mail-pl1-x629.google.com [IPv6:2607:f8b0:4864:20::629]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B46AD1C69D8 for ; Fri, 14 Oct 2022 02:41:31 -0700 (PDT) Received: by mail-pl1-x629.google.com with SMTP id l1so4216340pld.13 for ; Fri, 14 Oct 2022 02:41:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=KdrfFMceV7elb/rA2YCTui/ZhC4l1ygRRIMRPNusZhU=; b=tILJoqotiPdze1TCCwL+mF59nGOxYtncfKrouddVFax7Gcsz+crVbOVRREI0Tubts5 vV3yE8640V3CYRlQELJMmFfQQTczEty9SrunN7E3D3rGtjh2DtqxOyXcFHLCaUj06bmI n2K5LQbByQ+bIzM6sx24DqM+gWvxy+s7nteoahhh7v3RsKCxB7wfK3JM651TalbDEgCt iq+ySPOEI9A34HStVa3+jhYVticSF1HMoHESfANw0eGaYRCfMc6JZDSu2ulbuQq9aPiZ CEjBxmmj7CjShgC3jfj1T37OUIOlhGbi6AzfZNYGfNVtK/Ji6CXdTMJ7lej9vyZzHKB2 fE3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=KdrfFMceV7elb/rA2YCTui/ZhC4l1ygRRIMRPNusZhU=; b=O+RQNC8UHx/VCpLCnvLpoaCNiEetvTQNb3KUruw3lIrx2J4p7qKxRB3Ky6qV2TLaLB b3S+/AWm2DtGfD5890IsIcRQMXCzUt3D4VH4eIYPww2BuwBuRucFizho6t3kI622UhsL b0UpHauGm04mgNIJsvVHQjHbnU/Kl+MvnHrfnwj8dBxkJfbBP/rTt/v1JmUv2ntyLj00 c3zMSOY7j4STz/U8K7qgBXlC5B5b7e9veDgFAWJfVVQOxKloIxInguQPagnbzs9873ZW jAIyjGzcDxWErpb+3aFhaDvZ6/LNNIz5IuUmqBO7CjtTBewy5JI8iocbRgZtsjJsMmSL i8bw== X-Gm-Message-State: ACrzQf3BY8dFaLTQ3uPmHq/Vri/m5q7Eyj1WPj02ZNwqP5BjUPwvw1tG +8XYwxxn0xWLkGB1WQ5a7G4LHX8jLGxKg4tP4yCLKQ== X-Google-Smtp-Source: AMsMyM4UqG/vIk3S7koHuixUnBbVmb4fDf+woVq1uoMPm6jtVGqdAhA4fxEgS9gbQtKnvvq2Dv3MB4MMySXFLzb02qg= X-Received: by 2002:a17:90b:38c3:b0:20d:406e:26d9 with SMTP id nn3-20020a17090b38c300b0020d406e26d9mr4843007pjb.121.1665740490315; Fri, 14 Oct 2022 02:41:30 -0700 (PDT) MIME-Version: 1.0 References: <20220819174659.2427983-1-vannapurve@google.com> <20220819174659.2427983-7-vannapurve@google.com> In-Reply-To: From: Vishal Annapurve Date: Fri, 14 Oct 2022 15:11:19 +0530 Message-ID: Subject: Re: [RFC V3 PATCH 6/6] sefltests: kvm: x86: Add selftest for private memory To: Sean Christopherson Cc: x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, pbonzini@redhat.com, vkuznets@redhat.com, wanpengli@tencent.com, jmattson@google.com, joro@8bytes.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, shuah@kernel.org, yang.zhong@intel.com, drjones@redhat.com, ricarkol@google.com, aaronlewis@google.com, wei.w.wang@intel.com, kirill.shutemov@linux.intel.com, corbet@lwn.net, hughd@google.com, jlayton@kernel.org, bfields@fieldses.org, akpm@linux-foundation.org, chao.p.peng@linux.intel.com, yu.c.zhang@linux.intel.com, jun.nakajima@intel.com, dave.hansen@intel.com, michael.roth@amd.com, qperret@google.com, steven.price@arm.com, ak@linux.intel.com, david@redhat.com, luto@kernel.org, vbabka@suse.cz, marcorr@google.com, erdemaktas@google.com, pgonda@google.com, nikunj@amd.com, diviness@google.com, maz@kernel.org, dmatlack@google.com, axelrasmussen@google.com, maciej.szmigiero@oracle.com, mizhang@google.com, bgardon@google.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, Oct 7, 2022 at 1:54 AM Sean Christopherson wrote: > > On Fri, Aug 19, 2022, Vishal Annapurve wrote: > > +static bool verify_mem_contents(void *mem, uint32_t size, uint8_t pat) > > As per feedback in v1[*], spell out "pattern". > > [*] https://lore.kernel.org/all/YtiJx11AZHslcGnN@google.com > > > +{ > > + uint8_t *buf = (uint8_t *)mem; > > + > > + for (uint32_t i = 0; i < size; i++) { > > + if (buf[i] != pat) > > + return false; > > + } > > + > > + return true; > > +} > > + > > +/* > > + * Add custom implementation for memset to avoid using standard/builtin memset > > + * which may use features like SSE/GOT that don't work with guest vm execution > > + * within selftests. > > + */ > > +void *memset(void *mem, int byte, size_t size) > > +{ > > + uint8_t *buf = (uint8_t *)mem; > > + > > + for (uint32_t i = 0; i < size; i++) > > + buf[i] = byte; > > + > > + return buf; > > +} > > memset(), memcpy(), and memcmp() are safe to use as of commit 6b6f71484bf4 ("KVM: > selftests: Implement memcmp(), memcpy(), and memset() for guest use"). > This is much better. It made less sense to add a custom memset for a single selftest. > Note the "fun" with gcc "optimizing" into infinite recursion... :-) > > > + > > +static void populate_test_area(void *test_area_base, uint64_t pat) > > +{ > > + memset(test_area_base, pat, TEST_AREA_SIZE); > > +} > > + > > +static void populate_guest_test_mem(void *guest_test_mem, uint64_t pat) > > +{ > > + memset(guest_test_mem, pat, GUEST_TEST_MEM_SIZE); > > +} > > + > > +static bool verify_test_area(void *test_area_base, uint64_t area_pat, > > + uint64_t guest_pat) > > Again, avoid "pat". > > > +{ > > + void *test_area1_base = test_area_base; > > + uint64_t test_area1_size = GUEST_TEST_MEM_OFFSET; > > + void *guest_test_mem = test_area_base + test_area1_size; > > + uint64_t guest_test_size = GUEST_TEST_MEM_SIZE; > > + void *test_area2_base = guest_test_mem + guest_test_size; > > + uint64_t test_area2_size = (TEST_AREA_SIZE - (GUEST_TEST_MEM_OFFSET + > > + GUEST_TEST_MEM_SIZE)); > > This is all amazingly hard to read. AFAICT, the local variables are largely useless. > Actually, why even take in @test_area_base, isn't it hardcoded to TEST_AREA_GPA? > Then everything except the pattern can be hardcoded. > > > + return (verify_mem_contents(test_area1_base, test_area1_size, area_pat) && > > + verify_mem_contents(guest_test_mem, guest_test_size, guest_pat) && > > + verify_mem_contents(test_area2_base, test_area2_size, area_pat)); > > +} Ack. Will address these comments in the next series.