Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b9cef72f-0bcf-5bd6-e828-f11bd822fe33@mbosol.com>
Date:   Sun, 4 Jun 2023 07:02:33 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [patch 5/6] x86/smp: Split sending INIT IPI out into a helper
 function
Content-Language: en-US
To:     Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>
Cc:     x86@kernel.org, Ashok Raj <ashok.raj@linux.intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Tony Luck <tony.luck@intel.com>,
        Arjan van de Veen <arjan@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Eric Biederman <ebiederm@xmission.com>
References: <20230603193439.502645149@linutronix.de>
 <20230603200459.889612295@linutronix.de>
From:   =?UTF-8?Q?Mika_Penttil=c3=a4?= <mika.penttila@mbosol.com>
In-Reply-To: <20230603200459.889612295@linutronix.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VzVxVnYrUmlzQWt6NXVaMHRUWVlvUmJvcjZsdW42cWI0MG9CZkE2bzdwdGVV?=
 =?utf-8?B?K2llU3phNVFVTDJmQTF3d3c2Ymk1L2VoYjlJSHhNaTE5d2tzUWExWDliNDZX?=
 =?utf-8?B?WDdObkpNbys0d2lLQUd1RFNHTkZHM2FUdHJSRTB4bktYUU92WDVIcHdXR3BJ?=
 =?utf-8?B?bjR4TkFGNkZHbDdhQXgvcmlZTzM4alg1ZWF4MGNMeXhKYnBOQkJGVS94alps?=
 =?utf-8?B?Z3FKemk5N28vNmhnN1ZheHc3VXpKV1VaTzRRV2hyMGltcGVFWUwvUmpVS0RL?=
 =?utf-8?B?L3h4dDhYQkFrV3Q3MVhqWVdSTFJVVkNQYlRsNi85czBQa2ZGVzFhb1ZmY2F6?=
 =?utf-8?B?SW5CU05CRUhRK2tOSk5LODJrN1ZIT0ZWS1daanlOYWVCTi9tRVllWDQvS0k0?=
 =?utf-8?B?c25rZkpNbC8xMjNlMUp0UUN6dmVCbGwvNGdrcTNqWlJDL0EzMDF4ZkdZbUxo?=
 =?utf-8?B?MUlwTUVmbGZpdUJtS0tUQUxaTUZlSmVGNWV2NHowS3F0OEtBT0kxSFg5eSta?=
 =?utf-8?B?eUVvQkIrTFdIekY0aVR2N3BBbmZmUDdxbVhvVndjSVRmZEYwMWFiMmhBNVla?=
 =?utf-8?B?VFNwUVV3c0pyQXJ3Uld5YlhscThtZHA2VUpObnEyR1BxTm9IVld1eXFEcUlj?=
 =?utf-8?B?cTBRVlBBNktSaDFqcVMwS2lCZkplR3REYkNCbG1GMWhxM3NlREdNYVNwaTJY?=
 =?utf-8?B?OVgrUm1EN3JlWHRQUURaZ1lJS2dPSjROVEpJMzljUGFmV0RZWG8wdXJ0TU45?=
 =?utf-8?B?YWp5SjZjbEpWZldQR05jU2ZsYURTYzZmeG9ERVdmM2hGS0NNME16azJwTzVz?=
 =?utf-8?B?YjJEVDZOMFNYdTVCb1JrQnZ5bzdVUFRPTFBPNVZmaUJwVENKc2hQcGo3b09K?=
 =?utf-8?B?L2JzMElSVnp4OVhDTU9iRXlRUUNUYXpudVFYOEN2TWUwOUJtQlNkKzJwZHp0?=
 =?utf-8?B?dXpZYWkzRmx3d2VUMmtUTnJMMjZXZFh5dXlHVVVTM2tkUUJSRm5mQUZjZmxz?=
 =?utf-8?B?ZnJxeXh4SUs2ZWUwYVRObFdYOS9WSk11Z1pVc29jcWtxWjd4L09MOVB4L21s?=
 =?utf-8?B?STM4SGxndENQMHRlZjhweC8ySnZmT3BPTmdod2F0bGIySFRBazd4c2dXRnVY?=
 =?utf-8?B?bC9Zb3M3aEtIdGM4RHJNZkcxeEljMnRLcEtnRTk2cTh0cDlveEdMTGhMZHYz?=
 =?utf-8?B?R0VmYTJ1Y1U5YTU5VTZOSC83ZnJ6enNCQlo1M2N5WEF5WjYrRlNGa0l2dHh1?=
 =?utf-8?B?MVl0UU9ZY3hXRURMNDQyeUo5YzU2WmJVU0RLakM5ZGswSDk5NkVzZExOOTMz?=
 =?utf-8?B?ZnNQLzB0NHR0NmZ2Wm1wSU1xTElrTUh1NVNOZndxRUgyL2U2VFRXUGIvS2Fu?=
 =?utf-8?B?UGh3NHhHUnE4TGlLWDErZUlzVVZHUDFkVWhyQnRyUE52cTU3cmFvYnU5ekFv?=
 =?utf-8?B?b0lVbFEzZ1VLK3BGa1dLeTlDWldwbUducmp2eWtrdThFRlBrU1FoL09TWFZF?=
 =?utf-8?B?Y21VQXFndjQxM1BsM0VwN3AzYUdZcnRFL2cvUHJzQXJrUEtRejYzS3JaTGRh?=
 =?utf-8?B?NUdKbW5la0JpcUFKT1JHT2hLN0lBQVd5VlllTXlXSVFTaS9mMjZZQVpUMGFS?=
 =?utf-8?B?a0tScEFPWnlIMkhGWGJZVmt0Nk9XSkVsQnFMMUpqdVRxYnFiWTdpcEFBREhw?=
 =?utf-8?B?bk9ZS3laT1JMYS9Qc2JCMXhDcjFPMnZyTTIzMjY1UTRyUG91SUVENnZuc1ho?=
 =?utf-8?B?RjRXcXNyQTBJUUlmT29DRFpyamFBUjRXMWJiSGZyL1d0MDdmOEVxVzViYjBu?=
 =?utf-8?B?YXN4N1JFU2F5RjJjOTJGTmNHRnRhV0ExQXV0eWhNaDlFNzhvNU1MZjdNT2lB?=
 =?utf-8?B?SmtxSWxzMlNZSDJYTlRXWmFmYXZ4eDNobUJXRHMwdGRqNmJtTGg0QWIyaGVK?=
 =?utf-8?B?UGV4ZU0yTU90dStmN3k5OTE5TFMwZ20yc2V0TVBnK0JUNUhBTWVQVXQyK3JH?=
 =?utf-8?B?OHpjTFhOaUIwQjREdHhkMWZYbVJuUm5JbGRQdzJqWmtjUm1rcWo0TldqYmhP?=
 =?utf-8?B?WEJnMnNHMzA1SVBuWWZzbXpqR3dKZ0xIdGlyN2Q1Ky9BT2h1YWkvbHFyWjdz?=
 =?utf-8?B?VWJwUUoyT1ZpU0lyQnJDY3AxK3dDM2UyNjFON0JYdXcyU0k0c3Nackxhd2VD?=
 =?utf-8?B?MHc9PQ==?=
X-OriginatorOrg: mbosol.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f2553e03-57db-49d9-4576-08db64b087e3
X-MS-Exchange-CrossTenant-AuthSource: DB8P192MB0533.EURP192.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2023 04:02:36.1671
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 571b6760-44e0-4aae-b783-84984ac780c0
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9X/pwm3oyb5ZzBIoankfR7/ontJWNzf/l4+0yBO9gddlC8PnNclG4KDZfgCa9YV13DkMLGnbHhAQWMKoV/NMikhU2JgoJzQzbwupkuQ8enA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXP192MB1200
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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