From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 04655802A7 for ; Tue, 7 Mar 2017 09:20:19 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx16.intmail.prod.int.phx2.redhat.com [10.5.11.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8B3A680F75; Tue, 7 Mar 2017 17:20:19 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-184.phx2.redhat.com [10.3.116.184]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1D66D306C6; Tue, 7 Mar 2017 17:20:16 +0000 (UTC) To: Brijesh Singh , jordan.l.justen@intel.com, edk2-devel@ml01.01.org, "Jordan Justen (Intel address)" , "Gao, Liming" References: <148884284887.29188.7643544710695103939.stgit@brijesh-build-machine> <148884287496.29188.5155874233993236979.stgit@brijesh-build-machine> Cc: Thomas.Lendacky@amd.com, leo.duran@amd.com, brijesh.sing@amd.com From: Laszlo Ersek Message-ID: <2c6593dd-12f5-9277-0c36-ffd2d6c2cc55@redhat.com> Date: Tue, 7 Mar 2017 18:20:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <148884287496.29188.5155874233993236979.stgit@brijesh-build-machine> X-Scanned-By: MIMEDefang 2.74 on 10.5.11.28 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 07 Mar 2017 17:20:19 +0000 (UTC) Subject: Re: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import BaseIoLibIntrinsic package X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Mar 2017 17:20:19 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/07/17 00:27, Brijesh Singh wrote: > Imports IoLib into OvmfPkg to make the changes to support SEV guest. Ugh, this looks terrible. $ wc -l $(git ls-files MdePkg/Library/BaseIoLibIntrinsic/) 82 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf 24 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.uni 26 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicInternal.h 141 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm 137 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm 2356 MdePkg/Library/BaseIoLibIntrinsic/IoHighLevel.c 317 MdePkg/Library/BaseIoLibIntrinsic/IoLib.c 599 MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c 342 MdePkg/Library/BaseIoLibIntrinsic/IoLibEbc.c 196 MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c 214 MdePkg/Library/BaseIoLibIntrinsic/IoLibIcc.c 736 MdePkg/Library/BaseIoLibIntrinsic/IoLibIpf.c 411 MdePkg/Library/BaseIoLibIntrinsic/IoLibMmioBuffer.c 228 MdePkg/Library/BaseIoLibIntrinsic/IoLibMsc.c 127 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm 126 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm 6062 total Jordan, Liming, if I recall correctly, you guys were leading the IoFifoLib discussion a few weeks back. At that time, I would have preferred to add those functions to a separate IoFifoLib class (like Brijesh originally suggested), but seeing the consensus on adding the Fifo primitives to IoLib instead, I didn't speak up. So now that the Fifo primitives have to be customized (unrolled), and the selection should be made dynamically (at runtime), what do you guys suggest for the implementation, without importing six thousand lines into OvmfPkg? I think this patch should be dropped, and the next patch (#5) should be applied straight to MdePkg. SEV detection happens via the CPUID instruction, and it is specified by a public industry standard, so adding the code to MdePkg looks appropriate to me. If even the CPUID check should be omitted in the default case, then we should use a new FeaturePCD. Thanks, Laszlo > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Brijesh Singh > --- > .../BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 0 > .../BaseIoLibIntrinsic/BaseIoLibIntrinsic.uni | 0 > .../BaseIoLibIntrinsicInternal.h | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm | 0 > .../Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/IoHighLevel.c | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/IoLib.c | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/IoLibArm.c | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/IoLibEbc.c | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/IoLibGcc.c | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/IoLibIcc.c | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/IoLibIpf.c | 0 > .../Library/BaseIoLibIntrinsic/IoLibMmioBuffer.c | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/IoLibMsc.c | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm | 0 > OvmfPkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 0 > OvmfPkg/OvmfPkgIa32X64.dsc | 2 +- > OvmfPkg/OvmfPkgX64.dsc | 2 +- > 18 files changed, 2 insertions(+), 2 deletions(-) > copy MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf => OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.uni => OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.uni (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicInternal.h => OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicInternal.h (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm => OvmfPkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm => OvmfPkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoHighLevel.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoHighLevel.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoLib.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoLib.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoLibArm.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoLibEbc.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoLibEbc.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoLibGcc.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoLibIcc.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoLibIcc.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoLibIpf.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoLibIpf.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoLibMmioBuffer.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoLibMmioBuffer.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/IoLibMsc.c => OvmfPkg/Library/BaseIoLibIntrinsic/IoLibMsc.c (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm => OvmfPkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm (100%) > copy MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm => OvmfPkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm (100%) > > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf b/OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > copy to OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.uni b/OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.uni > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.uni > copy to OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.uni > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicInternal.h b/OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicInternal.h > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicInternal.h > copy to OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicInternal.h > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm b/OvmfPkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm > copy to OvmfPkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm b/OvmfPkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm > copy to OvmfPkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoHighLevel.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoHighLevel.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoHighLevel.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoHighLevel.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLib.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoLib.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoLib.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoLib.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoLibArm.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoLibArm.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibEbc.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoLibEbc.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoLibEbc.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoLibEbc.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoLibGcc.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoLibGcc.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibIcc.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoLibIcc.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoLibIcc.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoLibIcc.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibIpf.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoLibIpf.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoLibIpf.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoLibIpf.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibMmioBuffer.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoLibMmioBuffer.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoLibMmioBuffer.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoLibMmioBuffer.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibMsc.c b/OvmfPkg/Library/BaseIoLibIntrinsic/IoLibMsc.c > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/IoLibMsc.c > copy to OvmfPkg/Library/BaseIoLibIntrinsic/IoLibMsc.c > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm b/OvmfPkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm > copy to OvmfPkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm b/OvmfPkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm > similarity index 100% > copy from MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm > copy to OvmfPkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index a35e1d2..fd89518 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -106,7 +106,7 @@ > PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf > PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf > PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf > - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > + IoLib|OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf > SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf > MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 5d853d6..ce77170 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -106,7 +106,7 @@ > PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf > PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf > PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf > - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > + IoLib|OvmfPkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf > SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf > MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf >