From: Laszlo Ersek <lersek@redhat.com>
To: Brijesh Singh <brijesh.ksingh@gmail.com>,
jordan.l.justen@intel.com, edk2-devel@ml01.01.org,
"Jordan Justen (Intel address)" <jordan.l.justen@intel.com>,
"Gao, Liming" <liming.gao@intel.com>
Cc: Thomas.Lendacky@amd.com, leo.duran@amd.com, brijesh.sing@amd.com
Subject: Re: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import BaseIoLibIntrinsic package
Date: Tue, 7 Mar 2017 18:20:14 +0100 [thread overview]
Message-ID: <2c6593dd-12f5-9277-0c36-ffd2d6c2cc55@redhat.com> (raw)
In-Reply-To: <148884287496.29188.5155874233993236979.stgit@brijesh-build-machine>
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 <brijesh.singh@amd.com>
> ---
> .../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
>
next prev parent reply other threads:[~2017-03-07 17:20 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-06 23:27 [RFC PATCH v1 0/5] x86: Secure Encrypted Virtualization (AMD) Brijesh Singh
2017-03-06 23:27 ` [RFC PATCH v1 1/5] OvmfPkg/ResetVector: Set memory encryption when SEV is active Brijesh Singh
[not found] ` <3ec1cf2d-952d-97fa-108d-a6c70e613277@amd.com>
2017-03-07 16:34 ` Brijesh Singh
2017-03-07 16:35 ` Laszlo Ersek
2017-03-08 18:38 ` Jordan Justen
2017-03-08 18:42 ` Brijesh Singh
2017-03-06 23:27 ` [RFC PATCH v1 2/5] OvmfPkg/MemcryptSevLib: Add SEV helper library Brijesh Singh
2017-03-07 17:06 ` Laszlo Ersek
2017-03-07 19:14 ` Brijesh Singh
2017-03-07 22:08 ` Laszlo Ersek
2017-03-07 22:36 ` Brijesh Singh
2017-03-08 8:40 ` Laszlo Ersek
2017-03-17 2:02 ` Brijesh Singh
2017-03-17 10:29 ` Laszlo Ersek
2017-03-17 14:08 ` Brijesh Singh
2017-03-08 14:56 ` Duran, Leo
2017-03-08 15:19 ` Laszlo Ersek
2017-03-06 23:27 ` [RFC PATCH v1 3/5] OvmfPkg/PlatformPei: Initialize SEV support Brijesh Singh
2017-03-07 17:08 ` Laszlo Ersek
2017-03-07 19:17 ` Brijesh Singh
2017-03-06 23:27 ` [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import BaseIoLibIntrinsic package Brijesh Singh
2017-03-07 17:20 ` Laszlo Ersek [this message]
2017-03-07 20:06 ` Jordan Justen
2017-03-07 22:18 ` Laszlo Ersek
2017-03-08 15:41 ` Gao, Liming
2017-03-08 16:26 ` Brijesh Singh
2017-03-09 1:43 ` Gao, Liming
2017-03-08 18:58 ` Jordan Justen
2017-03-09 1:48 ` Gao, Liming
2017-03-09 15:36 ` Duran, Leo
2017-03-09 16:36 ` Laszlo Ersek
2017-03-06 23:28 ` [RFC PATCH v1 5/5] OvmfPkg/BaseIoLibIntrinsic: Unroll String I/O when SEV is active Brijesh Singh
[not found] ` <5a66f334-27e1-3b49-150e-c01209ecb2f6@amd.com>
2017-03-07 18:43 ` Brijesh Singh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2c6593dd-12f5-9277-0c36-ffd2d6c2cc55@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox