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 52142C433FE for ; Wed, 22 Dec 2021 15:35:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 97D7B6B0071; Wed, 22 Dec 2021 10:35:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 92CA06B0073; Wed, 22 Dec 2021 10:35:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7CCEE6B0074; Wed, 22 Dec 2021 10:35:27 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0177.hostedemail.com [216.40.44.177]) by kanga.kvack.org (Postfix) with ESMTP id 6AC156B0071 for ; Wed, 22 Dec 2021 10:35:27 -0500 (EST) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 0156D89063 for ; Wed, 22 Dec 2021 15:35:27 +0000 (UTC) X-FDA: 78945829494.13.5B44F3A Received: from mail-yb1-f172.google.com (mail-yb1-f172.google.com [209.85.219.172]) by imf16.hostedemail.com (Postfix) with ESMTP id 8E20D180044 for ; Wed, 22 Dec 2021 15:35:26 +0000 (UTC) Received: by mail-yb1-f172.google.com with SMTP id x32so7716471ybi.12 for ; Wed, 22 Dec 2021 07:35:26 -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=cwnIm+Pjf1RTUBBb5BRinhHvVqdyboPG2JJubsE04f4=; b=AG9Ug8PUwm0BhJU9O/vGcCjhoBDHg4gQq1bWPDSXIgj2KDi7CkFNOOv/u74Em02wkD os3Dou1fSrFsMcV35l/mRKBXXmx4l863Jip4zBJa8a1TLFmnPbO9Jd0vgmU4/ml+SJp0 AbJnXpaUOdNLQG66dPGZNaDsXdpZLZUvB9A0B+yi6pz2BuCXZlzfZ8wO435oxxb1nos/ Dx2yzjDENAgI2RyCbAgtcM/VZf8vakzzP+L9t5KekKiQzZ51av9j06vf6w1T/JHM5hvF Zp2YTI+rfHAO5KSW55ReosawsBPaxV2ZuuMmlvYvVHoHFiD3BN6TNjHpq7MrA88jUiOJ sX0g== 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=cwnIm+Pjf1RTUBBb5BRinhHvVqdyboPG2JJubsE04f4=; b=x8UhNBy7wNNn7iW9Xhaevqu+1m85jtk5JlKyBpWKcwd44YKwkUk2EwjMIDXo/JrmL7 pG0qsZ6YMipvmNgUUqdCVTMcjwj62vIhqI8vmnoGVgPXd63BctCBkXVErwM85eIdOwFY sIM9bS5zQ1bkIVwnucSijNPmkuPkXUfthZyMAb6daPL4J7/oJd/pcFVX82xVLHZNKJ5P xZzSw4hUFpFSbTtpZBmaR9Sk1DCs5JsfbW/oNWmWlAZ1I6k+OJ7Q7IApVJMS+tYfQNsq xO3evB0nPBoZN4U8dhcJKD8VWC2VTfNDeLElWQug42fO6zrxF+gMbwmGuvC8lJZa7yvj 6Qtw== X-Gm-Message-State: AOAM530mfjEsJQ5ozWp0gJyaUDxiVj/UJk8fS7uj4wKD/CR8mpekaQfX 6NsbIJ6Dyx0TtgPIqnHfpezfWKKvLLC1SR2qmnpg7g== X-Google-Smtp-Source: ABdhPJw7mITeuoygQFK1MhJbMGiGJKKDtAs86FI3UP7IrYs5kVS/hvOy/t4bTtqxhCrOxSYegmcjo3RH5D7EXfQqA5A= X-Received: by 2002:a25:3c07:: with SMTP id j7mr5487832yba.612.1640187325665; Wed, 22 Dec 2021 07:35:25 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a81:58c2:0:0:0:0:0 with HTTP; Wed, 22 Dec 2021 07:35:25 -0800 (PST) In-Reply-To: References: <20211216192222.127908-1-martin.fernandez@eclypsium.com> <20211216192222.127908-4-martin.fernandez@eclypsium.com> From: Martin Fernandez Date: Wed, 22 Dec 2021 12:35:25 -0300 Message-ID: Subject: Re: [PATCH v4 3/5] x86/e820: Tag e820_entry with crypto capabilities To: Greg KH 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, 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-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 8E20D180044 X-Stat-Signature: w8jzgxoe6d9uo8uwnpshizgonnoeyoie Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=eclypsium.com header.s=google header.b=AG9Ug8PU; dmarc=pass (policy=quarantine) header.from=eclypsium.com; spf=pass (imf16.hostedemail.com: domain of martin.fernandez@eclypsium.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=martin.fernandez@eclypsium.com X-HE-Tag: 1640187326-456425 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 12/21/21, Greg KH wrote: > On Mon, Dec 20, 2021 at 05:27:00PM -0300, Martin Fernandez wrote: >> On 12/20/21, Greg KH wrote: >> > On Thu, Dec 16, 2021 at 04:22:20PM -0300, Martin Fernandez wrote: >> >> diff --git a/arch/x86/include/asm/e820/types.h >> >> b/arch/x86/include/asm/e820/types.h >> >> index 314f75d886d0..7b510dffd3b9 100644 >> >> --- a/arch/x86/include/asm/e820/types.h >> >> +++ b/arch/x86/include/asm/e820/types.h >> >> @@ -56,6 +56,7 @@ struct e820_entry { >> >> u64 addr; >> >> u64 size; >> >> enum e820_type type; >> >> + u8 crypto_capable; >> > >> > Why isn't this a bool? >> >> It was a bool initially, but Andy Shevchenko told me that it couldn't >> be that way because boolean may not be part of firmware ABIs. > > Where does this structure hit an "ABI"? Looks internal to me. If not, > then something just broke anyway. > I prefer that Andy answers. Either way, I think that the enum will be the best option. >> >> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c >> >> index bc0657f0deed..001d64686938 100644 >> >> --- a/arch/x86/kernel/e820.c >> >> +++ b/arch/x86/kernel/e820.c >> >> @@ -163,7 +163,7 @@ int e820__get_entry_type(u64 start, u64 end) >> >> /* >> >> * Add a memory region to the kernel E820 map. >> >> */ >> >> -static void __init __e820__range_add(struct e820_table *table, u64 >> >> start, >> >> u64 size, enum e820_type type) >> >> +static void __init __e820__range_add(struct e820_table *table, u64 >> >> start, >> >> u64 size, enum e820_type type, u8 crypto_capable) >> > >> > Horrid api change, but it's internal to this file so oh well :( >> > >> > Hint, don't add flags to functions like this, it forces you to have to >> > always remember what those flags are when you read the code. Right now >> > you stuck "0" and "1" in the function call, which is not instructional >> > at all. >> > >> > Heck, why not make it an enum to have it be self-describing? Like the >> > type is here. that would make it much better and easier to understand >> > and maintain over time. >> > >> >> Yes, an enum will absolutely improve things. I'll do that. >> >> >> @@ -327,6 +330,7 @@ int __init e820__update_table(struct e820_table >> >> *table) >> >> unsigned long long last_addr; >> >> u32 new_nr_entries, overlap_entries; >> >> u32 i, chg_idx, chg_nr; >> >> + u8 current_crypto, last_crypto; >> >> >> >> /* If there's only one memory region, don't bother: */ >> >> if (table->nr_entries < 2) >> >> @@ -367,6 +371,7 @@ int __init e820__update_table(struct e820_table >> >> *table) >> >> new_nr_entries = 0; /* Index for creating new map entries */ >> >> last_type = 0; /* Start with undefined memory type */ >> >> last_addr = 0; /* Start with 0 as last starting address */ >> >> + last_crypto = 0; >> >> >> >> /* Loop through change-points, determining effect on the new map: */ >> >> for (chg_idx = 0; chg_idx < chg_nr; chg_idx++) { >> >> @@ -388,13 +393,17 @@ int __init e820__update_table(struct e820_table >> >> *table) >> >> * 1=usable, 2,3,4,4+=unusable) >> >> */ >> >> current_type = 0; >> >> + current_crypto = 1; >> >> for (i = 0; i < overlap_entries; i++) { >> >> + current_crypto = current_crypto && >> >> overlap_list[i]->crypto_capable; >> > >> > Is it a u8 or not? You treat it as a boolean a lot :( >> > >> >> if (overlap_list[i]->type > current_type) >> >> current_type = overlap_list[i]->type; >> >> } >> >> >> >> /* Continue building up new map based on this information: */ >> >> - if (current_type != last_type || e820_nomerge(current_type)) { >> >> + if (current_type != last_type || >> >> + current_crypto != last_crypto || >> >> + e820_nomerge(current_type)) { >> > >> > Why check it before calling e820_nomerge()? Is that required? >> > >> >> I don't see how the order of the checks matter, am I missing something? > > It might prevent this function from being called now when it previously > was. Is that ok? > Oh I see. No, that's not a problem. That if guard is to decide if you need to start/close a region. e820_nomerge is to prevent merging certain region types. In any case, the new check will cause less merging, complying with what e820_nomerge says.