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.5894.1612334742187278399 for ; Tue, 02 Feb 2021 22:45:42 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@oracle.com header.s=corp-2020-01-29 header.b=QctcY0wV; 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 1136UgwM170481; Wed, 3 Feb 2021 06:45:38 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=xAX8C1qlDjiVeDIhbau1VfG1xFuGkZkX6JHQ+OFnjVw=; b=QctcY0wVLw2ozLs51TB97Ray7Hxd9MXTSUUNkwnBOdHUa4BeCjAQA3T2UyBvnybpXuwI SAhdPQf/vOxc/iAb+lDEGcmF522j0VbFAOvhwp2JgxSunepzkTk8wIE2/qNCH8EN22OI WmuQqjvpx+GbUcD9qYb41JRKWGDVd2YqjdQ8NKhe4Hr1lJWjD6D/l0pw1SKywvU0Qg2U oynMjdkyHDsfBrCxAN9Ho8RdCnY+oEmDfR8zsDSs8n3ZCSr3NA0FwRsktOroJOKSSvaO PA7kRZseKxg8G4nSjc81PCHiApaUJ7rUOEcMJBdHwsTOWMObpAvHeoHPiD8lWGQH+Wng jQ== Received: from aserp3030.oracle.com (aserp3030.oracle.com [141.146.126.71]) by aserp2130.oracle.com with ESMTP id 36cvyaxm0j-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 03 Feb 2021 06:45:38 +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 1136VVri110782; Wed, 3 Feb 2021 06:45:37 GMT Received: from nam10-bn7-obe.outbound.protection.outlook.com (mail-bn7nam10lp2107.outbound.protection.outlook.com [104.47.70.107]) by aserp3030.oracle.com with ESMTP id 36dh1q9b4c-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 03 Feb 2021 06:45:37 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DJRGF71jRSbqGj+euWm461zsoM363EmIo/itYs0RJ+BjuXun9nLfZqa/7TqKKiXLH4rWy9NywIbwfp+PEb8yTWp/jgNGvl0ffmeGM4ym3mupMson2T3IjvkLmfohBnVyVJERVSaE0wI1LJyw6go2N9G+GkAZUSJtrqOTd5hNrUpgyvYAmw4FMI6ef/yxkH53YV6TsqVHi88ijRqvkcF9YT2TnyYl3FDnlpLq3SwisII6zymiFVPZuYfHEu2Y8YBixL8o648C8lmf5cCb8V6HqTzXnjA+YQDo6CryRYjChtTe+pSA2J0mUUnum2QlJpxSRHYEIi7wnH+DQKBwIAqnzw== 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=xAX8C1qlDjiVeDIhbau1VfG1xFuGkZkX6JHQ+OFnjVw=; b=ibhKXDimXZ0/Kvo6vaq37JAHEgMqtRuiVRoMbapEoj4MWwVOqaXpp8WTZ7i3Ykrl3Q8Dc1EOl/sVrQfgInIsz8/qW/+198v2gqa/DmelqH8wwop19TmuoGlQIg4VH+kwU/krT1wJjbtsTUhN3nKi21ABYfhscBkZ8jD4VzmDRz++MiQ3fKQFG2lk4aRMPy1na3MoXao4LFrzLwisUtjgdgVqdgB9THyL8L1wS01HcbX3K6s622eU2sIb5XLVEh/vweuD75V5k6g4V2wRSUPc0lCNRZpmj1rnimTH5HeMxyaR7xjnyuGP1p2l0Xn08gDV5/tOHlGSsDFK6H8XjF3D+w== 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=xAX8C1qlDjiVeDIhbau1VfG1xFuGkZkX6JHQ+OFnjVw=; b=PId6flFAE062v4aXz2OfSz8Ox8Z9EXMK5m5457XEwnJDwSxBuv0C69SQB5pdrUGr3x1OTS9VDLsEQc/Mj9w3q/5NjocknsjVmDIoUHOyuAWnG6sLD37b3c/jqHvsLvLtMfNVUHkGh2UR4V1Goazb/l1/IUS1sKW6qwiENy4QC1I= Received: from SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) by SJ0PR10MB4558.namprd10.prod.outlook.com (2603:10b6:a03:2d8::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.20; Wed, 3 Feb 2021 06:45:35 +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 06:45:35 +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: Date: Tue, 2 Feb 2021 22:45:30 -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: MWHPR12CA0050.namprd12.prod.outlook.com (2603:10b6:300:103::12) 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 MWHPR12CA0050.namprd12.prod.outlook.com (2603:10b6:300:103::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3805.17 via Frontend Transport; Wed, 3 Feb 2021 06:45:33 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: b4bf9b56-a020-42cf-e20a-08d8c80f4f4e X-MS-TrafficTypeDiagnostic: SJ0PR10MB4558: 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: HIGvmKgKWYL0ZPwpPk2dGDnwmX208aFIuPUpqLDtf2wY05NJ79c+lnnntoYTDYcLKC46CIHnrEVS4YIAhrtJQbSds1p4MkY9XGVQEmUUfwaiXzwaBgSn9s7rG6WrQRBwb1GOfjnI/4wcFTBMBlxjjyLsDrLyv0pyD0U6QOYKeBegpYZJcdG4E4eRO6kAnlJHJRkeR2KnuxAu9cQVI+4MUfv4NJyH44Noa8zSSb4uaq/TOl4S+U1kUmNL6Ga7WMBUdSZHBZazCemxQFvDP2y0qhpnAxs7AKwzCmi6z0jl7gtw+tvwQSyrR1i5WGRNfGWGeJnR6uYZVx9pVTBIHeBbaZ1BSGxuqnDFuYL3rHyOEaC792DiDjV+8KClq9RoZrwJ/9HQYzNNys86DDZ7nmSjviEj3BBjcD2H2/s6EVdcIjaBWm92i3Rol+13S+4VWbttNrD8qdHFQx1DplZj/P7MkYZoDn2J7Ni1KHM4NimuoXipCVvp/jPz3+WS7jINX2g/v8x67QTLqMgmjN2X+mtNOeHAH7Uq0LBHQ9ykrgUcC86L9VnZKNUIqD1UTSJX/56/cayhB4np1Fo/inqdR6BccKZHccUEYtALKAQ1nQv3rIk= 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:(136003)(346002)(366004)(376002)(39860400002)(396003)(8676002)(2616005)(2906002)(956004)(31696002)(86362001)(107886003)(36756003)(6486002)(53546011)(316002)(186003)(16526019)(83380400001)(26005)(54906003)(16576012)(5660300002)(478600001)(4326008)(31686004)(8936002)(66556008)(66476007)(66946007)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?RCt1czMva2VPOTdjbk1NZVNNQUF5UUsxRUtVOUlCWk5zVHdCbm5vQ2VyTHpn?= =?utf-8?B?QXllTFNuc3NNOVlSVmlJZFEwQW5BUmp5blV6K0J0ZXBBdVJlMUNyblVWN0pD?= =?utf-8?B?enhvWjM5WHRyRm1qaFg5TjRoa21MeU8yQndEbWNWcDBBR1huY3YwN0kzZ21k?= =?utf-8?B?ZnVHd0ZzQVprUzg1bjlWWlFCMThpN3VHZ3N6dEViOGgyZDE2RVRSaXVaM0Mv?= =?utf-8?B?MEVVTVI5UTF3YStSeHQ4WnlDNG1tMUt1OXVrazE2czJUVk5zMXd2TDc1UE1v?= =?utf-8?B?b2d5b3ZDNnVIZlRVMmFkTFhwelVlOVRLU3RucWV1eWFrOFhRYlZ5T25uR1hE?= =?utf-8?B?YVhUeGtDTHVOZkVIZUZTT1hyL2lwODJiaGJtbmVwRWtMZVE3S2txRTJRdm9z?= =?utf-8?B?M1ZKdWhhcGVHN2VGeEo5ckNFR28wc2V2cVZPRURaMlVpcko0M0JzZTJDdytj?= =?utf-8?B?citscU16UjJESUNoc2lrempOVDVoNGM5N2RhaDU3TENnR05TczZMTzdFY2Nl?= =?utf-8?B?ejFuZmQ5RHkxNjBqMy9JZHBybFhlRzZIdkgrNklkSS9obk1CMzV4SnNIK1F2?= =?utf-8?B?ZFMwR2lIMlpTRjNqeFUwOWtUakt3azBGR0psdEpyWklFKzAzQzY0NytpRmdB?= =?utf-8?B?V28rbWNERHBRdGVDdDQ3enpzWHdSZGxTemx4T0hPLzNGOWJBSzZkZ1Z5L2Z6?= =?utf-8?B?SklzaFV0UlhMazVydFNpMUg0TFdwY0lxWGprYmhzN25lNTNDQzR0R0UzVFFn?= =?utf-8?B?ZTEyM3owQ3FIVmZJRlFkWWJ1V0RpdXVLc2FjRjNKamZKekREWHArcnVCeDJh?= =?utf-8?B?dFZIQm5EbmJEeHd4L0t0NjVudVdjbURxb3dQbzNGUzc4NjRTclFyLzNUQUV5?= =?utf-8?B?elBSaGJFb01JR0duQk0wN0x6T1FTS2lmcngveFdnOTVwY0Y3NUx1NnZBTmxM?= =?utf-8?B?TmY1MTdQMlByZnRZcmhzejA4WXl2WXJhYmMvMGhYaXhTNS9nM1oyRDNmZ0tl?= =?utf-8?B?YmZKTHVNRnlqa3krWXRqMkhUWmpwWmtVTmVmbUJWUFJ2M3B2bnFJNjE4NVFx?= =?utf-8?B?d05lZk5mdm50YStLYkVOVzVCS2pkUm5IWDV6aUZ6NzNyUFFDMmZraG9xZlVS?= =?utf-8?B?YWFpRUZjbjVDL0JrdVB1Q3lhdnhMbmNpSmdkcWpwTXZXaVQvZEczdG5UOUVV?= =?utf-8?B?bHVkYzZDNjh1ZGFwMnFMbnk1OFcyZ2VMU0lnREl2SXB4WCtQYjF2TXdRQ0ZD?= =?utf-8?B?NTN6Q2lHZFhMUHd4anM3M0xHUEUzNUVZSDZSUHRVa2VSYVpUMkxnSkFTOGxE?= =?utf-8?B?R1NDdFJhNXhhTlFhK1o4YjNoTmxnVEhzMGVGbHVVRDA1cjhKS0pqVXNIZE1T?= =?utf-8?B?ckcvbTNZRHhpdkE3SGdTZG9QQ1BTNTdrQS8xc3RqdDFHSUFGblFrRkFldmZR?= =?utf-8?Q?OnHc/vhE?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: b4bf9b56-a020-42cf-e20a-08d8c80f4f4e X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4605.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Feb 2021 06:45:35.4724 (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: flzmkC2O65Lnb9GR9kJZ4GF4oB6955omRs50U7BEgtKKQ+eCDvnlO0/elRDUxOkT1yDj85nNNNofU6pS7xKLel8Ylwvw3BlBNNfEGJAcPro= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4558 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9883 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 spamscore=0 phishscore=0 suspectscore=0 mlxlogscore=903 bulkscore=0 mlxscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102030038 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9883 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-2102030038 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 streams >> 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 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 atomic > 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: - 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 cache the address of ApicIdMap and dereference it (thus obviating the need for volatile.) The compiler could, however, cache any assignments to ApicIdMap[Idx] (especially given LTO) and we could use a MemoryFence() (as the compiler barrier that it is) to force the store. - CPU pipeline: as you said, right now we basically depend on x86 store order semantics (and the CpuPause() loop around AllCpusInSync, kinda provides a barrier.) So the BSP writes in this order: ApicIdMap[Idx]=x; ... ->AllCpusInSync = true And whenever the AP sees ->AllCpusInSync == True, it has to now see ApicIdMap[Idx] == x. Maybe the thing to do is to detail this barrier in a commit note/comment? And add the MemoryFence() but not the volatile. Thanks Ankur > > Now the question is whether we have managed to *sufficiently* imitate > these patterns from PiSmmCpuDxeSmm, in this patch set. > > Making stuff volatile, and relying on the existent sync-up point, might > suffice. > > Thanks > Laszlo >