[5/6] x86/smp: Split sending INIT IPI out into a helper function
Commit Message
Putting CPUs into INIT is a safer place during kexec() to park CPUs.
Split the INIT assert/deassert sequence out so it can be reused.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/smpboot.c | 51 +++++++++++++++++++---------------------------
1 file changed, 22 insertions(+), 29 deletions(-)
Comments
Hi,
On 3.6.2023 23.07, Thomas Gleixner wrote:
> Putting CPUs into INIT is a safer place during kexec() to park CPUs.
>
> Split the INIT assert/deassert sequence out so it can be reused.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> arch/x86/kernel/smpboot.c | 51 +++++++++++++++++++---------------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -853,47 +853,40 @@ wakeup_secondary_cpu_via_nmi(int apicid,
> return (send_status | accept_status);
> }
>
> -static int
> -wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
> +static void send_init_sequence(int phys_apicid)
> {
> - unsigned long send_status = 0, accept_status = 0;
> - int maxlvt, num_starts, j;
> -
> - maxlvt = lapic_get_maxlvt();
> + int maxlvt = lapic_get_maxlvt();
>
> - /*
> - * Be paranoid about clearing APIC errors.
> - */
> + /* Be paranoid about clearing APIC errors. */
> if (APIC_INTEGRATED(boot_cpu_apic_version)) {
> - if (maxlvt > 3) /* Due to the Pentium erratum 3AP. */
> + /* Due to the Pentium erratum 3AP. */
> + if (maxlvt > 3)
> apic_write(APIC_ESR, 0);
> apic_read(APIC_ESR);
> }
>
> - pr_debug("Asserting INIT\n");
> -
> - /*
> - * Turn INIT on target chip
> - */
> - /*
> - * Send IPI
> - */
> - apic_icr_write(APIC_INT_LEVELTRIG | APIC_INT_ASSERT | APIC_DM_INIT,
> - phys_apicid);
> -
> - pr_debug("Waiting for send to finish...\n");
> - send_status = safe_apic_wait_icr_idle();
> + /* Assert INIT on the target CPU */
> + apic_icr_write(APIC_INT_LEVELTRIG | APIC_INT_ASSERT | APIC_DM_INIT, phys_apicid);
> + safe_apic_wait_icr_idle();
>
> udelay(init_udelay);
>
> - pr_debug("Deasserting INIT\n");
> -
> - /* Target chip */
> - /* Send IPI */
> + /* Deassert INIT on the target CPU */
> apic_icr_write(APIC_INT_LEVELTRIG | APIC_DM_INIT, phys_apicid);
> + safe_apic_wait_icr_idle();
> +}
> +
> +/*
> + * Wake up AP by INIT, INIT, STARTUP sequence.
> + */
> +static int wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
> +{
> + unsigned long send_status = 0, accept_status = 0;
> + int maxlvt, num_starts, j;
> +
> + preempt_disable();
This seems like an unbalanced preempt disable..
>
> - pr_debug("Waiting for send to finish...\n");
> - send_status = safe_apic_wait_icr_idle();
> + send_init_sequence(phys_apicid);
>
> mb();
>
>
--Mika
Hi
On Sun, Jun 04, 2023 at 07:02:33AM +0300, Mika Penttilä wrote:
> Hi,
>
> On 3.6.2023 23.07, Thomas Gleixner wrote:
> > Putting CPUs into INIT is a safer place during kexec() to park CPUs.
> >
> > Split the INIT assert/deassert sequence out so it can be reused.
> >
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > ---
> > arch/x86/kernel/smpboot.c | 51 +++++++++++++++++++---------------------------
> > 1 file changed, 22 insertions(+), 29 deletions(-)
> >
> > --- a/arch/x86/kernel/smpboot.c
> > +++ b/arch/x86/kernel/smpboot.c
> > @@ -853,47 +853,40 @@ wakeup_secondary_cpu_via_nmi(int apicid,
> > return (send_status | accept_status);
> > }
> > -static int
> > -wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
> > +static void send_init_sequence(int phys_apicid)
> > {
> > - unsigned long send_status = 0, accept_status = 0;
> > - int maxlvt, num_starts, j;
> > -
> > - maxlvt = lapic_get_maxlvt();
> > + int maxlvt = lapic_get_maxlvt();
> > - /*
> > - * Be paranoid about clearing APIC errors.
> > - */
> > + /* Be paranoid about clearing APIC errors. */
> > if (APIC_INTEGRATED(boot_cpu_apic_version)) {
> > - if (maxlvt > 3) /* Due to the Pentium erratum 3AP. */
> > + /* Due to the Pentium erratum 3AP. */
> > + if (maxlvt > 3)
> > apic_write(APIC_ESR, 0);
> > apic_read(APIC_ESR);
> > }
> > - pr_debug("Asserting INIT\n");
> > -
> > - /*
> > - * Turn INIT on target chip
> > - */
> > - /*
> > - * Send IPI
> > - */
> > - apic_icr_write(APIC_INT_LEVELTRIG | APIC_INT_ASSERT | APIC_DM_INIT,
> > - phys_apicid);
> > -
> > - pr_debug("Waiting for send to finish...\n");
> > - send_status = safe_apic_wait_icr_idle();
> > + /* Assert INIT on the target CPU */
> > + apic_icr_write(APIC_INT_LEVELTRIG | APIC_INT_ASSERT | APIC_DM_INIT, phys_apicid);
> > + safe_apic_wait_icr_idle();
> > udelay(init_udelay);
> > - pr_debug("Deasserting INIT\n");
> > -
> > - /* Target chip */
> > - /* Send IPI */
> > + /* Deassert INIT on the target CPU */
> > apic_icr_write(APIC_INT_LEVELTRIG | APIC_DM_INIT, phys_apicid);
> > + safe_apic_wait_icr_idle();
> > +}
> > +
> > +/*
> > + * Wake up AP by INIT, INIT, STARTUP sequence.
> > + */
> > +static int wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
> > +{
> > + unsigned long send_status = 0, accept_status = 0;
> > + int maxlvt, num_starts, j;
Also maxlvt used uninitialized.
> > +
> > + preempt_disable();
>
> This seems like an unbalanced preempt disable..
>
> > - pr_debug("Waiting for send to finish...\n");
> > - send_status = safe_apic_wait_icr_idle();
> > + send_init_sequence(phys_apicid);
> > mb();
> >
>
>
> --Mika
>
On Sun, Jun 04 2023 at 07:02, Mika Penttilä wrote:
> On 3.6.2023 23.07, Thomas Gleixner wrote:
>> +/*
>> + * Wake up AP by INIT, INIT, STARTUP sequence.
>> + */
>> +static int wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
>> +{
>> + unsigned long send_status = 0, accept_status = 0;
>> + int maxlvt, num_starts, j;
>> +
>> + preempt_disable();
>
> This seems like an unbalanced preempt disable..
Right. I screwed up a rebase completely....
@@ -853,47 +853,40 @@ wakeup_secondary_cpu_via_nmi(int apicid,
return (send_status | accept_status);
}
-static int
-wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
+static void send_init_sequence(int phys_apicid)
{
- unsigned long send_status = 0, accept_status = 0;
- int maxlvt, num_starts, j;
-
- maxlvt = lapic_get_maxlvt();
+ int maxlvt = lapic_get_maxlvt();
- /*
- * Be paranoid about clearing APIC errors.
- */
+ /* Be paranoid about clearing APIC errors. */
if (APIC_INTEGRATED(boot_cpu_apic_version)) {
- if (maxlvt > 3) /* Due to the Pentium erratum 3AP. */
+ /* Due to the Pentium erratum 3AP. */
+ if (maxlvt > 3)
apic_write(APIC_ESR, 0);
apic_read(APIC_ESR);
}
- pr_debug("Asserting INIT\n");
-
- /*
- * Turn INIT on target chip
- */
- /*
- * Send IPI
- */
- apic_icr_write(APIC_INT_LEVELTRIG | APIC_INT_ASSERT | APIC_DM_INIT,
- phys_apicid);
-
- pr_debug("Waiting for send to finish...\n");
- send_status = safe_apic_wait_icr_idle();
+ /* Assert INIT on the target CPU */
+ apic_icr_write(APIC_INT_LEVELTRIG | APIC_INT_ASSERT | APIC_DM_INIT, phys_apicid);
+ safe_apic_wait_icr_idle();
udelay(init_udelay);
- pr_debug("Deasserting INIT\n");
-
- /* Target chip */
- /* Send IPI */
+ /* Deassert INIT on the target CPU */
apic_icr_write(APIC_INT_LEVELTRIG | APIC_DM_INIT, phys_apicid);
+ safe_apic_wait_icr_idle();
+}
+
+/*
+ * Wake up AP by INIT, INIT, STARTUP sequence.
+ */
+static int wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
+{
+ unsigned long send_status = 0, accept_status = 0;
+ int maxlvt, num_starts, j;
+
+ preempt_disable();
- pr_debug("Waiting for send to finish...\n");
- send_status = safe_apic_wait_icr_idle();
+ send_init_sequence(phys_apicid);
mb();