From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web12.10372.1583324598951235114 for ; Wed, 04 Mar 2020 04:23:19 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=RqNWTBRh; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1583324598; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Eq2qUgopbdBKMT7NzH8wh0DnSKH5cB52T0ZO04JVGZc=; b=RqNWTBRhIsQi5mwXhYTppbv8J1ZJCOf1buokr65tJrL+Z3CIBmYuUbr7dAILLts6c9dKCZ WYYaMwXiB5XVYT1fiamShH23NbuEliUDs8Mz+FHHraFVRIojrzqN0W042+yMX+85nEAW/W pfmKq+biRpRsbFXIYcM10cfsiBNwzpU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-257-g67kLVBtPaejbMqOK3Dg1A-1; Wed, 04 Mar 2020 07:23:14 -0500 X-MC-Unique: g67kLVBtPaejbMqOK3Dg1A-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C32ADDB22; Wed, 4 Mar 2020 12:23:12 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-59.ams2.redhat.com [10.36.117.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 76541100194E; Wed, 4 Mar 2020 12:23:10 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 02/16] UefiCpuPkg/PiSmmCpuDxeSmm: fix S3 Resume for CPU hotplug To: devel@edk2.groups.io, eric.dong@intel.com Cc: Ard Biesheuvel , Igor Mammedov , "Yao, Jiewen" , "Justen, Jordan L" , "Kinney, Michael D" , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , "Ni, Ray" , Boris Ostrovsky References: <20200226221156.29589-1-lersek@redhat.com> <20200226221156.29589-3-lersek@redhat.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 4 Mar 2020 13:23:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Hi Eric, On 02/28/20 04:05, Dong, Eric wrote: > Hi Laszlo, > > Thanks for your patch. The change make sense base on the comments in > the data structure header file. > > I also checked all the code related to this data structure. The inputs > for this data structure are CpuS3DataDxe and RegisterCpuFeaturesLib. > Both these two drivers not support CPU hot plug feature, so the real > inputs for mAcpiCpuData.NumberOfCpus is the enabled CPU number in this > system. So before and after your code change, the CPU values are same. > But the data structure comments said it can support CPU hot plug, so I > agree your code change. > > Reviewed-by: Eric Dong while merging this patch, the edk2 CI system reported the following build warning (in the "Windows VS2019 PR" check): https://github.com/tianocore/edk2/pull/416/checks?check_run_id=484688972 https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=4568&view=logs&j=898a5c7a-7a49-5be1-c417-92a6761a8039&t=7c50f5c2-8a2c-50f9-5007-e26f12377af8&l=64 > 2020-03-04T11:06:29.7838532Z ERROR - Compiler #2220 from d:\a\1\s\UefiCpuPkg\PiSmmCpuDxeSmm\CpuS3.c(624): the following warning is treated as an error > 2020-03-04T11:06:29.7839570Z WARNING - Compiler #4244 from d:\a\1\s\UefiCpuPkg\PiSmmCpuDxeSmm\CpuS3.c(624): '=': conversion from 'UINTN' to 'volatile UINT32', possible loss of data > 2020-03-04T11:06:29.7841177Z WARNING - Compiler #4244 from d:\a\1\s\UefiCpuPkg\PiSmmCpuDxeSmm\CpuS3.c(659): '=': conversion from 'UINTN' to 'volatile UINT32', possible loss of data I've suppressed that by squashing the following hunks into the patch: > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > index 1e0840119724..29e9ba92b453 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > @@ -621,7 +621,7 @@ InitializeCpuBeforeRebase ( > } else { > ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus); > } > - mNumberToFinish = mNumberOfCpus - 1; > + mNumberToFinish = (UINT32)(mNumberOfCpus - 1); > mExchangeInfo->ApFunction = (VOID *) (UINTN) InitializeAp; > > // > @@ -656,7 +656,7 @@ InitializeCpuAfterRebase ( > } else { > ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus); > } > - mNumberToFinish = mNumberOfCpus - 1; > + mNumberToFinish = (UINT32)(mNumberOfCpus - 1); > > // > // Signal that SMM base relocation is complete and to continue initialization for all APs. Thanks, Laszlo