Subject: Re: [PATCH] cpu/hotplug: Allow the CPU in CPU_UP_PREPARE state to be
 brought up again.
To:     "Longpeng (Mike, Cloud Infrastructure Service Product Dept.)" 
        <longpeng2@huawei.com>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Gonglei (Arei)" <arei.gonglei@huawei.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Valentin Schneider <valentin.schneider@arm.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>
References: <20211122154714.xaoxok3fpk5bgznz@linutronix.de>
 <d5f43af4-f91f-ef9f-9c33-8d349f59b23c@oracle.com>
 <46f7dbb8c0cb42228ed028ed89af1282@huawei.com>
From:   Dongli Zhang <dongli.zhang@oracle.com>
Message-ID: <5c0b2ab4-7e97-b9a9-e1fa-d93323f23c1f@oracle.com>
Date:   Tue, 23 Nov 2021 21:24:32 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.1
In-Reply-To: <46f7dbb8c0cb42228ed028ed89af1282@huawei.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T0ZYdU9TVGZkeWZ6UVh4MTZBVzErdnFZYlhxYkdkb0JYNkl5N2J4RlRoTTBM?=
 =?utf-8?B?N1ZjYWxJcEZ1UERKZVBlUHZwQndMOFpUZy9uRXdLR2Z5ZTk1WlQ1Sk1FejdC?=
 =?utf-8?B?QmNKeFV6UHd0cW1qZjVMeDliM2VQbnF1UjYvY3V4dlprcitmQzFvL29zdnZ1?=
 =?utf-8?B?b3drMlFMc25LSW42R1ZDV3pNUSswc3FMUTJpYkxndDRVd2FVM3pFRmpwMWRa?=
 =?utf-8?B?eXk3bmwvbE8zek5oVVNhOTRYRUlSN2w2blBCcDBXOTdPbDBZWTRnNXFaRlMy?=
 =?utf-8?B?ZCtlWXJSODBqT1NlMG9GMHp5M3VTSVRoZnlyak9qb2JWWmp6S1g5QmlNZHJF?=
 =?utf-8?B?TjlJd3h3bWhhWFZDMGNvaXcvZURLK0gveHlaSm5iZ2ZjWjdRU0grNUZqKzls?=
 =?utf-8?B?N3FEbWVpaU0vRE5aZ1BlVHRzOVhFSzBERlNBRTZUeWlPM3R3ZzVUNTJkUkJ0?=
 =?utf-8?B?QzZOb2IvZFBwMHlqS1Z5SW9DeHVsQUNsWmo4ak5wSEtiVUR1UFhSa1k0Q0x5?=
 =?utf-8?B?T0x5cXJBdDc1MmliV3NRVEZhTTNIdC90dFFjUUhudVJ0a3lmc3owWkl4WFhj?=
 =?utf-8?B?U0xZK2kwZks1UTlsS1FuVDdaR3pkb0JlOEw3WHhUNVdxeS9YT3E5dGMydzBx?=
 =?utf-8?B?ejdrQXRVcE8weGZta1FHNTJnYS9vK3NwQXkzSmJUMmpVdXFwYjBYUnpPa3Jq?=
 =?utf-8?B?WE5QN1JKM05nbkwwNGNuMEVsNGEzSnU0cHhXYlRRVTR4SVJxcmxaZmxsajJm?=
 =?utf-8?B?TzJYNnQyb1l0blhzYlJuQW1FQWNYTG1IR1cwZXJjbEpMSnJENG5QK0lrS0Nn?=
 =?utf-8?B?UW9ReUovckZscHk0endTb3hWMWN2S3JvSWFFSzFMdm1zemVMMW5sZi9iL01T?=
 =?utf-8?B?WVRpaldPMzRzRDdUUHdIN00zVmdSTTlIanU0U0FxRE1RMlk3VnFGY095bTBV?=
 =?utf-8?B?cjBVNTM3YnJxNG5IelZUOVRwUVE1L0NoYWwzOWdoQ0VJalYrWUZXOHBKMi93?=
 =?utf-8?B?aGZKdDFSdE5FNEpLR3lJdlFhbG84cGcvMldpNEMyMU15YlNrMk83TUVyVTBo?=
 =?utf-8?B?UmlHRHhwWmJyM2tHTlNBY1crUU85UFgvNFBCL0ptMVlsUTNrZnhWVkJwYXg3?=
 =?utf-8?B?UndLb1hnYVgweEY0WVpHclJUZW44YTZRZ1RrWDJlY1JXSkdiUDNWOVEwNkNk?=
 =?utf-8?B?ckVqOGNKcDlzVEtGM1k5eXMwaFpMQlhVT2JNSnB6TEdjREtTY1RkWUU1S3Y4?=
 =?utf-8?B?RjlSVmVVQlJxUWdzRUZUSWh1YktkcERwOWc5Q1hTSHQxdkpWZVQveTBhV0VW?=
 =?utf-8?B?WFNGMXhvU0FWWUR4dCtYWjRKNytkNXAxOWF4UzFWRGJ5SWtwcDVDQWxLbXBz?=
 =?utf-8?B?MTJvTis3bUZ4N05Cd0hJR0taaWU5VDlYdE9OSTlaM2dHYk1HakRod1VCdlFB?=
 =?utf-8?B?YnNTWVM1MHFzdklyOU9TQXJjdnNGRUE5Vis2Zm1zZVRMMjhpNEdLKzNWY2FK?=
 =?utf-8?B?ZEQrdWtjODYzNmcvSjZsejBLZVZQK0ZsYjNwckNSdCtFZjVNLzNnVkVkL1ZO?=
 =?utf-8?B?K1NyMUlmbyt3amJQL3dZUy9iOG53aFo3N0RSN1RJKzh0SzF0MmVnVUpkb3Iy?=
 =?utf-8?B?QktRcHN0VFJJdWFMVGp4ZHhwY3ZjWkphV1hyTXBPUXJXdVpkUlo1NXQxNDJJ?=
 =?utf-8?B?Nnc1cXlWazNVOEpRRmxQRjBWaHJ3TXVwWHNYREJ2Q3dFMjlEbmRaTmVKWXB5?=
 =?utf-8?B?SEgwalNWcjBTamRtcFF3VzR3eng0dGNWSFIvV0V4dlVuRExMNW00NWVlSVFR?=
 =?utf-8?B?UjhxTGNlYzZaRUhyb0tIVjZIa1BoYncvVkJndWttbExJTnJJUmpKV285dFox?=
 =?utf-8?B?ZFFQM3FQb0x4SzV1aURmeEpjaFlKVXphUFlFZEJmM3dBTC8wT0MwUkVxQUNF?=
 =?utf-8?B?ckpqTEhESDJkZm9DUGtNM1VjbDlMa2RkRitCME8yV0xqYUNaTzBDNll3aU1T?=
 =?utf-8?B?SFIvVksySzhYQWNjUjkxU2IvN2d5SHk0dFBZSi9lZmNGR05URjIrL0UxdnI2?=
 =?utf-8?B?V0FmMThpc1hVelEvK3lPTGFNQlhsTC9CdThFYm0yVEpsVmxYZCs4V281UDJJ?=
 =?utf-8?B?Y0s1TG94T3Q3U3djT3l5Zno2U082UnlqWTVMTGVtN1phOGtvWkxqeUlEYnF6?=
 =?utf-8?B?WURIVXhaVE5ORTlpZkdqNlh3MVBmS2tmWFBQSExSTjVITnRHSkNLdVFza05I?=
 =?utf-8?B?TTNETTRuWm1FSjlJVW9pdnhGb1VBPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e662cf9b-db85-4d65-418d-08d9af0ab38f
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2663.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Nov 2021 05:24:35.0957
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: d8Rk8MR8pRYN0Y3ve4zUSA8F6cZWwPgZG19ikA3RXz8OwgGIskDcCdo0GMiBY+OefRi5RQVPNVitxz3DgEgRbA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB2453
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10177 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 bulkscore=0 spamscore=0
 phishscore=0 adultscore=0 suspectscore=0 malwarescore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2111240031
X-Proofpoint-GUID: fv6VGFIi2_PgJ8_BgZ_h_a-sdFFimo7F
X-Proofpoint-ORIG-GUID: fv6VGFIi2_PgJ8_BgZ_h_a-sdFFimo7F
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 11/23/21 3:50 PM, Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
wrote:
> 
> 
>> -----Original Message-----
>> From: Dongli Zhang [mailto:dongli.zhang@oracle.com]
>> Sent: Wednesday, November 24, 2021 5:22 AM
>> To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>; Longpeng (Mike, Cloud
>> Infrastructure Service Product Dept.) <longpeng2@huawei.com>
>> Cc: linux-kernel@vger.kernel.org; Gonglei (Arei) <arei.gonglei@huawei.com>;
>> x86@kernel.org; xen-devel@lists.xenproject.org; Peter Zijlstra
>> <peterz@infradead.org>; Ingo Molnar <mingo@kernel.org>; Valentin Schneider
>> <valentin.schneider@arm.com>; Boris Ostrovsky <boris.ostrovsky@oracle.com>;
>> Juergen Gross <jgross@suse.com>; Stefano Stabellini <sstabellini@kernel.org>;
>> Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; Borislav
>> Petkov <bp@alien8.de>; Dave Hansen <dave.hansen@linux.intel.com>; H. Peter
>> Anvin <hpa@zytor.com>
>> Subject: Re: [PATCH] cpu/hotplug: Allow the CPU in CPU_UP_PREPARE state to be
>> brought up again.
>>
>> Tested-by: Dongli Zhang <dongli.zhang@oracle.com>
>>
>>
>> The bug fixed by commit 53fafdbb8b21 ("KVM: x86: switch KVMCLOCK base to
>> monotonic raw clock") may leave the cpu_hotplug_state at CPU_UP_PREPARE. As a
>> result, to online this CPU again (even after removal) is always failed.
>>
>> I have tested that this patch works well to workaround the issue, by introducing
>> either a mdeley(11000) or while(1); to start_secondary(). That is, to online
>> the
>> same CPU again is successful even after initial do_boot_cpu() failure.
>>
>> 1. add mdelay(11000) or while(1); to the start_secondary().
>>
>> 2. to online CPU is failed at do_boot_cpu().
>>
> 
> Thanks for your testing :)
> 
> Does the cpu4 spin in wait_for_master_cpu() in your case ?

