Received-SPF: pass (google.com: domain of linux-kernel+bounces-18438-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sat, 6 Jan 2024 03:59:01 +0300
From: "kirill.shutemov@linux.intel.com" <kirill.shutemov@linux.intel.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc: "tglx@linutronix.de" <tglx@linutronix.de>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"x86@kernel.org" <x86@kernel.org>, "bp@alien8.de" <bp@alien8.de>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"kexec@lists.infradead.org" <kexec@lists.infradead.org>,
	"Reshetova, Elena" <elena.reshetova@intel.com>,
	"Nakajima, Jun" <jun.nakajima@intel.com>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"Huang, Kai" <kai.huang@intel.com>,
	"sathyanarayanan.kuppuswamy@linux.intel.com" <sathyanarayanan.kuppuswamy@linux.intel.com>,
	"Hunter, Adrian" <adrian.hunter@intel.com>,
	"thomas.lendacky@amd.com" <thomas.lendacky@amd.com>,
	"ashish.kalra@amd.com" <ashish.kalra@amd.com>,
	"linux-coco@lists.linux.dev" <linux-coco@lists.linux.dev>,
	"seanjc@google.com" <seanjc@google.com>,
	"bhe@redhat.com" <bhe@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCHv5 10/16] x86/tdx: Convert shared memory back to private
 on kexec
Message-ID: <20240106005901.jpdxd544ddih6nw7@box.shutemov.name>
References: <20231222235209.32143-1-kirill.shutemov@linux.intel.com>
 <20231222235209.32143-11-kirill.shutemov@linux.intel.com>
 <063a42bfe538710931fca183a8d28c8a3d011608.camel@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <063a42bfe538710931fca183a8d28c8a3d011608.camel@intel.com>

On Fri, Jan 05, 2024 at 07:38:48PM +0000, Edgecombe, Rick P wrote:
> The break apart looks good.
> 
> On Sat, 2023-12-23 at 02:52 +0300, Kirill A. Shutemov wrote:
> > +???????while (addr < end) {
> > +???????????????unsigned long size;
> > +???????????????unsigned int level;
> > +???????????????pte_t *pte;
> > +
> > +???????????????pte = lookup_address(addr, &level);
> > +???????????????size = page_level_size(level);
> > +
> > +???????????????if (pte && pte_decrypted(*pte)) {
> > +???????????????????????int pages = size / PAGE_SIZE;
> > +
> > +???????????????????????/*
> > +??????????????????????? * Touching memory with shared bit set
> > triggers implicit
> > +??????????????????????? * conversion to shared.
> > +??????????????????????? *
> > +??????????????????????? * Make sure nobody touches the shared range
> > from
> > +??????????????????????? * now on.
> > +??????????????????????? */
> > +???????????????????????set_pte(pte, __pte(0));
> 
> Does this need a full shootdown at this point, at least in the crash
> case? A local flush for the non-crash case might be nice too.

See __flush_tlb_all() after the loop.

> Also, probably a barrier() between set_pte() and
> tdx_enc_status_changed().

But why? There's no concurrency at this point. Interrupts are disabled and
only one CPU is active. Nobody can touch the memory relevant for the PTE.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov