From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=15.241.48.72; helo=g9t5008.houston.hpe.com; envelope-from=brian.johnson@hpe.com; receiver=edk2-devel@lists.01.org Received: from g9t5008.houston.hpe.com (g9t5008.houston.hpe.com [15.241.48.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 43D35202E60FA for ; Tue, 24 Oct 2017 15:26:41 -0700 (PDT) Received: from G9W9209.americas.hpqcorp.net (g9w9209.houston.hpecorp.net [16.220.66.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by g9t5008.houston.hpe.com (Postfix) with ESMTPS id 6238462; Tue, 24 Oct 2017 22:30:24 +0000 (UTC) Received: from G4W10205.americas.hpqcorp.net (2002:10cf:520f::10cf:520f) by G9W9209.americas.hpqcorp.net (2002:10dc:429c::10dc:429c) with Microsoft SMTP Server (TLS) id 15.0.1178.4; Tue, 24 Oct 2017 22:30:23 +0000 Received: from NAM01-BY2-obe.outbound.protection.outlook.com (15.241.52.11) by G4W10205.americas.hpqcorp.net (16.207.82.15) with Microsoft SMTP Server (TLS) id 15.0.1178.4 via Frontend Transport; Tue, 24 Oct 2017 22:30:23 +0000 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=brian.johnson@hpe.com; Received: from [10.0.2.15] (192.48.192.5) by DF4PR8401MB0411.NAMPRD84.PROD.OUTLOOK.COM (2a01:111:e400:7606::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.156.4; Tue, 24 Oct 2017 22:30:20 +0000 From: "Brian J. Johnson" To: Laszlo Ersek , "Dong, Eric" , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" , Paolo Bonzini References: <1508743358-3640-1-git-send-email-eric.dong@intel.com> <1508743358-3640-3-git-send-email-eric.dong@intel.com> <4fe39a52-0cd3-de2e-84f2-7363823a1b60@redhat.com> <1329f926-4c33-aaca-108a-7d15ae0503bc@redhat.com> Message-ID: <7cbc72b5-b829-079b-00ad-c46f224235f4@hpe.com> Date: Tue, 24 Oct 2017 17:30:14 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1329f926-4c33-aaca-108a-7d15ae0503bc@redhat.com> X-Originating-IP: [192.48.192.5] X-ClientProxiedBy: DM5PR0401CA0060.namprd04.prod.outlook.com (2603:10b6:4:73::37) To DF4PR8401MB0411.NAMPRD84.PROD.OUTLOOK.COM (2a01:111:e400:7606::12) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 3ddb1490-4c12-458d-9eab-08d51b2ed070 X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(48565401081)(4534020)(4602075)(4627075)(201703031133081)(201702281549075)(2017052603199); SRVR:DF4PR8401MB0411; X-Microsoft-Exchange-Diagnostics: 1; DF4PR8401MB0411; 3:Pug3f8kE9p6fPGKG/eVkkrsx7D+8vZwxLeBp1XOsM2D+hCX96E3E25zQH4teo0uo6tTRGwqv91pwf0DTD8nmbnw6h8woqxh1KjnAGi4kAALe8B1HKKSzhzsVV2Idj6CcoL/0+X8qjoEUh7TL04xlSD6LWsL+1uZikOFir+5Ab2wWuymaKBH6yZjZLJwkoykn4cbHNb63QUFG8QZJuyeWSlMSo4fKUtjkzc36HKgamzLwB6TKF8krsqvjlEG3dh22; 25:orB3f2YSnKQvT7fs2I8kT4+yAjm3S51k+SwMZnymrln+0+bzxSKXZMvc8pBdTMi8VIRWhrmYAfyRZ41xtqwPzOeg7qwyu5kWIAF0nIp0ECTDeYOaC/HBZ1NfhBBUajbc8YiwORk+X4TjGMLbtwRwJsKtL/4Tffn+6v7jIUmEuJzFFzHZ7xEImtJyWpJ4Nmac3xl0x4rbHrvAj+qw+uLKCK5Slc+ybvhCGzrR4DYIb1gxgGU2uNe7O1kFDeN1/8jtojDxJzX04d5EWWZ6vnDDignAnxjr2dywq5+84msPUftuCw5yJH3sjLDRQvx8C//k49jqoHsnLpKxTzYGWGJwjw==; 31:ek2//dKNZARFoR3M5+ZCZ6XK6pAGWkzrcqO+Jb6Bj1J7gDqT3cZSpIJgP0gSh+QCNgJ3LAxjxLJcmtncDU3KoyiBwjKZUW7LeVw1yZIFf4I70Zs9ME1D4zC/Sz6ytU8uWS7QFJZdfWoDxORNrM9CQXl4VTgIQD66C9Ly9m8+IMIoVYMvoHyhqGmUjA7C2MkZoMppWkIOsYb1Aj3k9gcOg5ID8K1U03cijK5t7EtcVyk= X-MS-TrafficTypeDiagnostic: DF4PR8401MB0411: X-Microsoft-Exchange-Diagnostics: 1; DF4PR8401MB0411; 20:MaI77UFVG3tTeyTZZ3YLwde9cwCI9Nnc/qvRGIPdMmbUjj5zjhkfXX3N/NcAMvr0A3ykw1XGW4uZTmIn2vW3ySSOUmCw4d5A6pcnzAsAqp2Foed+IIMuTUiUQuEsTZ1VZIG487JxyDndfM7Qxnb2wH9AKySWWS5vRD71708x7aWbuIk/EdDT/kKZx+0ORtQTV0+unDBupW3I2SPQRX45rkIvwJFXEGa+gYcWpVvaIxVIL2/+s7tSdHMjh+q8/yo9kjd1CXfBeRJBtDvdy5AsQ8VorwYt5OlypUW6lANhENyMYmLts87OD6m6DuDy1VaUUuxpWE2MLbujuRqFXPEo6xMHa7h8cfgl77esKzCW7WvihPyp+PZQwYBAICeMO8G5ddI2F5UsPog5/7TLpsOkd3ZvGS8bhyjQ2wtsAxqxOOvDaA1As1GxoDfu+VKGEGzcZK/F1Cnra2VKcM3rNCQrCszcbfXw+BCJ5bUxIauTOM+clt889YyIc5TSbMT4lqB0; 4:mtkedoCAaupo8Isg/Q5pVph5GznDoIz9x1osD93aGUaNyXx1jVNEDhTrYIaDmssoAQNYNbSwotq1ORShiLi75jsYhuTRLcFWaUJvAgrp2VTFMksj2Mo25VJ+skgKfWEDtO5yrk1+oyT95mhOXyIOu7IcXp/FDRIYlupN44r8Q7LojTqvcK8/wD+DadewDg5ia7n5lGBpcDo6/VVW4onSUfjMEcSfffb3nS+H+ZcVNSV9MOikD2CEXcHbnPWQ3ki7Y9p0UFzyYq2PCXJoEATPKr+rwqkGFNFFCgLzXoFw8IvEwdQM/DsoCbO04foGF9U2VWXgNIeuRV4VPaJ2LVaKlA== X-Exchange-Antispam-Report-Test: UriScan:(162533806227266)(228905959029699); X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(93006095)(93001095)(100000703101)(100105400095)(3231020)(3002001)(10201501046)(6055026)(6041248)(20161123558100)(20161123564025)(20161123555025)(20161123562025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:DF4PR8401MB0411; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:DF4PR8401MB0411; X-Forefront-PRVS: 047001DADA X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10019020)(6029001)(6009001)(6049001)(39860400002)(346002)(376002)(24454002)(13464003)(377424004)(199003)(189002)(51914003)(52314003)(4001150100001)(23676002)(106356001)(31686004)(3846002)(8676002)(6116002)(33646002)(36756003)(81156014)(68736007)(8936002)(81166006)(16526018)(77096006)(6246003)(105586002)(6486002)(229853002)(25786009)(230700001)(53936002)(2906002)(4326008)(6306002)(478600001)(2950100002)(2501003)(5660300001)(7736002)(16576012)(64126003)(65826007)(110136005)(966005)(54906003)(97736004)(6666003)(189998001)(305945005)(575784001)(83506002)(50466002)(86362001)(65956001)(53546010)(50986999)(76176999)(54356999)(101416001)(66066001)(47776003)(65806001)(316002)(58126008)(93886005)(31696002)(19627235001); DIR:OUT; SFP:1102; SCL:1; SRVR:DF4PR8401MB0411; H:[10.0.2.15]; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: hpe.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtERjRQUjg0MDFNQjA0MTE7MjM6WEU5L3QvQU1jK05PSGd2NUUrcWxkQkJT?= =?utf-8?B?YWFLUmxPMHMwdjM5akZOUlMxa0gwMThCS1ZaRjMwbG44TE4xNEhTT0FSa0xK?= =?utf-8?B?NzdrSzZlU3FxZ25LQVQwcVZ6d1ZWUHZGTXRsVGcycUhUK00xdUdydExpOElS?= =?utf-8?B?WkNHaG9DM1ZaM1RWdktPRFQ2Z254MFhzYmRBTlk5NENZN0tZY1RXU1kzMlJO?= =?utf-8?B?VmNUdURHTzBpVTJvVGh6OXcwc21BelJqRjk2YVpMQ2RiM29GUGJjeTU2Nkx3?= =?utf-8?B?VmtWSnBnOEVVZVl6NjVLMVhJUmMveVlnZHE5empiMHhvRWFQU2p6YmNxekt3?= =?utf-8?B?Mm1SR2FxN2lQeEVzdVA1RDNNYmZ0TzJxZSs1K0ViU3dOV3MxeDFodHFZOTRZ?= =?utf-8?B?OGNwdjN1ZVBTOVg4NHRtS1dMYXI1RW9XRDhtOEpXN0lhSkZWdDM0cE54NEFw?= =?utf-8?B?MzVLQzFXQlhMS3FpZ3l0VzAvMS9UTVFHRkhUa0JlTndQTjcrRWxoWVpGWUg0?= =?utf-8?B?M215Mko5RGdQTVpQVEdBcGprZ21qZmVsejBMa0pNakpjVXI1VjI2WnBnSjJZ?= =?utf-8?B?Z3J0QW1hZHhUdkpLWW40NEgyZlNyT1ZBSmlxazBXbHVkeE1Rb2VZZnUzdFhJ?= =?utf-8?B?ODhjNmMvUnBqcE1PQXRTdWlnb2FucXkyaXVXNFgvWTk0S3VFMEJQamZwU0Fj?= =?utf-8?B?bUtjWW1LbEZqWjZZYmMxOWF5RnFqekl1cCttT2dQZSs4MVIzMkcvOHlRMFVa?= =?utf-8?B?ajBpZFI0VUhMRmNaMzRVQ1lKdllnaFBaRk9uMVJlWklHUGx6emxrSTEyUG4y?= =?utf-8?B?RDQ4SmVoTTl3K0d1SEVJU0JPUGt3U1h4b3FKU3lLOUFZdGFIQnpGTnZHTFYr?= =?utf-8?B?YnZ0YTF2LzBpK3FnclBNenVPMlBHWmFyNkp6cEVZZ3RsSnRLNUlkMWovMklC?= =?utf-8?B?OW4xSXA1aGx2WHpOQW92aVJ3QVY3NEdNc1NHNXRWSlVkUzQrMkR3SmFEVlQy?= =?utf-8?B?Z3ZVeW0yWkkrVGs0WUdZY1VpTmd1TkdzdURwdDVRYXFCVGdHOXNvZDNrVlo4?= =?utf-8?B?cElQbnZuNnBNMSs2eVY1N1A2NjIvd1lHaU9rRFVvYVJVZ0ZObXpLWXRWajdC?= =?utf-8?B?Q3Z1b0Nmd2VtbHAreFJVWEt1Zmo1NVM0VW9SVFRPbGFqemoyQk9YNjh3Uk9M?= =?utf-8?B?aER4UGRHdlVsOGFDV0ZXRUE4Vm9oTmRUSDB5RzJjOC9FdWthS1AvdnJKWGIv?= =?utf-8?B?UTg4UmZLMVlOWHdCcS9VRHNiS0F3Qm45MTE2SlBSaXArZVZxcWFaRUZESExk?= =?utf-8?B?b0dScjJSckhvUXoxUmpSTFlyYnBURjBUdmNqamE2SXRxeXpGTDcwUFl0V0sx?= =?utf-8?B?ZWdaYmRsVnNheFZFdm02dERZaVNlRVpOSXNOMDlWQ3hFS2VQVW9EaVpoU2RU?= =?utf-8?B?OXc4WnZKUys2dU5tengvOVNXOEFsZUJBb1ZJamxZdzVYRUxpRWt6NitmTmY0?= =?utf-8?B?NVNhSWU1eWpWb3R0K0lhZTVzSVJGUUcwUGN4WmFrMHhPT2Mrd29wYWNUY056?= =?utf-8?B?eit1eEcreGY3WWZiOENyMTRyUU9abTcwREUzWFN0YU0wRks1TGRHWmNiZ2tk?= =?utf-8?B?NnBTQnUrR0gzOWtua24zakk3R0Z4blh2ZG9mT2tZMXFlY0Yyb01MN05VMTVL?= =?utf-8?B?eWZvMWRGVDhDZG1ISDBUdzVGOElibFNYbjIxM0RCMktvbWppcHVRaDZOQkRx?= =?utf-8?B?R3hoVWNHSWJTMmdvODBwQUZOSWVEekJTOEE1TUdCRW1jRWQ2ZkJybG1VOXYr?= =?utf-8?B?N0ZUZGxVVFFaK0NSQ0xVQVBHc1YyU3hrSk05VzEyU1VBR1BCSm9yUWhJWFZx?= =?utf-8?B?MzU5bm1uaVNTUEVnZTBVZUkxM2RKdWFhc1ZPOThKc0VLMHN6ZnVlQTg2S0Fo?= =?utf-8?B?WWNoWXArSzRleVl1T2I4RWJEcXlsMXkxMGUyek8rVWtoOXdCeHQxeXg5L2lP?= =?utf-8?B?OUYyVjFIZHduNEdROUNESkFQZ1dPVW1ENHlsMzQ0bkxVWjZ2Wkd2N3pUYy9U?= =?utf-8?B?RTh2a2xoRFFXc2UvWlBtVkJEK1RwZFIrWGFhVXlya3N0WGFsQ0Z3R2U0cnBl?= =?utf-8?B?QTVFZEpoZTZGbE5SZmhMTWkweWNyQUdFWEtZNTUwOCtNbHRFSHM0WXdKbWlr?= =?utf-8?Q?EcwLR+brjI/qI44DEudCyUrk/bqYbBvZ0gnNsTDfMMjo=3D?= X-Microsoft-Exchange-Diagnostics: 1; DF4PR8401MB0411; 6:3vb8DXpb48ewpdffY1KVpwSSuG9OlFewQ6nt5GvihXmn2DFcOP3xReJkKcw8bN7eagzGi5NSNtIq0u7muq5TMvp22NdjyTIFiwFYgtRDcqtlK3zox5IGVqmMv8dtBQHcOaAf6Chu5quRPg1+UPD8s/0s86BS/KmD7ePCXsrXwoOJu/cSJfo1sJL5gyApkrlcuFPhhPSG867GtBsttwzeod447U1VXikpHw99D0SXrK0J0ZXT85pCWLxiLprcLinssROP03zSGiK87dOhcBtihggGf3p6+G0+6b/C3eYR2488FhzG1wEkIQ1Dh5e9ztUPnYM/NLqSCZ1f8y7Uhb8X/w==; 5:aUStaXEsHeIUpMEzVjOyamuEuR/nR/Ji+h3x4H19xGryqJMriAOPf/njDKrVks3jwGMsiXaGhSikSKstzJ2S7TUf0XhNCYylThWqkLWaixsbOcoRDy0K6qCLC7dFriul+o4GKmuIExJxcb8akXrgyQ==; 24:O/+U4NxNuPPdGL+oadd0+BFtD8tglmlwaJgr6t5S9Pz912vWxPxRVJckvyp+mp2TMyW1bki74ouInu3CUKUeY9s4wJx0a35e8Rmsw+d+GsI=; 7:+TVzHB4dyJFrQPt+QhFcYUPLdl/Re+nAY8aJf3A5AX35UIt8o770D09Zgwq9tIWxX/LPpbL3XwxBuDRlizGFCkpqNAASg1Atlefyeoh6XU3jepe+TZWMZWKN/djUqwpBeJcYAg8HjZ2pmpAos+1nozzNvdUa1nn7D7W8UQAWB8Cztv6GHjPA1B2eQHTSduJdf3xPuLvNAdb9bsSGnZqq9lj2dHO9GILQqVECpf6l/CA= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Oct 2017 22:30:20.0717 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 3ddb1490-4c12-458d-9eab-08d51b2ed070 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR8401MB0411 X-OriginatorOrg: hpe.com Subject: Re: [Patch 2/2] UefiCpuPkg/MpInitLib: Enhance waiting for AP initialization logic. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Oct 2017 22:26:41 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/24/2017 12:40 PM, Laszlo Ersek wrote: > Hi Eric, > > On 10/24/17 17:23, Dong, Eric wrote: >> Laszlo, >> >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Tuesday, October 24, 2017 6:16 PM >>> To: Dong, Eric ; edk2-devel@lists.01.org >>> Cc: Ni, Ruiyu ; Paolo Bonzini >>> Subject: Re: [edk2] [Patch 2/2] UefiCpuPkg/MpInitLib: Enhance waiting for >>> AP initialization logic. >>> >>> CC Paolo >>> >>> On 10/23/17 09:22, Eric Dong wrote: > >>>> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc >>>> b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc >>>> index 976af1f..bdfe0d3 100644 >>>> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc >>>> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc >>>> @@ -40,4 +40,5 @@ EnableExecuteDisableLocation equ LockLocation >>> + 30h >>>> Cr3Location equ LockLocation + 34h >>>> InitFlagLocation equ LockLocation + 38h >>>> CpuInfoLocation equ LockLocation + 3Ch >>>> +NumApsExecutingLocation equ LockLocation + 40h >>>> >>>> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm >>>> b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm >>>> index 1b9c6a6..2b6c27d 100644 >>>> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm >>>> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm >>>> @@ -86,6 +86,12 @@ Flat32Start: ; protected mode >>> entry point >>>> >>>> mov esi, ebx >>>> >>>> + ; Increment the number of APs executing here as early as possible >>>> + ; This is decremented in C code when AP is finished executing >>>> + mov edi, esi >>>> + add edi, NumApsExecutingLocation >>>> + lock inc dword [edi] >>>> + >>>> mov edi, esi >>>> add edi, EnableExecuteDisableLocation >>>> cmp byte [edi], 0 >>>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c >>>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c >>>> index db923c9..48f930b 100644 >>>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >>>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >>>> @@ -662,6 +662,7 @@ ApWakeupFunction ( >>>> // AP finished executing C code >>>> // >>>> InterlockedIncrement ((UINT32 *) &CpuMpData->FinishedCount); >>>> + InterlockedDecrement ((UINT32 *) >>>> + &CpuMpData->MpCpuExchangeInfo->NumApsExecuting); >>>> >>>> // >>>> // Place AP is specified loop mode @@ -765,6 +766,7 @@ >>>> FillExchangeInfoData ( >>>> >>>> ExchangeInfo->CFunction = (UINTN) ApWakeupFunction; >>>> ExchangeInfo->ApIndex = 0; >>>> + ExchangeInfo->NumApsExecuting = 0; >>>> ExchangeInfo->InitFlag = (UINTN) CpuMpData->InitFlag; >>>> ExchangeInfo->CpuInfo = (CPU_INFO_IN_HOB *) (UINTN) >>> CpuMpData->CpuInfoInHob; >>>> ExchangeInfo->CpuMpData = CpuMpData; >>>> @@ -934,13 +936,19 @@ WakeUpAP ( >>>> } >>>> if (CpuMpData->InitFlag == ApInitConfig) { >>>> // >>>> - // Wait for all potential APs waken up in one specified period >>>> + // Wait for one potential AP waken up in one specified period >>>> // >>>> - TimedWaitForApFinish ( >>>> - CpuMpData, >>>> - PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, >>>> - PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) >>>> - ); >>>> + if (CpuMpData->CpuCount == 0) { >>>> + TimedWaitForApFinish ( >>>> + CpuMpData, >>>> + PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, >>>> + PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) >>>> + ); >>>> + } >>> >>> I don't understand this change. The new comment says, >>> >>> Wait for *one* potential AP waken up in one specified period >>> >>> However, the second parameter of TimedWaitForApFinish(), namely >>> "FinishedApLimit", gets the same value as before: >>> >>> PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1 >>> >>> It means that all of the (possible) APs are waited-for, just the same >>> as before. >> >> [[Eric]] This patch changes the collect AP count logic, original >> solution always waits for a specific time to let all APs start up. If >> the input CPU number (PcdGet32(PcdCpuMaxLogicalProcessorNumber) - 1) >> have been found or after a specific time(PcdGet32 >> (PcdCpuApInitTimeOutInMicroSeconds)). BSP will not wait anymore and use >> CpuMpData->CpuCount as the found AP count. >> >> New logic also wait for a specific time, but this time is smaller than >> the original one. It just wait for the first AP(any AP) begin to do the >> initialization( do CpuMpData->MpCpuExchangeInfo->NumApsExecuting++ means >> it begin to do the initialization). When Ap finishes initialization, it >> will do CpuMpData->MpCpuExchangeInfo->NumApsExecuting--. So after BSP >> waits for a specific time at first, it just needs to check whether >> CpuMpData->MpCpuExchangeInfo->NumApsExecuting == 0 to know whether all >> Aps have finished initialization. Here we still use the original PCD >> (PcdCpuApInitTimeOutInMicroSeconds) for the new time value. >> >> When one AP do the initialization, it will also do >> CpuMpData->CpuCount++. So here I check whether CpuMpData->CpuCount != 0 >> to know whether APs already begin to do the initialization. If yes, I >> not need to do the time out waiting anymore, just check the >> CpuMpData->MpCpuExchangeInfo->NumApsExecuting to know whether all Aps >> have finished initialization. > > Thanks for the explanation. > > The "NumApsExecuting" increment / decrement logic in this patch expects > that the APs work as follows: > > (1) After the INIT-SIPI-SIPI, there may be a delay before the APs start > running. During this delay, the BSP may or may not reach the code in > question. The (CpuMpData->CpuCount != 0) check is supposed to take this > into account. > > (2) After at least one AP has started running, the logic expects > "NumApsExecuting" to monotonically grow for a while, and then to > monotonically decrease, back to zero. For example, if we have 1 BSP and > 7 APs, the BSP logic more or less expects the following values in > "NumApsExecuting": > > 1; 2; 3; 4; 5; 6; 7; > 6; 5; 4; 3; 2; 1; 0 > > > While this may be a valid expectation for physical processors (which > actually run in parallel, in the physical world), in a virtual machine, > it is not guaranteed. Dependent on hypervisor scheduling artifacts, it > is possible that, say, three APs start up *and finish* before the > remaining four APs start up *at all*. The INIT-SIPI-SIPI marks all APs > for execution / scheduling in the hypervisor, yes, but the actual code > execution may commence a lot later. For example, the BSP may witness the > following series of values in "NumApsExecuting": > > 1; 2; 3; > 2; 1; 0; > 1; 2; 3; 4; > 3; 2; 1; 0 > > and the BSP could think that there are 3 APs only, when it sees the > first 0 value. > Eric, I agree with Laszlo -- this patch introduces race conditions and very machine-specific behavior assumptions. That's dangerous at best, and can easily break machines which are perfectly valid (such as VMs and node-controller based scalable systems) but don't meet the implicit assumptions. I'd rather see each AP get tracked individually. For example, have each AP set a bit in a global bitmap when it starts executing, and set a bit in another global bitmap when it completes. Then the BSP can compare the bitmaps to determine how many APs are running. That also provides good debug information on exactly which APs start running, and exactly which ones fail to complete. Otherwise that's quite difficult to determine. Thanks, Brian Johnson > > Now, let me get back to the use case that actually matters to OVMF and > QEMU. As I mentioned earlier, OVMF knows from QEMU the exact number of > APs. If there is a way for OVMF to tell MpInitLib to wait for this exact > number of APs -- no matter how long it takes --, then that's what I > would like to use. > > Please see the original discussion around OVMF commit 45a70db3c3a59: > > * In version 1, I introduced a new PCD called > > PcdCpuKnownLogicalProcessorNumber > > and I modified MpInitLib to wait for this AP number, ignoring timeout > completely, if the PCD was set: > > https://lists.01.org/pipermail/edk2-devel/2016-November/005076.html > > However, Jeff suggested to use the preexistent PCD > "PcdCpuMaxLogicalProcessorNumber" for this purpose: > > https://lists.01.org/pipermail/edk2-devel/2016-November/005092.html > > * In version 2, I used the PCD suggested by Jeff, but I also introduced > a new special value for the timeout. Timeout=0 would mean "infinity": > > https://lists.01.org/pipermail/edk2-devel/2016-November/005209.html > > Jeff didn't like the special value, and suggested that OVMF simply use > MAX_UINT32 microseconds (approx. 71 minutes) instead of "infinity": > > https://lists.01.org/pipermail/edk2-devel/2016-November/005266.html > > * In v3, I implemented that, and that was pushed as: > > - UefiCpuPkg commit 6e1987f19af7 ("UefiCpuPkg/MpInitLib: wait no > longer than necessary for initial AP startup", 2016-11-24) > > - and OvmfPkg commit 45a70db3c3a5 ("OvmfPkg/PlatformPei: take VCPU > count from QEMU and configure MpInitLib", 2016-11-24). > > > So, again, the use case that I would like to cover is: > > * the exact number of APs is known at boot, to OvmfPkg/PlatformPei, > > * after the very first INIT-SIPI-SIPI, MpInitLib should wait for exactly > this number of APs to "report in", regardless of: > > - how long it takes, > > - in what order / sequence the APs report in. (Again, please remember > that some APs may complete the initialization before other APs > execute their very first instruction.) > > * Preferably, the case should be handled when the processor count grows > from cold boot to S3 resume (VCPU hotplug). TianoCore BZ#251 used to > track this use case separately. > > ( > > Jeff closed BZ#251 today, with the argument that commit 0594ec417c89 > (this patch) finds the CPU count dynamically anyway, so a platform can > simply set "PcdCpuMaxLogicalProcessorNumber" to the maximum possible value. > > This argument does not work in a virtual machine, because commit > 0594ec417c89 (this patch) may in fact not find the VCPU count correctly > -- in a VM, (NumApsExecuting==0) does not imply that all APs have finished. > > ) > > Thanks! > Laszlo > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > -- Brian -------------------------------------------------------------------- "I don't believe personal letters sent bulk rate." -- me