I did two tests.

TEST 1.

I added "mdelay(11000);" as the first line in start_secondary(). Once the issue
was encountered, the RIP of CPU=4 was ffffffff8c242021 (from QEMU's "info
registers -a") which was in the range of wait_for_master_cpu().

# cat /proc/kallsyms | grep ffffffff8c2420
ffffffff8c242010 t wait_for_master_cpu
ffffffff8c242030 T load_fixmap_gdt
ffffffff8c242060 T native_write_cr4
ffffffff8c2420c0 T cr4_init


TEST 2.

I added "while(true);" as the first line in start_secondary(). Once the issue
was encountered, the RIP of CPU=4 was ffffffff91654c0a (from QEMU's "info
registers -a") which was in the range of start_secondary().

# cat /proc/kallsyms | grep ffffffff91654c0
ffffffff91654c00 t start_secondary

Dongli Zhang


> 
>> 3. to online CPU again is failed without this patch.
>>
>> # echo 1 > /sys/devices/system/cpu/cpu4/online
>> -su: echo: write error: Input/output error
>>
>> 4. to online CPU again is successful with this patch.
>>
>> Thank you very much!
>>
>> Dongli Zhang
>>
>> On 11/22/21 7:47 AM, Sebastian Andrzej Siewior wrote:
>>> From: "Longpeng(Mike)" <longpeng2@huawei.com>
>>>
>>> A CPU will not show up in virtualized environment which includes an
>>> Enclave. The VM splits its resources into a primary VM and a Enclave
>>> VM. While the Enclave is active, the hypervisor will ignore all requests
>>> to bring up a CPU and this CPU will remain in CPU_UP_PREPARE state.
>>> The kernel will wait up to ten seconds for CPU to show up
>>> (do_boot_cpu()) and then rollback the hotplug state back to
>>> CPUHP_OFFLINE leaving the CPU state in CPU_UP_PREPARE. The CPU state is
>>> set back to CPUHP_TEARDOWN_CPU during the CPU_POST_DEAD stage.
>>>
>>> After the Enclave VM terminates, the primary VM can bring up the CPU
>>> again.
>>>
>>> Allow to bring up the CPU if it is in the CPU_UP_PREPARE state.
>>>
>>> [bigeasy: Rewrite commit description.]
>>>
>>> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
>>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>> Link:
>> https://urldefense.com/v3/__https://lore.kernel.org/r/20210901051143.2752-1
>> -longpeng2@huawei.com__;!!ACWV5N9M2RV99hQ!d4sCCXMQV7ekFwpd21vo1_9K-m5h4VZ-g
>> E8Z62PLL58DT4VJ6StH57TR_KpBdbwhBE0$
>>> ---
>>>
>>> For XEN: this changes the behaviour as it allows to invoke
>>> cpu_initialize_context() again should it have have earlier. I *think*
>>> this is okay and would to bring up the CPU again should the memory
>>> allocation in cpu_initialize_context() fail.
>>>
>>>  kernel/smpboot.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/kernel/smpboot.c b/kernel/smpboot.c
>>> index f6bc0bc8a2aab..34958d7fe2c1c 100644
>>> --- a/kernel/smpboot.c
>>> +++ b/kernel/smpboot.c
>>> @@ -392,6 +392,13 @@ int cpu_check_up_prepare(int cpu)
>>>  		 */
>>>  		return -EAGAIN;
>>>
>>> +	case CPU_UP_PREPARE:
>>> +		/*
>>> +		 * Timeout while waiting for the CPU to show up. Allow to try
>>> +		 * again later.
>>> +		 */
>>> +		return 0;
>>> +
>>>  	default:
>>>
>>>  		/* Should not happen.  Famous last words. */
>>>