From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from aserp2120.oracle.com (aserp2120.oracle.com [141.146.126.78]) by mx.groups.io with SMTP id smtpd.web12.1540.1614033238385714953 for ; Mon, 22 Feb 2021 14:33:58 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@oracle.com header.s=corp-2020-01-29 header.b=ICu0If2h; spf=pass (domain: oracle.com, ip: 141.146.126.78, mailfrom: ankur.a.arora@oracle.com) Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 11MMXtqH125140; Mon, 22 Feb 2021 22:33:55 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=Uv8hX8iCr6zEjG22WfeUA1HgVmHf/8zHb8/GX8klXUA=; b=ICu0If2hBA1MNkeb82eIlgKmpo5VgaxuaQxRik03e+aQy4u6r3wKHbs0RV8G0mnPUrhj hgtI6gWaKWSLHgkaNOFueCkwXWwuUcqNRF3weQQ/EP7J/IRUEa6/7c3SKni62fARJ4rl QVcM81P90ZBi5i1S9FefYxbh/WY8EkrL17yPVJSlqVTLTj6pd8RIqTluj+W2/NdFli94 ToSCp2sdYt1VPRhNeoSeLfCEU/HB7/fjuAyDP8KdV+6TK9S04Ymf4S6+jKfaanj76a/6 8LIE17/vi5Tl7RpfWKiQHCpH4VGu2YYUPsGJ63wUx6FYzTRW1zY8fd9Vgmmp41BGhOXZ TQ== Received: from aserp3030.oracle.com (aserp3030.oracle.com [141.146.126.71]) by aserp2120.oracle.com with ESMTP id 36ttcm5bk1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 22 Feb 2021 22:33:54 +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 11MMUKm7167575; Mon, 22 Feb 2021 22:33:53 GMT Received: from nam02-cy1-obe.outbound.protection.outlook.com (mail-cys01nam02lp2054.outbound.protection.outlook.com [104.47.37.54]) by aserp3030.oracle.com with ESMTP id 36v9m3tw5r-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 22 Feb 2021 22:33:53 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CU3ze7CY1N/zX6Fd+1g+HOFl7JAUKmwDAv78CMHZXNVQlZ+MpHA6UPZcUokyOxYAj9QCpj6CoFxTXYjaOU6MitmlNehi1HpmmM418g4weTH2DmsZ1I4rdfPei4SiC73WEE26q/GAzKTUFKPOkZr06sLwJIuxasw7y5otZSWV0VU+Eme51BJz8SR0qXW19STz7Yg3ZbmyPWZ+/hioZwKkaYI9MDa8SbnOj15j36EIE9vsb/HSvPcLKJKKEl0i99rmaMsBKNhhBPcWLuvaWleufOxWC1xMdE6dcK1jmUsOnca18P8u6wZp9BReiEbrOzX5dK0i0t0WZxHbL6pi7dRQhA== 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=Uv8hX8iCr6zEjG22WfeUA1HgVmHf/8zHb8/GX8klXUA=; b=msLtc9l9LjHz356BSJAcuIYjJUSDLeRKMH3ORcZok/L/m5pj+UMvrhVOzldJZEmMiHHt1BIRkgVbOVbFJ9z//1rev2TfYCP37/wEMcTME4PAMEAzVFFEiAZGbzAyTTervjEeIL1+dYTgampGZJ/crCQNhoJHsqvQ/w/gnFLuhS+aY0XvXVC0bdMeeP7KT6iwEKoOq6Wt0ilSeG7Lpt8+NwtDFuQyb0EPbeW92YImRPB72aexCtIG2/Swa5A6gD0wCljpLEGhMdYNAnMD8JmT5za1o6dDFNFsI70tnUYgeJ7pMGmf88zyaC0jUDJdpJohMIE4DlfmH5G3WGSq2gheWQ== 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=Uv8hX8iCr6zEjG22WfeUA1HgVmHf/8zHb8/GX8klXUA=; b=SjdXrlHb2g2IUk3N4MvH/OuxFLoNO+GMtW/mZtNFaHRaTQKZiJR1MsR0XaKJlRasUbRy0tN/OUUbKEM3nFTifjYAgizMJZgoCOsNlQ6FfpJkFmE/3AZ3Pjeaj0vUP30lbraTdfhhgMqEeN09OW2g9uuL7U6dAyeTyKQgTXzHD9g= Received: from SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) by BY5PR10MB4162.namprd10.prod.outlook.com (2603:10b6:a03:20c::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3868.33; Mon, 22 Feb 2021 22:33:51 +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; Mon, 22 Feb 2021 22:33:51 +0000 Subject: Re: [edk2-devel] [PATCH v8 05/10] OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA To: Laszlo Ersek , devel@edk2.groups.io Cc: imammedo@redhat.com, boris.ostrovsky@oracle.com, Jordan Justen , Ard Biesheuvel , Aaron Young References: <20210222071928.1401820-1-ankur.a.arora@oracle.com> <20210222071928.1401820-6-ankur.a.arora@oracle.com> From: "Ankur Arora" Message-ID: <2e445f5f-1244-2992-7585-80f06508cd17@oracle.com> Date: Mon, 22 Feb 2021 14:33:48 -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.13] X-ClientProxiedBy: CO2PR06CA0067.namprd06.prod.outlook.com (2603:10b6:104:3::25) 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.13) by CO2PR06CA0067.namprd06.prod.outlook.com (2603:10b6:104:3::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3868.27 via Frontend Transport; Mon, 22 Feb 2021 22:33:50 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: a6a88064-940e-458d-1083-08d8d781edf8 X-MS-TrafficTypeDiagnostic: BY5PR10MB4162: 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: i44NBA79GbJA83MSFTEmWKUwNdwPy9OoNMrOI95+s6B92aqDBNp4ilWkvsV1UZSbQ+J+ZIEVeyT6xJThi44AL2v7ssGfWpwzcMNjXYZT+Cqv+yi7hPrans9txG7geTqBAnEX7aKvfRbLe+0TQ4By/3o1qAPLGizZOJ5K72KG8FXjKpP0fIusknfjLTbts4yYFH6vWncJV2DmcJx/QFTJKf0yWgfPxqLjSp1epkV0Jp0eiawpGcDwxkMqtcs2m8kho1Cuh1ozAmOUL9T9sCvcbDd/4sHDGnvIwgcJe4/kyxH9MGbOn0YjVkLUy5p1FccqIdAUDCUDltO2mI8e2na/patst/x9gYDWjfg7AS+FgoI2m1vxgG5V/N28ssy+PRX286+ynWEt7dRHRqVfQ3fw/vz8LEftYLaPFjndFOKKqLOETvo2AQdKywCfThyq+dLVf1kA6NehmIdAGC7PQcRwJpb6J7DBUWIUa5nI6Vp3Jsd8jnmWycmmMPDVNSkv4wWcZbzL6EueQz7qcWMKAgZYy++z4eJSTt9S7eQlC6mO81dVhCvfCVXWddyTTpbvqHbJGn/2cBDHQbhI5tCThvue584WaU7F9QelOxf1cRXlucCjJH5VOFMbStQHf1P3S5Y1VmbM4HksDxMSOGdd58QQRoKgOX35BTXmdk+hsTdwMSaU33dzaZKwvfnZaUKVNEd9 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)(396003)(346002)(366004)(136003)(376002)(86362001)(36756003)(4326008)(31696002)(107886003)(8676002)(53546011)(31686004)(2906002)(26005)(83380400001)(966005)(478600001)(54906003)(2616005)(16526019)(8936002)(316002)(66946007)(66476007)(66556008)(5660300002)(16576012)(186003)(956004)(6486002)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?RWJvVUtiTE1CNXB3WjJsTzZpVHNOTmE1bVFOTE9Xa3ZDMXV6dlU3Q1BxUHBr?= =?utf-8?B?MEZaRFJST2l4Z25ZY0hLLzhjRmhENzk2dStrWE9GT2MwUjBwN3hHQlJaQnhT?= =?utf-8?B?bUErWWVVQ3ovQnZINHRYOVZ5eVRDVXMvK3doMWZweE9HYXdQRFlDZFdjMlRX?= =?utf-8?B?K2NhdFRGakY1NWpOWXNnQzlZMnpzMGoyamcvekhZLzFMWXArZGZKUUJpdzFa?= =?utf-8?B?MlJ6K3I5NDYyT1B6clVRRWt2RGdsamRwUTNMcEd0YWVRN09TVHlUQWNIc1dv?= =?utf-8?B?RWZJM0xlOHkxZ011dGhoKzhhVVFCQWNiTVlQNzhyZzhGelJkaEp0WCtZeVhz?= =?utf-8?B?ZW8wL2F1TjNsTlRlV0VLcHRpRFVIN29zWTFyMFp6NjFZbm5oeDJxalFzc2Vz?= =?utf-8?B?WmgwNWErT1h6SHdVMEVEZFRkZE9lRVRHRTYzcHdoYm5ueXp2ckhwSFk1QWcz?= =?utf-8?B?M2w1Z2kwNTZaTVplVXFVN3hRL0ZJdEFYMDRmc3ltcEVBSFNoL2J3TU1ESXpB?= =?utf-8?B?clNOMEQ4S3pJd3BCN1A5WkFaSFpKeFl2cnFwRmRHRjF5VXFBOHh4bWsyMU5v?= =?utf-8?B?UlFmTzFsbGg1NU9BaU5BY0h2TUlBaWJMUVhvWGJkWVNPTkZwZG1FWUordmhP?= =?utf-8?B?YTBIN2ZZQkhDcWpYUk5OaWN4WkFjcm5RWVNwYUJMRC9IWFQ5WHRzOStPL09U?= =?utf-8?B?alh3VDZ5OUFQekZYdWx1Y1FNNDRWSFcyTFhFRkV3L2IvWlVMZ1o5T3BTMzJq?= =?utf-8?B?ZFhFWXJxZGROZm9ERTNCUHhWUFpIdWFjbER1eFNjM3VaU3A1OXRXU3FybkNE?= =?utf-8?B?ODhxVm1URGlNYm9CQWZWVURRcHhQaXd1aldFSks3U0liZmRid2M3NTAweDYx?= =?utf-8?B?WTJobVl6Y2lOOTAySnJuK2dzaldPOC9UTFFNTFpaci9ZbUd3RGN4N0l2c20z?= =?utf-8?B?N0VxQjAvd3F0a3JOU0JSZGZNeVp6Y25WSkVIRGx4UVBWOEVNV1FUWjRLWVFQ?= =?utf-8?B?cDhQVlE4VTBmQnVhUXBicDAxZm5SME5zYXptWVJwc0FTTVRUV1ZVVER3N1lE?= =?utf-8?B?YVliNHpySE5RY2JONE0xcnVyem5uWjZTNm1FN2M4MjZyNlVUNEIyRS9xWkhw?= =?utf-8?B?N1RndkhXWFpQWitoaWJOKzIzcitPU2tHZERzQUwxZ0dLZzlrYTN4OVR3akJB?= =?utf-8?B?VGhNNHp6aE95b29WSzFBNGNlcHlJVDFVV1VRT0pZTzBGWm93SnJhUmRPRnNs?= =?utf-8?B?VGRrdG56ZURzUkczeEp2ZlhRbmZmZ1IzNnJ4bmZ3ajhzbFY2N1NjWHR4SW1n?= =?utf-8?B?bTIybWxmS05URnBGNkdPakJXa3FlY1l2OFJXTlN1VERyQVhkQU5mb3JuejZp?= =?utf-8?B?MkNOTXZlQnVXdVRON2swS1ZQMSswMFMrQjdkV3g0UE91MHgvQmxLd0hvUkRk?= =?utf-8?B?VTR2UjRxK1VpK01yeUpFenpFdlJoekw2UjFPWnpEOFdLSW1MSWdnSlFZRjBy?= =?utf-8?B?NE9yNXZIdXlnQm9xSFpQRGVQaXhCbG1EUUx6UHhPaEJkRjErakpuNzRCWWg0?= =?utf-8?B?RFJjbjJsVlI0N0NUK2NTdUNFUlR5bGdWbnExWlhsYitTK2dvZzZpdnM4MHdH?= =?utf-8?B?WkhIb2NOZWZJUTYwN0xIL0RGTTZFYWtCWGgzei95WXNWckxNbDNlNU1kYTFo?= =?utf-8?B?UXJDS3hJVWNpOGtYSEZPQlJRU21XRlZIS2dUQXdlWnc1cHRiUHNTcGd3Y2Nu?= =?utf-8?Q?vYncsb8Pyvc9+hfcc23E7LQP2N309J65AR3huho?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: a6a88064-940e-458d-1083-08d8d781edf8 X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4605.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Feb 2021 22:33:51.7887 (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: EW5/E33SZ9HnqtGKjv4pia5iXu+XAg9B3JdAsm+MHZxItB4PKqverFxntyhUGG+Dg/L4OOFR/Qm4AFNWvWWnz8CFmcqZ7pZVyRsyACooO4s= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4162 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-2102220193 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9903 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 adultscore=0 lowpriorityscore=0 spamscore=0 mlxscore=0 bulkscore=0 clxscore=1015 priorityscore=1501 malwarescore=0 impostorscore=0 suspectscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102220194 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2021-02-22 5:06 a.m., Laszlo Ersek wrote: > On 02/22/21 08:19, Ankur Arora wrote: >> Define CPU_HOT_EJECT_DATA and add PCD PcdCpuHotEjectDataAddress, which >> will be used to share CPU ejection state between OvmfPkg/CpuHotPlugSmm >> and PiSmmCpuDxeSmm. >> >> 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: >> Addresses the following review comments in v6: >> (1) Dropped modifications to LibraryClasses in OvmfPkg.dec >> (2,3) Cleanup comments around PCD PcdCpuHotEjectDataAddress. >> (4) Move PCD PcdCpuHotEjectDataAddress declaration in CpuHotplugSmm.inf >> to a patch-7 where it actually gets used. >> (5a,5b) Change the comment in the top block to use Laszlo's language. >> Also detail when the PCD would contain a valid value. >> (6) Move Library/CpuHotEjectData.h to Pcd/CpuHotEjectData.h >> (7,15,16) Fixup guard macro to be C namespace compliant. Also fixup the >> comment style near the endif guard. >> (8-10) Rename CPU_HOT_EJECT_FN to a more EDK2 compliant style. Also add >> a comment block and fix spacing. >> () Rename ApicIdMap -> QemuSelectorMap while keeping the type as UINT64. >> Related to a comment in patch-8 ("... add worker to do CPU ejection".) >> (11a,11b) Rename CPU_EJECT_INVALID to CPU_EJECT_QEMU_SELECTOR_INVALID >> and add a comment about it. >> () Remove CPU_EJECT_WORKER based on review comment on a patch 8. >> (12,14) Remove CPU_HOT_EJECT_DATA fields Revision and Reserved. >> Reorder CPU_HOT_EJECT_DATA to minimize internal padding >> and ensure elements are properly aligned. >> (13a,13b) Change CpuIndex->ApicId map to ProcessorNum -> QemuSelector >> () Make CPU_HOT_EJECT_HANDLER->Handler, >> CPU_HOT_EJECT_HANDLER->QemuSelectorMap volatile. >> >> OvmfPkg/OvmfPkg.dec | 4 +++ >> OvmfPkg/Include/Pcd/CpuHotEjectData.h | 52 +++++++++++++++++++++++++++++++++++ >> 2 files changed, 56 insertions(+) >> create mode 100644 OvmfPkg/Include/Pcd/CpuHotEjectData.h > > Very nice updates; I only have superficial comments this time: > > (1) The patch -- correctly -- no longer touches CpuHotplugSmm, so please > remove "/CpuHotplugSmm" from the subject line. > > >> >> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >> index 4348bb45c64a..9629707020ba 100644 >> --- a/OvmfPkg/OvmfPkg.dec >> +++ b/OvmfPkg/OvmfPkg.dec >> @@ -352,6 +352,10 @@ [PcdsDynamic, PcdsDynamicEx] >> # This PCD is only accessed if PcdSmmSmramRequire is TRUE (see below). >> gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase|FALSE|BOOLEAN|0x34 >> >> + ## This PCD adds a communication channel between OVMF's SmmCpuFeaturesLib >> + # instance in PiSmmCpuDxeSmm, and CpuHotplugSmm. >> + gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress|0|UINT64|0x46 >> + >> [PcdsFeatureFlag] >> gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation|TRUE|BOOLEAN|0x1c >> gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderMmioTranslation|FALSE|BOOLEAN|0x1d >> diff --git a/OvmfPkg/Include/Pcd/CpuHotEjectData.h b/OvmfPkg/Include/Pcd/CpuHotEjectData.h >> new file mode 100644 >> index 000000000000..024a92726869 >> --- /dev/null >> +++ b/OvmfPkg/Include/Pcd/CpuHotEjectData.h >> @@ -0,0 +1,52 @@ >> +/** @file >> + Definition for the CPU_HOT_EJECT_DATA structure, which shares >> + CPU hot-eject state between OVMF's SmmCpuFeaturesLib instance in >> + PiSmmCpuDxeSmm, and CpuHotplugSmm. >> + >> + CPU_HOT_EJECT_DATA is allocated in SMRAM, and pointed-to by >> + PcdCpuHotEjectDataAddress. >> + >> + PcdCpuHotEjectDataAddress is valid when SMM_REQUIRE is TRUE >> + and MaxNumberOfCpus > 1. > > (2) Looks good, thanks; please clarify it as follows though: > > s/MaxNumberOfCpus/PcdCpuMaxLogicalProcessorNumber/ > > >> + >> + Copyright (C) 2021, Oracle Corporation. >> + >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> +**/ >> + >> +#ifndef CPU_HOT_EJECT_DATA_H_ >> +#define CPU_HOT_EJECT_DATA_H_ >> + >> +/** >> + CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit() >> + on each CPU at exit from SMM. >> + >> + @param[in] ProcessorNum ProcessorNum denotes the CPU exiting SMM, >> + and will be used as an index into >> + CPU_HOT_EJECT_DATA->QemuSelectorMap. It is >> + identical to the processor handle in >> + EFI_SMM_CPU_SERVICE_PROTOCOL. >> +**/ >> +typedef >> +VOID >> +(EFIAPI *CPU_HOT_EJECT_HANDLER) ( >> + IN UINTN ProcessorNum >> + ); >> + >> +// >> +// CPU_EJECT_QEMU_SELECTOR_INVALID marks CPUs not being ejected in >> +// CPU_HOT_EJECT_DATA->QemuSelectorMap. >> +// >> +// QEMU CPU Selector is UINT32, so we choose an invalid value larger >> +// than that type. >> +// >> +#define CPU_EJECT_QEMU_SELECTOR_INVALID (MAX_UINT64) >> + >> +typedef struct { >> + volatile UINT64 *QemuSelectorMap; // Maps ProcessorNum -> QemuSelector >> + // for pending hot-ejects >> + volatile CPU_HOT_EJECT_HANDLER Handler; // Handler to do the CPU ejection >> + UINT32 ArrayLength; // Entries in the QemuSelectorMap > Acking the comments above. > (3) Please move "*QemuSelectorMap" and "ArrayLength" to the right, by 9 > spaces, so that they line up with "Handler". > > Now... I realize that such an update would result in the awkwarly wide > code snippet: > >> typedef struct { >> volatile UINT64 *QemuSelectorMap; // Maps ProcessorNum -> QemuSelector >> // for pending hot-ejects >> volatile CPU_HOT_EJECT_HANDLER Handler; // Handler to do the CPU ejection >> UINT32 ArrayLength; // Entries in the QemuSelectorMap >> }; > > where it is not really possible to re-wrap the comments usefully. With > that in mind, the following is likely the best approach: > >> typedef struct { >> // >> // Maps ProcessorNum -> QemuSelector for pending hot-ejects >> // >> volatile UINT64 *QemuSelectorMap; >> // >> // Handler to do the CPU ejection >> // >> volatile CPU_HOT_EJECT_HANDLER Handler; >> // >> // Entries in the QemuSelectorMap >> // >> UINT32 ArrayLength; >> }; > > This way, you have ample room for the comments. Plus, the field names > need not care about any visual alignment, because they are separated by > the comments anyway. Thanks for clarifying. I was wondering if this comment style is fine for data structures or not. Should have checked other code. Thanks Ankur > > Thanks! > Laszlo > >> +} CPU_HOT_EJECT_DATA; >> + >> +#endif // CPU_HOT_EJECT_DATA_H_ >> >