From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (NAM12-BN8-obe.outbound.protection.outlook.com [40.107.237.50]) by mx.groups.io with SMTP id smtpd.web11.497.1632241981056941494 for ; Tue, 21 Sep 2021 09:33:01 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nvidia.com header.s=selector2 header.b=lshoS8w3; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: nvidia.com, ip: 40.107.237.50, mailfrom: jbrasen@nvidia.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=h16bngSJcctSnE72djlfUqwCP2cr+/ndSCHKcCAM0TnPlFlaqN8BoeAip0PGPT2Oi22UZCvelJwjOVRkmLLRdyWagFhyywLeuimOGGlQMj50EbV/jJzr/x+okDXlMPeHcR2oNgnRyyKA4YxxT3Vq8Uowuw5iKC9a5kEizylXgQv+J8nFDap7f+Z9OS5HnPeDTgway4MC/K7DosJB+mj3+RqKytX1a9zytblwl7ROwfJjh7BvsZFpZ1n4VO727Mj/tTKSbKRypvRetYbymXbbZIYMrmUxmUXudOlLXg5Wls89aeXK0nuGn7pfUH0qa5pkUDA528lOMdldIwuHzfw19Q== 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; bh=tERUsYRZPA+pyxiWXtrvC9B0PZZNAAAMzGa93hmYYZ0=; b=iwJcWzRvqm3Egk1Zdj40DwqqXg00ZAk9yfuns+Ro+57blNf3bVetZCFqHVRLgfQ4u5RNmUTwBMswkjrve12NmP5KjluOYaPf6AMstNKypRHXluDYPB0z2+fBqeotqNvvXTnzuDq0dRku/1CQk6Vm4TBkKdIZVYBFPPTLv9RhEBls7yK4MvNM/wEHMy1eT9A5K+9Dzab0STIzXBc22YoEGsps3RTUJKzDoq/0+D3wXwgNwx9yqJddCH6PshSRbf0uBnPmLgzA7AQe94e4nv9iK3dub70XhirrF52lo6dfUl3gTWz9YSyGZXbLxH/vs1w4ot+TzCGzfJ7HlHE3RIBTAA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=tERUsYRZPA+pyxiWXtrvC9B0PZZNAAAMzGa93hmYYZ0=; b=lshoS8w3ZzfrUCv3xDQWW8utBxpQrbUllG7vfELj9K4YAMm3HNEqKKa0Fd//iv41mDNDSaXfBuVBR0CG1S7GFT11jckN8KS5zpnNaHA8F+P9BWjkq9MOnb5ba7XlAVZjO6tjI9PowtU9qESSqFmOpDvF7t2yYq6vCKb07r0i6y+gPsp1y2vn0qVaSsNbNtccJ5ZZljImIorBfNBIJ85k7QVPxkKWdij8CH5Ixlwt7FiUxVfTQtzQ7QFtpLz02GloM/bYB3aKKgR8JsMfweNyhh8A9YL2hZuIuOzToahIioez3JLOaiJxvxZyMozozrlseqxPBKsV6+Ttqg3iix4Vzg== Received: from DM6PR12MB3340.namprd12.prod.outlook.com (2603:10b6:5:3d::24) by DM6PR12MB4233.namprd12.prod.outlook.com (2603:10b6:5:210::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4544.13; Tue, 21 Sep 2021 16:32:59 +0000 Received: from DM6PR12MB3340.namprd12.prod.outlook.com ([fe80::68ed:797b:18d4:848c]) by DM6PR12MB3340.namprd12.prod.outlook.com ([fe80::68ed:797b:18d4:848c%3]) with mapi id 15.20.4523.018; Tue, 21 Sep 2021 16:32:59 +0000 From: "Jeff Brasen" To: Leif Lindholm CC: "devel@edk2.groups.io" , "daniel.schaefer@hpe.com" , "abner.chang@hpe.com" , "ardb+tianocore@kernel.org" , Jun Nie Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements Thread-Topic: [PATCH v3 0/4] AndroidBootImgLib improvements Thread-Index: AQHXqPXBNs0khKNEzUWnxYBc56ObZKujoLoAgAAfOqeACvp2ww== Date: Tue, 21 Sep 2021 16:32:58 +0000 Message-ID: References: <20210914150058.nb5z4hz2a3e7ndts@leviathan> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: suggested_attachment_session_id: 0bde8836-9263-1499-16d9-0ecd60c16036 authentication-results: nuviainc.com; dkim=none (message not signed) header.d=none;nuviainc.com; dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 7f90ff7f-9670-4375-08fa-08d97d1d7966 x-ms-traffictypediagnostic: DM6PR12MB4233: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8273; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Kq5CUSA9PKoI3iOK76YbwPiOvlruO+sEz4mQkhWLZs3v3LsvkNxtoDsGzBXLxnLAZXGmTtMRUco2Mr7HdEO8/4Zu8LP6rjJKDQ9QQbVBId2PjZlYOgqyOX3mOe9jwwIdUa/ZYBQyCyXsmqPehJMrhRpKS/IQOtX6cX0Q8V6teqj9zOHw8OMZuIdFYkob1xGEKJ8LHbPjNkNMRUMLuTOTMEZHsToHezO+3wv/T6QYH/3NwbpVSKRz3g2LgVF9Z//CNeOlSDU6F0XlTiGlvxcDF7K9G652HzJy4u5AXsDUZCz2PK76jNjEWAKfM7myogdnS+tDkBZ70/Nxr/UOlw2jmIm8V/JURrKnAfSX2xZE08wWZRYEbHHQHcLV03KkyFn5CpvANenV3A2Nsxe8OJuTzyGAD24uqLn7LcHmLbAWrzhziNSQ621ekYdN+sKPjhS9C66PZV1uvg5WMbTk4GvA0g/NvwKIM1SzxoDnEfWM4APcXIGal+fwih+YHULHhCpEC4Apqv43cQ9lRlrByS2nrKO9V55MEJQaM8nidWaDbdHqYpKUII7lsg5/lnr+1xqn43af+HPxTo02aigzVjjriTtd+nTXqVPNZernHwmSLdhvoxdAsd1CbDkdYaNpSiQaQK6oAUIoCkCWa/rDJqtgPB/vC4OsV7g/o8Ez4hVTkinWW9Az6H3eJ9NhoKq2y5PPQAXnSYHQMeoLlOUprX1t4Q== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM6PR12MB3340.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(71200400001)(186003)(6506007)(86362001)(19627405001)(316002)(122000001)(38100700002)(9686003)(38070700005)(52536014)(64756008)(66446008)(53546011)(7696005)(66476007)(508600001)(66556008)(55016002)(76116006)(66946007)(6916009)(91956017)(4326008)(83380400001)(54906003)(5660300002)(33656002)(26005)(2906002)(8936002)(8676002);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?60S3DTo6YIT2BRxob6pM2nHFK3d0CylaP9GoeI2WsLrYuD1QM1RQLw79cqbl?= =?us-ascii?Q?x3zZiiM2jJSJO8+gJL4wQFrOnCq8rHuFwOjPK2c/QRr3JLBEyMQikVzw/QT6?= =?us-ascii?Q?yI856ABDTgLqGnVYHLOU6qFRdbFtuBO+YAdt/qUjEaA2/0/jJQVjCuInFnoK?= =?us-ascii?Q?S9BcE8f+A+NWrseqW+/ulRzxUDh+J5EDHQlsGU9gUgNduddZF2AyXX7iKaN2?= =?us-ascii?Q?aU/X4n661jUVpS/5clPbXzAOt0m3jacsAj0zXczas1ssHP5LaXX2DbP1Qn9N?= =?us-ascii?Q?y8EapnvXWngwqRokaebcuqR/4yUpQQVrOf3ppEMl7Ga5iBpv1H3ptudvI5YC?= =?us-ascii?Q?FcH+0I2EroETQvFMjnD+YZvQS1liQRJV76dyC6PkOMxKOmwrbi/CHeOAx1ir?= =?us-ascii?Q?/vJdZ+UwwbswfOOjYfHmtLoIEktkIzf7Q0Mfa9Xh021R95FdzuGlnh8mxn18?= =?us-ascii?Q?XqCrPsbdpgYcDHSIe39mybgpxDZumveNqXaOl0FIVDyN9TjGPYTBgaps4NZv?= =?us-ascii?Q?k1dyZBURvW/i+rSq7Zr6jA7KI1AbjDrJrVypWmsLvl/hct+Edyg+MWNgjTYh?= =?us-ascii?Q?VIEQxYju+O1Q4wFmVBYidauN9K2/Zr8+MR6ruZocRgN8NDkEkgQ7jOK18phj?= =?us-ascii?Q?t6hHls0w3/m1J5fJ7miKDSBE2Sh8pThSD9sQ9YFyPwjkuvA1Tp5/kaGK3IIj?= =?us-ascii?Q?3cl7TuFL4MDUSkNijEyCsaLNCBO9bkRIPa5mXxssipdG81hC1S3lRXWLwkON?= =?us-ascii?Q?By+ARAfy4orkMDGQ183b1Qrhhj6u0c0So+whVXB0dsR3olNDcy+97/CDmftM?= =?us-ascii?Q?CnQbogVn8jiZGE+51by+iXE9JMJXKwFMJBWFrusmLF5R9Xeq20e4G8nJcvt9?= =?us-ascii?Q?5a9ZEIFLLibf69hXmsqOj6jRbSdg0ofKG5xxZmtIT5kdJvfz+f5V1oTpzsdn?= =?us-ascii?Q?MFE34EOF9FwnkdGJ6nl3/ydEU1+Iygu96anjORp+R179281KfrL3WtkJsqBx?= =?us-ascii?Q?mDMZ4a8dhnHqj9uZtLreI6yDOI9IhoF/Yn+Y9MH6smtrpxIBMjDxKIwXHGeK?= =?us-ascii?Q?Rh5NBHjcMKWrhwYZQ+c9Wl2a2/YjTXxkZi3xTv7rAbp49Hb7QOgv9JtWGFkj?= =?us-ascii?Q?2fmOU1BtuEoGNYzBYqBZYJli5Azkyvqk0q4mhDqN6SYHw58vGL1ZQ5jL+4O5?= =?us-ascii?Q?ul0ybmw2jFZMHsFv1jvfFggZk0TShfN3Hm++Uj1LfbzhQLk9A9hJfS8ebgrj?= =?us-ascii?Q?hm2j6Pzfp3t3LZhd/Vg+qvr0ycNX9LVjUCjHmQ9wij8iHloLwBCcMrjUZxP0?= =?us-ascii?Q?9Bw=3D?= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3340.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7f90ff7f-9670-4375-08fa-08d97d1d7966 X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Sep 2021 16:32:58.5934 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: q7I7IIp/t68rL6vhMoOQjd+9phepoPhG9mHXnOW18dxQ8nMAaqoMetHwAjETw5myBHHndKxNUPbdaNgcxkA5LA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4233 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_DM6PR12MB334092FD2831E655025FC0BACBA19DM6PR12MB3340namp_" --_000_DM6PR12MB334092FD2831E655025FC0BACBA19DM6PR12MB3340namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jun/Others, Any additional comments on this patch series? Thanks, Jeff ________________________________ From: Jeff Brasen Sent: Tuesday, September 14, 2021 10:57 AM To: Leif Lindholm Cc: devel@edk2.groups.io ; daniel.schaefer@hpe.com ; abner.chang@hpe.com ; ardb+ti= anocore@kernel.org ; Jun Nie Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements So for patch 3: This is only a change if mAndroidBootImg->UpdateDtb =3D=3D = NULL. This seemed like a bug as we would not add the initrd values nor would we u= se the fdt from the BootImg if that is where the device tree was sourced fr= om. It seems like either we should require UpdateDtb to be implemented (which s= eems to cause greater compatibility issues) or we install the device tree w= e have if that function isn't implemented. As far as merging goes I am fine either way. Our particular flow won't hit = this path as we don't have a device tree in the bootimg (use the system con= fig table) and we will have the new pcd set, but this seemed like a bug whi= le I looking at this code. Thanks, Jeff ________________________________ From: Leif Lindholm Sent: Tuesday, September 14, 2021 9:00 AM To: Jeff Brasen Cc: devel@edk2.groups.io ; daniel.schaefer@hpe.com ; abner.chang@hpe.com ; ardb+ti= anocore@kernel.org ; Jun Nie Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements External email: Use caution opening links or attachments Hi Jeff, Thanks for this. This set looks good to me, with a slight question mark wrt behaviour compatibility with previous versions for 3/4. (I think it's fine, but I'm a bear of very little brain, and it's been several years since I reviewed this code, and even longer since I really interacted with Android. ^ | shameless plug for more EmbeddedPkg reviewer volunteers.) I've added Jun Nie, who wrote the original version of this code, to see if he has any comments. 1-2/4 are obviously unproblematic, and I could merge those ahead of time if preferred. You can add Reviewed-by: Leif Lindholm for those if there are any further revisions of the set. Best Regards, Leif On Mon, Sep 13, 2021 at 23:18:47 +0000, Jeff Brasen wrote: > Added support for using loadfile2 approach for passing ramdisk to linux. > Created patch series for general error handling improvments based on > review feedback. > If ACPI tables are in system table or PCD is defined the LoadFile2 method > of passing initrd will be used. > > [v3] > -Code review cleanup > -Removed duplicate header file > -Added change to allow FDT to install if UpdateDtb function is not define= d > -Added specific ACPI check > -Moved install functions to subfunctions > > [v2] > -Added review feedback > -General improvements to error handling > > [v1] > - Intial revision > > > Jeff Brasen (4): > EmbeddedPkg: Remove duplicate libfdt.h include > EmbeddedPkg: AndroidBootImgBoot error handling updates > EmbeddedPkg: Install FDT if UpdateDtb is not present > EmbeddedPkg: Add LoadFile2 for linux initrd > > EmbeddedPkg/EmbeddedPkg.dec | 1 + > .../AndroidBootImgLib/AndroidBootImgLib.inf | 4 + > .../AndroidBootImgLib/AndroidBootImgLib.c | 275 +++++++++++++++--- > 3 files changed, 233 insertions(+), 47 deletions(-) > > -- > 2.17.1 > --_000_DM6PR12MB334092FD2831E655025FC0BACBA19DM6PR12MB3340namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
Jun/Others,

  Any additional comments on this patch series?

