From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by mx.groups.io with SMTP id smtpd.web10.112.1632422303082012484 for ; Thu, 23 Sep 2021 11:38:23 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20210112.gappssmtp.com header.s=20210112 header.b=FB1xy7HK; spf=pass (domain: nuviainc.com, ip: 209.85.221.47, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f47.google.com with SMTP id d6so19763913wrc.11 for ; Thu, 23 Sep 2021 11:38:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=RQOjVreHt9nd4x47NlsR6hp1625D2fMh1N0UB2VjgTI=; b=FB1xy7HKmGyvNDBnublIgPLVTIXycrCgz9zRzCQVpP8x0AFkiWtQJ16mWOuCM+C27X qHX0CQ5Tuo95DZ1c8Yt3Lc+QO2yxW1K2P8YxV4kGbnNJiR7HQ/K/C7q4BHqyA9E7fiS7 XzUFo4EW0ZMToH0pUSSAHXo7JbWI0MYy1xgYNE6pEFlkgt1/GPpXmBiZWym/xIOJ1HmN ggHZYUwcs82giHymvDEB9KP/pc7aCSoEMbHnL7vfWMS2IUD5LXB0LpmtCCZarC6fV0Bj FbqOHkUlQvXU2di0oXF7jAb8YewaPzyTmOdz2GpqemKFBKkNajmrITe0guG24U9razjP TQpg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=RQOjVreHt9nd4x47NlsR6hp1625D2fMh1N0UB2VjgTI=; b=OXIFnK0KXrnN2k9SpqQOhFgsweWzdRf4vjkfddKylKwItX47ojWAQhgBiNo4PkB86I YGLRvhXHI3t+lW9ucaH5oIYaSXNsIboUyUQIAoiRXOlI1uTIEYXQcOFHQZvlRrMWJVOr smrR2sqzof6jzj973mNsGi6Hvi48lfrvn7Z2LR6HtNSZl6rXSAT5HDXNPLlOCLFCRSZH LauGJF7We7LAEH1YVjf3uUB5ye5S7xk9/uTDF1cPBRhlbg7AN+fDZ/DCmWT0NWYDPGFv 2NHAaR6DniU4RHLdA5j84QjE2Q67eusXpQAPBGax6xrc+AWBp5vxg3m1moXUef0ZrWRd NNow== X-Gm-Message-State: AOAM5319/F12vW62Z4LXzrOLwm0cQnY/TOYTmxrgyX/PGeOGNY2nJaoJ xh9mQcwVCPw6rEoEkgzUqo16Gw== X-Google-Smtp-Source: ABdhPJyR42aUQfarPa8NI8zP8NNg9CJcCXUrxEvjYdywVL4XiHnn868Zd+c1H2jrn84lPu2LENaKkA== X-Received: by 2002:a5d:4b4f:: with SMTP id w15mr6967291wrs.175.1632422301599; Thu, 23 Sep 2021 11:38:21 -0700 (PDT) Return-Path: Received: from leviathan (cpc92314-cmbg19-2-0-cust559.5-4.cable.virginm.net. [82.11.186.48]) by smtp.gmail.com with ESMTPSA id v21sm6289104wrv.3.2021.09.23.11.38.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Sep 2021 11:38:21 -0700 (PDT) Date: Thu, 23 Sep 2021 19:38:19 +0100 From: "Leif Lindholm" To: Jeff Brasen 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 Message-ID: <20210923183819.b4worro53hkltaqa@leviathan> References: <20210914150058.nb5z4hz2a3e7ndts@leviathan> <20210923110618.ty67i6wlyco63blr@leviathan> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Sep 23, 2021 at 15:20:41 +0000, Jeff Brasen wrote: > That looks like something I missed and looks good, do you need me to > make a v4 or will you just add that in. Nah, I just folded it in. Pushed as 79019c7a4228..7ea7f9c07757. Thanks! > When I built I just used our > application that has the guid in our inf so missed this. > > From my looking at it AndroidFastBootApp doesn't seem to use this lib. (That ought to have been obvious even to me, oh well, thanks :) / Leif > > -----Original Message----- > > From: Leif Lindholm > > Sent: Thursday, September 23, 2021 5:06 AM > > To: Jeff Brasen > > 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 > > > > External email: Use caution opening links or attachments > > > > > > Hi Jeff, > > > > I was about to say "no more issues", and then I went to build EmbeddedPkg, > > and it turns out this fails in Applications/AndroidBootApp due to the missing > > dependency on gEfiLoadFile2ProtocolGuid in AndroidBootImgLib.inf. > > > > (Why this doesn't break AndroidFastbootApp build as well is not immediately > > obvious to me.) > > > > Would you like to figure out why, or would you prefer me to just fold in > > > > diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf > > b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf > > index affde50f617a..8eefeef4f915 100644 > > --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf > > +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf > > @@ -39,6 +39,7 @@ [Packages] > > [Protocols] > > gAndroidBootImgProtocolGuid > > gEfiLoadedImageProtocolGuid > > + gEfiLoadFile2ProtocolGuid > > > > [Guids] > > gEfiAcpiTableGuid > > > > ? > > > > / > > Leif > > > > On Tue, Sep 21, 2021 at 16:32:58 +0000, Jeff Brasen wrote: > > > 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+tianocore@kernel.org > > > ; Jun Nie > > > Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements > > > > > > So for patch 3: This is only a change if mAndroidBootImg->UpdateDtb == > > NULL. > > > > > > This seemed like a bug as we would not add the initrd values nor would we > > use the fdt from the BootImg if that is where the device tree was sourced > > from. > > > > > > It seems like either we should require UpdateDtb to be implemented > > (which seems to cause greater compatibility issues) or we install the device > > tree we 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 > > config table) and we will have the new pcd set, but this seemed like a bug > > while 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+tianocore@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 > > > > defined -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 > > > >