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 X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28D08C11F68 for ; Wed, 14 Jul 2021 18:14:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B2470613BA for ; Wed, 14 Jul 2021 18:14:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B2470613BA Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id DD6BC6B0078; Wed, 14 Jul 2021 14:14:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D86DE6B007E; Wed, 14 Jul 2021 14:14:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C27026B0080; Wed, 14 Jul 2021 14:14:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0162.hostedemail.com [216.40.44.162]) by kanga.kvack.org (Postfix) with ESMTP id 9C9116B0078 for ; Wed, 14 Jul 2021 14:14:55 -0400 (EDT) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 7CD7718585199 for ; Wed, 14 Jul 2021 18:14:54 +0000 (UTC) X-FDA: 78361994508.25.F5F4A89 Received: from mail-qk1-f176.google.com (mail-qk1-f176.google.com [209.85.222.176]) by imf12.hostedemail.com (Postfix) with ESMTP id 3A23210000A9 for ; Wed, 14 Jul 2021 18:14:54 +0000 (UTC) Received: by mail-qk1-f176.google.com with SMTP id p202so2488085qka.12 for ; Wed, 14 Jul 2021 11:14:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3WKLZGhjXG90Fatu5wm7XoigXrQeEPeI0ZySxgtrf1E=; b=K34aiEbq6IThsKWJBJDDGsckcOYXonTslRsCf6xEi7ghhFV+UbwNIfzQP6Tq5ldAxz x/gfvgeDt8i9UmUFByP8JylSs7uppp+LheXconENxXR/wPDkP+0iUfG5YhfwPEmQU1fy 4tTjvgWpmY3o7IvTFaQR7Z34VuSgQbU+YmK0iN4AE41bZtFsm7nGfsIz6oRpkXpW6axf 6b+9SsmHKDPDXxNXQY2L0ih10uAe1HpuGUVydXvTdbXoxG4rjZrIfD2Nskrohj6TdZDI j2Q+YXsv7xUpGHDh1ZKrJ93dEG0SbTKyIkbyJdcn2EcTIo/QYOSql+60mVB0ixujp++K tFqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3WKLZGhjXG90Fatu5wm7XoigXrQeEPeI0ZySxgtrf1E=; b=L+LXDFUnhZX6xue/UCjv3uEuuG9uKYGgiKY7xzWCsB1EmGwVjb+CbN3Yp4oWA9chn2 tTDnL5u42afUOaOiQa45Jv28Y+m1J2pNB6TXjYJvq0NpvPgvJftfU4fn9FqV0HK6tuiL vmQiKKH6sR6VJezjzV5rm4WWtE4aoAqIAb6KMBDi/5Gb2X8L0sfqEB4v0cZZBj5im/X4 cFod6V8KUY6A5zdX+vtTbyf+AVrRwOm0EtyWguf48wiu4a3lLkaVr3pnn+7biqJgHJIw dn5MpcIZRCjw7dWa8vxg/Cb/zLHev0elT+8TZlgeuDUwfze4xZlUxtOawEC/n/tcNMei 3rPA== X-Gm-Message-State: AOAM532Q6uFdxeYnFBsuCdjZ563/oGexJuN5i7l0E6sp2YnBf1gw35Fd m1THGAop3mVYBhW/uJQhtP2OOmoECjenMG3nA7azpQ== X-Google-Smtp-Source: ABdhPJxM1wp8nSe3bkx8dg/pQpdxw78kl5esOD57NTfXsDoAxivKWcg01aZjKeev4L+fz4X+d7136TERrs+FRe6NY50= X-Received: by 2002:a05:620a:a90:: with SMTP id v16mr11085310qkg.150.1626286493309; Wed, 14 Jul 2021 11:14:53 -0700 (PDT) MIME-Version: 1.0 References: <20210707183616.5620-1-brijesh.singh@amd.com> <20210707183616.5620-16-brijesh.singh@amd.com> <98ac737d-83a8-6ee8-feac-554bab673191@amd.com> In-Reply-To: <98ac737d-83a8-6ee8-feac-554bab673191@amd.com> From: Marc Orr Date: Wed, 14 Jul 2021 11:14:41 -0700 Message-ID: Subject: Re: [PATCH Part2 RFC v4 15/40] crypto: ccp: Handle the legacy TMR allocation when SNP is enabled To: Brijesh Singh Cc: x86@kernel.org, linux-kernel@vger.kernel.org, kvm list , linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, Thomas Gleixner , Ingo Molnar , Joerg Roedel , Tom Lendacky , "H. Peter Anvin" , Ard Biesheuvel , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Andy Lutomirski , Dave Hansen , Sergio Lopez , Peter Gonda , Peter Zijlstra , Srinivas Pandruvada , David Rientjes , Dov Murik , Tobin Feldman-Fitzthum , Borislav Petkov , Michael Roth , Vlastimil Babka , tony.luck@intel.com, npmccallum@redhat.com, brijesh.ksingh@gmail.com, Alper Gun Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=K34aiEbq; spf=pass (imf12.hostedemail.com: domain of marcorr@google.com designates 209.85.222.176 as permitted sender) smtp.mailfrom=marcorr@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam05 X-Stat-Signature: wribetaru45yuq8woakoshm16fdz4z9p X-Rspamd-Queue-Id: 3A23210000A9 X-HE-Tag: 1626286494-924784 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: > > Should this return a non-zero value -- maybe `-ENODEV`? Otherwise, the > > `snp_alloc_firmware_page()` API will return a page that the caller > > believes is suitable to use with FW. My concern is that someone > > decides to use this API to stash a page very early on during kernel > > boot and that page becomes a time bomb. > > But that means the caller now need to know that SNP is enabled before > calling the APIs. The idea behind the API was that caller does not need > to know whether the firmware is in the INIT state. If the firmware has > initialized the SNP, then it will transparently set the immutable bit in > the RMP table. For SNP, isn't that already the case? There are three scenarios: #1: The PSP driver is loaded and `snp_inited` is `true`: These returns are never hit. #2: The PSP driver is not loaded. The first return, `!psp || !psp->sev_data` fires. As written, it returns `0`, indicating success. However, we never called RMPUPDATE on the page. Thus, later, when the PSP driver is loaded, the page that was previously returned as usable with FW is in fact not usable with FW. Unless SNP is disabled (e.g., SEV, SEV-ES only). In which case I guess the page is OK. #3 The PSP driver is loaded but the SNP_INIT command has not been issued. Looking at this again, I guess `return 0` is OK. Because if we got this far, then `sev_pci_init()` has been called, and the SNP_INIT command has been issued if we're supporting SNP VMs. So in summary, I think we should change the first return to return an error and leave the 2nd return as is. > > If we initialize `rc` to `-ENODEV` (or something similar), then every > > return in this function can be `return rc`. > > > >> + > >> + /* If SEV-SNP is initialized then add the page in RMP table. */ > >> + sev = psp->sev_data; > >> + if (!sev->snp_inited) > >> + return 0; > > > > Ditto. Should this turn a non-zero value? > > > >> + > >> + while (pfn < pfn_end) { > >> + if (need_reclaim) > >> + if (snp_reclaim_page(pfn_to_page(pfn), locked)) > >> + return -EFAULT; > >> + > >> + rc = rmpupdate(pfn_to_page(pfn), val); > >> + if (rc) > >> + return rc; > >> + > >> + pfn++; > >> + } > >> + > >> + return 0; > >> +}