Thanks,

Jeff

From: Jeff Brasen <jbras= en@nvidia.com>
Sent: Tuesday, September 14, 2021 10:57 AM
To: Leif Lindholm <leif@nuviainc.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; daniel.schaef= er@hpe.com <daniel.schaefer@hpe.com>; abner.chang@hpe.com <abner.c= hang@hpe.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org&g= t;; Jun Nie <jun.nie@linaro.org>
Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements
 
So for patch 3: This is only a change if mAndroidBootImg->UpdateDtb= =3D=3D NULL. 

This seemed like a bug as we would not add the initrd values nor would we u= se the fdt from the BootImg if that is where the device tree was sourced fr= om. 

It seems like either we should require UpdateDtb to be implemented (which s= eems to cause greater compatibility issues) or we install the device tree w= e have if that function isn't implemented.

As far as merging goes I am fine either way. Our particular flow won't hit = this path as we don't have a device tree in the bootimg (use the system con= fig table) and we will have the new pcd set, but this seemed like a bug whi= le I looking at this code.

Thanks,

Jeff

From: Leif Lindholm <l= eif@nuviainc.com>
Sent: Tuesday, September 14, 2021 9:00 AM
To: Jeff Brasen <jbrasen@nvidia.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; daniel.schaef= er@hpe.com <daniel.schaefer@hpe.com>; abner.chang@hpe.com <abner.c= hang@hpe.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org&g= t;; Jun Nie <jun.nie@linaro.org>
Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements
 
