On Fri, Oct 20, 2023 at 11:21:34AM +0000, Huang, Kai wrote:
>
> > --- /dev/null
> > +++ b/arch/x86/kernel/acpi/madt.S
> > @@ -0,0 +1,28 @@
> > +#include <linux/linkage.h>
> > +#include <asm/nospec-branch.h>
> > +#include <asm/page_types.h>
> > +#include <asm/processor-flags.h>
> > +
> > + .text
> > + .align PAGE_SIZE
> > +SYM_FUNC_START(asm_acpi_mp_play_dead)
> > + /* Load address of reset vector into RCX to jump when kernel is ready */
> > + movq acpi_mp_reset_vector_paddr(%rip), %rcx
> > +
> > + /* zero out flags, and disable interrupts */
> > + pushq $0
> > + popfq
> > +
> > + /* Turn off global entries. Following CR3 write will flush them. */
> > + movq %cr4, %rdx
> > + andq $~(X86_CR4_PGE), %rdx
> > + movq %rdx, %cr4
> > +
> > + /* Switch to identity mapping */
> > + movq acpi_mp_pgd(%rip), %rax
> > + movq %rax, %cr3
> > +
> > + /* Jump to reset vector */
> > + ANNOTATE_RETPOLINE_SAFE
> > + jmp *%rcx
> > +SYM_FUNC_END(asm_acpi_mp_play_dead)
> > diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/acpi/madt_wakeup.c
> > index 4e92d1d4a5fa..2cc8590ec7a5 100644
> > --- a/arch/x86/kernel/acpi/madt_wakeup.c
> > +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> > @@ -1,12 +1,162 @@
> > #include <linux/acpi.h>
> > #include <linux/cpu.h>
> > +#include <linux/delay.h>
> > +#include <linux/memblock.h>
> > +#include <linux/pgtable.h>
> > +#include <linux/sched/hotplug.h>
> > #include <asm/apic.h>
> > +#include <asm/init.h>
> >
> > /* Physical address of the Multiprocessor Wakeup Structure mailbox */
> > static u64 acpi_mp_wake_mailbox_paddr;
> > /* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> > static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> >
> > +unsigned long acpi_mp_pgd;
> > +u64 acpi_mp_reset_vector_paddr;
>
> Nit:
>
> They are both physical address. It's a little bit odd for one to use 'unsigned
> long' and the other to use 'u64'.
Okay, I will make them both u64.
> > +
> > +void asm_acpi_mp_play_dead(void);
> > +
> > +static void __init *alloc_pgt_page(void *context)
> > +{
> > + return memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> > +}
>
> If I am reading correclty, @context is never used. It's not used inside this
> function, and all the callers call this with @context = NULL.
Yes. We need the argument to conform to x86_mapping_info::alloc_pgt_page
interface.
> > +
> > +static void acpi_mp_play_dead(void)
> > +{
> > + idle_task_exit();
> > + cpuhp_ap_report_dead();
> > + asm_acpi_mp_play_dead();
> > +}
> > +
>
> Looks you can use play_dead_common() here, if you take IRQ disable part out from
> the assembly, because play_dead_common() does:
>
> void play_dead_common(void)
> {
> idle_task_exit();
>
> cpuhp_ap_report_dead();
>
> local_irq_disable();
> }
Okay, fair enough.
--
Kiryl Shutsemau / Kirill A. Shutemov
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
