From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-CO1-obe.outbound.protection.outlook.com (NAM04-CO1-obe.outbound.protection.outlook.com [40.107.69.53]) by mx.groups.io with SMTP id smtpd.web12.33070.1591003289699902074 for ; Mon, 01 Jun 2020 02:21:30 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@vmware.com header.s=selector2 header.b=LRdILJDC; spf=pass (domain: vmware.com, ip: 40.107.69.53, mailfrom: awarkentin@vmware.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nmU4gIbgGwJYrczACbHqZG0/3QUzpZFjxkJvLZHVzkSTgcjIcYzt4P5G2MGaCnckTf0wm3D3hSB8atY4ZKt18KFceULub0Cz3pERUjWTPdMKwXYZvyZGkHx0OjBkxajGeX0ZpfwB7D7ZhgAqhh/0P8B2aNYOD7VWutCOd19cgxqK+47Sd1ttK8pLvt7GOAiGRhLJs+kLtF5C6xPZSLzv4eETwn4yL8/838yf2bb3jcousm/kimhYFxPLcGFqYEuNZXKLdWKg9ZFSU5RogXCKFwhyWKSrcqzdO7pqnIu9lhfmduoMu59k99OqKJr5CJphz4iviuqyyNK6UEqOolJZ1Q== 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=PB3F3baK2eUafbwucHSFazdDwmLx1V8gUE6y04Bbbjg=; b=MjP+vV20naeiUgDL+cPGq2J7bgAEI9v4MLE4Dkmt5m8LSuUGw/ktMOv9eAV9jr9sWRViN/ebcEzvdPp9BJ6v3+9D3dCNDQ9d8xcaFRiSVm6bxbjPuDnEmIdIhC4emkGd7MwBUngW7pct8MbQqJwhoqlTPVvHi1wOla3C49QzQGiZthMTUpk68OeYhAPGatEyiiWZvZvqjMqS+jQZqaTmbT34gDSCqjIC46j/GrRJuLIELpdqYnr/+fWLLa0p/ylBUbvGjjxtx4BCMBSFgu5WqxpWqVDmvfewV8MTK5MwOu49IUyBZCBrL4chWVtKuuvMK8Elp0yq/Yk/gZAB8XsdZQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=vmware.com; dmarc=pass action=none header.from=vmware.com; dkim=pass header.d=vmware.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vmware.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=PB3F3baK2eUafbwucHSFazdDwmLx1V8gUE6y04Bbbjg=; b=LRdILJDC8jd1RmmApYgorY6acwrmKdsxu6iYIotVSshC8HlIA6NcHOlrOqupN8rEDmja3w9gXrbCGSyl9LSp2w4hKaVKB/wOZ3CV4vJ992JHf4VL1IsxSWtQx7y1h/j2Lz4Pj7Mhq4HNxwLKdAUVSjDA/qb7o59lgfMuGWJBSB4= Received: from BN6PR05MB3411.namprd05.prod.outlook.com (2603:10b6:405:43::23) by BN6PR05MB3217.namprd05.prod.outlook.com (2603:10b6:404:bb::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3066.7; Mon, 1 Jun 2020 09:21:28 +0000 Received: from BN6PR05MB3411.namprd05.prod.outlook.com ([fe80::e1ef:31eb:c802:aef0]) by BN6PR05MB3411.namprd05.prod.outlook.com ([fe80::e1ef:31eb:c802:aef0%3]) with mapi id 15.20.3066.007; Mon, 1 Jun 2020 09:21:27 +0000 From: "Andrei Warkentin" To: Andrei Warkentin , "devel@edk2.groups.io" , "ard.biesheuvel@arm.com" CC: "leif@nuviainc.com" , "pete@akeo.ie" , "philmd@redhat.com" Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys Thread-Topic: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys Thread-Index: AQHWN+bdOmv4mkn/eUKX9RuH1ZvwqqjDd6wAgAAEarM= Date: Mon, 1 Jun 2020 09:21:26 +0000 Message-ID: References: <20200601073238.101651-1-andrey.warkentin@gmail.com>,<48a133a5-6d57-9469-562e-73f9731693fa@arm.com> In-Reply-To: <48a133a5-6d57-9469-562e-73f9731693fa@arm.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=vmware.com; x-originating-ip: [98.214.99.181] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 82240457-b190-41e1-fa1b-08d8060d29d9 x-ms-traffictypediagnostic: BN6PR05MB3217: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:2276; x-forefront-prvs: 0421BF7135 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 8eKXIWX1ZpOV/stZtzlxnezIKzdAlaHLZOfuZO47Ihhrt2xL5KoyGbRwIT8/opjGTTAoiQRrnxN6i5FZxvkH5inm0KexAPqa9hnxnRNoxTxLLmJfH+VS9juYcMMzYaQSDm0yo5uuo+TPPmwp5sh5k+iu5Os3Re2t3YP+hTIb0E9QA7bJ5E4ehurwQd0bakxB6vG5viNGE5DWnIRv5F8zGAi0liEZDtc/TDgqyYueOdw0jJ8/dTJAcBrHqpwzebLO0/p9O0doz7GMjlWVmLDyAJGdyf2xmMfXDmyPWbPpYFNLuLq/ucw8ZR7MKKfm0Xas7zZ5s7g/sBCVsCtFS9nSDeelbvoCe/RjFut5olZCKxxK/b8Pk7QJJAh5HLNiMWEl0QrSD9f5MZNAvVtShaZLqdgeTnY9XGsx7/LCRSke7CLJzr5Ka7FfYY43NutpiXL7QvL87Dkdxys3PkqU+PDFdw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN6PR05MB3411.namprd05.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(346002)(39860400002)(136003)(396003)(366004)(376002)(9686003)(8936002)(5660300002)(4326008)(54906003)(71200400001)(8676002)(110136005)(55016002)(316002)(76116006)(53546011)(2906002)(26005)(33656002)(66556008)(7696005)(86362001)(66476007)(66946007)(6506007)(64756008)(966005)(478600001)(83380400001)(166002)(45080400002)(52536014)(186003)(19627405001)(66446008)(213903007);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: mGoGroxskYlf016kVamMekesIEq/KsObqgHGYRZ5ln7/PSJu5p7BmjW4nitiMiEN0WdcyvXTbpj3E3eInkNznMELXfAGzS9ZaEUNtaBw+PXB4u2xOsezd43BBhHO/sHxm6pNN4Zc0cYPapZ5WXi6st/z4LkCJrYoeqKKE40HN+gibFAe08uXMoxQxbs92/knWNOAGjQpOqAaiymbNYqlmz5HlKpv4sorSNDRv2JRxaVs5SUuMjhDGrZ99JoAwQKZGWo7u4viy50fF28Zh+DJROjWVlq9xV7Ilz/3+cS1oaMF/OXEX9HqPUOsmZGNaWTJZV53vzTRrUIrXTnBYNpy3Me+pcppPO+N/04hQPiee37UTxmbakSzYIvI6EENKEKhQDYTUVDWAoZiwgGN92suAxseC172dg7m7RdC6gRUbFWubAy1165399IHMO3rTpRjoRbC2bYIZIwpX4m1Lmh/MoYkzDTLINfhPSowPLF8HcI= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: vmware.com X-MS-Exchange-CrossTenant-Network-Message-Id: 82240457-b190-41e1-fa1b-08d8060d29d9 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jun 2020 09:21:26.9433 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: MtH58MBDddxo1RB0NSd6wgmylk3AkwWcT+5RLSzxj4YjrDTu3siwhxcFYKa1cNIeNAOP0D8f13jOAoI1vuWHCQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR05MB3217 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_BN6PR05MB341145CC8AB809B730196CEFB98A0BN6PR05MB3411namp_" --_000_BN6PR05MB341145CC8AB809B730196CEFB98A0BN6PR05MB3411namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Sure. Let me know if you want me to resend. A ________________________________ From: devel@edk2.groups.io on behalf of Ard Biesheu= vel via groups.io Sent: Monday, June 1, 2020 4:05 AM To: Andrei Warkentin ; devel@edk2.groups.io Cc: leif@nuviainc.com ; pete@akeo.ie ; ph= ilmd@redhat.com Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi4: add descriptiv= e errors in PlatformRegisterOptionsAndKeys On 6/1/20 9:32 AM, Andrei Warkentin wrote: > I have reports of debug builds sitting with an ASSERT inside > PlatformRegisterOptionsAndKeys, but have not been able to verify. > Let's log errors to help diagnose this in the future. > > Signed-off-by: Andrei Warkentin > --- > Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 25 = ++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/Platfor= mBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c > index 996ba8f3..f0a2fe1a 100644 > --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c > +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c > @@ -1,7 +1,7 @@ > /** @file > * > * Copyright (c) 2018, Pete Batard > - * Copyright (c) 2017-2018, Andrei Warkentin > + * Copyright (c) 2017-2020, Andrei Warkentin > * Copyright (c) 2016, Linaro Ltd. All rights reserved. > * Copyright (c) 2015-2016, Red Hat, Inc. > * Copyright (c) 2014, ARM Ltd. All rights reserved. > @@ -468,7 +468,13 @@ PlatformRegisterOptionsAndKeys ( > F1.ScanCode =3D SCAN_F1; > F1.UnicodeChar =3D CHAR_NULL; > Status =3D EfiBootManagerAddKeyOptionVariable (NULL, (UINT16)Shell= Option, 0, &F1, NULL); > - ASSERT (Status =3D=3D EFI_SUCCESS || Status =3D=3D EFI_ALREADY_STAR= TED); > + if (Status =3D=3D EFI_ALREADY_STARTED) { > + Status =3D EFI_SUCCESS; > + } > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failed to register F1 as UEFI Shell key: %r= \n", Status)); > + } > + ASSERT_EFI_ERROR (Status); Mind if we invert this 'success handling' pattern? if (EFI_ERROR (Status) && Status !=3D EFI_ALREADY_STARTED) { DEBUG ((DEBUG_ERROR, "Failed to register ... ASSERT_EFI_ERROR (Status); } (same below) > } > > // > @@ -477,6 +483,9 @@ PlatformRegisterOptionsAndKeys ( > Enter.ScanCode =3D SCAN_NULL; > Enter.UnicodeChar =3D CHAR_CARRIAGE_RETURN; > Status =3D EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL)= ; > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failed to register ENTER as CONTINUE key: %r\= n", Status)); > + } > ASSERT_EFI_ERROR (Status); > > // > @@ -485,9 +494,17 @@ PlatformRegisterOptionsAndKeys ( > Esc.ScanCode =3D SCAN_ESC; > Esc.UnicodeChar =3D CHAR_NULL; > Status =3D EfiBootManagerGetBootManagerMenu (&BootOption); > + if (!EFI_ERROR (Status)) { > + Status =3D EfiBootManagerAddKeyOptionVariable (NULL, (UINT16)BootOp= tion.OptionNumber, 0, &Esc, NULL); > + if (Status =3D=3D EFI_ALREADY_STARTED) { > + Status =3D EFI_SUCCESS; > + } > + } > + > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failed to register ESC as Boot Manager key: %= r\n", Status)); > + } > ASSERT_EFI_ERROR (Status); > - Status =3D EfiBootManagerAddKeyOptionVariable (NULL, (UINT16)BootOpti= on.OptionNumber, 0, &Esc, NULL); > - ASSERT (Status =3D=3D EFI_SUCCESS || Status =3D=3D EFI_ALREADY_STARTE= D); > } > > STATIC VOID > --_000_BN6PR05MB341145CC8AB809B730196CEFB98A0BN6PR05MB3411namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
Sure. Let me know if you want me to resend.

