From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from userp2120.oracle.com (userp2120.oracle.com [156.151.31.85]) by mx.groups.io with SMTP id smtpd.web11.5302.1612330873444960912 for ; Tue, 02 Feb 2021 21:41:13 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@oracle.com header.s=corp-2020-01-29 header.b=k1JLzGNa; spf=pass (domain: oracle.com, ip: 156.151.31.85, mailfrom: ankur.a.arora@oracle.com) Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 1135XqkS023979; Wed, 3 Feb 2021 05:41:12 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : in-reply-to : content-type : content-transfer-encoding : mime-version; s=corp-2020-01-29; bh=XTr7hfM/loIPZYF3pcxR/mdn9ADD2eXjRTweJQ2y9BQ=; b=k1JLzGNaQmmpGVqt1ROloEaxzo0lxseMkZtgyCSCFy7DMw4rRZkpYKxiQjaxwXDDLp09 padoLhsrKkjnX9npefmfybkN9BHdw8iKaoUXBkVTCwuzw4HjPgbYw0IJ76w1PDXfIwOr 8ldluZ+cpP6zxXfx30OSVfg5NtCgKDzfLfzxlUu0sLZS1pfjFZR0vqE+QcJ+GxVf99ga q0oK9HO6oXcawYov7+vqxqXOlwhkfcIAiuOBoWS0WXMiwFFza7lgOEY5tFigfiFiK7uq qxl5/qY/nrPcrLMRNUP6u7hY6fFxGCuyc2EnUUJdj7Ek/QWNskt1jEAXhF/XgtWdK3U6 1Q== Received: from userp3030.oracle.com (userp3030.oracle.com [156.151.31.80]) by userp2120.oracle.com with ESMTP id 36dn4wm1cs-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 03 Feb 2021 05:41:12 +0000 Received: from pps.filterd (userp3030.oracle.com [127.0.0.1]) by userp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 1135ebEl079342; Wed, 3 Feb 2021 05:41:11 GMT Received: from nam12-mw2-obe.outbound.protection.outlook.com (mail-mw2nam12lp2044.outbound.protection.outlook.com [104.47.66.44]) by userp3030.oracle.com with ESMTP id 36dhcxxsvk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 03 Feb 2021 05:41:11 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=D90Idrj90/0YA1nuHS23XiebpYUVNG68BVwl4TQPnQV60pBFlqpjKINO7npQwyIMlOVe4lR7i7CAZmZBf0qBMVr75gesp6yDFrmrr5MmSu3QGHl4bV+ATDqEjuOH69SIpPTKAxGQXnYQY726R3rRwd086o5nNl37FvtGrwwu9m5i8pVOZ9232ZDx8PYUdejJhuMsblkLJeB1sWw/jxafJvXVH0uRM9Hzsk3c9kckOjFYr0r6mlXljPznSWEuuEvi7oXvVin2eQgsVJWykpBwlcrdQ89NgzLiK9pv+6zgkT97jkHftawqNp+u2Ar3vM3JJ3aFF17HXKu58N9T7wQVCw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=XTr7hfM/loIPZYF3pcxR/mdn9ADD2eXjRTweJQ2y9BQ=; b=Qdopo5eQRBTCb1VejOu23ysiXUJm9HKHatJl3AuHqj0oq7Yxndl9rNHOmrQoFHc1+yZM4JMC0Tott6GweGKAa3Ee9BVG3QA+vEOS2Ms0dBtbOnSTApC2tdq4zst55fZZYSZpfsnF4c3DR/Mio5azfENL05o8KUR93bgGHFvA9Pu5m/KHC7n0w3JW/pr2mZdW7lrjUhoXKwnX6t/c0g9DNuU0t2LTZOctHXhgugeyiZOagjzRwbY9cy93J69/B4xVLNkYs2VfIVPeHKSeJNdyn2IHs1XqkwUt9yiShBcLsOCgF7mjkHL1bAX/+F5SAWkHzpMriCzIYh6YZ8kqLPNAww== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=XTr7hfM/loIPZYF3pcxR/mdn9ADD2eXjRTweJQ2y9BQ=; b=yiyvO5UjENi5C62Q+UNbehyALt06hRZjXBUH1Nqt0AxKehfjf/wOx8dI2K5l4pqZUozu2NGgIHT6p9TiFK3fAgEq7D/nt3QGMn1ZFyndpiSBdzYBHbwVhDyqgsr8re6m6AgIGifOBP0A+E21bD/L584Lk3KTjefqiRxu2ZV1xc4= Received: from SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) by SJ0PR10MB4525.namprd10.prod.outlook.com (2603:10b6:a03:2db::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3805.17; Wed, 3 Feb 2021 05:41:09 +0000 Received: from SJ0PR10MB4605.namprd10.prod.outlook.com ([fe80::b1e9:811d:aa8e:f62a]) by SJ0PR10MB4605.namprd10.prod.outlook.com ([fe80::b1e9:811d:aa8e:f62a%6]) with mapi id 15.20.3805.028; Wed, 3 Feb 2021 05:41:08 +0000 Subject: Re: [PATCH v6 8/9] OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection To: Laszlo Ersek , devel@edk2.groups.io Cc: imammedo@redhat.com, boris.ostrovsky@oracle.com, Jordan Justen , Ard Biesheuvel , Aaron Young References: <20210129005950.467638-1-ankur.a.arora@oracle.com> <20210129005950.467638-9-ankur.a.arora@oracle.com> <2c25a9d0-38dc-754d-fcef-b372e1211a92@redhat.com> From: "Ankur Arora" Message-ID: <905bc1e6-c499-6ae5-9e61-ca12c61a5c94@oracle.com> Date: Tue, 2 Feb 2021 21:41:03 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 In-Reply-To: X-Originating-IP: [148.87.23.11] X-ClientProxiedBy: MWHPR17CA0082.namprd17.prod.outlook.com (2603:10b6:300:c2::20) To SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [192.168.0.108] (148.87.23.11) by MWHPR17CA0082.namprd17.prod.outlook.com (2603:10b6:300:c2::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.17 via Frontend Transport; Wed, 3 Feb 2021 05:41:06 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: d16790c8-d62d-4b28-356e-08d8c8064e80 X-MS-TrafficTypeDiagnostic: SJ0PR10MB4525: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Xj9iwCrbHVPkjxVOa0FbtEqNADE/TvB096WLIT1LWPl/mjSqTGNBtZcnNCDL3SX132aJX8OCNi2cpAbirg4gSBqn8i2gzMaldVR7qApxNk3WW0/bUWbd6sK+j25CafV4jreFntn/BQKk+zgZgwOjZYHSEpovkkiNqkleHHulV61VK6djv4DyIUXCodsO1PsOXbOLcUPzMel72j4gMWPvwrUvePkQIy/uinoaOSE4d41Co0Xb03af2T3OFYBnjHWfQuT4g6vsqubkEpoRdlB8S3NF6112146Gf3B1pyObk/vN3Q6ixmwx+jhvXJ7ZzDd5Acag43Jwib6RAx8BQxVAtRZZx0AYw69Rl+Ep+JU3L/OY0PBCSmGdxP8cBWy8wYC4Cbb0TTjVAx/ispvyNlXMiPQx+ZWi4B6vgtxi+S7+wIsVcYQom/6Z2D3/ld7Uw3G2Zi93HLOsUwiY2ajDSVi2F6g5DjqVZfAU2vzKrKCNrXBi3usFPRTakrJ518K//z1oRAUrvxPUpKHCG3Oh7x9gs0PaVp6WDEQcd1tkimyeT0GAf8FyEfU3Kz13oJK5bmfUht8PGvN5K1DBdg4/vcAyHExfa86UHieYUaE0zPnbUeUkj6w+0+dJplAj49WkTeM1+2mWJyxw1sK+7dTI8qkZmiX6L+J+ZHR1IZumJYMMhBFy52ZhaKEzNcTNUESKk5nz X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SJ0PR10MB4605.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(39860400002)(136003)(366004)(376002)(396003)(346002)(86362001)(36756003)(2906002)(4326008)(53546011)(966005)(186003)(478600001)(16526019)(19627235002)(66476007)(316002)(66556008)(6486002)(26005)(8936002)(83380400001)(5660300002)(8676002)(16576012)(54906003)(107886003)(31696002)(2616005)(31686004)(956004)(6666004)(66946007)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?MXUvYk0xZml0RCt1RmNwOHBlczRkaXlYNWZxQWx5dEw0REpJanY1cS9kcmJr?= =?utf-8?B?Snd3MG9IMW5oVFJnN0YzN1Z4cDN6Y1lHNjVvYkpqVEd3ZGJoeDdvU3daQVQ1?= =?utf-8?B?NWZyQzJFQkJSQmNLYm1jU3JjRFNOMytCUnlJVUdUa2EyNFBmM3J2Z1M0eEx2?= =?utf-8?B?VWRvTmEwMzRGUFpqTFMrb1FtNENwSlMzNUw0ckQzVkE5N2VNOW5Uc1FsdXJV?= =?utf-8?B?ckI4TlZ4eU8yU2dGeFNBcXNna1d1bmFhYXJPQUZ5cWU0QWpmUUI4UWxla1NG?= =?utf-8?B?UEQrK1BNelJ5NnBiUlhBTkw5a1I3ZVlwTmxsOVc2Q0JYSVNuT0dFak50S3Nw?= =?utf-8?B?c0Y2dEs4ZmY5RWlBa1BzYmxNMzJySndIc2dGSGpQU3NqeVlDQmNub1AwVlNa?= =?utf-8?B?NlFQTkkzTGRIb2pCeVFNMk5IMXlOWTBaMU5iWFk1L0JudDNWYXd5VUNHOCsz?= =?utf-8?B?UTczUHBDejNrZittVVFRREFMbEVvMWNiNjNtUlBDYzBScDZVa3lrRU5hc1NN?= =?utf-8?B?M3FWVlhicEszM3hHWC9Za1pIRVVrK05WQkx0cENnZ1FBSEZIRTR1STk4VFlL?= =?utf-8?B?bGkwZ0htS0lsVWY1QW1sUUlYcmVhUXFzQXFPU015SUs1L2JmQkVzYjhTRGhr?= =?utf-8?B?MDMvM1M0c3FXSy9VYzR5SlJUbEc4b01aMGRPbE81T3VZWDZpZXB0ZStVazlt?= =?utf-8?B?ZndjMDg3NkZTQ1lydXRQNG1mcXNDRzNNeFdzOTFzcTlld2o4eTNNdUgrQUV3?= =?utf-8?B?WEE0M3RHU2I0clc4cTI4TWNmSnNJNVV4QTFjZWtCcE5ScHNJTmNkK3RVendS?= =?utf-8?B?cFczdFdnWS95bkNNdmRnejVKdHVsTFNHcldmWE54OWQzQUQrRjVPTkd6TzMz?= =?utf-8?B?MWpzMU9nWEdNdTh5RmZPKzJPcHd1Q2FSR0JWOEk3WFhmL29jbGpHRXJWVCtM?= =?utf-8?B?dzRPUUJxcnBDS2Yyek9NbloxVkI2M3BDVU5FdDNaSUhqdlJqUjJpdFB1OCtn?= =?utf-8?B?b0x4Y2loOStLeGZaT2FYUDkyTWh4VVRpTytWcXh4UjU4VHdvRkg5eFNXMkxX?= =?utf-8?B?TUpoZXg5eUpSOTMrWHN6WlhCajZFb1M2UnV6dU9JT1NWdW1JMVlxU2hBS0dP?= =?utf-8?B?Y2srQWFaZm93ajZuZFRvc01hR1IrUk1qRlFUTGFXSjZ2K2xqOWtzaEUwK2pP?= =?utf-8?B?WmU4MUFtWjhITDk0K0IxV2o0bHZPV1pUUDgrdkNhWllHNzRDTTZSUTkzbzJE?= =?utf-8?B?MXVRVEVlMnVwWTkvVDl0R2lMTUlwMU5zS0kyTjZJQXdkeTlreE9kSlRsOVVv?= =?utf-8?B?TmZRRmdSamY5a2ltR1RjdVJGaFM4OWt3bXJlaEdHMGJtbERNMHpFS0VNNFBx?= =?utf-8?B?WGxZT2kzbTkwVHhMOW9YMVhlSDd3dEQ1bHFtWEhNYU1KcVZaRkZmNW9QMU9i?= =?utf-8?Q?uCGLw89D?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: d16790c8-d62d-4b28-356e-08d8c8064e80 X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4605.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Feb 2021 05:41:08.7340 (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: DYquGsWacSg8hWFQhXqRUh/c3w1BiMfdn7CiTc/NXGa5uZhEam0fGkI5FtO0zfAQ8GEUPeeHBFD+qT+3kKMACmiaWgmOmj8doZeXGXLSuT0= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4525 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9883 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 mlxlogscore=999 phishscore=0 spamscore=0 suspectscore=0 malwarescore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102030034 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9883 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 impostorscore=0 lowpriorityscore=0 spamscore=0 priorityscore=1501 suspectscore=0 phishscore=0 mlxlogscore=999 malwarescore=0 clxscore=1015 bulkscore=0 adultscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102030033 X-MIME-Autoconverted: from 8bit to quoted-printable by userp2120.oracle.com id 1135XqkS023979 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 2021-02-02 5:23 a.m., Laszlo Ersek wrote: > On 02/01/21 20:21, Ankur Arora wrote: >> On 2021-02-01 9:22 a.m., Laszlo Ersek wrote: >>> On 01/29/21 01:59, Ankur Arora wrote: >>>> Designate a worker CPU (we use the one executing the root MMI >>>> handler), which will do the actual ejection via QEMU in CpuEject(). >>>> >>>> CpuEject(), on the worker CPU, ejects each marked CPU by first >>>> selecting its APIC ID and then sending the QEMU "eject" command. >>>> QEMU in-turn signals the remote VCPU thread which context-switches >>>> it out of the SMI. >>>> >>>> CpuEject(), on the CPU being ejected, spins around in its holding >>>> area until this final context-switch. This does mean that there is >>>> some CPU state that would ordinarily be restored (in SmiRendezvous() >>>> and in SmiEntry.nasm::CommonHandler), but will not be anymore. >>>> This unrestored state includes FPU state, CET enable, stuffing of >>>> RSB and the final RSM. Since the CPU state is destroyed by QEMU, >>>> this should be okay. >>>> >>>> Cc: Laszlo Ersek >>>> Cc: Jordan Justen >>>> Cc: Ard Biesheuvel >>>> Cc: Igor Mammedov >>>> Cc: Boris Ostrovsky >>>> Cc: Aaron Young >>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3132 >>>> Signed-off-by: Ankur Arora >>>> --- >>>> =C2=A0 OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 73 >>>> ++++++++++++++++++++++++++++++++++---- >>>> =C2=A0 1 file changed, 67 insertions(+), 6 deletions(-) >>> >>> (1) s/CpuEject/EjectCpu/g, per previous request (affects commit messa= ge >>> and code too). >>> >>> >>>> diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c >>>> b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c >>>> index 526f51faf070..bf91344eef9c 100644 >>>> --- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c >>>> +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c >>>> @@ -193,9 +193,12 @@ RevokeNewSlot: >>>> =C2=A0=C2=A0=C2=A0 CPU Hot-eject handler, called from SmmCpuFeature= sRendezvousExit(), >>>> =C2=A0=C2=A0=C2=A0 on each CPU at exit from SMM. >>>> >>>> -=C2=A0 If, the executing CPU is not being ejected, nothing to be do= ne. >>>> +=C2=A0 If, the executing CPU is neither a worker, nor being ejected= , nothing >>>> +=C2=A0 to be done. >>>> =C2=A0=C2=A0=C2=A0 If, the executing CPU is being ejected, wait in = a CpuDeadLoop() >>>> =C2=A0=C2=A0=C2=A0 until ejected. >>>> +=C2=A0 If, the executing CPU is a worker CPU, set QEMU CPU status t= o eject >>>> +=C2=A0 for CPUs being ejected. >>>> >>>> =C2=A0=C2=A0=C2=A0 @param[in] ProcessorNum=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 Index of executing CPU. >>>> >>>> @@ -217,6 +220,56 @@ CpuEject ( >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return; >>>> =C2=A0=C2=A0=C2=A0 } >>>> >>>> +=C2=A0 if (ApicId =3D=3D CPU_EJECT_WORKER) { >>> >>> (2) The CPU_EJECT_WORKER approach is needlessly complicated (speculat= ive >>> generality). I wish I understood this idea earlier in the patch set. >>> >>> (2a) In patch #5 (subject "OvmfPkg/CpuHotplugSmm: define >>> CPU_HOT_EJECT_DATA"), the CPU_EJECT_WORKER macro definition should be >>> dropped. >>> >>> (2b) In this patch, the question whether the executing CPU is the BSP= or >>> not, should be decided with the same logic that is visible in >>> PlatformSmmBspElection() >>> [OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.= c]: >>> >>> =C2=A0=C2=A0 MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; >>> =C2=A0=C2=A0 BOOLEAN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = IsBsp; >>> >>> =C2=A0=C2=A0 ApicBaseMsr.Uint64 =3D AsmReadMsr64 (MSR_IA32_APIC_BASE= ); >>> =C2=A0=C2=A0 IsBsp =3D (BOOLEAN)(ApicBaseMsr.Bits.BSP =3D=3D 1); >>> >>> (2c) Point (2b) obviates the explicit "mark as worker" logic entirely= , >>> in UnplugCpus() below. >>> >>> (2d) The "is worker" language (in comments etc) should be replaced wi= th >>> direct "is BSP" language. >>> >>> >>>> +=C2=A0=C2=A0=C2=A0 UINT32 CpuIndex; >>>> + >>>> +=C2=A0=C2=A0=C2=A0 for (CpuIndex =3D 0; CpuIndex < mCpuHotEjectData= ->ArrayLength; >>>> CpuIndex++) { >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 UINT64 RemoveApicId; >>>> + >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 RemoveApicId =3D mCpuHotEjectData->A= picIdMap[CpuIndex]; >>>> + >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if ((RemoveApicId !=3D CPU_EJECT_INV= ALID && >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Remove= ApicId !=3D CPU_EJECT_WORKER)) { >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // This to-be-ejected-CP= U has already received the BSP's SMI >>>> exit >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // signal and, will exec= ute SmmCpuFeaturesSmiRendezvousExit() >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // followed by this call= back or is already waiting in the >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // CpuDeadLoop() below. >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // Tell QEMU to context-= switch it out. >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 QemuCpuhpWriteCpuSelecto= r (mMmCpuIo, (APIC_ID) RemoveApicId); >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 QemuCpuhpWriteCpuStatus = (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); >>> >>> (3) While the QEMU CPU selector value *usually* matches the APIC ID, >>> it's not an invariant. APIC IDs have an internal structure, composed = of >>> bit-fields, where each bit-field accommodates one hierarchy level in = the >>> CPU topology (thread, core, die (maybe), and socket). >>> >>> However, this mapping need not be surjective. QEMU lets you create >>> "pathological" CPU topologies, for example one with: >>> - 3 threads/core, >>> - 5 cores/socket, >>> - (say) 2 sockets. >>> >>> Under that example, the bit-field standing for the "thread number" le= vel >>> would have 2 bits, theoretically permitting *4* threads/core, and the >>> bit-field standing for the "core number" level would have 3 bits, >>> theoretically allowing for *8* cores/socket. >>> >>> Considering the fully populated topology, you'd see the CPU selector >>> range from 0 to (3*5*2-1)=3D29, inclusive (corresponding to 30 logica= l >>> processors in total). However, the APIC ID *image* of this CPU select= or >>> *domain* would not be "contiguous" -- the APIC ID space, with the >>> above-described structure, would accommodate 4*8*2=3D64 logical >>> processors. For example, each APIC ID that stood for the nonexistent >>> "thread#3" on a particular core would be left unused (no CPU selector >>> would map to it). >>> >>> All in all, you can't write the APIC ID to the CPU selector register, >>> for ejection. You need to select the CPU whose APIC ID is the APIC ID >>> you want to eject, and then initiate ejection. >> >> Yeah, this is a clear bug. Should have seen it earlier. Thanks for >> pointing it out. >> >>> >>> This requires one of two alternatives: >>> >>> >>> (3a) The first option is to keep the change local to this patch. >>> >>> This alternative is the more CPU-hungry (and uglier) one. >>> >>> The idea is to perform a QEMU_CPUHP_CMD_GET_ARCH_ID loop over all >>> possible CPUs, somewhat similarly to QemuCpuhpCollectApicIds(). At ev= ery >>> CPU, knowing the APIC ID, try to find the APIC ID in "ApicIdMap". If >>> there is a match, eject. >>> >>> >>> (3b) The second option is much more elegant (and it's faster too), bu= t >>> it requires a much more intrusive update to the patch set. >>> >>> First, the *element type* of the arrays that QemuCpuhpCollectApicIds(= ) >>> operates on, has to be changed from APIC_ID to a structure type that >>> pairs APIC_ID with the QEMU CPU selector. [*] >>> >>> Second, whenever QemuCpuhpCollectApicIds() outputs an APIC_ID, it sho= uld >>> also save the "CurrentSelector" value (in the other field of the outp= ut >>> array element structure). >>> >>> Third, the element type of CPU_HOT_EJECT_DATA.ApicIdMap should be >>> replaced with a structure type similar (or identical) to the one >>> described at [*]. The ProcessorNumber lookup in UnplugCpus() would st= ill >>> be based upon the APIC ID, but CPU_HOT_EJECT_DATA should remember bot= h >>> the QEMU selector for that processor, and the APIC ID. >>> >>> Fourth, the actual ejection should use the selector. >>> >>> Fifth, the debug message (below) should continue logging the APIC ID,= to >>> mirror the DEBUG_INFO message in ProcessHotAddedCpus(). >>> >>> >>> Would you be willing to implement (3b)? >> >> 3b is clearly the better solution. However, is there enough value in >> the print message containing APIC ID, that CPU_HOT_EJECT_DATA.ApicIdMa= p >> carry both the cpu-selector and APIC ID? >> >> As you say, the ejection itself just needs the ProcessorNum -> QEMU >> cpu-selector mapping. >=20 > Good question, and I'm torn. >=20 > The default DEBUG build of OVMF enables INFO messages, and more severe > ones. It does not enable VERBOSE messages. >=20 > In such a DEBUG build (i.e., with otherwise default settings), the log > entries that relate to hot-plug do not report selectors. Conversely, on > hot-unplug, the log would report selectors *only* (not APIC IDs). I fee= l > this makes it more difficult to get useful OVMF debug logs in bug > reports, especially from users of distro-provided OVMF builds. >=20 > If we can ask the user to rebuild with VERBOSE enabled and re-run the > test, that's always great, but frequently users don't know how to > rebuild OVMF, plus usually OVMF is hidden so deep in their virt stack > that they have no idea how to make that stack use a custom OVMF build. >=20 > ... How about this: in UnplugCpus(), we could emit an INFO message abou= t > the ProcessorNumber <-> APIC ID <-> Selector correspondence, and when > the eject happens, it would suffice to log (at INFO level) only > ProcessorNumber <-> Selector. This makes sense to me. Will do. Thanks Ankur >=20 > Thanks, > Laszlo >=20