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.31346.1612760665590520187 for ; Sun, 07 Feb 2021 21:04:25 -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=QyogqTzZ; 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 11851PTV046347; Mon, 8 Feb 2021 05:04:20 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=ukywJ12H2R+PY5JPe8VmiTO+EPWNjUOUMMq7KCnl7kU=; b=QyogqTzZ+XRSnBsqRhuwrN4ezTfC7F3arcq+RMz9UBpFJFWYfpDz9rXGoT/1/hS69MDh cohJQsyxtFy8e5QzD+ClufrSkg4I4aAIavUE9WBuM3DnhGpQPjmZxfZ1IqhSyxaTzngc UQ7WtN9FQmLU6p8HOMAZUxj+aN25jbamiq8VvY/PBUJ0ktlq5TXQUjkZgYze6SQInRAy 07au8BWHoesq+AnAgujhqCqExSlef/1EYtS4QgGKQSVo5jpM1qyngkLOELxPLmHBlThv 9upsCoracfYuHKJByQ1qBmiHTY/6oxJwGf1RPPvVGxNygM77Imdla6mhA5XM1lct/P5P 5Q== Received: from userp3030.oracle.com (userp3030.oracle.com [156.151.31.80]) by userp2130.oracle.com with ESMTP id 36hjhqjq17-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 08 Feb 2021 05:04:20 +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 11850uMZ106291; Mon, 8 Feb 2021 05:04:20 GMT Received: from nam12-bn8-obe.outbound.protection.outlook.com (mail-bn8nam12lp2176.outbound.protection.outlook.com [104.47.55.176]) by userp3030.oracle.com with ESMTP id 36j51u3m91-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 08 Feb 2021 05:04:20 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dPuw29kikUcDT215C0JNrK0973sIA9UHey8KExfPjFXCK77EkSzUN6KV593b22OdKCtFSJD6GHsvo0jtIFbfOjpMGQw1XVyAc0L6oxpnF14MAF+loVBymS7OyJx7mUMt7VMfU00IkYlFHG04jMabDl/8wJiB7ZPTYGPjRzNvn43nSkr1M0ctzhr2exGcFA5Qx02F1KNTE51eaybojcSLuLGQDMaJ5Nw8lupNR2sCa4jRVM8l4H0GKmzTXgBsU6RSZMHegxdI4LHJ3IfrvG+5e5P/E0VOg7VbFbDs/2oURpGRCJ5Txes1nRNtSISnke2X0vlUHQ+58N1kveS/oFyKFg== 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=ukywJ12H2R+PY5JPe8VmiTO+EPWNjUOUMMq7KCnl7kU=; b=XFThna7589YOtEPYr1uAwOGlQpKalHIEW9fsxropzhFQ6iLXOtZSzXdD4zcyiViDLDYOovdKBsCd3B7+HFlPRAm5UnMwSQlGGlwUFnquRQuTyIuCdyUWBfP8PYsXf8Of1+O535a7zltJ/8my4kSHBWASqpFq+GyZWH9Hs3uzOBdYsTWBK+uS5iCsnwMxKD4Lq7kEIohcs0FOyljI0ieujX67ur7BInblMPn0WYPoRqXpGBzIpye6C+T+JT8RfzfDKNoY/VY6Jho/+7NCZt48z+/45ERF5qfsLypMB7j92FG8FfTHSoknKWTYVz4X+6Wp5j9QxVO8timXcRR/EF2vyA== 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=ukywJ12H2R+PY5JPe8VmiTO+EPWNjUOUMMq7KCnl7kU=; b=f1GxU9r9PgXkdS6sOfS++Oo5WTxlQHbnQTq7btNxGvQt4DJ/dHIGPPMQ/znh6S0aJYEqSvme+a934JZ5yYmFK7R3+p5BFgZmRiefuvaOnKNFlzaC51izcPaYjrLdj4wCqbqhTTMLpKm688NuhPTzN8hZGbbb/RY5L1Ju/yg6rw0= Received: from SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) by BY5PR10MB3780.namprd10.prod.outlook.com (2603:10b6:a03:1ff::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.30; Mon, 8 Feb 2021 05:04:18 +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.3825.030; Mon, 8 Feb 2021 05:04:17 +0000 Subject: Re: [edk2-devel] [PATCH v6 7/9] OvmfPkg/CpuHotplugSmm: add CpuEject() 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-8-ankur.a.arora@oracle.com> <14068eb6-dc43-c244-5985-709d685fc750@oracle.com> <039138dc-c20b-39fb-8c14-953c090b8961@oracle.com> <35f1e98b-7a6a-ca94-881a-59cf49015be7@redhat.com> From: "Ankur Arora" Message-ID: <47b590e4-edad-0c23-39f0-dfc0c23cfd9b@oracle.com> Date: Sun, 7 Feb 2021 21:04:15 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 In-Reply-To: <35f1e98b-7a6a-ca94-881a-59cf49015be7@redhat.com> X-Originating-IP: [70.36.60.91] X-ClientProxiedBy: MWHPR12CA0052.namprd12.prod.outlook.com (2603:10b6:300:103::14) 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 MWHPR12CA0052.namprd12.prod.outlook.com (2603:10b6:300:103::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.19 via Frontend Transport; Mon, 8 Feb 2021 05:04:17 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 632701a6-d8ef-4062-758c-08d8cbeefcb3 X-MS-TrafficTypeDiagnostic: BY5PR10MB3780: 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: mucpFcWKiinfLI3bhbRI1XGySEWTVZZNJT9ff5szehSRRCCoqfzcIQNfMMD8iggsF6uPU0Zh06BNxMqkJdB6K1kncBgTMR+5RcOTXJNjiBJbjKz+3p1qrJk5LUEBoCuYPR33Qm83BtXsScMFdKNHNMNAAPrCGeGYNSPcWxvOpo1CQs3mr4A7YhK52L6ieLlqkuwT0QHxioouh0gIDUqSBYjVvBu111kHJZYe7sjTcCjqsx4iLB8xo6tceamqiAanGCTS7yQcLKNPlDU3+wlZ/kxknPFVvXmZRnhxvJFOWplzvpCqpR8EHsEMGcVs8zSpjjQ+quq2ne7XSWCwgOnMN/eluP5k560xp0WsupUMujoSjZ474UWdx60HO1UrUp3TkmdMMxCj0F6Kf5McBlaPwQV39go31YmEpZTtHJp5O0YhrHos/Ner+DRzJYN9uXRSU/g+Qbt/ilr/CXFpTRDJHSv3lSWsvc73tnLFCIwOeSiifmipey6lWAJ3ePpxsYgputpq0g6fnvskyZ8+pPLi8r6vZQfra9sG7ZWNoyY+YMi/dOHSzt3lDY0Jz0JQsHXufKl5d0Decv7tukiqgVLWeSfKAoc+eJvHTIJ34gJoVjJVl3DE1HkkHyRq8cx5fhu5lezvA/HEYBhMmPTVW2zPosUkjmHnDakgWxHWrf4Vr4Su+bjnwLSo6Af4H8vWdSBR 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:(376002)(346002)(39860400002)(366004)(136003)(396003)(31686004)(316002)(5660300002)(16576012)(966005)(26005)(186003)(16526019)(54906003)(31696002)(36756003)(86362001)(6486002)(53546011)(83380400001)(4326008)(107886003)(2616005)(66556008)(66946007)(66476007)(478600001)(2906002)(8676002)(956004)(8936002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?NGhxVWJkdks1dXdIKzVKTENVdWVpcitKUTdnQityUTMvbGJkVHpGdXV0SzM0?= =?utf-8?B?M3B4MThONndXUFRnU1A4TGV3UzZFRlBscGlXcmtEOWtYVmxseS9ubFplNkZM?= =?utf-8?B?ZHcvN3dXQjFpRjVodWw4K2tBcVIwQmJTaFBGbUl3czFLd2psWE1sYzcvcnY5?= =?utf-8?B?NHdwWlcrL1JvSzRJaXowaG9seVIwbExWMTIvR3FLN01zdjJpYXZCQWpIOWpC?= =?utf-8?B?bFQyeUcycGcyQnBGY3VQWmdLOWNvOWVLZmtOT0F2Yk81WCtNblhtME9KYWJE?= =?utf-8?B?cEQyMU5FRnNvZHEvWjdDWWxnVjBhODkrMlpBL2NnbjZtMXZoYTU5dWVUWURv?= =?utf-8?B?ZHQzWGdYcnJjbVJBWHcxaEcxV2xWNVRiK0pRUHdsT1NBTXNNZUZqZkhNMVUv?= =?utf-8?B?M1JJWmJucytsRWp2c3lDUGNDbUkyWSt3WHFNUkFpY0pHWi9SWml0WkVtckpm?= =?utf-8?B?cE1sZWtzc1pTRVhtc0dIV3FZYzhvL28yVlFlcEtmQ1ZsNDdjK0tJODk0eU1X?= =?utf-8?B?VzdXU1F5cUZxU2Q4QW1iQVdqbUJNK3BHU3NsdkI2V3RIVEJzbEFHZXdXWWRp?= =?utf-8?B?WElZUmw2bzhHcDgxTkZaZnllYXRvb2J0eXFleHF5YUNXUWdXWnVsVkhYTkY5?= =?utf-8?B?eFRFeXJoemJkb0JqUCtCWjE3a2FqY0U5QS9FU1V1L1FFRm4rR3pKT0ZOUWFX?= =?utf-8?B?RW56L2VLMFlpYXVrUmNBenh0VHdXditRL2p6aUE5T3MreXRtL0M2VzIvRlNQ?= =?utf-8?B?QnNDWU0yaG5xMXBLNGV0cjhwWFozbVg4Q2lMa3VIK0VQTld6YmlBTERBN1Qw?= =?utf-8?B?OVJHSHBIcWZTTHRjUU95VE5sOFN1RU9pdVdFYzdnNWpkV1dNQlBVREN4cVAx?= =?utf-8?B?Snl1bllaY2pOOCtRS1habzBEVkR1OXcxOGxnc0tNcncxdExoTGZSaVhvaU9E?= =?utf-8?B?NDdnYmxySVhFRWw5dzRiNS9TaVBHbVlUdzlxWkd2MlhjVFJhK2xwNmNkNFR1?= =?utf-8?B?Y21nQXZiV0VxOXd5VC9Da2VHS2sxY2tNUHpCS0tvbHhHdDZiUU5EWHNyc092?= =?utf-8?B?dE96Z2l1ZThXUnFtWlpEWWNmWUNwRlA4am1kOU8zNnVhV0o1eGZFNlFsVVV2?= =?utf-8?B?MHlrN1JyWStkU1BhSU9CYVJzZUJieW5OZDRkRFNJNXgxYUVNa0R1NE1YUEYw?= =?utf-8?B?NFduTjBGc1g2clJGaDg2aVpaOFdTMTZBMC95WVNUL0pXV0FNck50SXBzQnVs?= =?utf-8?B?L2xVd3JpREpYOU0zdnhmdW0zb0kwMFI2UXZhb0lKY1RKMld3MlpZekIrTzZY?= =?utf-8?B?ZlpQMytPZUNVWE93VVBkRjQ5MGlRVWhHdUxmMFFjT3lnRk1pMGtXTDJ5NlFw?= =?utf-8?B?cTZ6QTF0SlF0Z01ZNVZhNXBBU2Vza2laUFlMRFNoS3JpamRDcnR4MlhEVFlF?= =?utf-8?B?UmtmSStZT08zTmdFY01GOXp6ZFpXUDgwQkNIS0YrZmFjTzBTSWxUZFlxVEdY?= =?utf-8?B?cWZydktXTTB3SzVQeU5EZCtBUWp0bDV1L0JpdjZMWmZ3aUtIRnFaUGZzZjhp?= =?utf-8?B?S0I2ZDZTMXFOKy82eTJTYnkzbFBmdlZxaGRoTC9scWpmTERrWHEvNkpzdDRX?= =?utf-8?B?TkFyd0RNRzJyaE5aZ3dtTjJ2eDRXTTRnbjJPYzhhbVVWZ21YbDFFdnNwQU1N?= =?utf-8?B?T0ZlRjR2a0xTSUlnQ2RLRXFGdGU0S2pxQS9DYng3MzlvRm9wQWtFSnk5SU9D?= =?utf-8?Q?y6ZwkSzPG14gl7Y4HOoWXvnTeYd5HIV580f+WaJ?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 632701a6-d8ef-4062-758c-08d8cbeefcb3 X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4605.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Feb 2021 05:04:17.7489 (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: 2J1CH55njMW0OfPJoXEGcX9pWe5LSBaGSOlqhNsYunS9ZonB79ZWVAmm+dUAGFUC+aEHsl5RqKIXDubH1WxlrVA0dgkizG/0XgUL+wvWMT4= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB3780 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9888 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 spamscore=0 phishscore=0 mlxscore=0 malwarescore=0 mlxlogscore=935 bulkscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102080034 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9888 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 impostorscore=0 priorityscore=1501 bulkscore=0 suspectscore=0 mlxscore=0 phishscore=0 lowpriorityscore=0 mlxlogscore=999 clxscore=1015 spamscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102080034 X-MIME-Autoconverted: from 8bit to quoted-printable by userp2130.oracle.com id 11851PTV046347 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 2021-02-05 8:06 a.m., Laszlo Ersek wrote: > Hi Ankur, >=20 > I figure it's prudent for me to follow up here too: >=20 > On 02/04/21 03:49, Ankur Arora wrote: >> On 2021-02-03 12:58 p.m., Laszlo Ersek wrote: >>> On 02/03/21 07:45, Ankur Arora wrote: >>>> On 2021-02-02 6:15 a.m., Laszlo Ersek wrote: >>>>> On 02/02/21 15:00, Laszlo Ersek wrote: >>>>> >>>>>> ... I guess that volatile-qualifying both CPU_HOT_EJECT_DATA, and = the >>>>>> array pointed-to by CPU_HOT_EJECT_DATA.ApicIdMap, should suffice. = In >>>>>> combination with the sync-up point that you quoted. This seems to >>>>>> match >>>>>> existing practice in PiSmmCpuDxeSmm -- there are no concurrent >>>>>> accesses, >>>>>> so atomicity is not a concern, and serializing the instruction str= eams >>>>>> coarsely, with the sync-up, in combination with volatile accesses, >>>>>> should presumably guarantee visibility (on x86 anyway). >>>>> >>>>> To summarize, this is what I would ask for: >>>>> >>>>> - make CPU_HOT_EJECT_DATA volatile >>>>> >>>>> - make (*CPU_HOT_EJECT_DATA.ApicIdMap) volatile >>>>> >>>>> - after storing something to CPU_HOT_EJECT_DATA or >>>>> CPU_HOT_EJECT_DATA.ApicIdMap on the BSP, execute a MemoryFence() >>>>> >>>>> - before fetching something from CPU_HOT_EJECT_DATA or >>>>> CPU_HOT_EJECT_DATA.ApicIdMap on an AP, execute a MemoryFence() >>>>> >>>>> >>>>> Except: MemoryFence() isn't a *memory fence* in fact. >>>>> >>>>> See "MdePkg/Library/BaseLib/X64/GccInline.c". >>>>> >>>>> It's just a compiler barrier, which may not add anything beyond wha= t >>>>> we'd already have from "volatile". >>>>> >>>>> Case in point: PiSmmCpuDxeSmm performs heavy multi-processing, but = does >>>>> not contain a single invocation of MemoryFence(). It uses volatile >>>>> objects, and a handful of InterlockedCompareExchangeXx() calls, for >>>>> implementing semaphores. (NB: there is no 8-bit variant of >>>>> InterlockedCompareExchange(), as "volatile UINT8" is considered ato= mic >>>>> in itself, and a suitable basis for a sempahore too.) And given the >>>>> synchronization from those semaphores, PiSmmCpuDpxeSmm trusts that >>>>> updates to the *other* volatile objects are both atomic and visible. >>>>> >>>>> I'm pretty sure this only works because x86 is in-order. There are >>>>> instruction stream barriers in place, and compiler barriers too, bu= t no >>>>> actual memory barriers. >>>> >>>> Right and just to separate them explicitly, there are two problems: >>>> >>>> =C2=A0=C2=A0- compiler: where the compiler caches stuff in or looks= at stale >>>> memory >>>> locations. Now, AFAICS, this should not happen because the ApicIdMap >>>> would >>>> never change once set so the compiler should reasonably be able to c= ache >>>> the address of ApicIdMap and dereference it (thus obviating the need= for >>>> volatile.) >>> >>> (CPU_HOT_EJECT_DATA.Handler does change though.) >> >> Yeah, I did kinda elide over that. Let me think this through in v7 >> and add more explicit comments and then we can see if it still looks >> fishy? >> >> Thanks >> Ankur >> >>> >>>> The compiler could, however, cache any assignments to ApicIdMap[Idx] >>>> (especially given LTO) and we could use a MemoryFence() (as the comp= iler >>>> barrier that it is) to force the store. >>>> >>>> =C2=A0=C2=A0- CPU pipeline: as you said, right now we basically dep= end on x86 >>>> store >>>> order semantics (and the CpuPause() loop around AllCpusInSync, kinda >>>> provides >>>> a barrier.) >>>> >>>> So the BSP writes in this order: >>>> ApicIdMap[Idx]=3Dx; ... ->AllCpusInSync =3D true >>>> >>>> And whenever the AP sees ->AllCpusInSync =3D=3D True, it has to now = see >>>> ApicIdMap[Idx] =3D=3D x. >>> >>> Yes. >>> >>>> >>>> Maybe the thing to do is to detail this barrier in a commit >>>> note/comment? >>> >>> That would be nice. >>> >>>> And add the MemoryFence() but not the volatile. >>> >>> Yes, that should work. >=20 > Please *do* add the volatile, and also the MemoryFence(). When built > with Visual Studio, MemoryFence() does nothing at all (at least when LT= O > is in effect -- which it almost always is). So we should have the > volatile for making things work, and MemoryFence() as a conceptual > reminder, so we know where to fix up things, when (if!) we come around > fixing this mess with MemoryFence(). Reference: >=20 > https://edk2.groups.io/g/rfc/message/500 > https://edk2.groups.io/g/rfc/message/501 > https://edk2.groups.io/g/rfc/message/502 > https://edk2.groups.io/g/rfc/message/503 Did see it on the thread. Yeah agreed, Visual Studio does necessitate vol= atile here. Will add. Thanks Ankur >=20 > Thanks! > Laszlo >=20