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=-3.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED 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 79324C4338F for ; Mon, 26 Jul 2021 23:54:22 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EF13B60F94 for ; Mon, 26 Jul 2021 23:54:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org EF13B60F94 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=shutemov.name Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 587488D0001; Mon, 26 Jul 2021 19:54:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 537D46B0036; Mon, 26 Jul 2021 19:54:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3FED48D0001; Mon, 26 Jul 2021 19:54:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0146.hostedemail.com [216.40.44.146]) by kanga.kvack.org (Postfix) with ESMTP id 21D306B0033 for ; Mon, 26 Jul 2021 19:54:21 -0400 (EDT) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 9CDDF8249980 for ; Mon, 26 Jul 2021 23:54:20 +0000 (UTC) X-FDA: 78406395480.06.EB3AEA4 Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) by imf05.hostedemail.com (Postfix) with ESMTP id 494DC5026DE7 for ; Mon, 26 Jul 2021 23:54:20 +0000 (UTC) Received: by mail-lf1-f51.google.com with SMTP id bp1so18615622lfb.3 for ; Mon, 26 Jul 2021 16:54:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shutemov-name.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=VMjZ1NiEINQ722sz2AvYEsOyg2jfEHnbAaMklK8Ptfk=; b=zonPvc/g9GVC1njP25RYe83HBENqz9bzp3czHEV24Wccan39b2QdG74JIpbgRf+Tbg lU72349t80EwtDCyovTnhozHfx1fg4C/WGkZmdltfSR8hDgdg6Z7xodH3Qmzf3pkbljY inQhLICfOlXg54G24QGuLdf+4rDUUlC3zjy5Jv/+2PgrqhNV46HUPtYlf2HxLtfwQgsz gAnDq7wTGZnx3D7KcS6RKND0REA/Qkywn6T3ACLNw6Ed2+vWG9rTwZLNflMlACq+2oC5 8AK+AsJYcaYcLq2Ot5IIZzpGIv2OMHCy4WNW1fyjuTMJ5tbvpVj+Dnu3RbD3y2iBLo9s om2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=VMjZ1NiEINQ722sz2AvYEsOyg2jfEHnbAaMklK8Ptfk=; b=JpAmXZd5AwoKIOnJgadatTndS4MWXZ9sL90VcfwqPqGC4LAYuwmWHoGONApF6vROLy +QT02rdjpLgw7SMoCw/WWA77vGFiPsif3nD51LFfdHfroe25yxHTE+u/ZBTiE+3sXrwn Ac+S9TUnYB3MQZSZVfm/vqDrvsnSBJr6MvIYQNztXwnMxZeoWA+eq2wmrZZ+f9lOo/on ET1TAlRJbhmmoB0VnhP1ZhMS3NCeAhrxAqr2NmVwsSl39bDm9Nl0+62Cu8Pq8msoeYDd CAsfyCHH7B41M+mW3YKrQYpMuKnEGXt/wkVbDQbMcOddl22S/AILhtoufHTyJU7kF3g/ wTZw== X-Gm-Message-State: AOAM532JXqSAQOIhDoOo1S4iuQWO8dJsk+daivO6IpQtG/pK6eSrmjYl OzqEAcsuPZ0oRmXhNZ5CoLKyTQ== X-Google-Smtp-Source: ABdhPJzPnJDZXAtNS611ZjKjr2hgziUksAB1ldetKqzrUGXa09V2wFzULChe9Z2x2Ot126UNAUnOVw== X-Received: by 2002:ac2:4c97:: with SMTP id d23mr14326530lfl.249.1627343658246; Mon, 26 Jul 2021 16:54:18 -0700 (PDT) Received: from box.localdomain ([86.57.175.117]) by smtp.gmail.com with ESMTPSA id br4sm133723lfb.33.2021.07.26.16.54.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Jul 2021 16:54:17 -0700 (PDT) Received: by box.localdomain (Postfix, from userid 1000) id 36495102968; Tue, 27 Jul 2021 02:54:17 +0300 (+03) Date: Tue, 27 Jul 2021 02:54:17 +0300 From: "Kirill A. Shutemov" To: Erdem Aktas Cc: Joerg Roedel , Andi Kleen , David Rientjes , Borislav Petkov , Andy Lutomirski , Sean Christopherson , Andrew Morton , Vlastimil Babka , "Kirill A. Shutemov" , Brijesh Singh , Tom Lendacky , Jon Grimm , Thomas Gleixner , Peter Zijlstra , Paolo Bonzini , Ingo Molnar , "Kaplan, David" , Varad Gautam , Dario Faggioli , x86 , linux-mm@kvack.org, linux-coco@lists.linux.dev Subject: Re: Runtime Memory Validation in Intel-TDX and AMD-SNP Message-ID: <20210726235417.fuxa5s32jta76lcv@box.shutemov.name> References: <20210722195130.beazbb5blvj3mruo@box> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 494DC5026DE7 Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=shutemov-name.20150623.gappssmtp.com header.s=20150623 header.b="zonPvc/g"; dmarc=none; spf=none (imf05.hostedemail.com: domain of kirill@shutemov.name has no SPF policy when checking 209.85.167.51) smtp.mailfrom=kirill@shutemov.name X-Stat-Signature: t4qejr4fanmmh3gnpuzdhi8uiaiiostx X-HE-Tag: 1627343660-910432 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 Mon, Jul 26, 2021 at 04:02:56PM -0700, Erdem Aktas wrote: > On Thu, Jul 22, 2021 at 12:51 PM Kirill A. Shutemov > wrote: > > +void mark_unaccepted(phys_addr_t start, phys_addr_t end) > > +{ > > + unsigned int npages; > > + > > + if (start & ~PMD_MASK) { > > + npages = (round_up(start, PMD_SIZE) - start) / PAGE_SIZE; > > + tdx_hcall_gpa_intent(start, npages, TDX_MAP_PRIVATE); > > + start = round_up(start, PMD_SIZE); > > + } > > + > > + if (end & ~PMD_MASK) { > > + npages = (end - round_down(end, PMD_SIZE)) / PAGE_SIZE; > > + end = round_down(end, PMD_SIZE); > > + tdx_hcall_gpa_intent(end, npages, TDX_MAP_PRIVATE); > > + } > > Is not the above code will accept the pages that are already accepted? No. This code will get called for all UNACCEPTED ranges in EFI table. If such memory is accepted it is a bug. > It is accepting the pages in the same 2MB region that is before start > and after end. We do not know what code/data is stored on those pages, > right? This might cause security issues depending on what is stored on > those pages. As I told above, it only get called for unaccepted memory and nothing can be stored there before the point. > > +static void __accept_pages(phys_addr_t start, phys_addr_t end) > > +{ > > + unsigned int rs, re; > > + > > + bitmap_for_each_set_region(unaccepted_memory, rs, re, > > + start / PMD_SIZE, end / PMD_SIZE) { > > + tdx_hcall_gpa_intent(rs * PMD_SIZE, (re - rs) * PMD_NR, > > + TDX_MAP_PRIVATE); > > + > This assumes that the granularity of the unaccepted pages is always in > PMD_SIZE. Yes, because we constructed the bitmap this way. Non-2M-aligned chunks get accepted when we accept upfront when we populate the bitmap. See mark_unaccepted(). (mark_unaccepted() has few bugs that will be fixed in the next version) > I have seen the answer above saying that mark_unaccepted > makes sure that we have only 2MB unaccepted pages in our bitmap but it > is not enough IMO. This function, as it is, will do double TDACCEPT > for the already accepted 4KB pages in the same 2MB region. > > +void maybe_set_page_offline(struct page *page, unsigned int order) > > +{ > > + phys_addr_t addr = page_to_phys(page); > > + bool unaccepted = true; > > + unsigned int i; > > + > > + spin_lock(&unaccepted_memory_lock); > > + if (order < PMD_ORDER) { > > + BUG_ON(test_bit(addr / PMD_SIZE, unaccepted_memory)); > > + goto out; > > + } > don't we need to throw a bug when order is < PMD_ORDER, independent of > what test_bit() is saying? If the page is accepted or not accepted, > there is a possibility of double accepting pages. No. maybe_set_page_offline() get called on all pages that get added to the free list on boot. Any pages with order < 9 has to belong to already accepted regions. > > + for (i = 0; i < (1 << (order - PMD_ORDER)); i++) { > > and if order < PMD_ORDER, this will be a wrong shift operation, right? order < PMD_ORDER handed by the 'if' above. > > + if (unaccepted) > > + __SetPageOffline(page); > > + else > > + __accept_pages(addr, addr + (PAGE_SIZE << order)); > so all the pages that were accepted will be reaccepted? Have you looked at what __accept_pages() does? It only accept unaccepted pages, according to the bitmap. -- Kirill A. Shutemov