From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by mx.groups.io with SMTP id smtpd.web12.2930.1632395182462654634 for ; Thu, 23 Sep 2021 04:06:22 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20210112.gappssmtp.com header.s=20210112 header.b=VkDFQScq; spf=pass (domain: nuviainc.com, ip: 209.85.221.44, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f44.google.com with SMTP id t28so2648058wra.7 for ; Thu, 23 Sep 2021 04:06: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=mQQsgHMT7h5xQDDdOu77Y23Qux+8Pyqk5MXQLHDiGqQ=; b=VkDFQScqTGtE69d6Fe9pwRF4zA7EbYdQ0cXHGy51Hg3oqk4GXyjEBcylWMgGy0q0dE WW0kixz4qsuCAI8PAGFdUDh3xxlUGMBiZTDrco02+rHxEu9cZP5V5gIvcMytADkIiYB8 qWk9AOmU1Du53qdhfnLxtCy6oHZq8bOx+OWl4fCWcjEn2CTN1TJBGpmVs7h+9si0c03c twSdgZWJzyb1S0xSr3aKec75/EYO+AGx6ULZZCC/cM0yIG/0G7frUGzrh6IYc526xhE5 IU9mgZW6v8YgGWvpe51of4rBxhFZ5PY1pEzxT4kp8MQJ5wbHORtTJTbGDLfCcrxeslLw CZmQ== 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=mQQsgHMT7h5xQDDdOu77Y23Qux+8Pyqk5MXQLHDiGqQ=; b=BiUptOFc5VzkAaTkoTniDxYm1UF+79vBwuAhfee8e2f/R1AKn6yS3S4r0xdpC7s5pd +xHuso2VpHgow1GwJ99OfBdYztviZUwlmR2/TB1ukKxWVjtENM5jO7xG/I8+F9Ehy4S5 4VyKakXtKko2Hd5UObGiMZUCSh+J/UkhQv/cFKizrphv2QwOJIMPSMRSDfmhxE+PLzNF FSN2phfeNP5XFuU5xpMGlDpcdtUtMCgGQR+aVi56sZrdJvlORNMi+VVwrWVbZpiiOyau JG51fmy7OVaECuO0VWXoKbcVUDM8oouERX1lmRQSgqrad+f6KzynI9w3PZvd7aoNv7NH +rmQ== X-Gm-Message-State: AOAM530BLJK6VoynzxcRKe2z3s6KMU+XYoS2H8OX3/Wcj3BvlxfI1z15 rMiQtyGLcHfxuFko9EOETG0HUQ== X-Google-Smtp-Source: ABdhPJz+h8ovU11pKRLOWEmJaxkPjfIae9vjWVFNC/wUAsXBjpAqlcFGQN5ha8JOS6tsv1pWTV03pQ== X-Received: by 2002:a5d:5552:: with SMTP id g18mr4426888wrw.188.1632395180854; Thu, 23 Sep 2021 04:06:20 -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 i187sm5271243wma.0.2021.09.23.04.06.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Sep 2021 04:06:20 -0700 (PDT) Date: Thu, 23 Sep 2021 12:06:18 +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: <20210923110618.ty67i6wlyco63blr@leviathan> References: <20210914150058.nb5z4hz2a3e7ndts@leviathan> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > >