On Tue, 2023-11-21 at 12:58 +0300, kirill.shutemov@xxxxxxxxxxxxxxx wrote:
> On Tue, Nov 21, 2023 at 03:46:41AM +0000, Huang, Kai wrote:
> >
> > >
> > > +static atomic_t conversions_in_progress;
> > > +static bool conversion_allowed = true;
> > >
> >
> > [...]
> >
> > > /* Used while preparing memory map entries for second kernel */
> > > struct crash_memmap_data {
> > > @@ -107,6 +108,9 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
> > >
> > > crash_smp_send_stop();
> > >
> > > + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> > > + x86_platform.guest.enc_kexec_unshare_mem(true);
> > > +
> > > cpu_emergency_disable_virtualization();
> > >
> > > /*
> > > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> > > index 830425e6d38e..9fb302562bfd 100644
> > > --- a/arch/x86/kernel/reboot.c
> > > +++ b/arch/x86/kernel/reboot.c
> > > @@ -12,6 +12,7 @@
> > > #include <linux/delay.h>
> > > #include <linux/objtool.h>
> > > #include <linux/pgtable.h>
> > > +#include <linux/kexec.h>
> > > #include <acpi/reboot.h>
> > > #include <asm/io.h>
> > > #include <asm/apic.h>
> > > @@ -31,6 +32,7 @@
> > > #include <asm/realmode.h>
> > > #include <asm/x86_init.h>
> > > #include <asm/efi.h>
> > > +#include <asm/tdx.h>
> > >
> > > /*
> > > * Power off function, if any
> > > @@ -716,6 +718,9 @@ static void native_machine_emergency_restart(void)
> > >
> > > void native_machine_shutdown(void)
> > > {
> > > + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) && kexec_in_progress)
> > > + x86_platform.guest.enc_kexec_unshare_mem(false);
> > > +
> > > /* Stop the cpus and apics */
> > > #ifdef CONFIG_X86_IO_APIC
> > > /*
> >
> > In native_machine_crash_shutdown() the "unshare" is called after
> > crash_smp_send_stop(), but here it's called before the stop_other_cpus().
> >
> > I am wondering if you call "unshare" after stop_other_cpus(), can we guarantee
> > there's only one cpu running in both normal and crash kexec in which case you
> > might be able to get rid of the "conversions_in_progress" and
> > "conversion_allowed" above?
>
> For normal kexec we need to keep other CPUs going so they can finish
> conversion cleanly and get us to the known state. Note that the conversion
> is not atomic wrt preemption.
Yeah makes sense.
Add a comment to explain this? That would help to remind us when we look at the
code like years later.
>
> In crash scenario we do the best effort: detect if we race with conversion
> and report. We cannot really wait for conversions to finish: we get into
> crash path with IRQs disabled and in generally unknown state.
>
Maybe it's good idea to add a TDVMCALL to query page status from the TDX module?
In that case we can avoid the inaccuracy when looking at the page table.
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
