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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 80E4EC433EF for ; Tue, 8 Feb 2022 21:09:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D8E5C6B0073; Tue, 8 Feb 2022 16:09:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D3E096B0075; Tue, 8 Feb 2022 16:09:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C05826B0078; Tue, 8 Feb 2022 16:09:33 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0097.hostedemail.com [216.40.44.97]) by kanga.kvack.org (Postfix) with ESMTP id B04566B0073 for ; Tue, 8 Feb 2022 16:09:33 -0500 (EST) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 6B76793D6A for ; Tue, 8 Feb 2022 21:09:33 +0000 (UTC) X-FDA: 79120853826.24.6D84CAE Received: from mail-yb1-f180.google.com (mail-yb1-f180.google.com [209.85.219.180]) by imf14.hostedemail.com (Postfix) with ESMTP id 00379100009 for ; Tue, 8 Feb 2022 21:09:32 +0000 (UTC) Received: by mail-yb1-f180.google.com with SMTP id p5so191916ybd.13 for ; Tue, 08 Feb 2022 13:09:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eclypsium.com; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Voz9w9zuaut5zzPtd6DrE9JLrle0cFN3rE5oW1AaY2M=; b=H86Fkha4NgU8OTWhaHjE4JlGaizgjjoCxAoxzH6KuMHph8fpqwf03RaVnEuLk2P/bp yY61kdvApYE6qxRpAaZcrq1arCGYhyP64OfxKZGk/ZOoOi/ltDcqSmSvBtCoQmeyEyLR FYKxeVXO0j/kENA0AQ+2vCrGOl0VUKu3hWvpH/6m/A6hDV0bT0QlgbAqfNsNBafihzt3 2DJiAdstGP57WhN+4kYnrr97E23PJoqPkDXeN4LhN/1bMFwtonqt5k3vmcslxFSVGhxw 1hFhSiwDhUci9XkHK8tdvuB1zULSZaI7zSssvL59P/voaJjScrGEK282mnejgUsEtNCb E5kA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Voz9w9zuaut5zzPtd6DrE9JLrle0cFN3rE5oW1AaY2M=; b=V9OQNej4arp507JwLZ09i0uKM7pcOXdn5fnYlY06MSngLrl2RcYPzJhK1GymNNf9qx vNe+BTG15oLLX6G9Hk7M65DHJVNnSFtkFztn45FOlwxWkELg36Lh53QgYUn+wjjDSd9/ C+y92FmbBPlorSWjRDyUSeYVP01uarqeBLUqOtBANR6SPDQz1N4M57UWv6EQiJQi1qJp LoWyH2E+SmwArD6Vg+cyptKsnuu2oP5Gi3lnmCVMw3ccmg85vZGpct0/W3RoW/mOGvX3 bgISZyfoBdj8YlD0lcjhfA77kVk6t91tal/Hp3FKykrL07T6T328DzQn1vC5EmpT236u 05lg== X-Gm-Message-State: AOAM533Cvk1JP2E33+QG7YOxqFt1TD/Ry5Bv4IX5IraG9CHGw/IrV2sv DKM78BhgcaqfF034h+HU945mVgqTtCSFqR6L0dZO4w== X-Google-Smtp-Source: ABdhPJwq0oxAvAIPreUivsgDVcSG53+v8LdcVF+reGL+9mzDGy2lOfjCUp4XCtrNh7PS7fr/4/oEr1Bunn9utBnAgNA= X-Received: by 2002:a25:9988:: with SMTP id p8mr6404360ybo.128.1644354572175; Tue, 08 Feb 2022 13:09:32 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a81:174d:0:0:0:0:0 with HTTP; Tue, 8 Feb 2022 13:09:31 -0800 (PST) In-Reply-To: <202202071325.F8450B3B2D@keescook> References: <20220203164328.203629-1-martin.fernandez@eclypsium.com> <20220203164328.203629-4-martin.fernandez@eclypsium.com> <202202071325.F8450B3B2D@keescook> From: Martin Fernandez Date: Tue, 8 Feb 2022 18:09:31 -0300 Message-ID: Subject: Re: [PATCH v6 3/6] x86/e820: Refactor range_update and range_remove To: Kees Cook Cc: linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-mm@kvack.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, ardb@kernel.org, dvhart@infradead.org, andy@infradead.org, gregkh@linuxfoundation.org, rafael@kernel.org, rppt@kernel.org, akpm@linux-foundation.org, daniel.gutson@eclypsium.com, hughsient@gmail.com, alex.bazhaniuk@eclypsium.com, alison.schofield@intel.com Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=eclypsium.com header.s=google header.b=H86Fkha4; spf=pass (imf14.hostedemail.com: domain of martin.fernandez@eclypsium.com designates 209.85.219.180 as permitted sender) smtp.mailfrom=martin.fernandez@eclypsium.com; dmarc=pass (policy=quarantine) header.from=eclypsium.com X-Stat-Signature: swghcutwtm6kiw44wn9otgmos6ckyimb X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 00379100009 X-Rspam-User: X-HE-Tag: 1644354572-219863 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2/7/22, Kees Cook wrote: > On Thu, Feb 03, 2022 at 01:43:25PM -0300, Martin Fernandez wrote: >> __e820__range_update and e820__range_remove had a very similar >> I propose a refactor of those functions, given that I need to create a >> similar one for this patchset. > > The diff here is pretty hard (for me) to review; I'll need more time > to check it. What might make review easier (at least for me), is to > incrementally change these routines. i.e. separate patches to: > > - add the new infrastructure > - replace e820__range_remove > - replace __e820__range_update > > If that's not actually useful, no worries. I'll just stare at it a bit > more. :) > Yep, that's a good idea. I'll keep that in mind for the next patch. >> >> Add a function to modify a E820 table in a given range. This >> modification is done backed up by two helper structs: >> e820_entry_updater and e820_*_data. >> >> The first one, e820_entry_updater, carries 3 callbacks which function >> as the actions to take on the table. >> >> The other one, e820_*_data carries information needed by the >> callbacks, for example in the case of range_update it will carry the >> type that we are targeting. > > Something I think would be really amazing here is if you could add KUnit > tests here to exercise the corner cases and validate the changes. It > should be pretty easy to add. Here's a quick example for the boilerplate > and testing a bit of __e820__range_add(): > > #ifdef CONFIG_E820_KUNIT_TEST > #include > > static void __init test_e820_range_add(struct kunit *context) > { > struct e820_table table; > u32 full; > > full = ARRAY_SIZE(table.entries); > /* Add last entry. */ > table->nr_entries = full - 1; > __e820__range_add(&table, 0, 15, 0); > KUNIT_EXPECT_EQ(table->nr_entries, full) > /* Skip new entry when full. */ > __e820__range_add(&table, 0, 15, 0); > KUNIT_EXPECT_EQ(table->nr_entries, full) > } > > static void __init test_e820_update(struct kunit *context) > { > ... > } > > static struct kunit_case __refdata e820_test_cases[] = { > KUNIT_CASE(test_e820_range_add), > KUNIT_CASE(test_e820_update), > ... > {} > }; > > static struct kunit_suite e820_test_suite = { > .name = "e820", > .test_cases = e820_test_cases, > }; > > kunit_test_suites(&e820_test_suite); > #endif > Oh that's awesome! I'll definitely take a look into KUnit and integrate it to this patch. Thanks for the code snippet!