From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=40.107.6.56; helo=eur01-db5-obe.outbound.protection.outlook.com; envelope-from=girish.pathak@arm.com; receiver=edk2-devel@lists.01.org Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-eopbgr60056.outbound.protection.outlook.com [40.107.6.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 58EB821F833B0 for ; Thu, 4 Jan 2018 10:53:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=N45Y+dYX4CJqMeI7AfIG/QsoWjubqRUuRIKQ6ypSP4s=; b=HkxoG7hURNNWzDtwvAJzOYte/BpZ0/tjEwkYusFdAB6eAoWXjmEeqmMq0cyibd5rvRzKeg5l0FcXK/iXE1Fg6uSAS4LOXOrl2hL4QuHiwxnk1sEU1yQOhkGREq/ZGAnYLCminXbkwStjebjMujEn+JZb/LQ5aYb58eBWzdZxhtQ= Received: from AM5PR0801MB1764.eurprd08.prod.outlook.com (10.169.247.18) by AM5PR0801MB1987.eurprd08.prod.outlook.com (10.168.158.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.386.5; Thu, 4 Jan 2018 18:55:01 +0000 Received: from AM5PR0801MB1764.eurprd08.prod.outlook.com ([fe80::2967:323c:241e:f763]) by AM5PR0801MB1764.eurprd08.prod.outlook.com ([fe80::2967:323c:241e:f763%14]) with mapi id 15.20.0386.006; Thu, 4 Jan 2018 18:55:01 +0000 From: Girish Pathak To: Ard Biesheuvel , Evan Lloyd CC: Matteo Carlini , nd , "edk2-devel@lists.01.org" , Thomas Abraham , Arvind Chauhan , "leif.lindholm@linaro.org" Thread-Topic: [edk2] [PATCH edk2-platforms v2 06/18] ARM/VExpressPkg: Add and update debug ASSERTS Thread-Index: AQHTe1hOWTzbO38tbEWaoUKASL6JGKNQ+WgAgBMbXaA= Date: Thu, 4 Jan 2018 18:55:00 +0000 Message-ID: References: <20171222190821.12440-1-evan.lloyd@arm.com> <20171222190821.12440-7-evan.lloyd@arm.com> In-Reply-To: Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Girish.Pathak@arm.com; x-originating-ip: [217.140.96.140] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM5PR0801MB1987; 7:hVb8zDOq7BJE+8bz6DrkCY1Gc6CkCrrg6BE36Nm/JWFvKCHpMAm7JbBLxJFy8q8b8YSlt2xNQkl29My8NPT9oT1GJI/UoU0MQydNc8syu8+noIwlR/zE8ueRfR1C+LP+nWjc2eFaQFNuTzZ1E2ZP0JwgG4FYBCxE6A1P4QSXctdPqKfQIMxoo+weIii1ayRA9nH66f92pEftQQM8cm3go9JXkeH1JVruGZEycsQATSo0S3jQyyIBE7xYmgSk0Tlh x-ms-exchange-antispam-srfa-diagnostics: SSOS;SSOR; x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 46254a57-102f-4c77-f8e9-08d553a4a891 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(48565401081)(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(5600026)(4604075)(3008032)(2017052603307)(7153060); SRVR:AM5PR0801MB1987; x-ms-traffictypediagnostic: AM5PR0801MB1987: nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917)(162533806227266); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040470)(2401047)(5005006)(8121501046)(10201501046)(3002001)(3231023)(944501075)(93006095)(93001095)(6055026)(6041268)(20161123564045)(20161123558120)(20161123560045)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(6072148)(201708071742011); SRVR:AM5PR0801MB1987; BCL:0; PCL:0; RULEID:(100000803101)(100110400095); SRVR:AM5PR0801MB1987; x-forefront-prvs: 054231DC40 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(396003)(366004)(376002)(39380400002)(39860400002)(24454002)(13464003)(199004)(189003)(7736002)(53546011)(305945005)(74316002)(33656002)(6506007)(106356001)(59450400001)(2906002)(110136005)(54906003)(72206003)(76176011)(966005)(478600001)(7696005)(105586002)(102836004)(53936002)(5250100002)(66066001)(6636002)(5660300001)(8676002)(8936002)(2900100001)(97736004)(68736007)(55016002)(9686003)(15650500001)(2950100002)(25786009)(99286004)(229853002)(81166006)(3660700001)(81156014)(3280700002)(6306002)(6246003)(14454004)(3846002)(86362001)(6116002)(4326008)(316002)(6436002)(575784001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0801MB1987; H:AM5PR0801MB1764.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: Fu/ro610wRCOCkUB7fhxUVesXonx2U5N9w3cL+YTRz59aAIwYAbgT+KgGeSZXjpOC7L+SVZr513pUcgssHc/nw== spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 46254a57-102f-4c77-f8e9-08d553a4a891 X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Jan 2018 18:55:01.0048 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB1987 Subject: Re: [PATCH edk2-platforms v2 06/18] ARM/VExpressPkg: Add and update debug ASSERTS X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Jan 2018 18:53:47 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Ard, > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: 23 December 2017 14:12 > To: Evan Lloyd > Cc: "Matteo.Carlini@arm.com"@arm.com; > "leif.lindholm@linaro.org"@arm.com; "nd@arm.com"@arm.com; edk2- > devel@lists.01.org; Thomas Abraham ; Arvind > Chauhan ; > "ard.biesheuvel@linaro.org"@arm.com > Subject: Re: [edk2] [PATCH edk2-platforms v2 06/18] ARM/VExpressPkg: Add > and update debug ASSERTS >=20 > On 22 December 2017 at 19:08, wrote: > > From: Girish Pathak > > > > This change adds some debug assertions e.g to catch NULL pointer > > errors missing in PL11Lcd and HdLcd platform libraries. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Girish Pathak > > Signed-off-by: Evan Lloyd > > --- > > > Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpr > ess.c | 22 +++++++++++++++++- > > > > > Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArm > VEx > > press.c | 24 +++++++++++++++++++- > > 2 files changed, 44 insertions(+), 2 deletions(-) > > > > diff --git > > > a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx > pres > > s.c > > > b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx > pres > > s.c index > > > 6afd764897f49c64490ce891682f99bb0f5d993b..a8fe8696da0653017ce9fa6e4a > 86 > > caf283bc04c9 100644 > > --- > > > a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx > pres > > s.c > > +++ > b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx > > +++ press.c > > @@ -153,6 +153,9 @@ LcdPlatformGetVram ( > > EFI_STATUS Status; > > EFI_ALLOCATE_TYPE AllocationType; > > > > + ASSERT (VramBaseAddress !=3D NULL); > > + ASSERT (VramSize !=3D NULL); > > + > > // Set the vram size > > *VramSize =3D LCD_VRAM_SIZE; > > > > @@ -171,6 +174,7 @@ LcdPlatformGetVram ( > > VramBaseAddress > > ); > > if (EFI_ERROR (Status)) { > > + ASSERT (FALSE); > > return Status; > > } > > > > @@ -181,8 +185,8 @@ LcdPlatformGetVram ( > > *VramSize, > > EFI_MEMORY_WC > > ); > > - ASSERT_EFI_ERROR (Status); > > if (EFI_ERROR (Status)) { > > + ASSERT (FALSE); >=20 > As in the sibling patch against EDK2, this patch makes it more difficult = to > figure out what went wrong when you hit the ASSERT. > ASSERT_EFI_ERROR prints the value of Status, ASSERT(FALSE) only prints > '0 !=3D 1' >=20 This change(and other similar changes) is in response to review comments on= patch v1 https://lists.01.org/pipermail/edk2-devel/2017-October/015995.html=20 with above reference, Can you please confirm if we should revert to the pat= ch v1 version ? > > gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize)); > > return Status; > > } > > @@ -221,6 +225,7 @@ LcdPlatformSetMode ( > > EFI_STATUS Status; > > > > if (ModeNumber >=3D LcdPlatformGetMaxMode ()) { > > + ASSERT (FALSE); >=20 > These are fine: the code itself explains adequately which condition trigg= ered > the ASSERT to fire. >=20 > > return EFI_INVALID_PARAMETER; > > } > > > > @@ -279,7 +284,10 @@ LcdPlatformQueryMode ( > > OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info > > ) > > { > > + ASSERT (Info !=3D NULL); > > + > > if (ModeNumber >=3D LcdPlatformGetMaxMode ()) { > > + ASSERT (FALSE); > > return EFI_INVALID_PARAMETER; > > } > > > > @@ -343,7 +351,18 @@ LcdPlatformGetTimings ( > > OUT UINT32 * CONST VFrontPorch > > ) > > { > > + // One of the pointers is NULL > > + ASSERT (HRes !=3D NULL); > > + ASSERT (HSync !=3D NULL); > > + ASSERT (HBackPorch !=3D NULL); > > + ASSERT (HFrontPorch !=3D NULL); > > + ASSERT (VRes !=3D NULL); > > + ASSERT (VSync !=3D NULL); > > + ASSERT (VBackPorch !=3D NULL); > > + ASSERT (VFrontPorch !=3D NULL); > > + > > if (ModeNumber >=3D LcdPlatformGetMaxMode ()) { > > + ASSERT (FALSE); > > return EFI_INVALID_PARAMETER; > > } > > > > @@ -376,6 +395,7 @@ LcdPlatformGetBpp ( > > ) > > { > > if (ModeNumber >=3D LcdPlatformGetMaxMode ()) { > > + ASSERT (FALSE); > > return EFI_INVALID_PARAMETER; > > } > > > > diff --git > > > a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr > mV > > Express.c > > > b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr > mV > > Express.c index > > > 799fb3fc781ce04bb64cb1fa0b87f262a670ed78..fd4eea8f8e2397bc7d4ddf4cfe > 3d > > cc97a5109edb 100644 > > --- > > > a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr > mV > > Express.c > > +++ > b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111Lcd > > +++ ArmVExpress.c > > @@ -205,6 +205,9 @@ LcdPlatformGetVram ( > > > > Status =3D EFI_SUCCESS; > > > > + ASSERT (VramBaseAddress !=3D NULL); > > + ASSERT (VramSize !=3D NULL); > > + > > // Is it on the motherboard or on the daughterboard? > > switch (PL111_CLCD_SITE) { > > > > @@ -225,6 +228,7 @@ LcdPlatformGetVram ( > > VramBaseAddress > > ); > > if (EFI_ERROR (Status)) { > > + ASSERT (FALSE); > > return Status; > > } > > > > @@ -235,8 +239,8 @@ LcdPlatformGetVram ( > > *VramSize, > > EFI_MEMORY_WC > > ); > > - ASSERT_EFI_ERROR (Status); > > if (EFI_ERROR (Status)) { > > + ASSERT (FALSE); > > gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES > (*VramSize)); > > return Status; > > } > > @@ -294,6 +298,7 @@ LcdPlatformSetMode ( > > UINT32 SysId; > > > > if (ModeNumber >=3D LcdPlatformGetMaxMode ()) { > > + ASSERT (FALSE); > > return EFI_INVALID_PARAMETER; > > } > > > > @@ -369,7 +374,10 @@ LcdPlatformQueryMode ( > > OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info > > ) > > { > > + ASSERT (Info !=3D NULL); > > + > > if (ModeNumber >=3D LcdPlatformGetMaxMode ()) { > > + ASSERT (FALSE); > > return EFI_INVALID_PARAMETER; > > } > > > > @@ -433,7 +441,18 @@ LcdPlatformGetTimings ( > > OUT UINT32 * CONST VFrontPorch > > ) > > { > > + // One of the pointers is NULL > > + ASSERT (HRes !=3D NULL); > > + ASSERT (HSync !=3D NULL); > > + ASSERT (HBackPorch !=3D NULL); > > + ASSERT (HFrontPorch !=3D NULL); > > + ASSERT (VRes !=3D NULL); > > + ASSERT (VSync !=3D NULL); > > + ASSERT (VBackPorch !=3D NULL); > > + ASSERT (VFrontPorch !=3D NULL); > > + > > if (ModeNumber >=3D LcdPlatformGetMaxMode ()) { > > + ASSERT (FALSE); > > return EFI_INVALID_PARAMETER; > > } > > > > @@ -465,7 +484,10 @@ LcdPlatformGetBpp ( > > OUT LCD_BPP * CONST Bpp > > ) > > { > > + ASSERT (Bpp !=3D NULL); > > + > > if (ModeNumber >=3D LcdPlatformGetMaxMode ()) { > > + ASSERT (FALSE); > > return EFI_INVALID_PARAMETER; > > } > > > > -- > > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel