From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (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 8494C821B8 for ; Sun, 26 Feb 2017 18:22:36 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Feb 2017 18:22:36 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,211,1484035200"; d="scan'208";a="62400370" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga004.jf.intel.com with ESMTP; 26 Feb 2017 18:22:36 -0800 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 26 Feb 2017 18:22:36 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 26 Feb 2017 18:22:35 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX104.ccr.corp.intel.com ([10.239.4.70]) with mapi id 14.03.0248.002; Mon, 27 Feb 2017 10:22:33 +0800 From: "Fan, Jeff" To: Laszlo Ersek , "edk2-devel@ml01.01.org" CC: "Zeng, Star" , "Tian, Feng" , "Kinney, Michael D" Thread-Topic: [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error Thread-Index: AQHSjoKINgv0sffzNkOY5VweWjzRbaF8IzvA Date: Mon, 27 Feb 2017 02:22:33 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4C5494BA@shsmsx102.ccr.corp.intel.com> References: <20170224061211.24720-1-jeff.fan@intel.com> <1c624860-9079-5c7a-9cfe-db4d3c8d34ec@redhat.com> In-Reply-To: <1c624860-9079-5c7a-9cfe-db4d3c8d34ec@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYzRjM2NjOGYtMWZjOC00N2YxLWE1MTgtYjkxOWM3NTVjMjY5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Im0xRFRjalwvOU03dStJT3NKWjdEaXQ1UlZNNFZRbkZnWjJJTTRaRCtUYVwvQT0ifQ== x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Feb 2017 02:22:36 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Yes. Some platform sets memory allocation HOB. It makes sense. So, I remove= ASSERT() from the code. -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Friday, February 24, 2017 5:44 PM To: Fan, Jeff; edk2-devel@ml01.01.org Cc: Zeng, Star; Tian, Feng; Kinney, Michael D Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpac= e() error On 02/24/17 07:12, Jeff Fan wrote: > Platform PEI may add LOCAL APIC memory mapped space into=20 > EFI_HOB_MEMORY_ALLOCATION. Or platform may allocate this range before. >=20 > So, we skip AllocateMemorySpace()'s return status checking. Instead,=20 > we add one DEBUG message for possible trace. >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D390 >=20 > This updating is suggested by Ersek's comments at=20 > https://www.mail-archive.com/edk2-devel@lists.01.org/msg22585.html >=20 > Cc: Laszlo Ersek > Cc: Star Zeng > Cc: Feng Tian > Cc: Michael D Kinney > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan > --- > UefiCpuPkg/CpuDxe/CpuDxe.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) >=20 > diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c=20 > index 2fd2f31..4a5e282 100644 > --- a/UefiCpuPkg/CpuDxe/CpuDxe.c > +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c > @@ -1075,6 +1075,11 @@ AddLocalApicMemorySpace ( > Status =3D AddMemoryMappedIoSpace (BaseAddress, SIZE_4KB, EFI_MEMORY_U= C); > ASSERT_EFI_ERROR (Status); > =20 > + // > + // Try to allocate APIC memory mapped space, does not check return =20 > + // status because it may be allocated by other driver, or DXE Core=20 > + if // this range is built into Memory Allocation HOB. > + // > Status =3D gDS->AllocateMemorySpace ( > EfiGcdAllocateAddress, > EfiGcdMemoryTypeMemoryMappedIo, @@ -1084,7 +1089,10=20 > @@ AddLocalApicMemorySpace ( > ImageHandle, > NULL > ); > - ASSERT_EFI_ERROR (Status); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_INFO, "%a: %a: AllocateMemorySpace() Status - %r\n", > + gEfiCallerBaseName, __FUNCTION__, Status)); =20 > + } > } > =20 > /** >=20 Did you actually hit the ASSERT on some platform soon after we discussed it= ? :) Either way, Reviewed-by: Laszlo Ersek Thanks! Laszlo