External email: Use caution opening links or att= achments


Hi Jeff,

Thanks for this.
This set looks good to me, with a slight question mark wrt behaviour
compatibility with previous versions for 3/4.
(I think it's fine, but I'm a bear of very little brain, and it's been
several years since I reviewed this code, and even longer since I
really interacted with Android.
^
| shameless plug for more EmbeddedPkg reviewer volunteers.)

I've added Jun Nie, who wrote the original version of this code, to
see if he has any comments.

1-2/4 are obviously unproblematic, and I could merge those ahead of
time if preferred. You can add
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
for those if there are any further revisions of the set.

Best Regards,

Leif

On Mon, Sep 13, 2021 at 23:18:47 +0000, Jeff Brasen wrote:
> Added support for using loadfile2 approach for passing ramdisk to linu= x.
> Created patch series for general error handling improvments based on > review feedback.
> If ACPI tables are in system table or PCD is defined the LoadFile2 met= hod
> of passing initrd will be used.
>
> [v3]
> -Code review cleanup
> -Removed duplicate header file
> -Added change to allow FDT to install if UpdateDtb function is not def= ined
> -Added specific ACPI check
> -Moved install functions to subfunctions
>
> [v2]
> -Added review feedback
> -General improvements to error handling
>
> [v1]
> - Intial revision
>
>
> Jeff Brasen (4):
>   EmbeddedPkg: Remove duplicate libfdt.h include
>   EmbeddedPkg: AndroidBootImgBoot error handling updates
>   EmbeddedPkg: Install FDT if UpdateDtb is not present
>   EmbeddedPkg: Add LoadFile2 for linux initrd
>
>  EmbeddedPkg/EmbeddedPkg.dec      &= nbsp;            |&n= bsp;  1 +
>  .../AndroidBootImgLib/AndroidBootImgLib.inf   | &= nbsp; 4 +
>  .../AndroidBootImgLib/AndroidBootImgLib.c    = ; | 275 +++++++++++++++---
>  3 files changed, 233 insertions(+), 47 deletions(-)
>
> --
> 2.17.1
>
--_000_DM6PR12MB334092FD2831E655025FC0BACBA19DM6PR12MB3340namp_--