A
From: devel@edk2.groups.io= <devel@edk2.groups.io> on behalf of Ard Biesheuvel via groups.io <= ;ard.biesheuvel=3Darm.com@groups.io>
Sent: Monday, June 1, 2020 4:05 AM
To: Andrei Warkentin <andrey.warkentin@gmail.com>; devel@edk2= .groups.io <devel@edk2.groups.io>
Cc: leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie <p= ete@akeo.ie>; philmd@redhat.com <philmd@redhat.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi4: add des= criptive errors in PlatformRegisterOptionsAndKeys
 
On 6/1/20 9:32 AM, Andrei Warkentin wrote:
> I have reports of debug builds sitting with an ASSERT inside
> PlatformRegisterOptionsAndKeys, but have not been able to verify.
> Let's log errors to help diagnose this in the future.
>
> Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com> > ---
>   Platform/RaspberryPi/Library/PlatformBootManagerLib/Platf= ormBm.c | 25 ++++++++++++&#= 43;+++----
>   1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/Plat= formBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c=
> index 996ba8f3..f0a2fe1a 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.= c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib= /PlatformBm.c
> @@ -1,7 +1,7 @@
>   /** @file
>    *
>    *  Copyright (c) 2018, Pete Batard <pete@ak= eo.ie>
> - *  Copyright (c) 2017-2018, Andrei Warkentin <andrey.warken= tin@gmail.com>
> + *  Copyright (c) 2017-2020, Andrei Warkentin <andrey.wa= rkentin@gmail.com>
>    *  Copyright (c) 2016, Linaro Ltd. All rights = reserved.
>    *  Copyright (c) 2015-2016, Red Hat, Inc.
>    *  Copyright (c) 2014, ARM Ltd. All rights res= erved.
> @@ -468,7 +468,13 @@ PlatformRegisterOptionsAndKeys (
>       F1.ScanCode =3D SCAN_F1;
>       F1.UnicodeChar =3D CHAR_NULL;
>       Status =3D EfiBootManagerAddKeyOp= tionVariable (NULL, (UINT16)ShellOption, 0, &F1, NULL);
> -    ASSERT (Status =3D=3D EFI_SUCCESS || Status =3D= =3D EFI_ALREADY_STARTED);
> +    if (Status =3D=3D EFI_ALREADY_STARTED) {
> +      Status =3D EFI_SUCCESS;
> +    }
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "Failed= to register F1 as UEFI Shell key: %r\n", Status));
> +    }
> +    ASSERT_EFI_ERROR (Status);

Mind if we invert this 'success handling' pattern?

if (EFI_ERROR (Status) && Status !=3D EFI_ALREADY_STARTED) {
   DEBUG ((DEBUG_ERROR, "Failed to register ...
   ASSERT_EFI_ERROR (Status);
}

(same below)

>     }
>  
>     //
> @@ -477,6 +483,9 @@ PlatformRegisterOptionsAndKeys (
>     Enter.ScanCode =3D SCAN_NULL;
>     Enter.UnicodeChar =3D CHAR_CARRIAGE_RETURN; >     Status =3D EfiBootManagerRegisterContinueKeyO= ption (0, &Enter, NULL);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to register= ENTER as CONTINUE key: %r\n", Status));
> +  }
>     ASSERT_EFI_ERROR (Status);
>  
>     //
> @@ -485,9 +494,17 @@ PlatformRegisterOptionsAndKeys (
>     Esc.ScanCode =3D SCAN_ESC;
>     Esc.UnicodeChar =3D CHAR_NULL;
>     Status =3D EfiBootManagerGetBootManagerMenu (= &BootOption);
> +  if (!EFI_ERROR (Status)) {
> +    Status =3D EfiBootManagerAddKeyOptionVariable= (NULL, (UINT16)BootOption.OptionNumber, 0, &Esc, NULL);
> +    if (Status =3D=3D EFI_ALREADY_STARTED) {
> +      Status =3D EFI_SUCCESS;
> +    }
> +  }
> +
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to register= ESC as Boot Manager key: %r\n", Status));
> +  }
>     ASSERT_EFI_ERROR (Status);
> -  Status =3D EfiBootManagerAddKeyOptionVariable (NULL, (UINT16)= BootOption.OptionNumber, 0, &Esc, NULL);
> -  ASSERT (Status =3D=3D EFI_SUCCESS || Status =3D=3D EFI_ALREAD= Y_STARTED);
>   }
>  
>   STATIC VOID
>




--_000_BN6PR05MB341145CC8AB809B730196CEFB98A0BN6PR05MB3411namp_--