From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from aserp2130.oracle.com (aserp2130.oracle.com [141.146.126.79]) by mx.groups.io with SMTP id smtpd.web09.2206.1612407000410446117 for ; Wed, 03 Feb 2021 18:50:00 -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=pYmaglgX; spf=pass (domain: oracle.com, ip: 141.146.126.79, mailfrom: ankur.a.arora@oracle.com) Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 1142nueZ052797; Thu, 4 Feb 2021 02:49:56 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=Vs9EH/21IpnJtdS17WVOB65yVi+fF+czdvoX60uPqY8=; b=pYmaglgXNE9MKNb7Ky/8DAwNP4wklUf/21o+YZfWDzozG5ibIZbZdVW075AjOd7sSTwS imrWGCB4SRFkcURiqiWiD0Ueoiz3OaPOUtU8Tqc+Q70qxPv0Q1V90knWWRGEW+q39nSY NreFJaaSQv99yrg2fW+Fg6InH80XZ/KxT4YLdbv+L6NiswvJeWcrdGccIWQPvX9k6Eij j6p9OXLUipSuyb0PFEP+Dij87wscnzNM9p4ycrlKd0ODa64kqcuoq/N5Er+X7tH4weZF Z95lIUDhl9uSfeCZ7tn02yiS7NXKHT2tE4gUqrHS7sbLPx3k7xcd8c04Jy2EtXe0c1uE 7w== Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by aserp2130.oracle.com with ESMTP id 36cvyb3b1e-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 04 Feb 2021 02:49:56 +0000 Received: from pps.filterd (userp3020.oracle.com [127.0.0.1]) by userp3020.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 1142kLrx076688; Thu, 4 Feb 2021 02:49:55 GMT Received: from nam02-sn1-obe.outbound.protection.outlook.com (mail-sn1nam02lp2052.outbound.protection.outlook.com [104.47.36.52]) by userp3020.oracle.com with ESMTP id 36dh7ug3n6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 04 Feb 2021 02:49:55 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DTiXEj6rpgye8EGx3N0GOm5en4ud52yBKDGrkXMIb3ylHUvWr3sBf4IV8f8emF4NTAU/0zgv5EUh5EXFbRu7yHgxFWKVaJGZP92EUAroa0KcZ9QpmAsywhHTEYLDQ8GXzqhSSbz01Cy9Sd4vPONB+XwjFvfJ32+Xge9XNlj4BZMjwLlJQaAUREJ7jIYR//S2e+VUrBUFQUR9dZ9nNW0AKyjDS0dK8fW/7JVyX7Lo8xJQTul/rWNSwLO+wduadW4Cz5VL30W8+ZR+EotTWCjlObMZ9Qlql0w+jsCSPAR/npx3FYxowMIhFoQoQVJU/pVXE16nEasqonuza2PmVVPfOw== 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=Vs9EH/21IpnJtdS17WVOB65yVi+fF+czdvoX60uPqY8=; b=decSDssYyf6snljxLurvVjAyGO/8BuG7qtTCXsaWUnkXUnWGP4T60fgwDh0lfa490PlyAnlXDw146cOhEsaM9ltZ2SgwvGf7sxfaEk0m0QAqi9CGdh5H+sSkKig+ie1IwYXjmTTd98GPNQZwHuvXMmnC1VDNhYx1Nq4kiFbikJEldegHUsemEtSXQlscr/VK3odbpEDVBJSyfghsTJngTGQfgIIl0N5jeCN5aB4A0WvYwYOPcbCF0Qen3C3V74SPZcwNTEvPzlkDIsNi7BIRAJWIy3nM955tZalQvafu9LrOiKL8MDVrP1DWSiyRzd2SpOEUceIzuldMtgUyE6ejWQ== 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=Vs9EH/21IpnJtdS17WVOB65yVi+fF+czdvoX60uPqY8=; b=QQPaekprMGzgeRdtZVlBJX2J7RwSom55qTCNgtQJdE6MVpufCeS26rdLXIEXX9aoD5e7GfWTCVieNycX083EImLJJobhdFvM2slr/le+o19SNOn9XRx6lVgyLH60zB/s5ChXtgUKfsi2Q8aIU3SRIneQakYA9z7O3fAG6wxfeos= Received: from SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) by BYAPR10MB2439.namprd10.prod.outlook.com (2603:10b6:a02:ba::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3805.17; Thu, 4 Feb 2021 02:49:48 +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; Thu, 4 Feb 2021 02:49:48 +0000 Subject: Re: [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> From: "Ankur Arora" Message-ID: <039138dc-c20b-39fb-8c14-953c090b8961@oracle.com> Date: Wed, 3 Feb 2021 18:49:45 -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.5] X-ClientProxiedBy: MW4PR03CA0311.namprd03.prod.outlook.com (2603:10b6:303:dd::16) 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.5) by MW4PR03CA0311.namprd03.prod.outlook.com (2603:10b6:303:dd::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.20 via Frontend Transport; Thu, 4 Feb 2021 02:49:47 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 39d32b99-b4fc-432d-0bf0-08d8c8b78960 X-MS-TrafficTypeDiagnostic: BYAPR10MB2439: 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: saMadC/Ouch8AXaafpFrXT5m4vMo0/pXy8+3c7JpUAonQWsKOq9yUQZyUTPw27vrE3172p32P5Mp6N43EripFRPl5pwIzSST8/Pk6TehxDQkFxzXg3ayDDtQ3yfTyUh4GFZsmsciGNLTMYtCCpBzJNyPINFxCuv9xDzgL2dSj4QkSCQoUoNMpnTg22y88nnXyhuv751qlHXhfoEiZBDe6LzJ8vzDdyRbSt0rtAo6WMELaawVsKgjtqIgWI4agCTDSBGFen/51gjOG/i5y6m3ewNT+dGQUxpaBlqla1AU+0nMwongX+jYdEFxXuMEhOg8Sku8dC+eklZKjLaPULDIg8IGL07W1wzWwX3zIrh3L/fws7as55TWNCKlk1uAU4YkC9rx55Av0RuQimKF2D/CUJ396rnLw1krJRmjyVIWky2jvzsglbXSuY5n4+SW28gcgT6l8Mwk96xAEF/o/ZouMrUYQ5045SBW2imQBrxd+cNsYmrcXmfP2MT9WF7avVDWjcADu82jGfA19E+a07TiiESlVTcE/qT3ZYtNRWnFKSywtEudAU9y0BkM612GKntbwt8oXoUqhi//OlTpj5k31VjOTDkPZ9Us6zChc698evA= 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)(396003)(366004)(136003)(346002)(39860400002)(6486002)(2616005)(66946007)(86362001)(31686004)(2906002)(83380400001)(956004)(5660300002)(36756003)(26005)(16526019)(186003)(53546011)(107886003)(8936002)(4326008)(478600001)(31696002)(16576012)(66556008)(8676002)(54906003)(66476007)(316002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?dHd1bWJWLy8rdGpBeEgwbmk1YTRXb0l5R3FBbVkzYUlVdTJFWW92N25mLzV3?= =?utf-8?B?ckpsaTEwQ2t3YytFbWY3RmxYR0UrQ0oyNFdYTHFBbHRtREE2aFRFOGdBTXZs?= =?utf-8?B?YVplZHZVa004RkNidDBVVGt1bytidXdEN3djSTNibXp6NElyTUpReVZjc1pm?= =?utf-8?B?VjFVMGpyc2x6SDFTb2RvZ2lLMjNObHNicGxiZ3NVd0l0bDlmalVaR2dXdDkr?= =?utf-8?B?NFdJZE02T09NNWpiWndJR3Ixa3cxcDYxTUZLT0YzL0U0LzhhRjRYVVplVjZV?= =?utf-8?B?UkFnYXJqaU9rUUV2ZFYxSUlHUnQ3NERBMEVZbUdyZFM1c25PbDlQZnl6aVE0?= =?utf-8?B?U3pKWDRzOWZxZStkTlBDMTVHUHZEaWgxd3g3MHF0WWp6MVZqTDJwL2I1N1lI?= =?utf-8?B?NzZXdHA5VlhQNFczZ2FyQTN0cWNlVjlHZjNkZjR6eEtXNnN0c1I1ejE3Uk51?= =?utf-8?B?aVE5RWdKcjRINVhMTkxGVm4xUGZPZzdlUmM4QlY3MmVBMU5VNjVyTlJ6Kzln?= =?utf-8?B?cU1WL2VFVTE5cm9zTDRWRHpISStSZUZxVXlFYks1dHR2SkRIb2E5cStTTFlT?= =?utf-8?B?c1Q1TzBBVHJMcnpadHFIR2ZjV1ZPQVNwN0VTMUo5b0Y2ZElSQlRmSmNhMjNy?= =?utf-8?B?bzVTOHhScTc0U3N3ODBWVEV5cVY0NzhUVHlJSTd1MTVKT0hUMmNwdFRIQnM3?= =?utf-8?B?TXNTci8ydXczZWRqUUIySWFuQzFGdWN0RXlEemZkTVBZaTZLb2lsMHBhZjRt?= =?utf-8?B?TXFUNjdSNmw3UG9JcW85cFVEdmtSMXhqekpBb0ZmQ2JYOUNPeDJnMXBrazVk?= =?utf-8?B?MVFnVEVPeXJ0YWFMbENRTXdrL1VkZE1NK2lIbUF4MmlWdTR4S1N1SEtwTFRI?= =?utf-8?B?d3R1STFmY0JFakU3dklGSUxVVitwb0lzZmV1d2YyZEk4VzFvNUU3WGhBek9l?= =?utf-8?B?RGVDcldsWkwralBQaHBSSUhwRjNzQWFvOU15RWtEWkdRVno0MkZLUG50TmUz?= =?utf-8?B?UVpRdTBsYU1FS3NNOVNvbHdsOHRkYWJqSzdidVNrbjdDeGIyZFNiQ1h6V3h0?= =?utf-8?B?WnhGTXYwY3R5WWpGck5yWlUrZ2YrZGhDc1J6bGU2NloyQU53N1dqTkNHK3Nr?= =?utf-8?B?T0oxN2xJa0ZBSVF5S2cvNzRzREJ0dTA5eFVrYW5SNm1ZOFlnOTM0dUptbjhu?= =?utf-8?B?ZzAwK2FsQVE2V2dMa3prMlFqcXREZUs4SytUbXFMeXFidFFwS0dIRlM4MDNV?= =?utf-8?B?c200UHZLWEJsOGdrUElOOW56SlMxMERzdmR2RExaZlR6M3pXamx6clY2b2RK?= =?utf-8?B?cFgyeDNHRjhoN2J4Z3NnaUV1a1YrdTRIaWNlMXRXNEdlZ2p3eHlEZ2k1OERu?= =?utf-8?B?SE5DeUF0YmVMZEJ2TjFhT3luamNOejhoajdiTzdrWjFlZFJlcXdBYnY4R2E1?= =?utf-8?Q?A6YOqeTf?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 39d32b99-b4fc-432d-0bf0-08d8c8b78960 X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4605.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Feb 2021 02:49:48.4867 (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: oxNxyoetHnx6QME8Fv/KKHGAnLfQ3qKSKfWEZzGAhPFLXbR2jW9scs9Kb7sFXNwkA5QuATtHtkYg/htYFIrTqgFysJbpxSj275zicKppWzA= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB2439 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9884 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 suspectscore=0 spamscore=0 mlxlogscore=920 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102040016 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9884 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 clxscore=1015 impostorscore=0 mlxscore=0 spamscore=0 bulkscore=0 priorityscore=1501 adultscore=0 lowpriorityscore=0 malwarescore=0 phishscore=0 mlxlogscore=999 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102040017 X-MIME-Autoconverted: from 8bit to quoted-printable by aserp2130.oracle.com id 1142nueZ052797 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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 th= e >>>> array pointed-to by CPU_HOT_EJECT_DATA.ApicIdMap, should suffice. In >>>> combination with the sync-up point that you quoted. This seems to ma= tch >>>> existing practice in PiSmmCpuDxeSmm -- there are no concurrent acces= ses, >>>> so atomicity is not a concern, and serializing the instruction strea= ms >>>> 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 what >>> we'd already have from "volatile". >>> >>> Case in point: PiSmmCpuDxeSmm performs heavy multi-processing, but do= es >>> 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 atomi= c >>> 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, but = no >>> actual memory barriers. >> >> Right and just to separate them explicitly, there are two problems: >> >> =C2=A0- compiler: where the compiler caches stuff in or looks at stal= e memory >> locations. Now, AFAICS, this should not happen because the ApicIdMap w= ould >> never change once set so the compiler should reasonably be able to cac= he >> the address of ApicIdMap and dereference it (thus obviating the need f= or >> volatile.) >=20 > (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 >=20 >> The compiler could, however, cache any assignments to ApicIdMap[Idx] >> (especially given LTO) and we could use a MemoryFence() (as the compil= er >> barrier that it is) to force the store. >> >> =C2=A0- CPU pipeline: as you said, right now we basically depend on x= 86 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 se= e >> ApicIdMap[Idx] =3D=3D x. >=20 > Yes. >=20 >> >> Maybe the thing to do is to detail this barrier in a commit note/comme= nt? >=20 > That would be nice. >=20 >> And add the MemoryFence() but not the volatile. >=20 > Yes, that should work. >=20 > Thanks, > Laszlo >=20