Re: [PATCH] x86: Disable kexec for TDX guests

[Baoquan He]

On 03/26/23 at 10:01am, Dave Hansen wrote:
> On 3/25/23 12:25, Kirill A. Shutemov wrote:
> > On Sat, Mar 25, 2023 at 09:25:36AM -0700, Dave Hansen wrote:
> >> On 3/25/23 09:01, Kirill A. Shutemov wrote:
> >>> The last item is tricky. TDX guests use ACPI MADT MPWK to bring up
> >>> secondary CPUs. The mechanism doesn't allow to put a CPU back offline if
> >>> it has woken up.
> >> ...
> >>> +int arch_kexec_load(void)
> >>> +{
> >>> +	if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
> >>> +		pr_warn_once("Disable kexec: not yet supported in TDX guest\n");
> >>> +		return -EOPNOTSUPP;
> >>> +	}
> >>> +
> >>> +	return 0;
> >>> +}
> >>
> >> So, let's put all this together:
> >>
> >> 1. TDX implementations use MADT for wakeup exclusively right now (but
> >>    are not necessarily _required_ to do so forever)
> >> 2. MADT doesn't support CPU offlining
> >> 3. kexec() requires offlining
> >>
> >> Thus, current TDX implementations can't support TDX guests.  This
> >> *doesn't* say that TDX will always use the MADT for wakeups.
> >>
> >> Yet, the check you have here is for TDX and *not* for the MADT.
> > 
> > As I described in the commit message there are more than MADT that is
> > required to get kexec in TDX guest.
> 
> I kinda think we should do both.
> 
> Let's make sure that all systems that depend on MADT wakeups can't
> kexec() until the ACPI folks work out what to do there.
> 
> Separately, let's either fix or *mark* the kexec()-incompatible pieces
> that *ARE* specific to TDX.
> 
> >> That seems wrong.
> >>
> >> Let's say SEV or arm64 comes along and uses the MADT for their guests.
> >> They'll add another arch_kexec_load(), with a check for *their* feature.
> >>
> >> This all seems like you should be disabling kexec() the moment the MADT
> >> CPU wakeup is used instead of making it based on TDX.
> > 
> > I guess we can go this path if you are fine with taking CR4.MCE and shared
> > memory reverting patches (they require some rework, but I can get them
> > into shape quickly). After that we can forbid kexec on machines with MADT
> > if nr_cpus > 1.
> 
> This goes back to what I asked before: is anyone actually going to *use*
> a single-processor system that wants to kexec()?  If not, let's not
> waste the time to introduce code that is just going to bitrot.  Just
> mark it broken and move on with life.

Now we have two API for kexec: kexec_load and kexec_file_load. They can
be used to do kexec reboot, or crash dumping. For crash dumping, we
usually only use one cpu to do the vmcore dumping. At least on our
Fedora/centos-stream/RHEL, we do like this with kernel parameter
'nr_cpus=1' added by default. Unless people explicitly remove the
'nr_cpus=1' restriction or set nr_cpus= to other number to persue
multithread dumping in kdump kernel.

So I think Kirill's idea looks good. Means on TDX guest kexec reboot
will be forbid, while crash dumping will function normally.

Thanks
Baoquan