Received-SPF: pass (google.com: domain of linux-kernel+bounces-173195-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 8 May 2024 14:04:22 +0200
From: Borislav Petkov <bp@alien8.de>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>,
	Elena Reshetova <elena.reshetova@intel.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Rick Edgecombe <rick.p.edgecombe@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	"Kalra, Ashish" <ashish.kalra@amd.com>,
	Sean Christopherson <seanjc@google.com>,
	"Huang, Kai" <kai.huang@intel.com>, Baoquan He <bhe@redhat.com>,
	kexec@lists.infradead.org, linux-coco@lists.linux.dev,
	linux-kernel@vger.kernel.org, Tao Liu <ltao@redhat.com>
Subject: Re: [PATCHv10 10/18] x86/tdx: Convert shared memory back to private
 on kexec
Message-ID: <20240508120422.GCZjtqRrSuJeUyByEM@fat_crate.local>
References: <20240409113010.465412-1-kirill.shutemov@linux.intel.com>
 <20240409113010.465412-11-kirill.shutemov@linux.intel.com>
 <20240505121319.GAZjd337gHC0uhk6aM@fat_crate.local>
 <bpmagk7v6swftzjjpchplzjwgx3pbdqzklcxybrmrhprqizk45@lxmvva42z36g>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <bpmagk7v6swftzjjpchplzjwgx3pbdqzklcxybrmrhprqizk45@lxmvva42z36g>

On Mon, May 06, 2024 at 06:37:19PM +0300, Kirill A. Shutemov wrote:
> "second kernel" is nomenclature kexec folks are using, but okay.

And the "third kernel" is the one which got kexec-ed the second time?

You can make it: "The second, kexec-ed kernel" and then it is perfectly
clear.

> > > +	/*
> > > +	 * Crash kernel reaches here with interrupts disabled: can't wait for
> > > +	 * conversions to finish.
> > > +	 *
> > > +	 * If race happened, just report and proceed.
> > > +	 */
> > > +	bool wait_for_lock = !crash;
> > 
> > You don't need that bool - use crash.
> 
> Dave suggested the variable for documentation purposes.
> 
> https://lore.kernel.org/all/0b70ee1e-4bb5-4867-9378-f5723ca091d5@intel.com
> 
> I'm fine either way.

But you have the comment above it which already explains what's going
on...

> > Why are we printing something here if we're not really acting up on it?
> > 
> > Who should care here?
> 
> The only thing we can do at this point on failure is panic. It think
> it is reasonable to proceed, especially for crash case.
> 
> The print leaves a trace in the log to give a clue for debug.

Sure but you'll leave a trace if you panic right then and there, on the
first failure. Why noodle through the pages if the first failure is
already fatal?

> One possible reason for the failure is if kdump raced with memory
> conversion. In this case shared bit in page table got set (or not cleared
> form shared->private conversion), but the page is actually private. So
> this failure is not going to affect the kexec'ed kernel.

Lemme make sure I understand what you're saying here:

1. This is a fatal failure and we should panic

However,

2. the kexec-ed kernel is using a different page table so there won't be
   a mismatch between shared/private marking of the page so it doesn't
   matter

Close?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette