From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta-01.yadro.com (mta-01.yadro.com [89.207.88.252]) by mx.groups.io with SMTP id smtpd.web10.12201.1589462863697718452 for ; Thu, 14 May 2020 06:27:44 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@yadro.com header.s=mta-01 header.b=Q4fzvu/B; spf=pass (domain: yadro.com, ip: 89.207.88.252, mailfrom: r.bolshakov@yadro.com) Received: from localhost (unknown [127.0.0.1]) by mta-01.yadro.com (Postfix) with ESMTP id 297554C832; Thu, 14 May 2020 13:27:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=yadro.com; h= in-reply-to:content-disposition:content-type:content-type :mime-version:references:message-id:subject:subject:from:from :date:date:received:received:received; s=mta-01; t=1589462856; x=1591277257; bh=MEcIJTmk7h5+33IQeEa2e0zkgQiqRw7ckPwN9pbK3d8=; b= Q4fzvu/BkwwPtstKq7lmELuc6WaYrRVcFBXArnq9n2aTEIIF7jA3sYBywzQMbn9K D9+rpI/SKqYcLqTK8kLbTSwj9dulnTwKC7yGaECxEzz3PG7meGVzubTQgK98XAPv hO3dc+VzDB6WSfLTd/lHA9j/Nqx6HpF4J4gKN83tChY= X-Virus-Scanned: amavisd-new at yadro.com Received: from mta-01.yadro.com ([127.0.0.1]) by localhost (mta-01.yadro.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6RVsBr_RP9Mu; Thu, 14 May 2020 16:27:36 +0300 (MSK) Received: from T-EXCH-02.corp.yadro.com (t-exch-02.corp.yadro.com [172.17.10.102]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mta-01.yadro.com (Postfix) with ESMTPS id 869A94C7BF; Thu, 14 May 2020 16:27:36 +0300 (MSK) Received: from localhost (172.17.204.212) by T-EXCH-02.corp.yadro.com (172.17.10.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.669.32; Thu, 14 May 2020 16:27:38 +0300 Date: Thu, 14 May 2020 16:27:37 +0300 From: Roman Bolshakov To: Laszlo Ersek CC: , Cameron Esfahani , LAHAYE Olivier , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= , Ard Biesheuvel , Liming Gao , Andrew Fish Subject: Re: [PATCH] OvmfPkg: Skip initrd command on Xcode toolchain Message-ID: <20200514132737.GF98158@SPB-NB-133.local> References: <20200512205832.17032-1-r.bolshakov@yadro.com> <54530251-0d9a-7be7-b51c-361a95a658e3@redhat.com> MIME-Version: 1.0 In-Reply-To: <54530251-0d9a-7be7-b51c-361a95a658e3@redhat.com> X-Originating-IP: [172.17.204.212] X-ClientProxiedBy: T-EXCH-01.corp.yadro.com (172.17.10.101) To T-EXCH-02.corp.yadro.com (172.17.10.102) Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline On Wed, May 13, 2020 at 12:23:33PM +0200, Laszlo Ersek wrote: > On 05/12/20 22:58, Roman Bolshakov wrote: > > OVMF booting stops with the assert if built with Xcode on macOS: > > > > Loading driver at 0x0001FAB8000 EntryPoint=0x0001FABF249 LinuxInitrdDynamicShellCommand.efi > > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 1F218398 > > ProtectUefiImageCommon - 0x1F218140 > > - 0x000000001FAB8000 - 0x0000000000008A60 > > > > ASSERT_EFI_ERROR (Status = Unsupported) > > ASSERT LinuxInitrdDynamicShellCommand.c(378): !EFI_ERROR (Status) > > > > The assert comes from InitializeHiiPackage() after an attempt to > > retrieve HII package list from ImageHandle. > > > > Xcode still doesn't support HII resource section and > > LinuxInitrdDynamicShellCommand depends on it. Likewise 277a3958d93a > > ("OvmfPkg: Don't include TftpDynamicCommand in XCODE5 tool chain"), > > disable initrd command if built with Xcode toolchain > > > > Fixes: 2632178bc683 ("OvmfPkg: add 'initrd' shell command to expose Linux initrd via device path") > > Cc: Ard Biesheuvel > > Cc: Liming Gao > > Cc: Andrew Fish > > Cc: Laszlo Ersek > > Signed-off-by: Roman Bolshakov > > --- > > OvmfPkg/OvmfPkgIa32.fdf | 2 +- > > OvmfPkg/OvmfPkgX64.fdf | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > Thanks for the patch! > > The reference to commit 277a3958d93a is very welcome. > > (1) However, this patch doesn't follow commit 277a3958d93a closely > enough. The present patch only conditionalizes some FDF files. We should > conditionalize the DSC files too, otherwise we're still going to build > LinuxInitrdDynamicShellCommand, just not include it in the firmware. > > (2) The "Fixes:" reference is incorrect. Technically, the issue (for > XCODE5) was introduced in the commit that modified the FDF file(s). > Therefore, based on "git-blame", we should have > > Fixes: ec41733cfd10 > > Now, if you look at that commit -- i.e., "OvmfPkg: add the 'initrd' > dynamic shell command" --, then not only will you find the parts in the > DSC files that are necessary to gate (per my point (1) above), you'll > also notice my next point: > > (3) The present patch only covers 2 out of the 4 OvmfPkg platforms; > namely, "OvmfPkgIa32X64" and "OvmfXen" are missed. > > Please extend the patch to those platforms as well. > > In total, the patch should modify 8 files: > > OvmfPkg/OvmfPkgIa32.dsc > OvmfPkg/OvmfPkgIa32.fdf > OvmfPkg/OvmfPkgIa32X64.dsc > OvmfPkg/OvmfPkgIa32X64.fdf > OvmfPkg/OvmfPkgX64.dsc > OvmfPkg/OvmfPkgX64.fdf > OvmfPkg/OvmfXen.dsc > OvmfPkg/OvmfXen.fdf > > because they all reference > "OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf". > Hi Laszlo, Thanks for the feedback and pointing me out to other places I missed. I will address the issues shortly. Regards, Roman