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 AF90EC433EF for ; Fri, 4 Mar 2022 20:32:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E24378D0003; Fri, 4 Mar 2022 15:32:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DA87C8D0001; Fri, 4 Mar 2022 15:32:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BFB8B8D0003; Fri, 4 Mar 2022 15:32:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0153.hostedemail.com [216.40.44.153]) by kanga.kvack.org (Postfix) with ESMTP id AE35C8D0001 for ; Fri, 4 Mar 2022 15:32:09 -0500 (EST) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 6C082181C9B8D for ; Fri, 4 Mar 2022 20:32:09 +0000 (UTC) X-FDA: 79207850778.26.C134CEB Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com [209.85.219.169]) by imf09.hostedemail.com (Postfix) with ESMTP id C803814001F for ; Fri, 4 Mar 2022 20:32:08 +0000 (UTC) Received: by mail-yb1-f169.google.com with SMTP id t7so15612776ybi.8 for ; Fri, 04 Mar 2022 12:32:08 -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=5Va0AzpfFrj2CIxQZYECbB+NvfeYwGm1BAKxA+n6QB4=; b=aqiySkX9xXwyx0pXK5VRBPZs+THKKNzuz8P4dXhtgCvyYD83k10XEbPITizjiFC5+1 +uJJ/WstvMVQTxU+LxuP8XvWADlDCdusBlVQa6BMMZVrW+OvyLaQ8uvd5/vC5a3PIP7E 7y8BB4T4piZJGQnj00J7i5/x2Tvr9Q2Vswp09xESARUUFkWHsHOeWImu8+bTyTXXx/eQ bg48wFjeuxt3dXnyUZI9XGuyC+9UU7Mxo4DmQDp55ReOUMP1xATcFBsHN4NlsMWvv7SO XFJtejMlR6DmbxBbJo6wJuIsEUUP9J9LGUSnI+XbbgA5eFTL5pRLbzkWbAK92pr+ZBX7 OlDg== 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=5Va0AzpfFrj2CIxQZYECbB+NvfeYwGm1BAKxA+n6QB4=; b=fF2UVS1cwXkMqXvE960fXu6O0aCxTv2qf0Emy4E3/lPLXlpBO1l79blDDS6CHniAPp x/lwnq92bsi3+L933+QaJjgQu5C15U4DXCtilM/ea7dXNgkMGRmUObROGHIenxLFhA7j EeCgDfV3RF3YDOHKwk/rKnCinnVRVLVtTJrq9pZEkN0H00fVrY3Ms0EhQHTVGObVIvXF QJWhk2fnYQmUKRhKPgPEUGON2AAmsXMzsgMQvqVDdm4ql6Hh1xCS3ZD4poZnez3xIIsU 2DeOh8CHolEViru77iRn4O3gQ7joea4QRcGUsFMZGS+T/9dnqJNlDJhfWphVkSz1Caw2 T1mA== X-Gm-Message-State: AOAM5331pgKRC+ItxPqM3X8U9krgJlH2H5QKecp41LGC9gHuzPAOdJUh F/Ro5FVM1K86wE/v+1vTP87VsvOLmima1zm0wOnq1w== X-Google-Smtp-Source: ABdhPJzxs23o8/HFTUjwCIU9Csa6E4rXqxc9gfNR5DpVCPks40rysm/OfOjtL91itmtSRi55jiNUior2jV/kxeE11DA= X-Received: by 2002:a25:c487:0:b0:61d:9570:e77f with SMTP id u129-20020a25c487000000b0061d9570e77fmr202540ybf.229.1646425927962; Fri, 04 Mar 2022 12:32:07 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a0d:df06:0:0:0:0:0 with HTTP; Fri, 4 Mar 2022 12:32:07 -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: Fri, 4 Mar 2022 17:32:07 -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" X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: C803814001F X-Stat-Signature: zkdwxfiqjo7iqgkdujqbht4diinead75 Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=eclypsium.com header.s=google header.b=aqiySkX9; spf=pass (imf09.hostedemail.com: domain of martin.fernandez@eclypsium.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=martin.fernandez@eclypsium.com; dmarc=pass (policy=quarantine) header.from=eclypsium.com X-HE-Tag: 1646425928-531592 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: > 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 I almost got it. Although when added the tests I have a warning when compiling, because KUnit doens't want to deal with __init things: WARNING: modpost: vmlinux.o(.data+0x26800): Section mismatch in reference from the variable __UNIQUE_ID_array286 to the variable .init.data:e820_test_suite The variable __UNIQUE_ID_array286 references the variable __initdata e820_test_suite If the reference is valid then annotate the variable with __init* or __refdata (see linux/init.h) or name the variable: *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console I need to test __init functions. I couldn't find any other similar cases in existant code. Is there a nice way to solve this? I'm adding the file that contains the tests just in case.. #include #include #include #define KUNIT_EXPECT_E820_ENTRY_EQ(test, entry, _addr, _size, _type, \ _crypto_capable) \ do { \ KUNIT_EXPECT_EQ((test), (entry).addr, (_addr)); \ KUNIT_EXPECT_EQ((test), (entry).size, (_size)); \ KUNIT_EXPECT_EQ((test), (entry).type, (_type)); \ KUNIT_EXPECT_EQ((test), (entry).crypto_capable, \ (_crypto_capable)); \ } while (0) struct e820_table test_table __initdata; static void __init test_e820_range_add(struct kunit *test) { u32 full; full = ARRAY_SIZE(test_table.entries); /* Add last entry. */ test_table.nr_entries = full - 1; __e820__range_add(&test_table, 0, 15, 0, 0); KUNIT_EXPECT_EQ(test, test_table.nr_entries, full); /* Skip new entry when full. */ __e820__range_add(&test_table, 0, 15, 0, 0); KUNIT_EXPECT_EQ(test, test_table.nr_entries, full); } static void __init test_e820_range_update(struct kunit *test) { u64 entry_size = 15; u64 updated_size = 0; /* Initialize table */ test_table.nr_entries = 0; __e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); __e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); __e820__range_add(&test_table, entry_size * 2, entry_size, E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE); updated_size = __e820__range_update(&test_table, 0, entry_size * 2, E820_TYPE_RAM, E820_TYPE_RESERVED); /* The first 2 regions were updated */ KUNIT_EXPECT_EQ(test, updated_size, entry_size * 2); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size, E820_TYPE_RESERVED, E820_NOT_CRYPTO_CAPABLE); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size, entry_size, E820_TYPE_RESERVED, E820_NOT_CRYPTO_CAPABLE); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2, entry_size, E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE); updated_size = __e820__range_update(&test_table, 0, entry_size * 3, E820_TYPE_RESERVED, E820_TYPE_RAM); /* Only the first 2 regions were updated */ KUNIT_EXPECT_EQ(test, updated_size, entry_size * 2); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2, entry_size, E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE); } static void __init test_e820_range_remove(struct kunit *test) { u64 entry_size = 15; u64 removed_size = 0; struct e820_entry_updater updater = { .should_update = remover__should_update, .update = remover__update, .new = remover__new }; struct e820_remover_data data = { .check_type = true, .old_type = E820_TYPE_RAM }; /* Initialize table */ test_table.nr_entries = 0; __e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); __e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); __e820__range_add(&test_table, entry_size * 2, entry_size, E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE); /* * Need to use __e820__handle_range_update because * e820__range_remove doesn't ask for the table */ removed_size = __e820__handle_range_update(&test_table, 0, entry_size * 2, &updater, &data); /* The first two regions were removed */ KUNIT_EXPECT_EQ(test, removed_size, entry_size * 2); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 0, 0, 0); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 0, 0, 0); removed_size = __e820__handle_range_update(&test_table, 0, entry_size * 3, &updater, &data); /* Nothing was removed */ KUNIT_EXPECT_EQ(test, removed_size, 0); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 0, 0, 0); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 0, 0, 0); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2, entry_size, E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE); } static void __init test_e820_range_crypto_update(struct kunit *test) { u64 entry_size = 15; u64 updated_size = 0; /* Initialize table */ test_table.nr_entries = 0; __e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM, E820_CRYPTO_CAPABLE); __e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); __e820__range_add(&test_table, entry_size * 2, entry_size, E820_TYPE_RAM, E820_CRYPTO_CAPABLE); updated_size = __e820__range_update_crypto(&test_table, 0, entry_size * 3, E820_CRYPTO_CAPABLE); /* Only the region in the middle was updated */ KUNIT_EXPECT_EQ(test, updated_size, entry_size); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size, E820_TYPE_RAM, E820_CRYPTO_CAPABLE); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size, entry_size, E820_TYPE_RAM, E820_CRYPTO_CAPABLE); KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2, entry_size, E820_TYPE_RAM, E820_CRYPTO_CAPABLE); } static void __init test_e820_handle_range_update_intersection(struct kunit *test) { struct e820_entry_updater updater = { .should_update = type_updater__should_update, .update = type_updater__update, .new = type_updater__new }; struct e820_type_updater_data data = { .old_type = E820_TYPE_RAM, .new_type = E820_TYPE_RESERVED }; u64 entry_size = 15; u64 updated_size = 0; /* Initialize table */ test_table.nr_entries = 0; __e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); updated_size = __e820__handle_range_update(&test_table, 0, entry_size - 2, &updater, &data); KUNIT_EXPECT_EQ(test, updated_size, entry_size - 2); /* There is a new entry */ KUNIT_EXPECT_EQ(test, test_table.nr_entries, 2); /* The original entry now is moved */ KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], entry_size - 2, 2, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); /* The new entry has the correct values */ KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 13, E820_TYPE_RESERVED, E820_NOT_CRYPTO_CAPABLE); } static void __init test_e820_handle_range_update_inside(struct kunit *test) { struct e820_entry_updater updater = { .should_update = type_updater__should_update, .update = type_updater__update, .new = type_updater__new }; struct e820_type_updater_data data = { .old_type = E820_TYPE_RAM, .new_type = E820_TYPE_RESERVED }; u64 entry_size = 15; u64 updated_size = 0; /* Initialize table */ test_table.nr_entries = 0; __e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); updated_size = __e820__handle_range_update(&test_table, 5, entry_size - 10, &updater, &data); KUNIT_EXPECT_EQ(test, updated_size, entry_size - 10); /* There are two new entrie */ KUNIT_EXPECT_EQ(test, test_table.nr_entries, 3); /* The original entry now shrinked */ KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 5, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); /* The new entries have the correct values */ KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 5, entry_size - 10, E820_TYPE_RESERVED, E820_NOT_CRYPTO_CAPABLE); /* Left over of the original region */ KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size - 5, 5, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE); } static struct kunit_case e820_test_cases[] __initdata = { KUNIT_CASE(test_e820_range_add), KUNIT_CASE(test_e820_range_update), KUNIT_CASE(test_e820_range_remove), KUNIT_CASE(test_e820_range_crypto_update), KUNIT_CASE(test_e820_handle_range_update_intersection), KUNIT_CASE(test_e820_handle_range_update_inside), {} }; static struct kunit_suite e820_test_suite __initdata = { .name = "e820", .test_cases = e820_test_cases, }; kunit_test_suite(e820_test_suite);