From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from userp2130.oracle.com (userp2130.oracle.com [156.151.31.86]) by mx.groups.io with SMTP id smtpd.web11.6901.1614065865084357637 for ; Mon, 22 Feb 2021 23:37:45 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@oracle.com header.s=corp-2020-01-29 header.b=Cg0vkQWU; spf=pass (domain: oracle.com, ip: 156.151.31.86, mailfrom: ankur.a.arora@oracle.com) Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 11N7ZbOu117925; Tue, 23 Feb 2021 07:37:41 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=iid59wkOr2YIX9IbehpdiJka+0JT1ODnnEMopMKtuuk=; b=Cg0vkQWUKHnADN8wShbYeyTmTAAxaBCHyt4vYrc9NVN2OfR/i513C+Bh7v/SToQJouup 3MVKYMDA9NiHt/pzh57MgDOHtAfDu28rcI375IaVKvHykSz8W2pVzI8D5yaBwNTYPvML rLv4VPKkNOX4beK1FCS06V6WQnfKu/Rr49V2SAdDAcq2028Ye6Yn62NjLLTLS+WQRV5p 7wRLCfSKTKKqYQcTrEfzRo7Z+aaypRkYTl4SPEm6Xe14TPBBMhH4gV2+eZf2warClncC nwmolFRzCdnR68teoOofozzUOdQKskS4T++MCVUWByIpI9iieS3g9ShOfs3HInifTuSY Hw== Received: from aserp3030.oracle.com (aserp3030.oracle.com [141.146.126.71]) by userp2130.oracle.com with ESMTP id 36tsuqx7m8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 23 Feb 2021 07:37:41 +0000 Received: from pps.filterd (aserp3030.oracle.com [127.0.0.1]) by aserp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 11N7aL3W151939; Tue, 23 Feb 2021 07:37:40 GMT Received: from nam12-dm6-obe.outbound.protection.outlook.com (mail-dm6nam12lp2173.outbound.protection.outlook.com [104.47.59.173]) by aserp3030.oracle.com with ESMTP id 36v9m47egq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 23 Feb 2021 07:37:40 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UjBn1QiE7wdrDCeVoPOEMznTjP/Onxi2D8ILM4f0m8g83cqh3ZrSWcRrsRSafIUGMu70qDu4WKFPf8GUMZ3nVVBYa/Q21DmwOS6Y4KfazIH3dbOmvj/KblcZw+lw0TzTp0ajEiIF2P3JvmDKRloIIODBysmJxBFaY06FBmRdRZ3qE+C90bzRH+BhHR/qwdXa5lw0/qM21SPCCa9kF8ieLiWh2jPUAu4zuTx4u4hA49gBk5w2lZKOkmd4O7pWXzHob54QqpQgzAPqEUExzW4sJpiwmkfejgFSGCq0RkRd51x105ds2z/hIDzw44iL0lfXHxM5gW0x/etL+EfyBKIrtA== 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=iid59wkOr2YIX9IbehpdiJka+0JT1ODnnEMopMKtuuk=; b=ASGAtR0gY5o8DSyih2sPknUf8nc8avhKqqCjxEeMUnf7UGXry1MI8vL2RXukbK7uAWmO7pSGCyJ4HqsJkwpFvJstSMhAGMD117oOXMkpwRZSSuIvZUgv19W5WhyQLVa0Yqm45LR3XzvSP9AuOmbWwvU9ASNXRJc+AZbwiLZ0VU0nHlLEwkZS1gok5zFxVCGnWCvXMu0tUj6rfOLG3uJJWhglG717aqslod58+uxTJWa1hDG2VATUL+eJlFV51vQ5QnlwdE8X2gbZwhHwhFW+Uu/ogJJA3jb4IDHxbCcEIt7uUOm1W0pmnPthuc9lH5jQoUZhtZQNsJR/3ulYs3nNkQ== 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=iid59wkOr2YIX9IbehpdiJka+0JT1ODnnEMopMKtuuk=; b=JFD1dPE8HSEHDrWD0GRH5P2Et/ng4n5wV8LsS3MeAL7XKo90HJTQGX35PkrOdGsrOIQdBJUiHLa88OGk6MtBwmpgfG3RqxxbNHuVuaedk6lXEjBgmw/eeHbew7Xm0acq4lWERL8lFtW4aKK/6qSNMfE3aub5cLNrNAYupgrJHzE= Authentication-Results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=oracle.com; Received: from SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) by BY5PR10MB4259.namprd10.prod.outlook.com (2603:10b6:a03:212::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3868.33; Tue, 23 Feb 2021 07:37:38 +0000 Received: from SJ0PR10MB4605.namprd10.prod.outlook.com ([fe80::a021:790:7ce6:6f16]) by SJ0PR10MB4605.namprd10.prod.outlook.com ([fe80::a021:790:7ce6:6f16%6]) with mapi id 15.20.3868.033; Tue, 23 Feb 2021 07:37:38 +0000 Subject: Re: [edk2-devel] [PATCH v8 07/10] OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler To: Laszlo Ersek , devel@edk2.groups.io Cc: imammedo@redhat.com, boris.ostrovsky@oracle.com, Jordan Justen , Ard Biesheuvel , Aaron Young , Paolo Bonzini References: <20210222071928.1401820-1-ankur.a.arora@oracle.com> <20210222071928.1401820-8-ankur.a.arora@oracle.com> From: "Ankur Arora" Message-ID: <75a58bb2-d5a0-965e-e5e9-ceb2af6c0410@oracle.com> Date: Mon, 22 Feb 2021 23:37:35 -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: [70.36.60.91] X-ClientProxiedBy: MWHPR11CA0025.namprd11.prod.outlook.com (2603:10b6:300:115::11) 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] (70.36.60.91) by MWHPR11CA0025.namprd11.prod.outlook.com (2603:10b6:300:115::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3868.28 via Frontend Transport; Tue, 23 Feb 2021 07:37:37 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 0065142c-3316-4eb3-07ab-08d8d7cde517 X-MS-TrafficTypeDiagnostic: BY5PR10MB4259: 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: sFCU9Amxk7eFpYrNA+4tBTqPLsMpt4uXOn1+KgsUqrXAUiUjOI12d1xpRWYaqGs8A1Q36YQDt9c/5/dc5e+nT1BaQdt5pOkH53YlUIq8pLIB6b9ngME0p8mtd+eJZV1lr1PhFIImTKSiGit+UKYHzZlwns1t8Wpw99OWVzpVqpCfV7VM5/sT9fgtMNcVcYhW3LVXd505H6GYfCFOaeKOLLXilmJhShEyE7gP/ho74dInkfAxYgAVJa5jAEr3y31PvUa10RWZETfDImn+RqQvpXVEKS+h+AUXCdX/YHHmbnLCHOUpmkClasTox+FDhJU/T74J5xoUv6rADWdOdk/ae0FFi3Ojc5V4Uh/YGawqtkX0BlZGC3LT2wHVtLqazVGWcAw4AK68vMaN8hNpAGTQsPfuypTf+JkI6P4EdSTt9BOdeLBWvkqBCNPQ9DMAt/QTB94pL/9RPE4PfFOzyxjV7/kXUrKe6Yz8+78N/xdibXvMAODNhB+OZ4utTLtcKwxRA1s0ZRqxNwOWOjZQOy/4UgU5D+gAf/Q1CG2/5ZDMxxxHgsgyyQOUaff3GXba3EFIEnFJ89GZ2/Wuy4JsXIDKMQX+8zOUsTAgip9+AGbXcMgIOMyIQLjbKct0PmwmO8UCukInMSMVv8iS/cbkBveroo1NszQuv3lW6ZcDbWk8i079V06V1mRh1daWhEbZtiii 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:(396003)(376002)(136003)(366004)(39860400002)(346002)(956004)(83380400001)(6486002)(316002)(16576012)(5660300002)(4326008)(2616005)(36756003)(478600001)(86362001)(2906002)(31686004)(8676002)(186003)(8936002)(53546011)(66476007)(16526019)(66946007)(26005)(66556008)(54906003)(966005)(31696002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?YVRsRW44S1dhVlIrOXNBcms0Uk1SMUZ3cXQ0ZnlLbm5FZHR5Q1Y1MzlyRnlN?= =?utf-8?B?RDNZR0IrZHdCYXd1WlRKVkRsWnFKQVdBaW82UGZNMWVEeDVaU0IwWDhQYzhl?= =?utf-8?B?bDB5WHd6SkgzQkcwYnZ0NlluekVhYmR4SXpJNFI1VXdOeDN3RnZSOFJiT3Mw?= =?utf-8?B?bGIvNVVVMHNyVWVudWo5TzE1MzRrRU9xREZXcERSWjd1MkloOFkwNTlER25C?= =?utf-8?B?Mk0yZFZ0VUpaVUNDdldjazBDenlMZU1lK2IzWXc3MTF0aFJqOVhVcHRsbXFu?= =?utf-8?B?TS9TdUNUWVpEOFZsV2J3OHhYWFR5VzNiaDl5SWh1Nit2Vnkrd3FEVG9hcG82?= =?utf-8?B?ck5GQUdrK28zUjFQbkx5T0JRcEZmQUhqd2YzOHovVHFtY3E3OGcxbndYb1dl?= =?utf-8?B?NlFPZ2o5SGUvYkpQdFlHOXhuVWlPM25LNFZNbEloUzJlSG5PcEV5Y0hyeWZR?= =?utf-8?B?d3ppdHNGM0c2bWlDR1NkU3B2RURBOThQSFdnZkFzR1UrRjQ3czdiTk92ZGVN?= =?utf-8?B?SFJEdUdPa05sMXY5WjRqN0pTQkFMeXVjMXY0NXh3bHdkNGRiNzRsNkplSVlM?= =?utf-8?B?ZHFBbFBYTG8va3RMdWh1a0NIczFLd2ZKK29TQ2xHb2NQbXRuRnd6NFJXUkZ6?= =?utf-8?B?c3dWTC9xaEJURzNrNjVQTjZoeWlGcWRwZnJrRk8zS2NRWmNjV244YVNFOUNZ?= =?utf-8?B?N3Vua21CdzIyWnZXNjBUc0hzL0Eydi9oa3gvcmNVTlBtNE9ZVFVIam1LRFlJ?= =?utf-8?B?VTVpRzVFNkROK1hTZWpTNFJ4bmxqQ1lHelpETXFCci9Oa2R0L2VGMG1PK2hE?= =?utf-8?B?a09ySVlQbFhsMU51RWgySHptMDAvVXhINXAxOEZMaklBck9yWk5LNEZvbG43?= =?utf-8?B?SjBCbTYzR1dsY3g0UzkwdFZQdk9JUXZpdGgyVjZJVkdqNURaUXQ2M2NUK1NF?= =?utf-8?B?MHlZbGFRV0NOaCtTNFdETHAyMFgzYjZFN3pDeHRIS1o2WlBnZGRtU1NmendX?= =?utf-8?B?ZkZjWjZESk9DRkk4TEtuQlk5cHhENEk3U201NlVjL0d3cUVUNWgzUUxweHF0?= =?utf-8?B?U2JoaUhoNW5YMGlMbW1Hc3N2ZHd6UEdWUjBsRHc3Sit1ZEc1VmVnNW1JZW5u?= =?utf-8?B?WlFzS1pHZ1Vtd3ozS3Frd0ZOZ1UzZGJjYk05MElFQlBVU0prVFFXM3RsUzJ4?= =?utf-8?B?RllESGJFc2dScG44UXhEbE9HZzlqbVpvQ2FmSkVSS2Z4dnljRitLTVIvd1Yy?= =?utf-8?B?YXM0Z3MvWFd5ajZyNjJqRlMrNDVHQ2VHamVIc21vM2hWa3FNL29GbGtTL1hR?= =?utf-8?B?bEZxcVd2Tk5sdmxJcFpwMmpPeVFwWVZ5SVkzS0pmT1dKTGJrVUlWejNGdnU5?= =?utf-8?B?ZWdUaFRDMHVjU1BqZ3RyL1c5cGovZWdHSXpXMjJodUZlYnc5TVU1V1NUbngv?= =?utf-8?B?eVBMQS80K2R4TUZ5Qi9qZU9SWDFTWWVsdHdBY1J4YllkSFVFcGtFN2NZUytu?= =?utf-8?B?MkR6WjFKY2k3L0VUTlFKSlN1U3UxQ3ZuY3JKNDhtR041bmVrMlordVJzZWcw?= =?utf-8?B?ckZnUVFibklhU0pDZjZwN1RISjN2UWpqYU84OFA5UjBiNTZlNTFJQ05CZG1S?= =?utf-8?B?UXNQZmdqYzh2ZldNZi9tRlQ5NVBvUDFUQkRkTURCTVROUmlDUndTODVldWpT?= =?utf-8?B?Sis2UWtDb3dlL0RTelI1NTJTaGtyM21rQjU0d0pzWUdLbVdabW9HeDJrWDRY?= =?utf-8?Q?OKGJNFrLv6UtpepLrzSS7/cKNGS2GHLtyv9zICu?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0065142c-3316-4eb3-07ab-08d8d7cde517 X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4605.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Feb 2021 07:37:38.6277 (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: 2bNttdJ7tIITPQRNKhyT2p9BUaYZTybGwQvkhEO2tQ3QYhe7X7WnnBZCVpoS2uyeXMvuguFaINwcJARyO/DPWnbuNQmEbrjm/rak3P9SCuE= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4259 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9903 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0 suspectscore=0 mlxlogscore=999 mlxscore=0 spamscore=0 bulkscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102230063 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9903 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 spamscore=0 priorityscore=1501 impostorscore=0 bulkscore=0 mlxscore=0 malwarescore=0 clxscore=1015 phishscore=0 mlxlogscore=999 lowpriorityscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102230063 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2021-02-22 6:53 a.m., Laszlo Ersek wrote: > Adding Paolo, one comment below: > > On 02/22/21 08:19, Ankur Arora wrote: >> Call the CPU hot-eject handler if one is installed. The condition for >> installation is (PcdCpuMaxLogicalProcessorNumber > 1), and there's >> a hot-unplug request. >> >> The handler executes in context of SmmCpuFeaturesRendezvousExit(), >> which is called at the tail end of SmiRendezvous() after the BSP has >> given the signal to exit via the "AllCpusInSync" loop. >> >> 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=3132 >> Signed-off-by: Ankur Arora >> --- >> >> Notes: >> Address the following review comments from v6, patch-6: >> (19a) Move the call to the ejection handler to a separate patch. >> (19b) Describe the calling context of SmmCpuFeaturesRendezvousExit(). >> (20) Add comment describing the state when the Handler is not armed. >> >> OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c >> index adbfc90ad46e..99988285b6a2 100644 >> --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c >> +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c >> @@ -467,6 +467,21 @@ SmmCpuFeaturesRendezvousExit ( >> IN UINTN CpuIndex >> ) >> { >> + // >> + // We only call the Handler if CPU hot-eject is enabled >> + // (PcdCpuMaxLogicalProcessorNumber > 1), and hot-eject is needed >> + // in this SMI exit (otherwise mCpuHotEjectData->Handler is not armed.) >> + // >> + >> + if (mCpuHotEjectData != NULL) { >> + CPU_HOT_EJECT_HANDLER Handler; >> + >> + Handler = mCpuHotEjectData->Handler; > > This patch looks otherwise OK to me, but: > > In patch v8 08/10, we have a ReleaseMemoryFence(). (For now, it is only > expressed as a MemoryFence() call; we'll make that more precise later.) > > (1) I think that should be paired with an AcquireMemoryFence() call, > just before loading "mCpuHotEjectData->Handler" above -- for now, also > expressed as a MemoryFence() call only. > > BTW the first article in Paolo's series has been published: > > https://lwn.net/Articles/844224/ > > so in terms of that, we have something similar to this diagram: > > thread 1 thread 2 > -------------------------------- ------------------------ > a.x = 1; > smp_wmb(); > WRITE_ONCE(message, &a); datum = READ_ONCE(message); > smp_rmb(); > if (datum != NULL) > printk("%x\n", datum->x); Thanks for the link (and Paolo for writing it.) This is great. > > In patch 8, UnplugCpus() does the first two lines of the "thread 1" > (BSP) actions, and the third line is covered by the final "AllCpusInSync > = FALSE" assignment in BSPHandler() [UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c]. > > Regarding the thread#2 (AP) actions, line#1 is covered by the > "AllCpusInSync loop" near the end of SmiRendezvous(). Lines 3+ are > covered by our SmmCpuFeaturesRendezvousExit() implementation here. But > line#2 (the AcquireMemoryFence()) is missing. Yeah you are right. Just think out aloud here... without this it is possible that on the the AP, the CPU could reorder loads on line-1 and line-3. This patch does need an AcquireMemoryFence() (or a MemoryFence() and a comment stating that it needs acquire semantics. This also makes me realize that although I have somewhat detailed comments in patches 8 and 9, but I do need to specify which fence needs to have acquire semantics and which release. > ... I'll suspend the review at this point for today; let's see whether > we agree on the comments I've made so far. I hope to continue the review > tomorrow. Agreed so far! And, thanks. Ankur > > Thanks! > Laszlo > >> + >> + if (Handler != NULL) { >> + Handler (CpuIndex); >> + } >> + } >> } >> >> /** >> >