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.web08.5521.1612331204963118271 for ; Tue, 02 Feb 2021 21:46:45 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@oracle.com header.s=corp-2020-01-29 header.b=ARPiVWpp; 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 1135iuol136868; Wed, 3 Feb 2021 05:46:41 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=lH/w6cClVyP8cHQEatcWDyokC5cetll/LU9mUvPc05w=; b=ARPiVWppG7gZMsbRIyEFrVNLgGTlar3j+FY5MCkaeC4X96lgJVEUdtnV2j/bM8J42K6k tw58/nGorjb5raCaxBU9qEdk+xmXpYyufUfVuJbRdt5BBvs1m+gn8rzeSXVJ/g3L4rOV K+tWEhRKYJC8Uw0aHTqDkPKYhCUnq6TwGmSWbP9vndTwEzKRX5FnpVX3ANXGAVvMms6U X1bjLBZzMEKPZCxnVnkSPxaTjldVodjliqqo4yyo9LrhWpG+BIIjPaH4R2V2G04kHu7c 9Y29iwLhKWBYROLSJqBuI11EFCLEzqBaaXEARcpOErxf5gqFTZdchm4kP7Mn64GGJJAa Zg== Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by aserp2120.oracle.com with ESMTP id 36cydkx5c5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 03 Feb 2021 05:46:41 +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 1135eGdO193867; Wed, 3 Feb 2021 05:46:40 GMT Received: from nam12-mw2-obe.outbound.protection.outlook.com (mail-mw2nam12lp2040.outbound.protection.outlook.com [104.47.66.40]) by userp3020.oracle.com with ESMTP id 36dh7sr9jh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 03 Feb 2021 05:46:40 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AycPHuS9BqyqXnwdDRL5iOHFjYdA3+XPvYjxyQksdxITA5o04da5e7DuyD000rD1XmloWXOiLbkumQwNvEXcBBi86amZH87gMl+IhKvLBSkWoQ78aubNHOF5sf1qyfSvp7hmhtJyd7CisgZWe5xOLyl0RhLcCMMdrYkUYSfYDq7euoHZKEdZwImbSa0HfLCkvgysANNAgjQC3046bqD6GMtPDTFLP1GCyhECropF6jk73IrGp9qyp/0XcMQw7dvmj8anJ6FRS2xGki4J8jjQx0fN7tobGT7Ji0Ax9oGR6GrqjmFhe+wLmL5Mwdwh30yrc3/NP3ssKWfhh4h59wzilA== 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=lH/w6cClVyP8cHQEatcWDyokC5cetll/LU9mUvPc05w=; b=mjkQ2lpye/LSmtBY+GbkiaHlRTlAeiroATzhNXuHMf3dEuNjGZ0EF6CJsWGGxgUcJ8yPdb5+otME6b3WsuyX2eDwHq9E82m9wmt+0kTzObfHq1v8vIU35SCBgLOCe5KR4+xN/ULctIx8DYHQkGyOqwKxc2kGyJM34RzZybYu2PYtpW/kvF4HlJj/fGEVG3ucdpqSjA1DAoNhbdVKBKZDKG4gNkW2IhQoMnZJPoWLoaxhtkDKrvBXJOIV7b03SzZUhz7WhKrSeCL6E2JskHl4HXGTcpVj3aL1ECu+oLO1Bv6HJryWtdpPWtCZ7OpQ+mcz23o2Bt995eOrDpWfAZl+3g== 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=lH/w6cClVyP8cHQEatcWDyokC5cetll/LU9mUvPc05w=; b=EoLpxXykNmGCl9zyIUMvOIBnlLDST0ExeZFGnr4Wd+E9p2NHUhK48AvWHJzJjR66wQ0tfylOpsqdja4aPiAbXuBkluLb0TOQ32Kxpzii98R0UykRM8PIl+wH+mAY6Mqklquwd/CYC7zd0L8KxvZkZcYn8jGQIrXupHSdU4qNzCo= Received: from SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) by SJ0PR10MB4525.namprd10.prod.outlook.com (2603:10b6:a03:2db::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3805.17; Wed, 3 Feb 2021 05:46:38 +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 05:46:38 +0000 Subject: Re: [PATCH v6 9/9] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug 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-10-ankur.a.arora@oracle.com> <92a29bb5-fc0e-c11f-2702-07c7729a11b4@redhat.com> From: "Ankur Arora" Message-ID: <90fcd0ff-8065-9575-c5ec-820176a4e713@oracle.com> Date: Tue, 2 Feb 2021 21:46:32 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 In-Reply-To: <92a29bb5-fc0e-c11f-2702-07c7729a11b4@redhat.com> X-Originating-IP: [148.87.23.11] X-ClientProxiedBy: CO2PR06CA0053.namprd06.prod.outlook.com (2603:10b6:104:3::11) 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 CO2PR06CA0053.namprd06.prod.outlook.com (2603:10b6:104:3::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.17 via Frontend Transport; Wed, 3 Feb 2021 05:46:35 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 65d38182-9d1c-4353-33b7-08d8c80712cd X-MS-TrafficTypeDiagnostic: SJ0PR10MB4525: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: oSSmthDvikSQ814qM1OQbfG8rBBG4QGazxRXwLpWXoRDjHnOvFVXXeHC6QQkMQ8IKYOKCbTi0L9ATQhRWcSN2RwGFGXNCjzHJiBRlrRQcMX4+BldKMihX4zmzlS41IYyiwtguu4DLDwNQSuLljMh3JK/QkrgpAauFlnwZL138/Nlnkk5dbfwmnnG/JCqaAjaNRZZm0wVgw+ZRMVhm1IvjkCHa7Gkpc/LB4o+OD1HiPEsIOt7Q0GWQCwddmx9cmJnOyJ1TAZeajVtjmoLWGX7nSVYk9jN82b7Y8HvsfljQXqRiuc3idp3RuWK2j3xb8z/m0282PKbeD9v+kxX4UU4aniM/5hf9QbOZ0hYt0L86y2wMza/HCscU6ra74EDAQNyoxsFtzwOtOmBCSs/jQ0O5dQMVEyxjh84RTscwwJEgbl7y4nRXSJo4kIFPEFnfDgoqtyp1nXZNwNjmQt79CE+kEeNNwjbJ+H8H2za8X1em9HkSZk9GnHS50zPoFkEDhc4UhwHTDM/T2fQey2ywX1xnorBGxsecVt10K7jNUtQbKZQYdYZmf4B+VMXzH5Xsxw5qs120+jUI4qDunV2bWFfpFjHAnSsmnm4OhkOA+9EY7vx/siWdCmu7AfFmBdJCI/kzGRcMqytK4NdIon1txa2qPXYDcjPZMq7HmT2I2SaNVodcPmJo/z9J6hFEgukvA80 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:(346002)(396003)(376002)(136003)(39860400002)(366004)(26005)(8936002)(6486002)(66556008)(316002)(66476007)(31686004)(956004)(2616005)(31696002)(66946007)(6666004)(107886003)(5660300002)(83380400001)(54906003)(8676002)(16576012)(2906002)(86362001)(36756003)(186003)(478600001)(19627235002)(16526019)(4326008)(53546011)(966005)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?S0daYjgwQlFsd3NKODVsVk9CVUlnbnZDdzhOc2hkYUFUOGpnelBiWGVwbldR?= =?utf-8?B?UW1GakhmZGZaMVNrTmk5NlpWMDdyL0VQdktCYy9Ib1JEQXRTMzg0TzcvWktv?= =?utf-8?B?Qm1TTWZPaFhJVXc0MFNWSjgvVC96SGUxZ0RmL2l0UUZGQy9HUU9ldVRXVGRR?= =?utf-8?B?V09PQndmVlJsVnBtaUUxMlBmRTFPZDdsMThlWTFGTU9BWHFYZ2sxd0tDZjg4?= =?utf-8?B?a0NuUEFYZTdEY3NVNXJkWi9HK1FUVGl3Z09vSUh2MnVnT0Y3dzI5RlppeDNu?= =?utf-8?B?L0R6Y1JvSFhWVHQ4ZFlBbkYvNUs4c2ZhQkp2b3ZPMEpNUG1OUHp5V2UyYVBu?= =?utf-8?B?WWlXc0R4UjA3dEhBTXFxNHZUVFBIYTdzMi82VklnTzIxclVvOUd2OU82YmdR?= =?utf-8?B?eEVnR3pDZzdtQzBpYTAvdGVDTXgzZFJHN09hNjkrdXEyV2ZmUUhmaytFc0pU?= =?utf-8?B?YlN4Ums3MHNvNEFUZm9CTTRiNlBVOFducXZLVVJBclc0bm9lK3FtUzRTRGJ3?= =?utf-8?B?Qnk3bi9oYlRQanhpT2pNelRleFlYdnV1TDdYN1RNWjZ0Q01VdytTTFIveTh1?= =?utf-8?B?U2JZaG1FOVd1UG1rN1dJTnhhdlFxS1YweTNWblI3Q1gyWktaVEtETnRHK3cw?= =?utf-8?B?T0N6dGRvNXNEelpKZGU4Z2JnVStJenJHRkFMdytpamVDV0E5cDNoeFMwYXJL?= =?utf-8?B?Vm4wdVh1ZWU5ZGxjN1JlWlUyUFJkY256WWN3YkhaRlVVTTRlaWY0RW9IWEVx?= =?utf-8?B?b1phZGlhVEZXTmRObnhiWUtaeW1YTVV1dm9tcVVBQ3BnL0J3ejB2SWlKbVZB?= =?utf-8?B?allBdlV1dDFDMGJMa2d1M3BiU0dDNEhzVmlQOCtpSlNUQ21vcG40cTZ2MVhR?= =?utf-8?B?Q2RvYTRKTEdzOHYrUHpiZ3BJeFJBNUsxeHBhQUZONTZ1dFZBOVl5ZEtISFB2?= =?utf-8?B?Zy9VVlM1YlZaVnlEVlF5MWZFeXcvMmdaZWh0bUJFSUtza1lzaWp2WU80aXBU?= =?utf-8?B?VnpibS9jbkNzNXVEYU5rMjcvT1hjUzNZQlU5cmZ2WjB5QUNDdEV6OUltOFgv?= =?utf-8?B?Q0pMVG44alNVbmJwejhNbGpxeDlGbkZvWUI4eGwvR2F1QXhYLzlOSkJlcisy?= =?utf-8?B?ZWVzdFFKd2dFUnNnMUQyTExHVFhubG5uRWFWeWQxbkZFVVA0Q3BuNE9JaFpT?= =?utf-8?B?aEpjczAxdkpMSFMxZ0krMzNyaW5RbGVvWldmZUNITnZzS29kZUZldzJWNFUx?= =?utf-8?B?MXBxSjRqYVR4bnlBZ210VUJhNVhRMDRETklNaFc2RWs1dDlaL21XYWsxeFFw?= =?utf-8?B?UjRMcUNrYzdyUTg2M1VUVm9BeWdZaVdpQmlqRGVTWDhwcTQ0WW9pTGpHQTlE?= =?utf-8?B?TCtqaDV6VEYrNCtjbDBDVGlzRjQvcFc4OXFRL2wrUUMxVlp3eDBuMnovcHNG?= =?utf-8?Q?DYg8bK+H?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 65d38182-9d1c-4353-33b7-08d8c80712cd X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4605.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Feb 2021 05:46:38.0750 (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: 8OnOxS+pO6frz7JlpPwTq2mKOOtlDb9YWoVLcAsnLecN71QSrIKM6ooUmE8HtJycag7vQtfk885CUsnujx7DcBiXN51/7sKFleQcVr742xg= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4525 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9883 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 suspectscore=0 spamscore=0 mlxlogscore=999 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102030034 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9883 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 adultscore=0 priorityscore=1501 impostorscore=0 malwarescore=0 clxscore=1015 spamscore=0 lowpriorityscore=0 phishscore=0 mlxlogscore=999 mlxscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102030034 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2021-02-01 9:37 a.m., Laszlo Ersek wrote: > On 01/29/21 01:59, Ankur Arora wrote: >> As part of the negotiation treat ICH9_LPC_SMI_F_CPU_HOT_UNPLUG as a >> subfeature of feature flag ICH9_LPC_SMI_F_CPU_HOTPLUG, so enable it >> only if the other is also being negotiated. >> >> 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 >> --- >> OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c >> index c9d875543205..e70f3f8b58cb 100644 >> --- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c >> +++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c >> @@ -29,6 +29,13 @@ >> // >> #define ICH9_LPC_SMI_F_CPU_HOTPLUG BIT1 >> >> +// The following bit value stands for "enable CPU hot unplug, and inject an SMI > > (1) s/hot unplug/hot-unplug/ > > >> +// with control value ICH9_APM_CNT_CPU_HOT_UNPLUG upon hot unplug", in the > > (2) There is no such thing as ICH9_APM_CNT_CPU_HOT_UNPLUG; we use the > same SMI command value ICH9_APM_CNT_CPU_HOTPLUG (= 4) for unplug. > > In QEMU, the macro is called OVMF_CPUHP_SMI_CMD. > > > (3) s/hot unplug/hot-unplug/. > > >> +// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files. >> +// Is only negotiated alongside ICH9_LPC_SMI_F_CPU_HOTPLUG. > > (4) Please drop the last sentence (see more on it below). > > >> +// >> +#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG BIT2 >> + >> // >> // Provides a scratch buffer (allocated in EfiReservedMemoryType type memory) >> // for the S3 boot script fragment to write to and read from. >> @@ -112,7 +119,8 @@ NegotiateSmiFeatures ( >> QemuFwCfgReadBytes (sizeof mSmiFeatures, &mSmiFeatures); >> >> // >> - // We want broadcast SMI, SMI on CPU hotplug, and nothing else. >> + // We want broadcast SMI, SMI on CPU hotplug, on CPU hot-unplug >> + // and nothing else. >> // >> RequestedFeaturesMask = ICH9_LPC_SMI_F_BROADCAST; >> if (!MemEncryptSevIsEnabled ()) { > > (5) Please spell out the full expression "SMI on CPU hot-unplug". > > >> @@ -120,8 +128,18 @@ NegotiateSmiFeatures ( >> // For now, we only support hotplug with SEV disabled. >> // >> RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOTPLUG; >> + RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOT_UNPLUG; >> } >> mSmiFeatures &= RequestedFeaturesMask; >> + >> + if (!(mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOTPLUG) && >> + (mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOT_UNPLUG)) { >> + DEBUG ((DEBUG_WARN, "%a CPU host-features %Lx, requested mask %Lx\n", >> + __FUNCTION__, mSmiFeatures, RequestedFeaturesMask)); >> + >> + mSmiFeatures &= ~ICH9_LPC_SMI_F_CPU_HOT_UNPLUG; >> + } >> + >> QemuFwCfgSelectItem (mRequestedFeaturesItem); >> QemuFwCfgWriteBytes (sizeof mSmiFeatures, &mSmiFeatures); >> > > (6) Please drop this hunk. We don't try to be smarter than QEMU, in > general, whenever we perform feature negotiation. Also, AFAICS, we will do the hotplug (and now hot-unplug) even if it wasn't negotiated? > > For example, the pre-patch code doesn't attempt to notice if QEMU > acknowledges ICH9_LPC_SMI_F_CPU_HOTPLUG but not ICH9_LPC_SMI_F_BROADCAST. > > >> @@ -162,8 +180,9 @@ NegotiateSmiFeatures ( >> if ((mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOTPLUG) == 0) { >> DEBUG ((DEBUG_INFO, "%a: CPU hotplug not negotiated\n", __FUNCTION__)); >> } else { >> - DEBUG ((DEBUG_INFO, "%a: CPU hotplug with SMI negotiated\n", >> - __FUNCTION__)); >> + DEBUG ((DEBUG_INFO, "%a: CPU hotplug%s with SMI negotiated\n", >> + __FUNCTION__, >> + (mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOT_UNPLUG) ? ", unplug" : "")); >> } >> >> // >> > > (7) Rather than combining these two in a common debug message, please > just add a separate "if" that follows the whole pattern seen with > ICH9_LPC_SMI_F_CPU_HOTPLUG. Thus, for each feature bit we care about, > we'll have a dedicated log message, saying yes or no. Acking this set (and the ones down-thread.) Will fix. Thanks Ankur > > Thanks! > Laszlo >