From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web12.35110.1661508791078463177 for ; Fri, 26 Aug 2022 03:13:11 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=M3UGAfZe; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 6AEA960B52 for ; Fri, 26 Aug 2022 10:13:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CEC54C433B5 for ; Fri, 26 Aug 2022 10:13:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1661508789; bh=cKFu/ZhtmIeZx1XvNLWNy/V3oinz6kLcmrGYHdFeyCg=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=M3UGAfZeqericdGkVXmz0AbZSkTOJxa6R06OaHRsSYnn1KymlAPQSi1I3wuGGiUW6 y5Qazw2FNkh/fT6/MlJCI4HeRRFb2NaVdGKqv+vfwUsV46KPTIC2ke87ThP44bJwCR WB+6G/UHqQNq9aZchyAWvmDwx6E0tL8HJuf0TFYR0morAFqJXYwBUw5AbmS18/6D62 x+VHI8PiTvrZwYTwGTvfFGOC5t3Z93I3l2z9UiqiymgCSx65vAV5DKCQcNi9Chs7ku zTvzFZM9zN2xNuggCjF+YFbE0lDZXzhKL30mLqjviDaTUD+IGcNrHQJNMWacRG3sU8 E1NIV2Bc2w3cA== Received: by mail-wr1-f53.google.com with SMTP id e13so325360wrm.1 for ; Fri, 26 Aug 2022 03:13:09 -0700 (PDT) X-Gm-Message-State: ACgBeo08qr6kAx+8kEBX5QADzW5Z2YrStktvvBwDogbwBtEtyxmFHqnU aaAvlFjE5rAPgZki1NQ9h1WtRM93aKkUrBp4zow= X-Google-Smtp-Source: AA6agR7htjpfvUwl2nbnWUl+xW2iaB6pSx/C0Xir9XB0/xk8GmUoZHL3ioxdCJiAuRU98WxZcuFtGdrYWTaolf7ImYo= X-Received: by 2002:a05:6000:786:b0:225:4f77:5d24 with SMTP id bu6-20020a056000078600b002254f775d24mr4297929wrb.180.1661508787938; Fri, 26 Aug 2022 03:13:07 -0700 (PDT) MIME-Version: 1.0 References: <41a26d57-4fae-f53f-29ae-d6c7b258eaee@bsdio.com> <000a01d8b693$58d87c60$0a897520$@byosoft.com.cn> <010801d8b789$6a19ade0$3e4d09a0$@byosoft.com.cn> <6fb68a04-df82-d32f-bb46-0a7969c0275b@quicinc.com> In-Reply-To: From: "Ard Biesheuvel" Date: Fri, 26 Aug 2022 12:12:56 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04 To: "Huang, Long1" Cc: "Feng, Bob C" , "Kinney, Michael D" , "devel@edk2.groups.io" , "quic_llindhol@quicinc.com" , "Gao, Liming" , "rebecca@bsdio.com" , "Chen, Christine" , Andrew Fish , "Wang, Jian J" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable comments inline On Fri, 26 Aug 2022 at 05:28, Huang, Long1 wrote: > > Hi All, > > I'm the submitter. > Hello Huang, Long, Thanks for providing this background - it is very helpful. Some of this should have been recorded in the bugzilla entry or the patch commit log, though, as now, nobody understood the problem or why the fix was necessary to begin with. > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3DHere's the DSC prec= edence rule from DSC Spec=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > The Library Instances will be selected using the following rules to satis= fy a library class for > each module listed in the [Components] section (in order of highest prece= dence): > 1. associated with the INF file in the [Components] sect= ion > 2. [LibraryClasses.$(Arch).$(MODULE_TYPE), LibraryClasses.$(Arch).$(MODUL= E_TYPE)] > 3. [LibraryClasses.$(Arch).$(MODULE_TYPE)] > 4. [LibraryClasses.common.$(MODULE_TYPE)] > 5. [LibraryClasses.$(Arch)] > 6. [LibraryClasses.common] or [LibraryClasses] > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D > This is incomplete. Before that, the DSC spec says """ The first globally defined library instance, defined in a DSC file, that satisfies a module's requirement for a Library Class, unless specifically overridden by the module in the [Components] section, will be used. """ Should we simply drop that part from the DSC spec? Also, do you happen to have any idea for the rationale of having 2. supersede 3. explicltly? > Background: > Our system takes DXE and later stages as 64-bit by default, using [Librar= yClasses.X64] or appending a more detailed section "MODULE_TYPE"(e.g. [Libr= aryClasses.X64.DXE_CORE], [LibraryClasses.X64.DXE_SMM_DRIVER]). For SEC or = PEI, use [LibraryClasses.IA32] or [LibraryClasses.IA32.SEC], [LibraryClass= es.IA32.PEIM], etc. It works fine for traditional. > > With the arrival of pure X64, there will be some linking issues in this w= ay. Because which library to link against cannot be determined simply using= ARCH. > During the development of X64 PEI, we found that the actual behavior of B= aseTools for DSC precedence: > [LibraryClasses.common.PEIM] < [LibraryClasses.X64] > [LibraryClasses.IA32.PEIM] > [LibraryClasses.X64] > [LibraryClasses.X64.PEIM] > [LibraryClasses.X64] > > We can see that this does not match the order of 4 and 5 in the spec. > > So when we tried to use "[LibraryClasses.common.MODULE_TYPE]" such the co= mmon ways to support both X64 and IA32 SEC & PEI and avoid duplication, bef= ore the fixed patch, modules in the PEI stage will linking DXE and later st= age's libraries first(under [LibraryClasses.X64]), it will cause many puzzl= ing linking issues. After the fixed patch downstream, we setup the X64 PEI = build target, and works well for several days(both for IA32 and X64 SEC & P= EI build). > OK. The problem, however, is that many platform definitions may exist that actually rely on this behavior. And the symptom could be something that only manifests itself at runtime. > For Ard Biesheuvel's question: > I think it may be the LockBoxLib under [LibraryClasses.ARM, LibraryClasse= s.AARCH64] doesn't provide the MODULE_TYPE section, it can work before this= patch as I said above. But now it should be replaced with [LibraryClasses.= ARM.MODULE_TYPE, LibraryClasses.AARCH64.MODULE_TYPE] to get higher priority= than LockBoxLib under [LibraryClasses.comon.MODULE_TYPE]. > Thanks for clarifying that. But fixing the MdeModulePkg build is not the priority here. The MdeModulePkg breakage is the canary in the coalmine here, and tells us that making changes to the build tools like this is dangerous, and may affect many users of this code base. > Conclusion: > From my point of view, I think the DSC precedence rule defined by spec is= reasonable and make sense. MinPlatform gives us a good example, using the = method of [LibraryClasses.common.MODULE_TYPE]. Do not assume the current ar= ch, use "common" instead, then always append "MODULE_TYPE" section. It's cl= ear, compatible and unambiguous. So we strongly support the idea of the DSC= spec: Unless specified under [Components], then more detailed sections, hi= gher priority. I think it will benefit future development. > Doesn't this mean that you should not be using [LibraryClasses.X64] to begin with? Or am I misunderstanding the point you are making? Because in that case, the problem would not exist either. > Short term solution: > we can agree to temporarily revert this patch for stable tag if we can't = fix the MdeModulePkg build issues or other potential problems immediately. = For us, we will downstream overwrite it to keep this change to support the = X64 build target. > Fixing MdeModulePkg is easy. Fixing all the DSCs out in the world that may silently break because of this change is much harder. > Long term solution: > We suggest that we can discuss the rationale of this rule and evaluate if= this patch should be reintroduced after the stable tag. > I agree. But we should clarify the DSC spec first, and at least consider aligning the spec with the existing code (which has been in wide use for many years) instead of the other way around. Thanks, Ard.