* [PATCH v3 0/4] BaseIoFifoLib
@ 2017-01-05 21:49 Leo Duran
2017-01-05 21:49 ` [PATCH v3 1/4] MdePkg: Add BaseIoFifoLib library Leo Duran
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Leo Duran @ 2017-01-05 21:49 UTC (permalink / raw)
To: edk2-devel
Cc: brijesh.singh, lersek, jordan.l.justen, jeff.fan,
michael.d.kinney, liming.gao, maurice.ma, prince.agyeman,
ruiyu.ni, kelly.steele, david.wei, mang.guo, Leo Duran
The UefiCpuPkg/CpuIo2Dxe driver and the QemuCfgLib library have duplicate
implementations of I/O Fifo routines. The patch series moves the I/O Fifo
routines into a common BaseIofifoLib library supporting IA32 and X64
architectures under MdePkg.
The intent of this patch series is twofold:
1) Consolidate I/O Fifo routines into a common BaseIofifoLib library.
2) Allow override of BaseIofifoLib for specific platform implementations.
For example, the OVMF package can provide its own version of BaseIoFifoLib
to support Secure Encrypted Virtualization (SEV) guests, since SEV does not
support string I/O instructions (rep ins/outs), and requires unrolled loops
of single in/out instructions.
Brijesh Singh (4):
MdePkg: Add BaseIoFifoLib library
Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library.
Modify QemuFwCfgLib to use new BaseIoFifoLib library.
CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 +
CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +
DuetPkg/DuetPkgIa32.dsc | 2 +
DuetPkg/DuetPkgX64.dsc | 2 +
MdePkg/Include/Library/IoFifoLib.h | 176 +++++++++++++
MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf | 44 ++++
.../Library/BaseIoFifoLib}/Ia32/IoFifo.asm | 280 ++++++++++-----------
.../Library/BaseIoFifoLib}/Ia32/IoFifo.nasm | 272 ++++++++++----------
.../Library/BaseIoFifoLib}/X64/IoFifo.asm | 252 +++++++++----------
.../Library/BaseIoFifoLib}/X64/IoFifo.nasm | 250 +++++++++---------
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 2 +
OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm | 55 ----
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 54 +---
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 8 +-
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf | 8 +-
OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm | 52 ----
OvmfPkg/OvmfPkgIa32.dsc | 2 +
OvmfPkg/OvmfPkgIa32X64.dsc | 2 +
OvmfPkg/OvmfPkgX64.dsc | 2 +
QuarkPlatformPkg/Quark.dsc | 2 +
QuarkPlatformPkg/QuarkMin.dsc | 2 +
UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c | 3 +-
UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h | 3 +
UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf | 12 +-
UefiCpuPkg/UefiCpuPkg.dsc | 2 +
Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 +
Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 2 +
Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 2 +
29 files changed, 794 insertions(+), 709 deletions(-)
create mode 100644 MdePkg/Include/Library/IoFifoLib.h
create mode 100644 MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
rename {UefiCpuPkg/CpuIo2Dxe => MdePkg/Library/BaseIoFifoLib}/Ia32/IoFifo.asm (96%)
rename {UefiCpuPkg/CpuIo2Dxe => MdePkg/Library/BaseIoFifoLib}/Ia32/IoFifo.nasm (96%)
rename {UefiCpuPkg/CpuIo2Dxe => MdePkg/Library/BaseIoFifoLib}/X64/IoFifo.asm (97%)
rename {UefiCpuPkg/CpuIo2Dxe => MdePkg/Library/BaseIoFifoLib}/X64/IoFifo.nasm (97%)
delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
--
1.9.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 1/4] MdePkg: Add BaseIoFifoLib library
2017-01-05 21:49 [PATCH v3 0/4] BaseIoFifoLib Leo Duran
@ 2017-01-05 21:49 ` Leo Duran
2017-01-05 21:49 ` [PATCH v3 2/4] Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf Leo Duran
` (3 subsequent siblings)
4 siblings, 0 replies; 23+ messages in thread
From: Leo Duran @ 2017-01-05 21:49 UTC (permalink / raw)
To: edk2-devel
Cc: brijesh.singh, lersek, jordan.l.justen, jeff.fan,
michael.d.kinney, liming.gao, maurice.ma, prince.agyeman,
ruiyu.ni, kelly.steele, david.wei, mang.guo, Leo Duran
From: Brijesh Singh <brijesh.singh@amd.com>
The UefiCpuPkg/CpuIo2Dxe driver and the QemuCfgLib library have duplicate
implementations of I/O Fifo routines. The patch moves the I/O Fifo
routines into a common BaseIofifoLib library supporting IA32 and X64
architectures under MdePkg.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
MdePkg/Include/Library/IoFifoLib.h | 176 +++++++++++++++++++++++++
MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf | 44 +++++++
MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.asm | 140 ++++++++++++++++++++
MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.nasm | 136 +++++++++++++++++++
MdePkg/Library/BaseIoFifoLib/X64/IoFifo.asm | 126 ++++++++++++++++++
MdePkg/Library/BaseIoFifoLib/X64/IoFifo.nasm | 125 ++++++++++++++++++
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 2 +
8 files changed, 753 insertions(+)
create mode 100644 MdePkg/Include/Library/IoFifoLib.h
create mode 100644 MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
create mode 100644 MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.asm
create mode 100644 MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.nasm
create mode 100644 MdePkg/Library/BaseIoFifoLib/X64/IoFifo.asm
create mode 100644 MdePkg/Library/BaseIoFifoLib/X64/IoFifo.nasm
diff --git a/MdePkg/Include/Library/IoFifoLib.h b/MdePkg/Include/Library/IoFifoLib.h
new file mode 100644
index 0000000..f606a41
--- /dev/null
+++ b/MdePkg/Include/Library/IoFifoLib.h
@@ -0,0 +1,176 @@
+/** @file
+ I/O FIFO routines
+
+ Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+
+ This program and the accompanying materials are licensed and made available
+ under the terms and conditions of the BSD License which accompanies this
+ distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __IO_FIFO_LIB_H__
+#define __IO_FIFO_LIB_H__
+
+/**
+ Reads an 8-bit I/O port fifo into a block of memory.
+
+ Reads the 8-bit I/O fifo port specified by Port.
+
+ The port is read Count times, and the read data is
+ stored in the provided Buffer.
+
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 8-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+ @param Count The number of times to read I/O port.
+ @param Buffer The buffer to store the read data into.
+
+**/
+VOID
+EFIAPI
+IoReadFifo8 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ );
+
+/**
+ Reads a 16-bit I/O port fifo into a block of memory.
+
+ Reads the 16-bit I/O fifo port specified by Port.
+
+ The port is read Count times, and the read data is
+ stored in the provided Buffer.
+
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 16-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+ @param Count The number of times to read I/O port.
+ @param Buffer The buffer to store the read data into.
+
+**/
+VOID
+EFIAPI
+IoReadFifo16 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ );
+
+/**
+ Reads a 32-bit I/O port fifo into a block of memory.
+
+ Reads the 32-bit I/O fifo port specified by Port.
+
+ The port is read Count times, and the read data is
+ stored in the provided Buffer.
+
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 32-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+ @param Count The number of times to read I/O port.
+ @param Buffer The buffer to store the read data into.
+
+**/
+VOID
+EFIAPI
+IoReadFifo32 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ );
+
+/**
+ Writes a block of memory into an 8-bit I/O port fifo.
+
+ Writes the 8-bit I/O fifo port specified by Port.
+
+ The port is written Count times, and the write data is
+ retrieved from the provided Buffer.
+
+ This function must guarantee that all I/O write and write operations are
+ serialized.
+
+ If 8-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Count The number of times to write I/O port.
+ @param Buffer The buffer to store the write data into.
+
+**/
+VOID
+EFIAPI
+IoWriteFifo8 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ );
+
+/**
+ Writes a block of memory into a 16-bit I/O port fifo.
+
+ Writes the 16-bit I/O fifo port specified by Port.
+
+ The port is written Count times, and the write data is
+ retrieved from the provided Buffer.
+
+ This function must guarantee that all I/O write and write operations are
+ serialized.
+
+ If 16-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Count The number of times to write I/O port.
+ @param Buffer The buffer to store the write data into.
+
+**/
+VOID
+EFIAPI
+IoWriteFifo16 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ );
+
+/**
+ Writes a block of memory into a 32-bit I/O port fifo.
+
+ Writes the 32-bit I/O fifo port specified by Port.
+
+ The port is written Count times, and the write data is
+ retrieved from the provided Buffer.
+
+ This function must guarantee that all I/O write and write operations are
+ serialized.
+
+ If 32-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Count The number of times to write I/O port.
+ @param Buffer The buffer to store the write data into.
+
+**/
+VOID
+EFIAPI
+IoWriteFifo32 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ );
+
+#endif
diff --git a/MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf b/MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
new file mode 100644
index 0000000..91ee89f
--- /dev/null
+++ b/MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
@@ -0,0 +1,44 @@
+## @file
+# Base I/O FiFo Library using REP string instructions.
+#
+# Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+#
+# This program and the accompanying materials
+# are licensed and made available under the terms and conditions of the BSD License
+# which accompanies this distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+# Derived from:
+# UefiCpuPkg/CpuIo2Dxe
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = BaseIoFifoLib
+ FILE_GUID = 5591c2ef-cb95-4b56-a907-f057b1b96a3d
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = IoFifoLib
+
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources.IA32]
+ Ia32/IoFifo.nasm
+ Ia32/IoFifo.asm
+
+[Sources.X64]
+ X64/IoFifo.nasm
+ X64/IoFifo.asm
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ DebugLib
diff --git a/MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.asm b/MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.asm
new file mode 100644
index 0000000..96286c2
--- /dev/null
+++ b/MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.asm
@@ -0,0 +1,140 @@
+;------------------------------------------------------------------------------
+;
+; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+;
+; This program and the accompanying materials are licensed and made available
+; under the terms and conditions of the BSD License which accompanies this
+; distribution. The full text of the license may be found at
+; http://opensource.org/licenses/bsd-license.php.
+;
+; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+;
+;------------------------------------------------------------------------------
+
+ .586P
+ .model flat,C
+ .code
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo8 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+IoReadFifo8 PROC
+ push edi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov edi, [esp + 16]
+rep insb
+ pop edi
+ ret
+IoReadFifo8 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo16 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+IoReadFifo16 PROC
+ push edi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov edi, [esp + 16]
+rep insw
+ pop edi
+ ret
+IoReadFifo16 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo32 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+IoReadFifo32 PROC
+ push edi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov edi, [esp + 16]
+rep insd
+ pop edi
+ ret
+IoReadFifo32 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo8 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+IoWriteFifo8 PROC
+ push esi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov esi, [esp + 16]
+rep outsb
+ pop esi
+ ret
+IoWriteFifo8 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo16 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+IoWriteFifo16 PROC
+ push esi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov esi, [esp + 16]
+rep outsw
+ pop esi
+ ret
+IoWriteFifo16 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo32 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+IoWriteFifo32 PROC
+ push esi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov esi, [esp + 16]
+rep outsd
+ pop esi
+ ret
+IoWriteFifo32 ENDP
+
+ END
diff --git a/MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.nasm b/MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.nasm
new file mode 100644
index 0000000..f4210cb
--- /dev/null
+++ b/MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.nasm
@@ -0,0 +1,136 @@
+;------------------------------------------------------------------------------
+;
+; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+;
+; This program and the accompanying materials are licensed and made available
+; under the terms and conditions of the BSD License which accompanies this
+; distribution. The full text of the license may be found at
+; http://opensource.org/licenses/bsd-license.php.
+;
+; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+;
+;------------------------------------------------------------------------------
+
+ SECTION .text
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo8 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoReadFifo8)
+ASM_PFX(IoReadFifo8):
+ push edi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov edi, [esp + 16]
+rep insb
+ pop edi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo16 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoReadFifo16)
+ASM_PFX(IoReadFifo16):
+ push edi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov edi, [esp + 16]
+rep insw
+ pop edi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo32 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoReadFifo32)
+ASM_PFX(IoReadFifo32):
+ push edi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov edi, [esp + 16]
+rep insd
+ pop edi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo8 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoWriteFifo8)
+ASM_PFX(IoWriteFifo8):
+ push esi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov esi, [esp + 16]
+rep outsb
+ pop esi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo16 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoWriteFifo16)
+ASM_PFX(IoWriteFifo16):
+ push esi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov esi, [esp + 16]
+rep outsw
+ pop esi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo32 (
+; IN UINTN Port,
+; IN UINTN Size,
+; IN VOID *Buffer
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoWriteFifo32)
+ASM_PFX(IoWriteFifo32):
+ push esi
+ cld
+ mov dx, [esp + 8]
+ mov ecx, [esp + 12]
+ mov esi, [esp + 16]
+rep outsd
+ pop esi
+ ret
diff --git a/MdePkg/Library/BaseIoFifoLib/X64/IoFifo.asm b/MdePkg/Library/BaseIoFifoLib/X64/IoFifo.asm
new file mode 100644
index 0000000..dfdaeea
--- /dev/null
+++ b/MdePkg/Library/BaseIoFifoLib/X64/IoFifo.asm
@@ -0,0 +1,126 @@
+;------------------------------------------------------------------------------
+;
+; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+;
+; This program and the accompanying materials are licensed and made available
+; under the terms and conditions of the BSD License which accompanies this
+; distribution. The full text of the license may be found at
+; http://opensource.org/licenses/bsd-license.php.
+;
+; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+;
+;------------------------------------------------------------------------------
+
+ .code
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo8 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+IoReadFifo8 PROC
+ cld
+ xchg rcx, rdx
+ xchg rdi, r8 ; rdi: buffer address; r8: save rdi
+rep insb
+ mov rdi, r8 ; restore rdi
+ ret
+IoReadFifo8 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo16 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+IoReadFifo16 PROC
+ cld
+ xchg rcx, rdx
+ xchg rdi, r8 ; rdi: buffer address; r8: save rdi
+rep insw
+ mov rdi, r8 ; restore rdi
+ ret
+IoReadFifo16 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo32 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+IoReadFifo32 PROC
+ cld
+ xchg rcx, rdx
+ xchg rdi, r8 ; rdi: buffer address; r8: save rdi
+rep insd
+ mov rdi, r8 ; restore rdi
+ ret
+IoReadFifo32 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo8 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+IoWriteFifo8 PROC
+ cld
+ xchg rcx, rdx
+ xchg rsi, r8 ; rsi: buffer address; r8: save rsi
+rep outsb
+ mov rsi, r8 ; restore rsi
+ ret
+IoWriteFifo8 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo16 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+IoWriteFifo16 PROC
+ cld
+ xchg rcx, rdx
+ xchg rsi, r8 ; rsi: buffer address; r8: save rsi
+rep outsw
+ mov rsi, r8 ; restore rsi
+ ret
+IoWriteFifo16 ENDP
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo32 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+IoWriteFifo32 PROC
+ cld
+ xchg rcx, rdx
+ xchg rsi, r8 ; rsi: buffer address; r8: save rsi
+rep outsd
+ mov rsi, r8 ; restore rsi
+ ret
+IoWriteFifo32 ENDP
+
+ END
diff --git a/MdePkg/Library/BaseIoFifoLib/X64/IoFifo.nasm b/MdePkg/Library/BaseIoFifoLib/X64/IoFifo.nasm
new file mode 100644
index 0000000..40f90f9
--- /dev/null
+++ b/MdePkg/Library/BaseIoFifoLib/X64/IoFifo.nasm
@@ -0,0 +1,125 @@
+;------------------------------------------------------------------------------
+;
+; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+;
+; This program and the accompanying materials are licensed and made available
+; under the terms and conditions of the BSD License which accompanies this
+; distribution. The full text of the license may be found at
+; http://opensource.org/licenses/bsd-license.php.
+;
+; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+;
+;------------------------------------------------------------------------------
+
+ DEFAULT REL
+ SECTION .text
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo8 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoReadFifo8)
+ASM_PFX(IoReadFifo8):
+ cld
+ xchg rcx, rdx
+ xchg rdi, r8 ; rdi: buffer address; r8: save rdi
+rep insb
+ mov rdi, r8 ; restore rdi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo16 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoReadFifo16)
+ASM_PFX(IoReadFifo16):
+ cld
+ xchg rcx, rdx
+ xchg rdi, r8 ; rdi: buffer address; r8: save rdi
+rep insw
+ mov rdi, r8 ; restore rdi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoReadFifo32 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoReadFifo32)
+ASM_PFX(IoReadFifo32):
+ cld
+ xchg rcx, rdx
+ xchg rdi, r8 ; rdi: buffer address; r8: save rdi
+rep insd
+ mov rdi, r8 ; restore rdi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo8 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoWriteFifo8)
+ASM_PFX(IoWriteFifo8):
+ cld
+ xchg rcx, rdx
+ xchg rsi, r8 ; rsi: buffer address; r8: save rsi
+rep outsb
+ mov rsi, r8 ; restore rsi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo16 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoWriteFifo16)
+ASM_PFX(IoWriteFifo16):
+ cld
+ xchg rcx, rdx
+ xchg rsi, r8 ; rsi: buffer address; r8: save rsi
+rep outsw
+ mov rsi, r8 ; restore rsi
+ ret
+
+;------------------------------------------------------------------------------
+; VOID
+; EFIAPI
+; IoWriteFifo32 (
+; IN UINTN Port, // rcx
+; IN UINTN Size, // rdx
+; IN VOID *Buffer // r8
+; );
+;------------------------------------------------------------------------------
+global ASM_PFX(IoWriteFifo32)
+ASM_PFX(IoWriteFifo32):
+ cld
+ xchg rcx, rdx
+ xchg rsi, r8 ; rsi: buffer address; r8: save rsi
+rep outsd
+ mov rsi, r8 ; restore rsi
+ ret
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index f2bdb30..4475d4d 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -7,6 +7,7 @@
# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
# Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials are licensed and made available under
# the terms and conditions of the BSD License which accompanies this distribution.
@@ -254,6 +255,9 @@ [LibraryClasses.IA32, LibraryClasses.X64]
#
RngLib|Include/Library/RngLib.h
+ ## @libraryclass Provide services to access I/O Fifo Ports
+ IoFifoLib|Include/Library/IoFifoLib.h
+
[LibraryClasses.IPF]
## @libraryclass The SAL Library provides a service to make a SAL CALL.
SalLib|Include/Library/SalLib.h
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index b4575cd..93c5ee4 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -3,6 +3,7 @@
#
# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
# Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -155,6 +156,7 @@ [Components.IA32, Components.X64]
MdePkg/Library/SmmMemLib/SmmMemLib.inf
MdePkg/Library/BaseRngLib/BaseRngLib.inf
MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf
+ MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
[Components.IPF]
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
--
1.9.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 2/4] Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
2017-01-05 21:49 [PATCH v3 0/4] BaseIoFifoLib Leo Duran
2017-01-05 21:49 ` [PATCH v3 1/4] MdePkg: Add BaseIoFifoLib library Leo Duran
@ 2017-01-05 21:49 ` Leo Duran
2017-01-06 11:23 ` Laszlo Ersek
2017-01-05 21:49 ` [PATCH v3 3/4] Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library Leo Duran
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Leo Duran @ 2017-01-05 21:49 UTC (permalink / raw)
To: edk2-devel
Cc: brijesh.singh, lersek, jordan.l.justen, jeff.fan,
michael.d.kinney, liming.gao, maurice.ma, prince.agyeman,
ruiyu.ni, kelly.steele, david.wei, mang.guo, Leo Duran
From: Brijesh Singh <brijesh.singh@amd.com>
This patch adds the new BaseIoFifoLib (IoFifoLib class) library
consumed by the UefiCpuPkg/CpuIo2Dxe driver.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 +++
CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +++
DuetPkg/DuetPkgIa32.dsc | 2 ++
DuetPkg/DuetPkgX64.dsc | 2 ++
OvmfPkg/OvmfPkgIa32.dsc | 2 ++
OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
OvmfPkg/OvmfPkgX64.dsc | 2 ++
QuarkPlatformPkg/Quark.dsc | 2 ++
QuarkPlatformPkg/QuarkMin.dsc | 2 ++
UefiCpuPkg/UefiCpuPkg.dsc | 2 ++
Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 ++
Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 2 ++
Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 2 ++
13 files changed, 28 insertions(+)
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index cdfcb75..1647b38 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -4,6 +4,8 @@
# Provides drivers and definitions to create uefi payload for coreboot.
#
# Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+#
# This program and the accompanying materials are licensed and made available under
# the terms and conditions of the BSD License that accompanies this distribution.
# The full text of the license may be found at
@@ -129,6 +131,7 @@ [LibraryClasses]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
!if $(PCIE_BASE) == 0
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
index c35f261..6c6ad94 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
@@ -4,6 +4,8 @@
# Provides drivers and definitions to create uefi payload for coreboot.
#
# Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+#
# This program and the accompanying materials are licensed and made available under
# the terms and conditions of the BSD License that accompanies this distribution.
# The full text of the license may be found at
@@ -131,6 +133,7 @@ [LibraryClasses]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
!if $(PCIE_BASE) == 0
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
diff --git a/DuetPkg/DuetPkgIa32.dsc b/DuetPkg/DuetPkgIa32.dsc
index 7dd963b..f9efabc 100644
--- a/DuetPkg/DuetPkgIa32.dsc
+++ b/DuetPkg/DuetPkgIa32.dsc
@@ -5,6 +5,7 @@
# to help developing and debugging native EFI/UEFI drivers.
#
# Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -59,6 +60,7 @@ [LibraryClasses]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
diff --git a/DuetPkg/DuetPkgX64.dsc b/DuetPkg/DuetPkgX64.dsc
index 1b08a95..1fd96ce 100644
--- a/DuetPkg/DuetPkgX64.dsc
+++ b/DuetPkg/DuetPkgX64.dsc
@@ -5,6 +5,7 @@
# to help developing and debugging native EFI/UEFI drivers.
#
# Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -59,6 +60,7 @@ [LibraryClasses]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 81f7521..bd76113 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -3,6 +3,7 @@
#
# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -101,6 +102,7 @@ [LibraryClasses]
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index f7855b6..1bc0c85 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -3,6 +3,7 @@
#
# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -106,6 +107,7 @@ [LibraryClasses]
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.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 e933a41..3e1c0fa 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -3,6 +3,7 @@
#
# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -106,6 +107,7 @@ [LibraryClasses]
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/QuarkPlatformPkg/Quark.dsc b/QuarkPlatformPkg/Quark.dsc
index d36fd6e..57230dd 100644
--- a/QuarkPlatformPkg/Quark.dsc
+++ b/QuarkPlatformPkg/Quark.dsc
@@ -3,6 +3,7 @@
#
# This package provides Clanton Peak CRB platform specific modules.
# Copyright (c) 2013 - 2016 Intel Corporation.
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -94,6 +95,7 @@ [LibraryClasses]
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
diff --git a/QuarkPlatformPkg/QuarkMin.dsc b/QuarkPlatformPkg/QuarkMin.dsc
index be85e3f..de7a281 100644
--- a/QuarkPlatformPkg/QuarkMin.dsc
+++ b/QuarkPlatformPkg/QuarkMin.dsc
@@ -3,6 +3,7 @@
#
# This package provides Clanton Peak CRB platform specific modules.
# Copyright (c) 2013 - 2016 Intel Corporation.
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -82,6 +83,7 @@ [LibraryClasses]
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index 3922f2d..42d4154 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -2,6 +2,7 @@
# UefiCpuPkg Package
#
# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -37,6 +38,7 @@ [LibraryClasses]
DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index cdde337..79bf900 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -2,6 +2,7 @@
# Platform description.
#
# Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials are licensed and made available under
# the terms and conditions of the BSD License that accompanies this distribution.
@@ -118,6 +119,7 @@ [LibraryClasses.common]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index ed519bd..a4b743e 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -2,6 +2,7 @@
# Platform description.
#
# Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials are licensed and made available under
# the terms and conditions of the BSD License that accompanies this distribution.
@@ -118,6 +119,7 @@ [LibraryClasses.common]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 91f018a..7d65e82 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -2,6 +2,7 @@
# Platform description.
#
# Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials are licensed and made available under
# the terms and conditions of the BSD License that accompanies this distribution.
@@ -118,6 +119,7 @@ [LibraryClasses.common]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoFifoLib|MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
--
1.9.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 3/4] Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library.
2017-01-05 21:49 [PATCH v3 0/4] BaseIoFifoLib Leo Duran
2017-01-05 21:49 ` [PATCH v3 1/4] MdePkg: Add BaseIoFifoLib library Leo Duran
2017-01-05 21:49 ` [PATCH v3 2/4] Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf Leo Duran
@ 2017-01-05 21:49 ` Leo Duran
2017-01-05 21:49 ` [PATCH v3 4/4] Modify QemuFwCfgLib " Leo Duran
2017-01-06 6:02 ` [PATCH v3 0/4] BaseIoFifoLib Gao, Liming
4 siblings, 0 replies; 23+ messages in thread
From: Leo Duran @ 2017-01-05 21:49 UTC (permalink / raw)
To: edk2-devel
Cc: brijesh.singh, lersek, jordan.l.justen, jeff.fan,
michael.d.kinney, liming.gao, maurice.ma, prince.agyeman,
ruiyu.ni, kelly.steele, david.wei, mang.guo, Leo Duran
From: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c | 3 +-
UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h | 3 +
UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf | 12 +--
UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.asm | 140 ----------------------------------
UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm | 136 ---------------------------------
UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.asm | 126 ------------------------------
UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm | 125 ------------------------------
7 files changed, 8 insertions(+), 537 deletions(-)
delete mode 100644 UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.asm
delete mode 100644 UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm
delete mode 100644 UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.asm
delete mode 100644 UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm
diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c
index 6ccfc40..7727878 100644
--- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c
+++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c
@@ -2,6 +2,8 @@
Produces the CPU I/O 2 Protocol.
Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -13,7 +15,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
#include "CpuIo2Dxe.h"
-#include "IoFifo.h"
//
// Handle for the CPU I/O 2 Protocol
diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h
index 7d00da1..c7789fa 100644
--- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h
+++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h
@@ -2,6 +2,8 @@
Internal include file for the CPU I/O 2 Protocol.
Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -22,6 +24,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Library/BaseLib.h>
#include <Library/DebugLib.h>
#include <Library/IoLib.h>
+#include <Library/IoFifoLib.h>
#include <Library/UefiBootServicesTableLib.h>
#define MAX_IO_PORT_ADDRESS 0xFFFF
diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
index 920ede7..1a608f4 100644
--- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
+++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
@@ -2,6 +2,8 @@
# Produces the CPU I/O 2 Protocol by using the services of the I/O Library.
#
# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -30,15 +32,6 @@ [Defines]
[Sources]
CpuIo2Dxe.c
CpuIo2Dxe.h
- IoFifo.h
-
-[Sources.IA32]
- Ia32/IoFifo.nasm
- Ia32/IoFifo.asm
-
-[Sources.X64]
- X64/IoFifo.nasm
- X64/IoFifo.asm
[Packages]
MdePkg/MdePkg.dec
@@ -48,6 +41,7 @@ [LibraryClasses]
BaseLib
DebugLib
IoLib
+ IoFifoLib
UefiBootServicesTableLib
[Protocols]
diff --git a/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.asm b/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.asm
deleted file mode 100644
index b1cc25e..0000000
--- a/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.asm
+++ /dev/null
@@ -1,140 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-;
-; This program and the accompanying materials are licensed and made available
-; under the terms and conditions of the BSD License which accompanies this
-; distribution. The full text of the license may be found at
-; http://opensource.org/licenses/bsd-license.php.
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-;
-;------------------------------------------------------------------------------
-
- .586P
- .model flat,C
- .code
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-IoReadFifo8 PROC
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insb
- pop edi
- ret
-IoReadFifo8 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo16 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-IoReadFifo16 PROC
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insw
- pop edi
- ret
-IoReadFifo16 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo32 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-IoReadFifo32 PROC
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insd
- pop edi
- ret
-IoReadFifo32 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-IoWriteFifo8 PROC
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsb
- pop esi
- ret
-IoWriteFifo8 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo16 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-IoWriteFifo16 PROC
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsw
- pop esi
- ret
-IoWriteFifo16 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo32 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-IoWriteFifo32 PROC
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsd
- pop esi
- ret
-IoWriteFifo32 ENDP
-
- END
-
diff --git a/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm b/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm
deleted file mode 100644
index daa90a9..0000000
--- a/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm
+++ /dev/null
@@ -1,136 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-;
-; This program and the accompanying materials are licensed and made available
-; under the terms and conditions of the BSD License which accompanies this
-; distribution. The full text of the license may be found at
-; http://opensource.org/licenses/bsd-license.php.
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-;
-;------------------------------------------------------------------------------
-
- SECTION .text
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo8)
-ASM_PFX(IoReadFifo8):
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insb
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo16 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo16)
-ASM_PFX(IoReadFifo16):
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insw
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo32 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo32)
-ASM_PFX(IoReadFifo32):
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insd
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo8)
-ASM_PFX(IoWriteFifo8):
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsb
- pop esi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo16 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo16)
-ASM_PFX(IoWriteFifo16):
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsw
- pop esi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo32 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo32)
-ASM_PFX(IoWriteFifo32):
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsd
- pop esi
- ret
-
diff --git a/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.asm b/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.asm
deleted file mode 100644
index 1a3f0ef..0000000
--- a/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.asm
+++ /dev/null
@@ -1,126 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-;
-; This program and the accompanying materials are licensed and made available
-; under the terms and conditions of the BSD License which accompanies this
-; distribution. The full text of the license may be found at
-; http://opensource.org/licenses/bsd-license.php.
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-;
-;------------------------------------------------------------------------------
-
- .code
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-IoReadFifo8 PROC
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insb
- mov rdi, r8 ; restore rdi
- ret
-IoReadFifo8 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo16 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-IoReadFifo16 PROC
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insw
- mov rdi, r8 ; restore rdi
- ret
-IoReadFifo16 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo32 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-IoReadFifo32 PROC
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insd
- mov rdi, r8 ; restore rdi
- ret
-IoReadFifo32 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-IoWriteFifo8 PROC
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsb
- mov rsi, r8 ; restore rsi
- ret
-IoWriteFifo8 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo16 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-IoWriteFifo16 PROC
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsw
- mov rsi, r8 ; restore rsi
- ret
-IoWriteFifo16 ENDP
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo32 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-IoWriteFifo32 PROC
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsd
- mov rsi, r8 ; restore rsi
- ret
-IoWriteFifo32 ENDP
-
- END
-
diff --git a/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm b/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm
deleted file mode 100644
index bb3d1da..0000000
--- a/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm
+++ /dev/null
@@ -1,125 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-;
-; This program and the accompanying materials are licensed and made available
-; under the terms and conditions of the BSD License which accompanies this
-; distribution. The full text of the license may be found at
-; http://opensource.org/licenses/bsd-license.php.
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-;
-;------------------------------------------------------------------------------
-
- DEFAULT REL
- SECTION .text
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo8)
-ASM_PFX(IoReadFifo8):
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insb
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo16 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo16)
-ASM_PFX(IoReadFifo16):
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insw
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo32 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo32)
-ASM_PFX(IoReadFifo32):
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insd
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo8)
-ASM_PFX(IoWriteFifo8):
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsb
- mov rsi, r8 ; restore rsi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo16 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo16)
-ASM_PFX(IoWriteFifo16):
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsw
- mov rsi, r8 ; restore rsi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo32 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo32)
-ASM_PFX(IoWriteFifo32):
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsd
- mov rsi, r8 ; restore rsi
- ret
-
--
1.9.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 4/4] Modify QemuFwCfgLib to use new BaseIoFifoLib library.
2017-01-05 21:49 [PATCH v3 0/4] BaseIoFifoLib Leo Duran
` (2 preceding siblings ...)
2017-01-05 21:49 ` [PATCH v3 3/4] Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library Leo Duran
@ 2017-01-05 21:49 ` Leo Duran
2017-01-06 11:36 ` Laszlo Ersek
2017-01-06 6:02 ` [PATCH v3 0/4] BaseIoFifoLib Gao, Liming
4 siblings, 1 reply; 23+ messages in thread
From: Leo Duran @ 2017-01-05 21:49 UTC (permalink / raw)
To: edk2-devel
Cc: brijesh.singh, lersek, jordan.l.justen, jeff.fan,
michael.d.kinney, liming.gao, maurice.ma, prince.agyeman,
ruiyu.ni, kelly.steele, david.wei, mang.guo, Leo Duran
From: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm | 55 -----------------------
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 54 +---------------------
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 8 +---
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf | 8 +---
OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm | 52 ---------------------
5 files changed, 5 insertions(+), 172 deletions(-)
delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
diff --git a/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm b/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
deleted file mode 100644
index faa22e9..0000000
--- a/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
+++ /dev/null
@@ -1,55 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
-; This program and the accompanying materials
-; are licensed and made available under the terms and conditions of the BSD License
-; which accompanies this distribution. The full text of the license may be found at
-; http://opensource.org/licenses/bsd-license.php.
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-;
-;------------------------------------------------------------------------------
-
- SECTION .text
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo8)
-ASM_PFX(IoReadFifo8):
-
- mov dx, [esp + 4]
- mov ecx, [esp + 8]
- push edi
- mov edi, [esp + 16]
-rep insb
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo8)
-ASM_PFX(IoWriteFifo8):
-
- mov dx, [esp + 4]
- mov ecx, [esp + 8]
- push esi
- mov esi, [esp + 16]
-rep outsb
- pop esi
- ret
-
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
index 0bbf121..68fbade 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
@@ -2,6 +2,7 @@
Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
Copyright (C) 2013, Red Hat, Inc.
+ Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -26,59 +27,6 @@
/**
- Reads an 8-bit I/O port fifo into a block of memory.
-
- Reads the 8-bit I/O fifo port specified by Port.
-
- The port is read Count times, and the read data is
- stored in the provided Buffer.
-
- This function must guarantee that all I/O read and write operations are
- serialized.
-
- If 8-bit I/O port operations are not supported, then ASSERT().
-
- @param Port The I/O port to read.
- @param Count The number of times to read I/O port.
- @param Buffer The buffer to store the read data into.
-
-**/
-VOID
-EFIAPI
-IoReadFifo8 (
- IN UINTN Port,
- IN UINTN Count,
- OUT VOID *Buffer
- );
-
-/**
- Writes an 8-bit I/O port fifo from a block of memory.
-
- Writes the 8-bit I/O fifo port specified by Port.
-
- The port is written Count times, and the data are obtained
- from the provided Buffer.
-
- This function must guarantee that all I/O read and write operations are
- serialized.
-
- If 8-bit I/O port operations are not supported, then ASSERT().
-
- @param Port The I/O port to read.
- @param Count The number of times to read I/O port.
- @param Buffer The buffer to store the read data into.
-
-**/
-VOID
-EFIAPI
-IoWriteFifo8 (
- IN UINTN Port,
- IN UINTN Count,
- OUT VOID *Buffer
- );
-
-
-/**
Selects a firmware configuration item for reading.
Following this call, any data read from this item will start from
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
index 66ac778..e48c639 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
@@ -4,6 +4,7 @@
#
# Copyright (C) 2013, Red Hat, Inc.
# Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -36,12 +37,6 @@ [Sources]
QemuFwCfgLib.c
QemuFwCfgPeiDxe.c
-[Sources.IA32]
- Ia32/IoLibExAsm.nasm
-
-[Sources.X64]
- X64/IoLibExAsm.nasm
-
[Packages]
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
@@ -51,5 +46,6 @@ [LibraryClasses]
BaseMemoryLib
DebugLib
IoLib
+ IoFifoLib
MemoryAllocationLib
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
index c1d6a54..6275ba9 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
@@ -4,6 +4,7 @@
#
# Copyright (C) 2013, Red Hat, Inc.
# Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -34,12 +35,6 @@ [Sources]
QemuFwCfgLib.c
QemuFwCfgSec.c
-[Sources.IA32]
- Ia32/IoLibExAsm.nasm
-
-[Sources.X64]
- X64/IoLibExAsm.nasm
-
[Packages]
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
@@ -49,5 +44,6 @@ [LibraryClasses]
BaseMemoryLib
DebugLib
IoLib
+ IoFifoLib
MemoryAllocationLib
diff --git a/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm b/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
deleted file mode 100644
index f1078f2..0000000
--- a/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
+++ /dev/null
@@ -1,52 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
-; This program and the accompanying materials
-; are licensed and made available under the terms and conditions of the BSD License
-; which accompanies this distribution. The full text of the license may be found at
-; http://opensource.org/licenses/bsd-license.php.
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-;
-;------------------------------------------------------------------------------
-
- DEFAULT REL
- SECTION .text
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo8)
-ASM_PFX(IoReadFifo8):
-
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insb
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo8)
-ASM_PFX(IoWriteFifo8):
-
- xchg rcx, rdx
- xchg rsi, r8 ; rdi: buffer address; r8: save rdi
-rep outsb
- mov rsi, r8 ; restore rdi
- ret
-
--
1.9.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-05 21:49 [PATCH v3 0/4] BaseIoFifoLib Leo Duran
` (3 preceding siblings ...)
2017-01-05 21:49 ` [PATCH v3 4/4] Modify QemuFwCfgLib " Leo Duran
@ 2017-01-06 6:02 ` Gao, Liming
2017-01-06 11:12 ` Laszlo Ersek
4 siblings, 1 reply; 23+ messages in thread
From: Gao, Liming @ 2017-01-06 6:02 UTC (permalink / raw)
To: Leo Duran, edk2-devel@lists.01.org
Cc: brijesh.singh@amd.com, lersek@redhat.com, Justen, Jordan L,
Fan, Jeff, Kinney, Michael D, Ma, Maurice, Agyeman, Prince,
Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
Leo:
FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how about add new APIs into IoLib together with other Io APIs? If so, no new library class is required. Platform DSC files are not required to be changed.
Thanks
Liming
>-----Original Message-----
>From: Leo Duran [mailto:leo.duran@amd.com]
>Sent: Friday, January 06, 2017 5:49 AM
>To: edk2-devel@lists.01.org
>Cc: brijesh.singh@amd.com; lersek@redhat.com; Justen, Jordan L
><jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Kinney, Michael
>D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Ma,
>Maurice <maurice.ma@intel.com>; Agyeman, Prince
><prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
><kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo, Mang
><mang.guo@intel.com>; Leo Duran <leo.duran@amd.com>
>Subject: [PATCH v3 0/4] BaseIoFifoLib
>
>The UefiCpuPkg/CpuIo2Dxe driver and the QemuCfgLib library have duplicate
>implementations of I/O Fifo routines. The patch series moves the I/O Fifo
>routines into a common BaseIofifoLib library supporting IA32 and X64
>architectures under MdePkg.
>
>The intent of this patch series is twofold:
>1) Consolidate I/O Fifo routines into a common BaseIofifoLib library.
>2) Allow override of BaseIofifoLib for specific platform implementations.
>For example, the OVMF package can provide its own version of BaseIoFifoLib
>to support Secure Encrypted Virtualization (SEV) guests, since SEV does not
>support string I/O instructions (rep ins/outs), and requires unrolled loops
>of single in/out instructions.
>
>Brijesh Singh (4):
> MdePkg: Add BaseIoFifoLib library
> Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
> Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library.
> Modify QemuFwCfgLib to use new BaseIoFifoLib library.
>
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +
> DuetPkg/DuetPkgIa32.dsc | 2 +
> DuetPkg/DuetPkgX64.dsc | 2 +
> MdePkg/Include/Library/IoFifoLib.h | 176 +++++++++++++
> MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf | 44 ++++
> .../Library/BaseIoFifoLib}/Ia32/IoFifo.asm | 280 ++++++++++-----------
> .../Library/BaseIoFifoLib}/Ia32/IoFifo.nasm | 272 ++++++++++----------
> .../Library/BaseIoFifoLib}/X64/IoFifo.asm | 252 +++++++++----------
> .../Library/BaseIoFifoLib}/X64/IoFifo.nasm | 250 +++++++++---------
> MdePkg/MdePkg.dec | 4 +
> MdePkg/MdePkg.dsc | 2 +
> OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm | 55 ----
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 54 +---
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 8 +-
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf | 8 +-
> OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm | 52 ----
> OvmfPkg/OvmfPkgIa32.dsc | 2 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +
> OvmfPkg/OvmfPkgX64.dsc | 2 +
> QuarkPlatformPkg/Quark.dsc | 2 +
> QuarkPlatformPkg/QuarkMin.dsc | 2 +
> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c | 3 +-
> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h | 3 +
> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf | 12 +-
> UefiCpuPkg/UefiCpuPkg.dsc | 2 +
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 +
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 2 +
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 2 +
> 29 files changed, 794 insertions(+), 709 deletions(-)
> create mode 100644 MdePkg/Include/Library/IoFifoLib.h
> create mode 100644 MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
> rename {UefiCpuPkg/CpuIo2Dxe =>
>MdePkg/Library/BaseIoFifoLib}/Ia32/IoFifo.asm (96%)
> rename {UefiCpuPkg/CpuIo2Dxe =>
>MdePkg/Library/BaseIoFifoLib}/Ia32/IoFifo.nasm (96%)
> rename {UefiCpuPkg/CpuIo2Dxe =>
>MdePkg/Library/BaseIoFifoLib}/X64/IoFifo.asm (97%)
> rename {UefiCpuPkg/CpuIo2Dxe =>
>MdePkg/Library/BaseIoFifoLib}/X64/IoFifo.nasm (97%)
> delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
>
>--
>1.9.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-06 6:02 ` [PATCH v3 0/4] BaseIoFifoLib Gao, Liming
@ 2017-01-06 11:12 ` Laszlo Ersek
2017-01-06 15:23 ` Duran, Leo
0 siblings, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2017-01-06 11:12 UTC (permalink / raw)
To: Gao, Liming, Leo Duran, edk2-devel@lists.01.org
Cc: brijesh.singh@amd.com, Justen, Jordan L, Fan, Jeff,
Kinney, Michael D, Ma, Maurice, Agyeman, Prince, Ni, Ruiyu,
Steele, Kelly, Wei, David, Guo, Mang
On 01/06/17 07:02, Gao, Liming wrote:
> Leo:
> FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how about
> add new APIs into IoLib together with other Io APIs? If so, no new
> library class is required. Platform DSC files are not required to be
> changed.
But then all of the IoLib instances will have to be extended too:
IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
Thanks,
Laszlo
>
> Thanks
> Liming
>> -----Original Message-----
>> From: Leo Duran [mailto:leo.duran@amd.com]
>> Sent: Friday, January 06, 2017 5:49 AM
>> To: edk2-devel@lists.01.org
>> Cc: brijesh.singh@amd.com; lersek@redhat.com; Justen, Jordan L
>> <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Kinney, Michael
>> D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Ma,
>> Maurice <maurice.ma@intel.com>; Agyeman, Prince
>> <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
>> <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo, Mang
>> <mang.guo@intel.com>; Leo Duran <leo.duran@amd.com>
>> Subject: [PATCH v3 0/4] BaseIoFifoLib
>>
>> The UefiCpuPkg/CpuIo2Dxe driver and the QemuCfgLib library have duplicate
>> implementations of I/O Fifo routines. The patch series moves the I/O Fifo
>> routines into a common BaseIofifoLib library supporting IA32 and X64
>> architectures under MdePkg.
>>
>> The intent of this patch series is twofold:
>> 1) Consolidate I/O Fifo routines into a common BaseIofifoLib library.
>> 2) Allow override of BaseIofifoLib for specific platform implementations.
>> For example, the OVMF package can provide its own version of BaseIoFifoLib
>> to support Secure Encrypted Virtualization (SEV) guests, since SEV does not
>> support string I/O instructions (rep ins/outs), and requires unrolled loops
>> of single in/out instructions.
>>
>> Brijesh Singh (4):
>> MdePkg: Add BaseIoFifoLib library
>> Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>> Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library.
>> Modify QemuFwCfgLib to use new BaseIoFifoLib library.
>>
>> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 +
>> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +
>> DuetPkg/DuetPkgIa32.dsc | 2 +
>> DuetPkg/DuetPkgX64.dsc | 2 +
>> MdePkg/Include/Library/IoFifoLib.h | 176 +++++++++++++
>> MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf | 44 ++++
>> .../Library/BaseIoFifoLib}/Ia32/IoFifo.asm | 280 ++++++++++-----------
>> .../Library/BaseIoFifoLib}/Ia32/IoFifo.nasm | 272 ++++++++++----------
>> .../Library/BaseIoFifoLib}/X64/IoFifo.asm | 252 +++++++++----------
>> .../Library/BaseIoFifoLib}/X64/IoFifo.nasm | 250 +++++++++---------
>> MdePkg/MdePkg.dec | 4 +
>> MdePkg/MdePkg.dsc | 2 +
>> OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm | 55 ----
>> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 54 +---
>> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 8 +-
>> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf | 8 +-
>> OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm | 52 ----
>> OvmfPkg/OvmfPkgIa32.dsc | 2 +
>> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +
>> OvmfPkg/OvmfPkgX64.dsc | 2 +
>> QuarkPlatformPkg/Quark.dsc | 2 +
>> QuarkPlatformPkg/QuarkMin.dsc | 2 +
>> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c | 3 +-
>> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h | 3 +
>> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf | 12 +-
>> UefiCpuPkg/UefiCpuPkg.dsc | 2 +
>> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 +
>> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 2 +
>> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 2 +
>> 29 files changed, 794 insertions(+), 709 deletions(-)
>> create mode 100644 MdePkg/Include/Library/IoFifoLib.h
>> create mode 100644 MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
>> rename {UefiCpuPkg/CpuIo2Dxe =>
>> MdePkg/Library/BaseIoFifoLib}/Ia32/IoFifo.asm (96%)
>> rename {UefiCpuPkg/CpuIo2Dxe =>
>> MdePkg/Library/BaseIoFifoLib}/Ia32/IoFifo.nasm (96%)
>> rename {UefiCpuPkg/CpuIo2Dxe =>
>> MdePkg/Library/BaseIoFifoLib}/X64/IoFifo.asm (97%)
>> rename {UefiCpuPkg/CpuIo2Dxe =>
>> MdePkg/Library/BaseIoFifoLib}/X64/IoFifo.nasm (97%)
>> delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
>> delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
>>
>> --
>> 1.9.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/4] Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
2017-01-05 21:49 ` [PATCH v3 2/4] Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf Leo Duran
@ 2017-01-06 11:23 ` Laszlo Ersek
2017-01-06 15:28 ` Duran, Leo
0 siblings, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2017-01-06 11:23 UTC (permalink / raw)
To: Leo Duran, edk2-devel
Cc: ruiyu.ni, brijesh.singh, jordan.l.justen, liming.gao,
michael.d.kinney, jeff.fan, prince.agyeman, david.wei
On 01/05/17 22:49, Leo Duran wrote:
> From: Brijesh Singh <brijesh.singh@amd.com>
>
> This patch adds the new BaseIoFifoLib (IoFifoLib class) library
> consumed by the UefiCpuPkg/CpuIo2Dxe driver.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> ---
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 +++
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +++
> DuetPkg/DuetPkgIa32.dsc | 2 ++
> DuetPkg/DuetPkgX64.dsc | 2 ++
> OvmfPkg/OvmfPkgIa32.dsc | 2 ++
> OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
> OvmfPkg/OvmfPkgX64.dsc | 2 ++
> QuarkPlatformPkg/Quark.dsc | 2 ++
> QuarkPlatformPkg/QuarkMin.dsc | 2 ++
> UefiCpuPkg/UefiCpuPkg.dsc | 2 ++
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 ++
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 2 ++
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 2 ++
> 13 files changed, 28 insertions(+)
The OvmfPkg hunks look good to me, but I believe you missed my request
that the patch be split per top level Pkg directory (that is, into 6
patchets).
https://lists.01.org/pipermail/edk2-devel/2017-January/006193.html
(I do see you implemented the xfuncname configuration; thanks for that!)
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/4] Modify QemuFwCfgLib to use new BaseIoFifoLib library.
2017-01-05 21:49 ` [PATCH v3 4/4] Modify QemuFwCfgLib " Leo Duran
@ 2017-01-06 11:36 ` Laszlo Ersek
2017-01-06 15:31 ` Duran, Leo
0 siblings, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2017-01-06 11:36 UTC (permalink / raw)
To: Leo Duran, edk2-devel
Cc: ruiyu.ni, brijesh.singh, jordan.l.justen, liming.gao,
michael.d.kinney, jeff.fan, prince.agyeman, david.wei
On 01/05/17 22:49, Leo Duran wrote:
> From: Brijesh Singh <brijesh.singh@amd.com>
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> ---
> OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm | 55 -----------------------
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 54 +---------------------
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 8 +---
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf | 8 +---
> OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm | 52 ---------------------
> 5 files changed, 5 insertions(+), 172 deletions(-)
> delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
Differences vs. v1:
- git metadata & commit msg tags fixed, good
- copyright notices added, okay
- now unnecessary prototypes removed, good
- QemuFwCfgSecLib.inf now handled (I didn't notice it in v1, sorry!), OK
However, the #include <Library/IoFifoLib.h> directive is also gone, from
"QemuFwCfgLib.c". I think you may have moved that directive to
"QemuFwCfgLibInternal.h", but forgotten to stage that change for commit,
with "git add"?
(Anyway, what was wrong with the #include directive being in
"QemuFwCfgLib.c"?)
Apart from the #include directive, it looks good to me. (I know Liming
is proposing a different avenue, and here I'm not arguing against that;
just saying that under this approach, the patch seems mostly okay.)
Thanks,
Laszlo
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm b/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> deleted file mode 100644
> index faa22e9..0000000
> --- a/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -;------------------------------------------------------------------------------
> -;
> -; Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
> -; This program and the accompanying materials
> -; are licensed and made available under the terms and conditions of the BSD License
> -; which accompanies this distribution. The full text of the license may be found at
> -; http://opensource.org/licenses/bsd-license.php.
> -;
> -; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -;
> -;------------------------------------------------------------------------------
> -
> - SECTION .text
> -
> -;------------------------------------------------------------------------------
> -; VOID
> -; EFIAPI
> -; IoReadFifo8 (
> -; IN UINTN Port,
> -; IN UINTN Size,
> -; IN VOID *Buffer
> -; );
> -;------------------------------------------------------------------------------
> -global ASM_PFX(IoReadFifo8)
> -ASM_PFX(IoReadFifo8):
> -
> - mov dx, [esp + 4]
> - mov ecx, [esp + 8]
> - push edi
> - mov edi, [esp + 16]
> -rep insb
> - pop edi
> - ret
> -
> -;------------------------------------------------------------------------------
> -; VOID
> -; EFIAPI
> -; IoWriteFifo8 (
> -; IN UINTN Port,
> -; IN UINTN Size,
> -; IN VOID *Buffer
> -; );
> -;------------------------------------------------------------------------------
> -global ASM_PFX(IoWriteFifo8)
> -ASM_PFX(IoWriteFifo8):
> -
> - mov dx, [esp + 4]
> - mov ecx, [esp + 8]
> - push esi
> - mov esi, [esp + 16]
> -rep outsb
> - pop esi
> - ret
> -
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> index 0bbf121..68fbade 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> @@ -2,6 +2,7 @@
>
> Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
> Copyright (C) 2013, Red Hat, Inc.
> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -26,59 +27,6 @@
>
>
> /**
> - Reads an 8-bit I/O port fifo into a block of memory.
> -
> - Reads the 8-bit I/O fifo port specified by Port.
> -
> - The port is read Count times, and the read data is
> - stored in the provided Buffer.
> -
> - This function must guarantee that all I/O read and write operations are
> - serialized.
> -
> - If 8-bit I/O port operations are not supported, then ASSERT().
> -
> - @param Port The I/O port to read.
> - @param Count The number of times to read I/O port.
> - @param Buffer The buffer to store the read data into.
> -
> -**/
> -VOID
> -EFIAPI
> -IoReadFifo8 (
> - IN UINTN Port,
> - IN UINTN Count,
> - OUT VOID *Buffer
> - );
> -
> -/**
> - Writes an 8-bit I/O port fifo from a block of memory.
> -
> - Writes the 8-bit I/O fifo port specified by Port.
> -
> - The port is written Count times, and the data are obtained
> - from the provided Buffer.
> -
> - This function must guarantee that all I/O read and write operations are
> - serialized.
> -
> - If 8-bit I/O port operations are not supported, then ASSERT().
> -
> - @param Port The I/O port to read.
> - @param Count The number of times to read I/O port.
> - @param Buffer The buffer to store the read data into.
> -
> -**/
> -VOID
> -EFIAPI
> -IoWriteFifo8 (
> - IN UINTN Port,
> - IN UINTN Count,
> - OUT VOID *Buffer
> - );
> -
> -
> -/**
> Selects a firmware configuration item for reading.
>
> Following this call, any data read from this item will start from
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> index 66ac778..e48c639 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> @@ -4,6 +4,7 @@
> #
> # Copyright (C) 2013, Red Hat, Inc.
> # Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> @@ -36,12 +37,6 @@ [Sources]
> QemuFwCfgLib.c
> QemuFwCfgPeiDxe.c
>
> -[Sources.IA32]
> - Ia32/IoLibExAsm.nasm
> -
> -[Sources.X64]
> - X64/IoLibExAsm.nasm
> -
> [Packages]
> MdePkg/MdePkg.dec
> OvmfPkg/OvmfPkg.dec
> @@ -51,5 +46,6 @@ [LibraryClasses]
> BaseMemoryLib
> DebugLib
> IoLib
> + IoFifoLib
> MemoryAllocationLib
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> index c1d6a54..6275ba9 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> @@ -4,6 +4,7 @@
> #
> # Copyright (C) 2013, Red Hat, Inc.
> # Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> @@ -34,12 +35,6 @@ [Sources]
> QemuFwCfgLib.c
> QemuFwCfgSec.c
>
> -[Sources.IA32]
> - Ia32/IoLibExAsm.nasm
> -
> -[Sources.X64]
> - X64/IoLibExAsm.nasm
> -
> [Packages]
> MdePkg/MdePkg.dec
> OvmfPkg/OvmfPkg.dec
> @@ -49,5 +44,6 @@ [LibraryClasses]
> BaseMemoryLib
> DebugLib
> IoLib
> + IoFifoLib
> MemoryAllocationLib
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm b/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
> deleted file mode 100644
> index f1078f2..0000000
> --- a/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
> +++ /dev/null
> @@ -1,52 +0,0 @@
> -;------------------------------------------------------------------------------
> -;
> -; Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
> -; This program and the accompanying materials
> -; are licensed and made available under the terms and conditions of the BSD License
> -; which accompanies this distribution. The full text of the license may be found at
> -; http://opensource.org/licenses/bsd-license.php.
> -;
> -; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -;
> -;------------------------------------------------------------------------------
> -
> - DEFAULT REL
> - SECTION .text
> -
> -;------------------------------------------------------------------------------
> -; VOID
> -; EFIAPI
> -; IoReadFifo8 (
> -; IN UINTN Port, // rcx
> -; IN UINTN Size, // rdx
> -; IN VOID *Buffer // r8
> -; );
> -;------------------------------------------------------------------------------
> -global ASM_PFX(IoReadFifo8)
> -ASM_PFX(IoReadFifo8):
> -
> - xchg rcx, rdx
> - xchg rdi, r8 ; rdi: buffer address; r8: save rdi
> -rep insb
> - mov rdi, r8 ; restore rdi
> - ret
> -
> -;------------------------------------------------------------------------------
> -; VOID
> -; EFIAPI
> -; IoWriteFifo8 (
> -; IN UINTN Port, // rcx
> -; IN UINTN Size, // rdx
> -; IN VOID *Buffer // r8
> -; );
> -;------------------------------------------------------------------------------
> -global ASM_PFX(IoWriteFifo8)
> -ASM_PFX(IoWriteFifo8):
> -
> - xchg rcx, rdx
> - xchg rsi, r8 ; rdi: buffer address; r8: save rdi
> -rep outsb
> - mov rsi, r8 ; restore rdi
> - ret
> -
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-06 11:12 ` Laszlo Ersek
@ 2017-01-06 15:23 ` Duran, Leo
2017-01-07 0:49 ` Jordan Justen
0 siblings, 1 reply; 23+ messages in thread
From: Duran, Leo @ 2017-01-06 15:23 UTC (permalink / raw)
To: 'Laszlo Ersek', Gao, Liming, edk2-devel@lists.01.org
Cc: Singh, Brijesh, Justen, Jordan L, Fan, Jeff, Kinney, Michael D,
Ma, Maurice, Agyeman, Prince, Ni, Ruiyu, Steele, Kelly,
Wei, David, Guo, Mang
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, January 06, 2017 5:12 AM
> To: Gao, Liming <liming.gao@intel.com>; Duran, Leo <leo.duran@amd.com>;
> edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
>
> On 01/06/17 07:02, Gao, Liming wrote:
> > Leo:
> > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how about
> > add new APIs into IoLib together with other Io APIs? If so, no new
> > library class is required. Platform DSC files are not required to be
> > changed.
>
> But then all of the IoLib instances will have to be extended too:
>
> IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
> MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
> MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
>
> Thanks,
> Laszlo
>
[Duran, Leo] Correct.
As I mentioned, one of the reasons for the new IoFifo library is to be able to override it without having to duplicate the complete IoLib.
> >
> > Thanks
> > Liming
> >> -----Original Message-----
> >> From: Leo Duran [mailto:leo.duran@amd.com]
> >> Sent: Friday, January 06, 2017 5:49 AM
> >> To: edk2-devel@lists.01.org
> >> Cc: brijesh.singh@amd.com; lersek@redhat.com; Justen, Jordan L
> >> <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Kinney,
> >> Michael D <michael.d.kinney@intel.com>; Gao, Liming
> >> <liming.gao@intel.com>; Ma, Maurice <maurice.ma@intel.com>;
> Agyeman,
> >> Prince <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> >> Steele, Kelly <kelly.steele@intel.com>; Wei, David
> >> <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>; Leo Duran
> >> <leo.duran@amd.com>
> >> Subject: [PATCH v3 0/4] BaseIoFifoLib
> >>
> >> The UefiCpuPkg/CpuIo2Dxe driver and the QemuCfgLib library have
> >> duplicate implementations of I/O Fifo routines. The patch series
> >> moves the I/O Fifo routines into a common BaseIofifoLib library
> >> supporting IA32 and X64 architectures under MdePkg.
> >>
> >> The intent of this patch series is twofold:
> >> 1) Consolidate I/O Fifo routines into a common BaseIofifoLib library.
> >> 2) Allow override of BaseIofifoLib for specific platform implementations.
> >> For example, the OVMF package can provide its own version of
> >> BaseIoFifoLib to support Secure Encrypted Virtualization (SEV)
> >> guests, since SEV does not support string I/O instructions (rep
> >> ins/outs), and requires unrolled loops of single in/out instructions.
> >>
> >> Brijesh Singh (4):
> >> MdePkg: Add BaseIoFifoLib library
> >> Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
> >> Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library.
> >> Modify QemuFwCfgLib to use new BaseIoFifoLib library.
> >>
> >> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 +
> >> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +
> >> DuetPkg/DuetPkgIa32.dsc | 2 +
> >> DuetPkg/DuetPkgX64.dsc | 2 +
> >> MdePkg/Include/Library/IoFifoLib.h | 176 +++++++++++++
> >> MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf | 44 ++++
> >> .../Library/BaseIoFifoLib}/Ia32/IoFifo.asm | 280 ++++++++++-----------
> >> .../Library/BaseIoFifoLib}/Ia32/IoFifo.nasm | 272 ++++++++++----------
> >> .../Library/BaseIoFifoLib}/X64/IoFifo.asm | 252 +++++++++----------
> >> .../Library/BaseIoFifoLib}/X64/IoFifo.nasm | 250 +++++++++---------
> >> MdePkg/MdePkg.dec | 4 +
> >> MdePkg/MdePkg.dsc | 2 +
> >> OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm | 55 ----
> >> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 54 +---
> >> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 8 +-
> >> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf | 8 +-
> >> OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm | 52 ----
> >> OvmfPkg/OvmfPkgIa32.dsc | 2 +
> >> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +
> >> OvmfPkg/OvmfPkgX64.dsc | 2 +
> >> QuarkPlatformPkg/Quark.dsc | 2 +
> >> QuarkPlatformPkg/QuarkMin.dsc | 2 +
> >> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c | 3 +-
> >> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h | 3 +
> >> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf | 12 +-
> >> UefiCpuPkg/UefiCpuPkg.dsc | 2 +
> >> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 +
> >> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 2 +
> >> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 2 +
> >> 29 files changed, 794 insertions(+), 709 deletions(-) create mode
> >> 100644 MdePkg/Include/Library/IoFifoLib.h
> >> create mode 100644 MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf
> >> rename {UefiCpuPkg/CpuIo2Dxe =>
> >> MdePkg/Library/BaseIoFifoLib}/Ia32/IoFifo.asm (96%) rename
> >> {UefiCpuPkg/CpuIo2Dxe =>
> >> MdePkg/Library/BaseIoFifoLib}/Ia32/IoFifo.nasm (96%) rename
> >> {UefiCpuPkg/CpuIo2Dxe =>
> MdePkg/Library/BaseIoFifoLib}/X64/IoFifo.asm
> >> (97%) rename {UefiCpuPkg/CpuIo2Dxe =>
> >> MdePkg/Library/BaseIoFifoLib}/X64/IoFifo.nasm (97%) delete mode
> >> 100644 OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> >> delete mode 100644
> OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
> >>
> >> --
> >> 1.9.1
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/4] Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
2017-01-06 11:23 ` Laszlo Ersek
@ 2017-01-06 15:28 ` Duran, Leo
0 siblings, 0 replies; 23+ messages in thread
From: Duran, Leo @ 2017-01-06 15:28 UTC (permalink / raw)
To: 'Laszlo Ersek', edk2-devel@ml01.01.org
Cc: ruiyu.ni@intel.com, Singh, Brijesh, jordan.l.justen@intel.com,
liming.gao@intel.com, michael.d.kinney@intel.com,
jeff.fan@intel.com, prince.agyeman@intel.com, david.wei@intel.com
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, January 06, 2017 5:24 AM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
> Cc: ruiyu.ni@intel.com; Singh, Brijesh <brijesh.singh@amd.com>;
> jordan.l.justen@intel.com; liming.gao@intel.com;
> michael.d.kinney@intel.com; jeff.fan@intel.com;
> prince.agyeman@intel.com; david.wei@intel.com
> Subject: Re: [edk2] [PATCH v3 2/4] Modify .DSC files that include
> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>
> On 01/05/17 22:49, Leo Duran wrote:
> > From: Brijesh Singh <brijesh.singh@amd.com>
> >
> > This patch adds the new BaseIoFifoLib (IoFifoLib class) library
> > consumed by the UefiCpuPkg/CpuIo2Dxe driver.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> > Signed-off-by: Leo Duran <leo.duran@amd.com>
> > ---
> > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 +++
> > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +++
> > DuetPkg/DuetPkgIa32.dsc | 2 ++
> > DuetPkg/DuetPkgX64.dsc | 2 ++
> > OvmfPkg/OvmfPkgIa32.dsc | 2 ++
> > OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
> > OvmfPkg/OvmfPkgX64.dsc | 2 ++
> > QuarkPlatformPkg/Quark.dsc | 2 ++
> > QuarkPlatformPkg/QuarkMin.dsc | 2 ++
> > UefiCpuPkg/UefiCpuPkg.dsc | 2 ++
> > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 ++
> > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 2 ++
> > Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 2 ++
> > 13 files changed, 28 insertions(+)
>
> The OvmfPkg hunks look good to me, but I believe you missed my request
> that the patch be split per top level Pkg directory (that is, into 6 patchets).
[Duran, Leo]
Yes, I did miss that... I'll send out a v4. Thanks.
>
> https://lists.01.org/pipermail/edk2-devel/2017-January/006193.html
>
> (I do see you implemented the xfuncname configuration; thanks for that!)
>
> Thanks!
> Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/4] Modify QemuFwCfgLib to use new BaseIoFifoLib library.
2017-01-06 11:36 ` Laszlo Ersek
@ 2017-01-06 15:31 ` Duran, Leo
0 siblings, 0 replies; 23+ messages in thread
From: Duran, Leo @ 2017-01-06 15:31 UTC (permalink / raw)
To: 'Laszlo Ersek', edk2-devel@ml01.01.org
Cc: ruiyu.ni@intel.com, Singh, Brijesh, jordan.l.justen@intel.com,
liming.gao@intel.com, michael.d.kinney@intel.com,
jeff.fan@intel.com, prince.agyeman@intel.com, david.wei@intel.com
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, January 06, 2017 5:36 AM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
> Cc: ruiyu.ni@intel.com; Singh, Brijesh <brijesh.singh@amd.com>;
> jordan.l.justen@intel.com; liming.gao@intel.com;
> michael.d.kinney@intel.com; jeff.fan@intel.com;
> prince.agyeman@intel.com; david.wei@intel.com
> Subject: Re: [edk2] [PATCH v3 4/4] Modify QemuFwCfgLib to use new
> BaseIoFifoLib library.
>
> On 01/05/17 22:49, Leo Duran wrote:
> > From: Brijesh Singh <brijesh.singh@amd.com>
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> > Signed-off-by: Leo Duran <leo.duran@amd.com>
> > ---
> > OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm | 55 ----------------
> -------
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 54 +-----------------
> ----
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 8 +---
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf | 8 +---
> > OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm | 52
> > ---------------------
> > 5 files changed, 5 insertions(+), 172 deletions(-) delete mode
> > 100644 OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> > delete mode 100644
> OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
>
> Differences vs. v1:
> - git metadata & commit msg tags fixed, good
> - copyright notices added, okay
> - now unnecessary prototypes removed, good
> - QemuFwCfgSecLib.inf now handled (I didn't notice it in v1, sorry!), OK
>
> However, the #include <Library/IoFifoLib.h> directive is also gone, from
> "QemuFwCfgLib.c".
[Duran, Leo] OK, I'll fix that on the next revision. Thanks.
> I think you may have moved that directive to
> "QemuFwCfgLibInternal.h", but forgotten to stage that change for commit,
> with "git add"?
>
> (Anyway, what was wrong with the #include directive being in
> "QemuFwCfgLib.c"?)
>
> Apart from the #include directive, it looks good to me. (I know Liming is
> proposing a different avenue, and here I'm not arguing against that; just
> saying that under this approach, the patch seems mostly okay.)
>
> Thanks,
> Laszlo
>
>
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> > b/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> > deleted file mode 100644
> > index faa22e9..0000000
> > --- a/OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
> > +++ /dev/null
> > @@ -1,55 +0,0 @@
> > -;--------------------------------------------------------------------
> > ----------
> > -;
> > -; Copyright (c) 2006 - 2013, Intel Corporation. All rights
> > reserved.<BR> -; This program and the accompanying materials -; are
> > licensed and made available under the terms and conditions of the BSD
> > License -; which accompanies this distribution. The full text of the
> > license may be found at -; http://opensource.org/licenses/bsd-
> license.php.
> > -;
> > -; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS, -; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> > -;
> > -;--------------------------------------------------------------------
> > ----------
> > -
> > - SECTION .text
> > -
> > -;--------------------------------------------------------------------
> > ----------
> > -; VOID
> > -; EFIAPI
> > -; IoReadFifo8 (
> > -; IN UINTN Port,
> > -; IN UINTN Size,
> > -; IN VOID *Buffer
> > -; );
> > -;--------------------------------------------------------------------
> > ----------
> > -global ASM_PFX(IoReadFifo8)
> > -ASM_PFX(IoReadFifo8):
> > -
> > - mov dx, [esp + 4]
> > - mov ecx, [esp + 8]
> > - push edi
> > - mov edi, [esp + 16]
> > -rep insb
> > - pop edi
> > - ret
> > -
> > -;--------------------------------------------------------------------
> > ----------
> > -; VOID
> > -; EFIAPI
> > -; IoWriteFifo8 (
> > -; IN UINTN Port,
> > -; IN UINTN Size,
> > -; IN VOID *Buffer
> > -; );
> > -;--------------------------------------------------------------------
> > ----------
> > -global ASM_PFX(IoWriteFifo8)
> > -ASM_PFX(IoWriteFifo8):
> > -
> > - mov dx, [esp + 4]
> > - mov ecx, [esp + 8]
> > - push esi
> > - mov esi, [esp + 16]
> > -rep outsb
> > - pop esi
> > - ret
> > -
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> > b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> > index 0bbf121..68fbade 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> > @@ -2,6 +2,7 @@
> >
> > Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
> > Copyright (C) 2013, Red Hat, Inc.
> > + Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> >
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of
> > the BSD License @@ -26,59 +27,6 @@
> >
> >
> > /**
> > - Reads an 8-bit I/O port fifo into a block of memory.
> > -
> > - Reads the 8-bit I/O fifo port specified by Port.
> > -
> > - The port is read Count times, and the read data is
> > - stored in the provided Buffer.
> > -
> > - This function must guarantee that all I/O read and write operations
> > are
> > - serialized.
> > -
> > - If 8-bit I/O port operations are not supported, then ASSERT().
> > -
> > - @param Port The I/O port to read.
> > - @param Count The number of times to read I/O port.
> > - @param Buffer The buffer to store the read data into.
> > -
> > -**/
> > -VOID
> > -EFIAPI
> > -IoReadFifo8 (
> > - IN UINTN Port,
> > - IN UINTN Count,
> > - OUT VOID *Buffer
> > - );
> > -
> > -/**
> > - Writes an 8-bit I/O port fifo from a block of memory.
> > -
> > - Writes the 8-bit I/O fifo port specified by Port.
> > -
> > - The port is written Count times, and the data are obtained
> > - from the provided Buffer.
> > -
> > - This function must guarantee that all I/O read and write operations
> > are
> > - serialized.
> > -
> > - If 8-bit I/O port operations are not supported, then ASSERT().
> > -
> > - @param Port The I/O port to read.
> > - @param Count The number of times to read I/O port.
> > - @param Buffer The buffer to store the read data into.
> > -
> > -**/
> > -VOID
> > -EFIAPI
> > -IoWriteFifo8 (
> > - IN UINTN Port,
> > - IN UINTN Count,
> > - OUT VOID *Buffer
> > - );
> > -
> > -
> > -/**
> > Selects a firmware configuration item for reading.
> >
> > Following this call, any data read from this item will start from
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> > b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> > index 66ac778..e48c639 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> > @@ -4,6 +4,7 @@
> > #
> > # Copyright (C) 2013, Red Hat, Inc.
> > # Copyright (c) 2008 - 2012, Intel Corporation. All rights
> > reserved.<BR>
> > +# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> > #
> > # This program and the accompanying materials # are licensed and
> > made available under the terms and conditions of the BSD License @@
> > -36,12 +37,6 @@ [Sources]
> > QemuFwCfgLib.c
> > QemuFwCfgPeiDxe.c
> >
> > -[Sources.IA32]
> > - Ia32/IoLibExAsm.nasm
> > -
> > -[Sources.X64]
> > - X64/IoLibExAsm.nasm
> > -
> > [Packages]
> > MdePkg/MdePkg.dec
> > OvmfPkg/OvmfPkg.dec
> > @@ -51,5 +46,6 @@ [LibraryClasses]
> > BaseMemoryLib
> > DebugLib
> > IoLib
> > + IoFifoLib
> > MemoryAllocationLib
> >
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > index c1d6a54..6275ba9 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > @@ -4,6 +4,7 @@
> > #
> > # Copyright (C) 2013, Red Hat, Inc.
> > # Copyright (c) 2008 - 2012, Intel Corporation. All rights
> > reserved.<BR>
> > +# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> > #
> > # This program and the accompanying materials # are licensed and
> > made available under the terms and conditions of the BSD License @@
> > -34,12 +35,6 @@ [Sources]
> > QemuFwCfgLib.c
> > QemuFwCfgSec.c
> >
> > -[Sources.IA32]
> > - Ia32/IoLibExAsm.nasm
> > -
> > -[Sources.X64]
> > - X64/IoLibExAsm.nasm
> > -
> > [Packages]
> > MdePkg/MdePkg.dec
> > OvmfPkg/OvmfPkg.dec
> > @@ -49,5 +44,6 @@ [LibraryClasses]
> > BaseMemoryLib
> > DebugLib
> > IoLib
> > + IoFifoLib
> > MemoryAllocationLib
> >
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
> > b/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
> > deleted file mode 100644
> > index f1078f2..0000000
> > --- a/OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
> > +++ /dev/null
> > @@ -1,52 +0,0 @@
> > -;--------------------------------------------------------------------
> > ----------
> > -;
> > -; Copyright (c) 2006 - 2013, Intel Corporation. All rights
> > reserved.<BR> -; This program and the accompanying materials -; are
> > licensed and made available under the terms and conditions of the BSD
> > License -; which accompanies this distribution. The full text of the
> > license may be found at -; http://opensource.org/licenses/bsd-
> license.php.
> > -;
> > -; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS, -; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> > -;
> > -;--------------------------------------------------------------------
> > ----------
> > -
> > - DEFAULT REL
> > - SECTION .text
> > -
> > -;--------------------------------------------------------------------
> > ----------
> > -; VOID
> > -; EFIAPI
> > -; IoReadFifo8 (
> > -; IN UINTN Port, // rcx
> > -; IN UINTN Size, // rdx
> > -; IN VOID *Buffer // r8
> > -; );
> > -;--------------------------------------------------------------------
> > ----------
> > -global ASM_PFX(IoReadFifo8)
> > -ASM_PFX(IoReadFifo8):
> > -
> > - xchg rcx, rdx
> > - xchg rdi, r8 ; rdi: buffer address; r8: save rdi
> > -rep insb
> > - mov rdi, r8 ; restore rdi
> > - ret
> > -
> > -;--------------------------------------------------------------------
> > ----------
> > -; VOID
> > -; EFIAPI
> > -; IoWriteFifo8 (
> > -; IN UINTN Port, // rcx
> > -; IN UINTN Size, // rdx
> > -; IN VOID *Buffer // r8
> > -; );
> > -;--------------------------------------------------------------------
> > ----------
> > -global ASM_PFX(IoWriteFifo8)
> > -ASM_PFX(IoWriteFifo8):
> > -
> > - xchg rcx, rdx
> > - xchg rsi, r8 ; rdi: buffer address; r8: save rdi
> > -rep outsb
> > - mov rsi, r8 ; restore rdi
> > - ret
> > -
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-06 15:23 ` Duran, Leo
@ 2017-01-07 0:49 ` Jordan Justen
2017-01-07 17:16 ` Duran, Leo
0 siblings, 1 reply; 23+ messages in thread
From: Jordan Justen @ 2017-01-07 0:49 UTC (permalink / raw)
To: Duran, Leo, 'Laszlo Ersek', Gao, Liming,
edk2-devel@lists.01.org
Cc: Singh, Brijesh, Fan, Jeff, Kinney, Michael D, Ma, Maurice,
Agyeman, Prince, Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
On 2017-01-06 07:23:47, Duran, Leo wrote:
>
>
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Friday, January 06, 2017 5:12 AM
> > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo <leo.duran@amd.com>;
> > edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
> > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Kinney,
> > Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> > <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> > David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> > Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
> >
> > On 01/06/17 07:02, Gao, Liming wrote:
> > > Leo:
> > > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how about
> > > add new APIs into IoLib together with other Io APIs? If so, no new
> > > library class is required. Platform DSC files are not required to be
> > > changed.
> >
> > But then all of the IoLib instances will have to be extended too:
> >
> > IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
> > MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
> > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
> >
> > Thanks,
> > Laszlo
> >
> [Duran, Leo] Correct.
> As I mentioned, one of the reasons for the new IoFifo library is to
> be able to override it without having to duplicate the complete
> IoLib.
>
I agree with Liming about adding the functions to IoLib instead.
Perhaps a PCD could be added to control if rep i/o instructions are
used.
-Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-07 0:49 ` Jordan Justen
@ 2017-01-07 17:16 ` Duran, Leo
2017-01-09 3:10 ` Gao, Liming
0 siblings, 1 reply; 23+ messages in thread
From: Duran, Leo @ 2017-01-07 17:16 UTC (permalink / raw)
To: Jordan Justen, 'Laszlo Ersek', Gao, Liming,
edk2-devel@lists.01.org
Cc: Singh, Brijesh, Fan, Jeff, Kinney, Michael D, Ma, Maurice,
Agyeman, Prince, Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
Jordan, Liming, et al,
It turns out that the runtime enablement of SEV feature that I referred to can be detected in hardware;
so instead of requiring 'driver' code to set a dynamic PCD, the override Fifo routines could do a runtime check like this:
// In override version of the Fifo library
fifo_foo()
{
If (SEV_Enabled()) {
// don't use REP ins/outs
} else {
// use REP ins/outs
}
}
In essence we already have a hardware-based dynamic PCD, so the idea is to leverage it.
And since we're interested in overriding just the Fifo routines, it would make better sense to keep them in a separate library (as proposed in the patch set).
Leo.
> -----Original Message-----
> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
> Sent: Friday, January 06, 2017 6:50 PM
> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek' <lersek@redhat.com>;
> Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>
> On 2017-01-06 07:23:47, Duran, Leo wrote:
> >
> >
> > > -----Original Message-----
> > > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > > Sent: Friday, January 06, 2017 5:12 AM
> > > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
> > > <leo.duran@amd.com>; edk2-devel@lists.01.org
> > > <edk2-devel@ml01.01.org>
> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> > > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Kinney,
> > > Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> > > <maurice.ma@intel.com>; Agyeman, Prince
> <prince.agyeman@intel.com>;
> > > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
> > > <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> > > Mang <mang.guo@intel.com>
> > > Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
> > >
> > > On 01/06/17 07:02, Gao, Liming wrote:
> > > > Leo:
> > > > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how
> > > > about add new APIs into IoLib together with other Io APIs? If so,
> > > > no new library class is required. Platform DSC files are not
> > > > required to be changed.
> > >
> > > But then all of the IoLib instances will have to be extended too:
> > >
> > > IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
> > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> > > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
> > > MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
> > > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> > > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
> > >
> > > Thanks,
> > > Laszlo
> > >
> > [Duran, Leo] Correct.
> > As I mentioned, one of the reasons for the new IoFifo library is to be
> > able to override it without having to duplicate the complete IoLib.
> >
>
> I agree with Liming about adding the functions to IoLib instead.
>
> Perhaps a PCD could be added to control if rep i/o instructions are
> used.
>
> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-07 17:16 ` Duran, Leo
@ 2017-01-09 3:10 ` Gao, Liming
2017-01-09 14:22 ` Duran, Leo
0 siblings, 1 reply; 23+ messages in thread
From: Gao, Liming @ 2017-01-09 3:10 UTC (permalink / raw)
To: Duran, Leo, Justen, Jordan L, 'Laszlo Ersek',
edk2-devel@lists.01.org
Cc: Singh, Brijesh, Fan, Jeff, Kinney, Michael D, Ma, Maurice,
Agyeman, Prince, Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
Leo:
IoLib Library class is designed from the functionality, not code implementation. So, many IO operations are included in this library class. If developers want to use IO API, they only need to check IoLib library class. After add new APIs, we need to update all IoLib library instances to implement them. And, if any library API implementation has the different version, the full library instance will have to be copied to another instance. I know your concern is to duplicate the library implementation. But, I think this is the separate topic to optimize the library implementation and reuse the same source file. Other library instances may have the same issue. So, I suggest you submit bugzilla for this optimization request. We will figure out the solution and review it in this mail list.
Thanks
Liming
>-----Original Message-----
>From: Duran, Leo [mailto:leo.duran@amd.com]
>Sent: Sunday, January 08, 2017 1:17 AM
>To: Justen, Jordan L <jordan.l.justen@intel.com>; 'Laszlo Ersek'
><lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
>devel@lists.01.org
>Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
>Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
><maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>; Ni,
>Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
>David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
>Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>
>Jordan, Liming, et al,
>
>It turns out that the runtime enablement of SEV feature that I referred to can
>be detected in hardware;
>so instead of requiring 'driver' code to set a dynamic PCD, the override Fifo
>routines could do a runtime check like this:
>
>// In override version of the Fifo library
>fifo_foo()
>{
> If (SEV_Enabled()) {
> // don't use REP ins/outs
> } else {
> // use REP ins/outs
> }
>}
>In essence we already have a hardware-based dynamic PCD, so the idea is to
>leverage it.
>
>And since we're interested in overriding just the Fifo routines, it would make
>better sense to keep them in a separate library (as proposed in the patch set).
>Leo.
>
>> -----Original Message-----
>> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
>> Sent: Friday, January 06, 2017 6:50 PM
>> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek' <lersek@redhat.com>;
>> Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
>> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
>> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
>> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
>> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>>
>> On 2017-01-06 07:23:47, Duran, Leo wrote:
>> >
>> >
>> > > -----Original Message-----
>> > > From: Laszlo Ersek [mailto:lersek@redhat.com]
>> > > Sent: Friday, January 06, 2017 5:12 AM
>> > > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
>> > > <leo.duran@amd.com>; edk2-devel@lists.01.org
>> > > <edk2-devel@ml01.01.org>
>> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
>> > > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Kinney,
>> > > Michael D <michael.d.kinney@intel.com>; Ma, Maurice
>> > > <maurice.ma@intel.com>; Agyeman, Prince
>> <prince.agyeman@intel.com>;
>> > > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
>> > > <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
>> > > Mang <mang.guo@intel.com>
>> > > Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
>> > >
>> > > On 01/06/17 07:02, Gao, Liming wrote:
>> > > > Leo:
>> > > > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how
>> > > > about add new APIs into IoLib together with other Io APIs? If so,
>> > > > no new library class is required. Platform DSC files are not
>> > > > required to be changed.
>> > >
>> > > But then all of the IoLib instances will have to be extended too:
>> > >
>> > > IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
>> > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> > > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
>> > > MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
>> > > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
>> > > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
>> > >
>> > > Thanks,
>> > > Laszlo
>> > >
>> > [Duran, Leo] Correct.
>> > As I mentioned, one of the reasons for the new IoFifo library is to be
>> > able to override it without having to duplicate the complete IoLib.
>> >
>>
>> I agree with Liming about adding the functions to IoLib instead.
>>
>> Perhaps a PCD could be added to control if rep i/o instructions are
>> used.
>>
>> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-09 3:10 ` Gao, Liming
@ 2017-01-09 14:22 ` Duran, Leo
2017-01-09 14:30 ` Duran, Leo
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Duran, Leo @ 2017-01-09 14:22 UTC (permalink / raw)
To: 'Gao, Liming', Justen, Jordan L, 'Laszlo Ersek',
edk2-devel@lists.01.org
Cc: Singh, Brijesh, Fan, Jeff, Kinney, Michael D, Ma, Maurice,
Agyeman, Prince, Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
> -----Original Message-----
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Sunday, January 08, 2017 9:11 PM
> To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
> devel@lists.01.org
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>
> Leo:
> IoLib Library class is designed from the functionality, not code
> implementation. So, many IO operations are included in this library class. If
> developers want to use IO API, they only need to check IoLib library class.
> After add new APIs, we need to update all IoLib library instances to
> implement them. And, if any library API implementation has the different
> version, the full library instance will have to be copied to another instance. I
> know your concern is to duplicate the library implementation. But, I think this
> is the separate topic to optimize the library implementation and reuse the
> same source file. Other library instances may have the same issue. So, I
> suggest you submit bugzilla for this optimization request. We will figure out
> the solution and review it in this mail list.
>
> Thanks
> Liming
[Duran, Leo]
Hi Liming,
I'm not sure I follow what you mean by an 'optimization request'.
At present IoLIb does *not* include the Fifo routines that I've referred to, so I'm simply proposing to wrap the Fifo routines into in a library.
Moreover, as you just said, I’m also proposing not using IoLib to avoid having to duplicate all of the functionality in IoLib.
Can you please give me a bit more detail as to what the 'optimization request' would be?
(i.e., should that request read exactly as I've proposed so far, proposing the creation of an IoFifoLib?)
I'll submit Bugzilla once I better understand what needs to be in it.
Thanks,
Leo
> >-----Original Message-----
> >From: Duran, Leo [mailto:leo.duran@amd.com]
> >Sent: Sunday, January 08, 2017 1:17 AM
> >To: Justen, Jordan L <jordan.l.justen@intel.com>; 'Laszlo Ersek'
> ><lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
> >devel@lists.01.org
> >Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> ><jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> >Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> ><prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> >Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> >Mang <mang.guo@intel.com>
> >Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >
> >Jordan, Liming, et al,
> >
> >It turns out that the runtime enablement of SEV feature that I referred
> >to can be detected in hardware; so instead of requiring 'driver' code
> >to set a dynamic PCD, the override Fifo routines could do a runtime
> >check like this:
> >
> >// In override version of the Fifo library
> >fifo_foo()
> >{
> > If (SEV_Enabled()) {
> > // don't use REP ins/outs
> > } else {
> > // use REP ins/outs
> > }
> >}
> >In essence we already have a hardware-based dynamic PCD, so the idea is
> >to leverage it.
> >
> >And since we're interested in overriding just the Fifo routines, it
> >would make better sense to keep them in a separate library (as proposed in
> the patch set).
> >Leo.
> >
> >> -----Original Message-----
> >> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
> >> Sent: Friday, January 06, 2017 6:50 PM
> >> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek'
> >> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
> >> edk2-devel@lists.01.org
> >> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> >> <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> >> Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> >> <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> >> Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
> >> Guo, Mang <mang.guo@intel.com>
> >> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >>
> >> On 2017-01-06 07:23:47, Duran, Leo wrote:
> >> >
> >> >
> >> > > -----Original Message-----
> >> > > From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> > > Sent: Friday, January 06, 2017 5:12 AM
> >> > > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
> >> > > <leo.duran@amd.com>; edk2-devel@lists.01.org
> >> > > <edk2-devel@ml01.01.org>
> >> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> >> > > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
> >> > > Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> >> > > <maurice.ma@intel.com>; Agyeman, Prince
> >> <prince.agyeman@intel.com>;
> >> > > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
> >> > > <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> >> > > Mang <mang.guo@intel.com>
> >> > > Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
> >> > >
> >> > > On 01/06/17 07:02, Gao, Liming wrote:
> >> > > > Leo:
> >> > > > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how
> >> > > > about add new APIs into IoLib together with other Io APIs? If
> >> > > > so, no new library class is required. Platform DSC files are
> >> > > > not required to be changed.
> >> > >
> >> > > But then all of the IoLib instances will have to be extended too:
> >> > >
> >> > > IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
> >> > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> >> > > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
> >> > > MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
> >> > > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> >> > > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
> >> > >
> >> > > Thanks,
> >> > > Laszlo
> >> > >
> >> > [Duran, Leo] Correct.
> >> > As I mentioned, one of the reasons for the new IoFifo library is to
> >> > be able to override it without having to duplicate the complete IoLib.
> >> >
> >>
> >> I agree with Liming about adding the functions to IoLib instead.
> >>
> >> Perhaps a PCD could be added to control if rep i/o instructions are
> >> used.
> >>
> >> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-09 14:22 ` Duran, Leo
@ 2017-01-09 14:30 ` Duran, Leo
2017-01-09 14:36 ` Duran, Leo
2017-01-09 15:13 ` Laszlo Ersek
2017-01-10 5:33 ` Gao, Liming
2 siblings, 1 reply; 23+ messages in thread
From: Duran, Leo @ 2017-01-09 14:30 UTC (permalink / raw)
To: 'Gao, Liming', 'Justen, Jordan L',
'Laszlo Ersek', 'edk2-devel@lists.01.org'
Cc: Singh, Brijesh, 'Fan, Jeff', 'Kinney, Michael D',
'Ma, Maurice', 'Agyeman, Prince',
'Ni, Ruiyu', 'Steele, Kelly',
'Wei, David', 'Guo, Mang'
BTW,
I also you should have mentioned that the proposed IoFifoLib is intended to support just x86 (IA32 + X64)... Whereas IoLIb is universal.
Leo.
> -----Original Message-----
> From: Duran, Leo
> Sent: Monday, January 09, 2017 8:23 AM
> To: 'Gao, Liming' <liming.gao@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
> devel@lists.01.org
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>
>
> > -----Original Message-----
> > From: Gao, Liming [mailto:liming.gao@intel.com]
> > Sent: Sunday, January 08, 2017 9:11 PM
> > To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
> > <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
> > devel@lists.01.org
> > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> > <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> > Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> > <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> > Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> > Mang <mang.guo@intel.com>
> > Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >
> > Leo:
> > IoLib Library class is designed from the functionality, not code
> > implementation. So, many IO operations are included in this library
> > class. If developers want to use IO API, they only need to check IoLib library
> class.
> > After add new APIs, we need to update all IoLib library instances to
> > implement them. And, if any library API implementation has the
> > different version, the full library instance will have to be copied to
> > another instance. I know your concern is to duplicate the library
> > implementation. But, I think this is the separate topic to optimize
> > the library implementation and reuse the same source file. Other
> > library instances may have the same issue. So, I suggest you submit
> > bugzilla for this optimization request. We will figure out the solution and
> review it in this mail list.
> >
> > Thanks
> > Liming
> [Duran, Leo]
> Hi Liming,
>
> I'm not sure I follow what you mean by an 'optimization request'.
> At present IoLIb does *not* include the Fifo routines that I've referred to, so
> I'm simply proposing to wrap the Fifo routines into in a library.
> Moreover, as you just said, I’m also proposing not using IoLib to avoid having
> to duplicate all of the functionality in IoLib.
>
> Can you please give me a bit more detail as to what the 'optimization
> request' would be?
> (i.e., should that request read exactly as I've proposed so far, proposing the
> creation of an IoFifoLib?) I'll submit Bugzilla once I better understand what
> needs to be in it.
>
> Thanks,
> Leo
>
>
>
> > >-----Original Message-----
> > >From: Duran, Leo [mailto:leo.duran@amd.com]
> > >Sent: Sunday, January 08, 2017 1:17 AM
> > >To: Justen, Jordan L <jordan.l.justen@intel.com>; 'Laszlo Ersek'
> > ><lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
> > >devel@lists.01.org
> > >Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> > ><jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> > >Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> > ><prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> > >Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
> > >Guo, Mang <mang.guo@intel.com>
> > >Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> > >
> > >Jordan, Liming, et al,
> > >
> > >It turns out that the runtime enablement of SEV feature that I
> > >referred to can be detected in hardware; so instead of requiring
> > >'driver' code to set a dynamic PCD, the override Fifo routines could
> > >do a runtime check like this:
> > >
> > >// In override version of the Fifo library
> > >fifo_foo()
> > >{
> > > If (SEV_Enabled()) {
> > > // don't use REP ins/outs
> > > } else {
> > > // use REP ins/outs
> > > }
> > >}
> > >In essence we already have a hardware-based dynamic PCD, so the idea
> > >is to leverage it.
> > >
> > >And since we're interested in overriding just the Fifo routines, it
> > >would make better sense to keep them in a separate library (as
> > >proposed in
> > the patch set).
> > >Leo.
> > >
> > >> -----Original Message-----
> > >> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
> > >> Sent: Friday, January 06, 2017 6:50 PM
> > >> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek'
> > >> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
> > >> edk2-devel@lists.01.org
> > >> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> > >> <jeff.fan@intel.com>; Kinney, Michael D
> > >> <michael.d.kinney@intel.com>; Ma, Maurice <maurice.ma@intel.com>;
> > >> Agyeman, Prince <prince.agyeman@intel.com>; Ni, Ruiyu
> > >> <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> > >> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> > >> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> > >>
> > >> On 2017-01-06 07:23:47, Duran, Leo wrote:
> > >> >
> > >> >
> > >> > > -----Original Message-----
> > >> > > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > >> > > Sent: Friday, January 06, 2017 5:12 AM
> > >> > > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
> > >> > > <leo.duran@amd.com>; edk2-devel@lists.01.org
> > >> > > <edk2-devel@ml01.01.org>
> > >> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> > >> > > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
> > >> > > Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> > >> > > <maurice.ma@intel.com>; Agyeman, Prince
> > >> <prince.agyeman@intel.com>;
> > >> > > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
> > >> > > <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
> > >> > > Guo, Mang <mang.guo@intel.com>
> > >> > > Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
> > >> > >
> > >> > > On 01/06/17 07:02, Gao, Liming wrote:
> > >> > > > Leo:
> > >> > > > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So,
> > >> > > > how about add new APIs into IoLib together with other Io
> > >> > > > APIs? If so, no new library class is required. Platform DSC
> > >> > > > files are not required to be changed.
> > >> > >
> > >> > > But then all of the IoLib instances will have to be extended too:
> > >> > >
> > >> > > IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
> > >> > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> > >> > > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
> > >> > > MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
> > >> > > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> > >> > > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
> > >> > >
> > >> > > Thanks,
> > >> > > Laszlo
> > >> > >
> > >> > [Duran, Leo] Correct.
> > >> > As I mentioned, one of the reasons for the new IoFifo library is
> > >> > to be able to override it without having to duplicate the complete
> IoLib.
> > >> >
> > >>
> > >> I agree with Liming about adding the functions to IoLib instead.
> > >>
> > >> Perhaps a PCD could be added to control if rep i/o instructions are
> > >> used.
> > >>
> > >> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-09 14:30 ` Duran, Leo
@ 2017-01-09 14:36 ` Duran, Leo
0 siblings, 0 replies; 23+ messages in thread
From: Duran, Leo @ 2017-01-09 14:36 UTC (permalink / raw)
To: 'Gao, Liming', 'Justen, Jordan L',
'Laszlo Ersek', 'edk2-devel@lists.01.org'
Cc: Singh, Brijesh, 'Fan, Jeff', 'Kinney, Michael D',
'Ma, Maurice', 'Agyeman, Prince',
'Ni, Ruiyu', 'Steele, Kelly',
'Wei, David', 'Guo, Mang'
Please pardon the churn...
It just occurred to me that perhaps renaming the library may justify its standalone existence... How about if I call it: X86IoFifoLib?
(since the intended consumers are x86 packages)
Leo.
> -----Original Message-----
> From: Duran, Leo
> Sent: Monday, January 09, 2017 8:31 AM
> To: 'Gao, Liming' <liming.gao@intel.com>; 'Justen, Jordan L'
> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; 'edk2-
> devel@lists.01.org' <edk2-devel@lists.01.org>
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; 'Fan, Jeff'
> <jeff.fan@intel.com>; 'Kinney, Michael D' <michael.d.kinney@intel.com>;
> 'Ma, Maurice' <maurice.ma@intel.com>; 'Agyeman, Prince'
> <prince.agyeman@intel.com>; 'Ni, Ruiyu' <ruiyu.ni@intel.com>; 'Steele,
> Kelly' <kelly.steele@intel.com>; 'Wei, David' <david.wei@intel.com>; 'Guo,
> Mang' <mang.guo@intel.com>
> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>
> BTW,
> I also you should have mentioned that the proposed IoFifoLib is intended to
> support just x86 (IA32 + X64)... Whereas IoLIb is universal.
> Leo.
>
> > -----Original Message-----
> > From: Duran, Leo
> > Sent: Monday, January 09, 2017 8:23 AM
> > To: 'Gao, Liming' <liming.gao@intel.com>; Justen, Jordan L
> > <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
> > devel@lists.01.org
> > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> > <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> > Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> > <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> > Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> > Mang <mang.guo@intel.com>
> > Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >
> >
> > > -----Original Message-----
> > > From: Gao, Liming [mailto:liming.gao@intel.com]
> > > Sent: Sunday, January 08, 2017 9:11 PM
> > > To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
> > > <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>;
> > > edk2- devel@lists.01.org
> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> > > <jeff.fan@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>; Ma, Maurice <maurice.ma@intel.com>;
> > > Agyeman, Prince <prince.agyeman@intel.com>; Ni, Ruiyu
> > > <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> > > David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> > > Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> > >
> > > Leo:
> > > IoLib Library class is designed from the functionality, not code
> > > implementation. So, many IO operations are included in this library
> > > class. If developers want to use IO API, they only need to check
> > > IoLib library
> > class.
> > > After add new APIs, we need to update all IoLib library instances to
> > > implement them. And, if any library API implementation has the
> > > different version, the full library instance will have to be copied
> > > to another instance. I know your concern is to duplicate the library
> > > implementation. But, I think this is the separate topic to optimize
> > > the library implementation and reuse the same source file. Other
> > > library instances may have the same issue. So, I suggest you submit
> > > bugzilla for this optimization request. We will figure out the
> > > solution and
> > review it in this mail list.
> > >
> > > Thanks
> > > Liming
> > [Duran, Leo]
> > Hi Liming,
> >
> > I'm not sure I follow what you mean by an 'optimization request'.
> > At present IoLIb does *not* include the Fifo routines that I've
> > referred to, so I'm simply proposing to wrap the Fifo routines into in a
> library.
> > Moreover, as you just said, I’m also proposing not using IoLib to
> > avoid having to duplicate all of the functionality in IoLib.
> >
> > Can you please give me a bit more detail as to what the 'optimization
> > request' would be?
> > (i.e., should that request read exactly as I've proposed so far,
> > proposing the creation of an IoFifoLib?) I'll submit Bugzilla once I
> > better understand what needs to be in it.
> >
> > Thanks,
> > Leo
> >
> >
> >
> > > >-----Original Message-----
> > > >From: Duran, Leo [mailto:leo.duran@amd.com]
> > > >Sent: Sunday, January 08, 2017 1:17 AM
> > > >To: Justen, Jordan L <jordan.l.justen@intel.com>; 'Laszlo Ersek'
> > > ><lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
> > > >devel@lists.01.org
> > > >Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> > > ><jeff.fan@intel.com>; Kinney, Michael D
> > > ><michael.d.kinney@intel.com>; Ma, Maurice <maurice.ma@intel.com>;
> > > >Agyeman, Prince <prince.agyeman@intel.com>; Ni, Ruiyu
> > > ><ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> > > >David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> > > >Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> > > >
> > > >Jordan, Liming, et al,
> > > >
> > > >It turns out that the runtime enablement of SEV feature that I
> > > >referred to can be detected in hardware; so instead of requiring
> > > >'driver' code to set a dynamic PCD, the override Fifo routines
> > > >could do a runtime check like this:
> > > >
> > > >// In override version of the Fifo library
> > > >fifo_foo()
> > > >{
> > > > If (SEV_Enabled()) {
> > > > // don't use REP ins/outs
> > > > } else {
> > > > // use REP ins/outs
> > > > }
> > > >}
> > > >In essence we already have a hardware-based dynamic PCD, so the
> > > >idea is to leverage it.
> > > >
> > > >And since we're interested in overriding just the Fifo routines, it
> > > >would make better sense to keep them in a separate library (as
> > > >proposed in
> > > the patch set).
> > > >Leo.
> > > >
> > > >> -----Original Message-----
> > > >> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
> > > >> Sent: Friday, January 06, 2017 6:50 PM
> > > >> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek'
> > > >> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
> > > >> edk2-devel@lists.01.org
> > > >> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> > > >> <jeff.fan@intel.com>; Kinney, Michael D
> > > >> <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>;
> > > >> Agyeman, Prince <prince.agyeman@intel.com>; Ni, Ruiyu
> > > >> <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>;
> > > >> Wei, David <david.wei@intel.com>; Guo, Mang
> <mang.guo@intel.com>
> > > >> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> > > >>
> > > >> On 2017-01-06 07:23:47, Duran, Leo wrote:
> > > >> >
> > > >> >
> > > >> > > -----Original Message-----
> > > >> > > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > > >> > > Sent: Friday, January 06, 2017 5:12 AM
> > > >> > > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
> > > >> > > <leo.duran@amd.com>; edk2-devel@lists.01.org
> > > >> > > <edk2-devel@ml01.01.org>
> > > >> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> > > >> > > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
> > > >> > > Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> > > >> > > <maurice.ma@intel.com>; Agyeman, Prince
> > > >> <prince.agyeman@intel.com>;
> > > >> > > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
> > > >> > > <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
> > > >> > > Guo, Mang <mang.guo@intel.com>
> > > >> > > Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
> > > >> > >
> > > >> > > On 01/06/17 07:02, Gao, Liming wrote:
> > > >> > > > Leo:
> > > >> > > > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So,
> > > >> > > > how about add new APIs into IoLib together with other Io
> > > >> > > > APIs? If so, no new library class is required. Platform DSC
> > > >> > > > files are not required to be changed.
> > > >> > >
> > > >> > > But then all of the IoLib instances will have to be extended too:
> > > >> > >
> > > >> > > IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
> > > >> > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> > > >> > > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
> > > >> > > MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
> > > >> > > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> > > >> > > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
> > > >> > >
> > > >> > > Thanks,
> > > >> > > Laszlo
> > > >> > >
> > > >> > [Duran, Leo] Correct.
> > > >> > As I mentioned, one of the reasons for the new IoFifo library
> > > >> > is to be able to override it without having to duplicate the
> > > >> > complete
> > IoLib.
> > > >> >
> > > >>
> > > >> I agree with Liming about adding the functions to IoLib instead.
> > > >>
> > > >> Perhaps a PCD could be added to control if rep i/o instructions
> > > >> are used.
> > > >>
> > > >> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-09 14:22 ` Duran, Leo
2017-01-09 14:30 ` Duran, Leo
@ 2017-01-09 15:13 ` Laszlo Ersek
2017-01-09 16:37 ` Duran, Leo
2017-01-10 5:33 ` Gao, Liming
2 siblings, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2017-01-09 15:13 UTC (permalink / raw)
To: Duran, Leo, 'Gao, Liming', Justen, Jordan L,
edk2-devel@lists.01.org
Cc: Singh, Brijesh, Fan, Jeff, Kinney, Michael D, Ma, Maurice,
Agyeman, Prince, Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
On 01/09/17 15:22, Duran, Leo wrote:
>
>> -----Original Message-----
>> From: Gao, Liming [mailto:liming.gao@intel.com]
>> Sent: Sunday, January 08, 2017 9:11 PM
>> To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
>> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
>> devel@lists.01.org
>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
>> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
>> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
>> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
>> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>>
>> Leo:
>> IoLib Library class is designed from the functionality, not code
>> implementation. So, many IO operations are included in this library class. If
>> developers want to use IO API, they only need to check IoLib library class.
>> After add new APIs, we need to update all IoLib library instances to
>> implement them. And, if any library API implementation has the different
>> version, the full library instance will have to be copied to another instance. I
>> know your concern is to duplicate the library implementation. But, I think this
>> is the separate topic to optimize the library implementation and reuse the
>> same source file. Other library instances may have the same issue. So, I
>> suggest you submit bugzilla for this optimization request. We will figure out
>> the solution and review it in this mail list.
>>
>> Thanks
>> Liming
> [Duran, Leo]
> Hi Liming,
>
> I'm not sure I follow what you mean by an 'optimization request'.
I think under "optimization", Liming means eliminating code duplication
between IoLib library instances. That is, if I understand correctly,
Liming and Jordan suggest that you first add the new interfaces to the
IoLib class header, add (--> duplicate) the implementation to all
library instances, then file a BZ about eliminating code duplication
between the library instances.
This is my understanding anyway.
Thanks
Laszlo
> At present IoLIb does *not* include the Fifo routines that I've referred to, so I'm simply proposing to wrap the Fifo routines into in a library.
> Moreover, as you just said, I’m also proposing not using IoLib to avoid having to duplicate all of the functionality in IoLib.
>
> Can you please give me a bit more detail as to what the 'optimization request' would be?
> (i.e., should that request read exactly as I've proposed so far, proposing the creation of an IoFifoLib?)
> I'll submit Bugzilla once I better understand what needs to be in it.
>
> Thanks,
> Leo
>
>
>
>>> -----Original Message-----
>>> From: Duran, Leo [mailto:leo.duran@amd.com]
>>> Sent: Sunday, January 08, 2017 1:17 AM
>>> To: Justen, Jordan L <jordan.l.justen@intel.com>; 'Laszlo Ersek'
>>> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
>>> devel@lists.01.org
>>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
>>> <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>>> Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
>>> <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
>>> Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
>>> Mang <mang.guo@intel.com>
>>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>>>
>>> Jordan, Liming, et al,
>>>
>>> It turns out that the runtime enablement of SEV feature that I referred
>>> to can be detected in hardware; so instead of requiring 'driver' code
>>> to set a dynamic PCD, the override Fifo routines could do a runtime
>>> check like this:
>>>
>>> // In override version of the Fifo library
>>> fifo_foo()
>>> {
>>> If (SEV_Enabled()) {
>>> // don't use REP ins/outs
>>> } else {
>>> // use REP ins/outs
>>> }
>>> }
>>> In essence we already have a hardware-based dynamic PCD, so the idea is
>>> to leverage it.
>>>
>>> And since we're interested in overriding just the Fifo routines, it
>>> would make better sense to keep them in a separate library (as proposed in
>> the patch set).
>>> Leo.
>>>
>>>> -----Original Message-----
>>>> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
>>>> Sent: Friday, January 06, 2017 6:50 PM
>>>> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek'
>>>> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
>>>> edk2-devel@lists.01.org
>>>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
>>>> <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>>>> Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
>>>> <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
>>>> Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
>>>> Guo, Mang <mang.guo@intel.com>
>>>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>>>>
>>>> On 2017-01-06 07:23:47, Duran, Leo wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>>>>>> Sent: Friday, January 06, 2017 5:12 AM
>>>>>> To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
>>>>>> <leo.duran@amd.com>; edk2-devel@lists.01.org
>>>>>> <edk2-devel@ml01.01.org>
>>>>>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
>>>>>> <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
>>>>>> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
>>>>>> <maurice.ma@intel.com>; Agyeman, Prince
>>>> <prince.agyeman@intel.com>;
>>>>>> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
>>>>>> <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
>>>>>> Mang <mang.guo@intel.com>
>>>>>> Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
>>>>>>
>>>>>> On 01/06/17 07:02, Gao, Liming wrote:
>>>>>>> Leo:
>>>>>>> FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how
>>>>>>> about add new APIs into IoLib together with other Io APIs? If
>>>>>>> so, no new library class is required. Platform DSC files are
>>>>>>> not required to be changed.
>>>>>>
>>>>>> But then all of the IoLib instances will have to be extended too:
>>>>>>
>>>>>> IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
>>>>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>>>>>> MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
>>>>>> MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
>>>>>> MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
>>>>>> MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
>>>>>>
>>>>>> Thanks,
>>>>>> Laszlo
>>>>>>
>>>>> [Duran, Leo] Correct.
>>>>> As I mentioned, one of the reasons for the new IoFifo library is to
>>>>> be able to override it without having to duplicate the complete IoLib.
>>>>>
>>>>
>>>> I agree with Liming about adding the functions to IoLib instead.
>>>>
>>>> Perhaps a PCD could be added to control if rep i/o instructions are
>>>> used.
>>>>
>>>> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-09 15:13 ` Laszlo Ersek
@ 2017-01-09 16:37 ` Duran, Leo
2017-01-09 22:41 ` Jordan Justen
0 siblings, 1 reply; 23+ messages in thread
From: Duran, Leo @ 2017-01-09 16:37 UTC (permalink / raw)
To: 'Laszlo Ersek', 'Gao, Liming', Justen, Jordan L,
edk2-devel@lists.01.org
Cc: Singh, Brijesh, Fan, Jeff, Kinney, Michael D, Ma, Maurice,
Agyeman, Prince, Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, January 09, 2017 9:14 AM
> To: Duran, Leo <leo.duran@amd.com>; 'Gao, Liming'
> <liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
>
> On 01/09/17 15:22, Duran, Leo wrote:
> >
> >> -----Original Message-----
> >> From: Gao, Liming [mailto:liming.gao@intel.com]
> >> Sent: Sunday, January 08, 2017 9:11 PM
> >> To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
> >> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>;
> >> edk2- devel@lists.01.org
> >> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> >> <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> >> Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> >> <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> >> Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
> >> Guo, Mang <mang.guo@intel.com>
> >> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >>
> >> Leo:
> >> IoLib Library class is designed from the functionality, not code
> >> implementation. So, many IO operations are included in this library
> >> class. If developers want to use IO API, they only need to check IoLib
> library class.
> >> After add new APIs, we need to update all IoLib library instances to
> >> implement them. And, if any library API implementation has the
> >> different version, the full library instance will have to be copied
> >> to another instance. I know your concern is to duplicate the library
> >> implementation. But, I think this is the separate topic to optimize
> >> the library implementation and reuse the same source file. Other
> >> library instances may have the same issue. So, I suggest you submit
> >> bugzilla for this optimization request. We will figure out the solution and
> review it in this mail list.
> >>
> >> Thanks
> >> Liming
> > [Duran, Leo]
> > Hi Liming,
> >
> > I'm not sure I follow what you mean by an 'optimization request'.
>
> I think under "optimization", Liming means eliminating code duplication
> between IoLib library instances. That is, if I understand correctly, Liming and
> Jordan suggest that you first add the new interfaces to the IoLib class header,
> add (--> duplicate) the implementation to all library instances, then file a BZ
> about eliminating code duplication between the library instances.
>
> This is my understanding anyway.
>
> Thanks
> Laszlo
>
[Duran, Leo] Thanks Lazlo... Unfortunately the 'ask' is still not clear to me.
Liming, I would appreciate some clarification.
Thanks,
Leo
>
> > At present IoLIb does *not* include the Fifo routines that I've referred to,
> so I'm simply proposing to wrap the Fifo routines into in a library.
> > Moreover, as you just said, I’m also proposing not using IoLib to avoid
> having to duplicate all of the functionality in IoLib.
> >
> > Can you please give me a bit more detail as to what the 'optimization
> request' would be?
> > (i.e., should that request read exactly as I've proposed so far,
> > proposing the creation of an IoFifoLib?) I'll submit Bugzilla once I better
> understand what needs to be in it.
> >
> > Thanks,
> > Leo
> >
> >
> >
> >>> -----Original Message-----
> >>> From: Duran, Leo [mailto:leo.duran@amd.com]
> >>> Sent: Sunday, January 08, 2017 1:17 AM
> >>> To: Justen, Jordan L <jordan.l.justen@intel.com>; 'Laszlo Ersek'
> >>> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
> >>> devel@lists.01.org
> >>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> >>> <jeff.fan@intel.com>; Kinney, Michael D
> >>> <michael.d.kinney@intel.com>; Ma, Maurice <maurice.ma@intel.com>;
> >>> Agyeman, Prince <prince.agyeman@intel.com>; Ni, Ruiyu
> >>> <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> >>> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> >>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >>>
> >>> Jordan, Liming, et al,
> >>>
> >>> It turns out that the runtime enablement of SEV feature that I
> >>> referred to can be detected in hardware; so instead of requiring
> >>> 'driver' code to set a dynamic PCD, the override Fifo routines could
> >>> do a runtime check like this:
> >>>
> >>> // In override version of the Fifo library
> >>> fifo_foo()
> >>> {
> >>> If (SEV_Enabled()) {
> >>> // don't use REP ins/outs
> >>> } else {
> >>> // use REP ins/outs
> >>> }
> >>> }
> >>> In essence we already have a hardware-based dynamic PCD, so the idea
> >>> is to leverage it.
> >>>
> >>> And since we're interested in overriding just the Fifo routines, it
> >>> would make better sense to keep them in a separate library (as
> >>> proposed in
> >> the patch set).
> >>> Leo.
> >>>
> >>>> -----Original Message-----
> >>>> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
> >>>> Sent: Friday, January 06, 2017 6:50 PM
> >>>> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek'
> >>>> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
> >>>> edk2-devel@lists.01.org
> >>>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> >>>> <jeff.fan@intel.com>; Kinney, Michael D
> >>>> <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>;
> >>>> Agyeman, Prince <prince.agyeman@intel.com>; Ni, Ruiyu
> >>>> <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> >>>> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> >>>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >>>>
> >>>> On 2017-01-06 07:23:47, Duran, Leo wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >>>>>> Sent: Friday, January 06, 2017 5:12 AM
> >>>>>> To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
> >>>>>> <leo.duran@amd.com>; edk2-devel@lists.01.org
> >>>>>> <edk2-devel@ml01.01.org>
> >>>>>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> >>>>>> <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
> >>>>>> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> >>>>>> <maurice.ma@intel.com>; Agyeman, Prince
> >>>> <prince.agyeman@intel.com>;
> >>>>>> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
> >>>>>> <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> >>>>>> Mang <mang.guo@intel.com>
> >>>>>> Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
> >>>>>>
> >>>>>> On 01/06/17 07:02, Gao, Liming wrote:
> >>>>>>> Leo:
> >>>>>>> FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So,
> how
> >>>>>>> about add new APIs into IoLib together with other Io APIs? If
> >>>>>>> so, no new library class is required. Platform DSC files are not
> >>>>>>> required to be changed.
> >>>>>>
> >>>>>> But then all of the IoLib instances will have to be extended too:
> >>>>>>
> >>>>>> IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
> >>>>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> >>>>>> MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
> >>>>>> MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
> >>>>>> MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> >>>>>> MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Laszlo
> >>>>>>
> >>>>> [Duran, Leo] Correct.
> >>>>> As I mentioned, one of the reasons for the new IoFifo library is
> >>>>> to be able to override it without having to duplicate the complete
> IoLib.
> >>>>>
> >>>>
> >>>> I agree with Liming about adding the functions to IoLib instead.
> >>>>
> >>>> Perhaps a PCD could be added to control if rep i/o instructions are
> >>>> used.
> >>>>
> >>>> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-09 16:37 ` Duran, Leo
@ 2017-01-09 22:41 ` Jordan Justen
0 siblings, 0 replies; 23+ messages in thread
From: Jordan Justen @ 2017-01-09 22:41 UTC (permalink / raw)
To: Duran, Leo, 'Laszlo Ersek', 'Gao, Liming',
edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Singh, Brijesh, Kinney, Michael D, Fan, Jeff,
Agyeman, Prince, Wei, David
On 2017-01-09 08:37:38, Duran, Leo wrote:
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Monday, January 09, 2017 9:14 AM
> >
> > On 01/09/17 15:22, Duran, Leo wrote:
> > >
> > >> -----Original Message-----
> > >> From: Gao, Liming [mailto:liming.gao@intel.com]
> > >> Sent: Sunday, January 08, 2017 9:11 PM
> > >>
> > >> Leo:
> > >> IoLib Library class is designed from the functionality, not code
> > >> implementation. So, many IO operations are included in this library
> > >> class. If developers want to use IO API, they only need to check IoLib
> > library class.
> > >> After add new APIs, we need to update all IoLib library instances to
> > >> implement them. And, if any library API implementation has the
> > >> different version, the full library instance will have to be copied
> > >> to another instance. I know your concern is to duplicate the library
> > >> implementation. But, I think this is the separate topic to optimize
> > >> the library implementation and reuse the same source file. Other
> > >> library instances may have the same issue. So, I suggest you submit
> > >> bugzilla for this optimization request. We will figure out the solution and
> > review it in this mail list.
> > >>
> > >
> > > I'm not sure I follow what you mean by an 'optimization request'.
> >
> > I think under "optimization", Liming means eliminating code duplication
> > between IoLib library instances. That is, if I understand correctly, Liming and
> > Jordan suggest that you first add the new interfaces to the IoLib class header,
> > add (--> duplicate) the implementation to all library instances, then file a BZ
> > about eliminating code duplication between the library instances.
> >
> > This is my understanding anyway.
> >
> [Duran, Leo] Thanks Lazlo... Unfortunately the 'ask' is still not clear to me.
> Liming, I would appreciate some clarification.
Can you simplify the request down to what is needed, and not worry
about the 'IoFifoLib' implementation idea? Then put that into a
bugzilla feature request? I think that was Liming's request.
Would a reasonable example feature request be: "OVMF should detect
Secure Encrypted Virtualization mode, and if detected, it should not
use rep i/o instructions"
I think you should also try to provide details for how to detect SEV
mode in the feature request.
One situation that sounds familiar to me is
881813d7a93d9009c873515b043c41c4554779e4.
By the way, is this true? You simply cannot use those instructions?
It's not a matter of them being really slow, but they just don't work
at all?
-Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-09 14:22 ` Duran, Leo
2017-01-09 14:30 ` Duran, Leo
2017-01-09 15:13 ` Laszlo Ersek
@ 2017-01-10 5:33 ` Gao, Liming
2017-01-10 5:48 ` Duran, Leo
2 siblings, 1 reply; 23+ messages in thread
From: Gao, Liming @ 2017-01-10 5:33 UTC (permalink / raw)
To: 'Duran, Leo', Justen, Jordan L, 'Laszlo Ersek',
edk2-devel@lists.01.org
Cc: Singh, Brijesh, Fan, Jeff, Kinney, Michael D, Ma, Maurice,
Agyeman, Prince, Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
Leo:
I suggest to add new IoFifo APIs into IoLib library class, not add IoFifoLib class. After add IoFifo APIs into IoLib library class, we have to update all IoLib library instances to include IoFifo API implementation. For IA32 and X64 arch, your proposed implementation will be used. For other arch, normal implementation (read same IO port in one loop) can be used. And, you say you want to implement another version IoFifo APIs. If it is requested, I suggest you add new IoLib instance. For IoFifo APIs, your provide new implementation. For other Io APIs, you can copy the implementation from BaseIoLibIntrinsic library instance, then new IoLib instance can provide full IO APIs. That's my proposal for IoFifo APIs.
Besides, I raise one generic problem. In edk2 project, every library instance must provide the full API implementation. If one library instance only overrides some API implementation, it will have to copy other API implementation. It will bring the duplicated code in the different library instance. Current six Io library instances are just a case. I think we can figure out new solution to share the same logic between the different library instance. If this solution is ready, you can easily add new IoLib library instance with the override APIs only. So, I suggest you submit new request to support the library instance inherit from another instance.
Thanks
Liming
>-----Original Message-----
>From: Duran, Leo [mailto:leo.duran@amd.com]
>Sent: Monday, January 09, 2017 10:23 PM
>To: Gao, Liming <liming.gao@intel.com>; Justen, Jordan L
><jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
>devel@lists.01.org
>Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
>Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
><maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>; Ni,
>Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
>David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
>Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>
>
>> -----Original Message-----
>> From: Gao, Liming [mailto:liming.gao@intel.com]
>> Sent: Sunday, January 08, 2017 9:11 PM
>> To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
>> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
>> devel@lists.01.org
>> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
>> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
>> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
>> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
>> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>>
>> Leo:
>> IoLib Library class is designed from the functionality, not code
>> implementation. So, many IO operations are included in this library class. If
>> developers want to use IO API, they only need to check IoLib library class.
>> After add new APIs, we need to update all IoLib library instances to
>> implement them. And, if any library API implementation has the different
>> version, the full library instance will have to be copied to another instance. I
>> know your concern is to duplicate the library implementation. But, I think
>this
>> is the separate topic to optimize the library implementation and reuse the
>> same source file. Other library instances may have the same issue. So, I
>> suggest you submit bugzilla for this optimization request. We will figure out
>> the solution and review it in this mail list.
>>
>> Thanks
>> Liming
>[Duran, Leo]
>Hi Liming,
>
>I'm not sure I follow what you mean by an 'optimization request'.
>At present IoLIb does *not* include the Fifo routines that I've referred to, so
>I'm simply proposing to wrap the Fifo routines into in a library.
>Moreover, as you just said, I’m also proposing not using IoLib to avoid having
>to duplicate all of the functionality in IoLib.
>
>Can you please give me a bit more detail as to what the 'optimization request'
>would be?
>(i.e., should that request read exactly as I've proposed so far, proposing the
>creation of an IoFifoLib?)
>I'll submit Bugzilla once I better understand what needs to be in it.
>
>Thanks,
>Leo
>
>
>
>> >-----Original Message-----
>> >From: Duran, Leo [mailto:leo.duran@amd.com]
>> >Sent: Sunday, January 08, 2017 1:17 AM
>> >To: Justen, Jordan L <jordan.l.justen@intel.com>; 'Laszlo Ersek'
>> ><lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
>> >devel@lists.01.org
>> >Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
>> ><jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>> >Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
>> ><prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
>> >Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
>> >Mang <mang.guo@intel.com>
>> >Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>> >
>> >Jordan, Liming, et al,
>> >
>> >It turns out that the runtime enablement of SEV feature that I referred
>> >to can be detected in hardware; so instead of requiring 'driver' code
>> >to set a dynamic PCD, the override Fifo routines could do a runtime
>> >check like this:
>> >
>> >// In override version of the Fifo library
>> >fifo_foo()
>> >{
>> > If (SEV_Enabled()) {
>> > // don't use REP ins/outs
>> > } else {
>> > // use REP ins/outs
>> > }
>> >}
>> >In essence we already have a hardware-based dynamic PCD, so the idea is
>> >to leverage it.
>> >
>> >And since we're interested in overriding just the Fifo routines, it
>> >would make better sense to keep them in a separate library (as proposed
>in
>> the patch set).
>> >Leo.
>> >
>> >> -----Original Message-----
>> >> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
>> >> Sent: Friday, January 06, 2017 6:50 PM
>> >> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek'
>> >> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
>> >> edk2-devel@lists.01.org
>> >> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
>> >> <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>> >> Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
>> >> <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
>> >> Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
>> >> Guo, Mang <mang.guo@intel.com>
>> >> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>> >>
>> >> On 2017-01-06 07:23:47, Duran, Leo wrote:
>> >> >
>> >> >
>> >> > > -----Original Message-----
>> >> > > From: Laszlo Ersek [mailto:lersek@redhat.com]
>> >> > > Sent: Friday, January 06, 2017 5:12 AM
>> >> > > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
>> >> > > <leo.duran@amd.com>; edk2-devel@lists.01.org
>> >> > > <edk2-devel@ml01.01.org>
>> >> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
>> >> > > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
>> >> > > Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
>> >> > > <maurice.ma@intel.com>; Agyeman, Prince
>> >> <prince.agyeman@intel.com>;
>> >> > > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
>> >> > > <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
>> >> > > Mang <mang.guo@intel.com>
>> >> > > Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
>> >> > >
>> >> > > On 01/06/17 07:02, Gao, Liming wrote:
>> >> > > > Leo:
>> >> > > > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So,
>how
>> >> > > > about add new APIs into IoLib together with other Io APIs? If
>> >> > > > so, no new library class is required. Platform DSC files are
>> >> > > > not required to be changed.
>> >> > >
>> >> > > But then all of the IoLib instances will have to be extended too:
>> >> > >
>> >> > > IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
>> >> > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> >> > > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
>> >> > > MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
>> >> > > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
>> >> > > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
>> >> > >
>> >> > > Thanks,
>> >> > > Laszlo
>> >> > >
>> >> > [Duran, Leo] Correct.
>> >> > As I mentioned, one of the reasons for the new IoFifo library is to
>> >> > be able to override it without having to duplicate the complete IoLib.
>> >> >
>> >>
>> >> I agree with Liming about adding the functions to IoLib instead.
>> >>
>> >> Perhaps a PCD could be added to control if rep i/o instructions are
>> >> used.
>> >>
>> >> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/4] BaseIoFifoLib
2017-01-10 5:33 ` Gao, Liming
@ 2017-01-10 5:48 ` Duran, Leo
0 siblings, 0 replies; 23+ messages in thread
From: Duran, Leo @ 2017-01-10 5:48 UTC (permalink / raw)
To: Gao, Liming, Justen, Jordan L, 'Laszlo Ersek',
edk2-devel@lists.01.org
Cc: Singh, Brijesh, Fan, Jeff, Kinney, Michael D, Ma, Maurice,
Agyeman, Prince, Ni, Ruiyu, Steele, Kelly, Wei, David, Guo, Mang
OK Liming, I think I understand now.
Thanks,
Leo
> -----Original Message-----
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Monday, January 09, 2017 11:34 PM
> To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
> devel@lists.01.org
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
>
> Leo:
> I suggest to add new IoFifo APIs into IoLib library class, not add IoFifoLib
> class. After add IoFifo APIs into IoLib library class, we have to update all IoLib
> library instances to include IoFifo API implementation. For IA32 and X64 arch,
> your proposed implementation will be used. For other arch, normal
> implementation (read same IO port in one loop) can be used. And, you say
> you want to implement another version IoFifo APIs. If it is requested, I
> suggest you add new IoLib instance. For IoFifo APIs, your provide new
> implementation. For other Io APIs, you can copy the implementation from
> BaseIoLibIntrinsic library instance, then new IoLib instance can provide full IO
> APIs. That's my proposal for IoFifo APIs.
>
> Besides, I raise one generic problem. In edk2 project, every library instance
> must provide the full API implementation. If one library instance only
> overrides some API implementation, it will have to copy other API
> implementation. It will bring the duplicated code in the different library
> instance. Current six Io library instances are just a case. I think we can figure
> out new solution to share the same logic between the different library
> instance. If this solution is ready, you can easily add new IoLib library instance
> with the override APIs only. So, I suggest you submit new request to support
> the library instance inherit from another instance.
>
> Thanks
> Liming
> >-----Original Message-----
> >From: Duran, Leo [mailto:leo.duran@amd.com]
> >Sent: Monday, January 09, 2017 10:23 PM
> >To: Gao, Liming <liming.gao@intel.com>; Justen, Jordan L
> ><jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
> >devel@lists.01.org
> >Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> ><jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> >Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> ><prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> >Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> >Mang <mang.guo@intel.com>
> >Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >
> >
> >> -----Original Message-----
> >> From: Gao, Liming [mailto:liming.gao@intel.com]
> >> Sent: Sunday, January 08, 2017 9:11 PM
> >> To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
> >> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>;
> >> edk2- devel@lists.01.org
> >> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> >> <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> >> Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> >> <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> >> Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
> >> Guo, Mang <mang.guo@intel.com>
> >> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >>
> >> Leo:
> >> IoLib Library class is designed from the functionality, not code
> >> implementation. So, many IO operations are included in this library
> >> class. If developers want to use IO API, they only need to check IoLib
> library class.
> >> After add new APIs, we need to update all IoLib library instances to
> >> implement them. And, if any library API implementation has the
> >> different version, the full library instance will have to be copied
> >> to another instance. I know your concern is to duplicate the library
> >> implementation. But, I think
> >this
> >> is the separate topic to optimize the library implementation and
> >> reuse the same source file. Other library instances may have the same
> >> issue. So, I suggest you submit bugzilla for this optimization
> >> request. We will figure out the solution and review it in this mail list.
> >>
> >> Thanks
> >> Liming
> >[Duran, Leo]
> >Hi Liming,
> >
> >I'm not sure I follow what you mean by an 'optimization request'.
> >At present IoLIb does *not* include the Fifo routines that I've
> >referred to, so I'm simply proposing to wrap the Fifo routines into in a
> library.
> >Moreover, as you just said, I’m also proposing not using IoLib to avoid
> >having to duplicate all of the functionality in IoLib.
> >
> >Can you please give me a bit more detail as to what the 'optimization
> request'
> >would be?
> >(i.e., should that request read exactly as I've proposed so far,
> >proposing the creation of an IoFifoLib?) I'll submit Bugzilla once I
> >better understand what needs to be in it.
> >
> >Thanks,
> >Leo
> >
> >
> >
> >> >-----Original Message-----
> >> >From: Duran, Leo [mailto:leo.duran@amd.com]
> >> >Sent: Sunday, January 08, 2017 1:17 AM
> >> >To: Justen, Jordan L <jordan.l.justen@intel.com>; 'Laszlo Ersek'
> >> ><lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
> >> >devel@lists.01.org
> >> >Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> >> ><jeff.fan@intel.com>; Kinney, Michael D
> >> ><michael.d.kinney@intel.com>; Ma, Maurice <maurice.ma@intel.com>;
> >> >Agyeman, Prince <prince.agyeman@intel.com>; Ni, Ruiyu
> >> ><ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> >> >David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> >> >Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >> >
> >> >Jordan, Liming, et al,
> >> >
> >> >It turns out that the runtime enablement of SEV feature that I
> >> >referred to can be detected in hardware; so instead of requiring
> >> >'driver' code to set a dynamic PCD, the override Fifo routines could
> >> >do a runtime check like this:
> >> >
> >> >// In override version of the Fifo library
> >> >fifo_foo()
> >> >{
> >> > If (SEV_Enabled()) {
> >> > // don't use REP ins/outs
> >> > } else {
> >> > // use REP ins/outs
> >> > }
> >> >}
> >> >In essence we already have a hardware-based dynamic PCD, so the idea
> >> >is to leverage it.
> >> >
> >> >And since we're interested in overriding just the Fifo routines, it
> >> >would make better sense to keep them in a separate library (as
> >> >proposed
> >in
> >> the patch set).
> >> >Leo.
> >> >
> >> >> -----Original Message-----
> >> >> From: Jordan Justen [mailto:jordan.l.justen@intel.com]
> >> >> Sent: Friday, January 06, 2017 6:50 PM
> >> >> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek'
> >> >> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
> >> >> edk2-devel@lists.01.org
> >> >> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> >> >> <jeff.fan@intel.com>; Kinney, Michael D
> >> >> <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>;
> >> >> Agyeman, Prince <prince.agyeman@intel.com>; Ni, Ruiyu
> >> >> <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> >> >> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> >> >> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib
> >> >>
> >> >> On 2017-01-06 07:23:47, Duran, Leo wrote:
> >> >> >
> >> >> >
> >> >> > > -----Original Message-----
> >> >> > > From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> >> > > Sent: Friday, January 06, 2017 5:12 AM
> >> >> > > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
> >> >> > > <leo.duran@amd.com>; edk2-devel@lists.01.org
> >> >> > > <edk2-devel@ml01.01.org>
> >> >> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> >> >> > > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
> >> >> > > Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> >> >> > > <maurice.ma@intel.com>; Agyeman, Prince
> >> >> <prince.agyeman@intel.com>;
> >> >> > > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
> >> >> > > <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
> >> >> > > Guo, Mang <mang.guo@intel.com>
> >> >> > > Subject: Re: [PATCH v3 0/4] BaseIoFifoLib
> >> >> > >
> >> >> > > On 01/06/17 07:02, Gao, Liming wrote:
> >> >> > > > Leo:
> >> >> > > > FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So,
> >how
> >> >> > > > about add new APIs into IoLib together with other Io APIs?
> >> >> > > > If so, no new library class is required. Platform DSC files
> >> >> > > > are not required to be changed.
> >> >> > >
> >> >> > > But then all of the IoLib instances will have to be extended too:
> >> >> > >
> >> >> > > IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf
> >> >> > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> >> >> > > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
> >> >> > > MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf
> >> >> > > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> >> >> > > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
> >> >> > >
> >> >> > > Thanks,
> >> >> > > Laszlo
> >> >> > >
> >> >> > [Duran, Leo] Correct.
> >> >> > As I mentioned, one of the reasons for the new IoFifo library is
> >> >> > to be able to override it without having to duplicate the complete
> IoLib.
> >> >> >
> >> >>
> >> >> I agree with Liming about adding the functions to IoLib instead.
> >> >>
> >> >> Perhaps a PCD could be added to control if rep i/o instructions
> >> >> are used.
> >> >>
> >> >> -Jordan
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-01-10 5:49 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-05 21:49 [PATCH v3 0/4] BaseIoFifoLib Leo Duran
2017-01-05 21:49 ` [PATCH v3 1/4] MdePkg: Add BaseIoFifoLib library Leo Duran
2017-01-05 21:49 ` [PATCH v3 2/4] Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf Leo Duran
2017-01-06 11:23 ` Laszlo Ersek
2017-01-06 15:28 ` Duran, Leo
2017-01-05 21:49 ` [PATCH v3 3/4] Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library Leo Duran
2017-01-05 21:49 ` [PATCH v3 4/4] Modify QemuFwCfgLib " Leo Duran
2017-01-06 11:36 ` Laszlo Ersek
2017-01-06 15:31 ` Duran, Leo
2017-01-06 6:02 ` [PATCH v3 0/4] BaseIoFifoLib Gao, Liming
2017-01-06 11:12 ` Laszlo Ersek
2017-01-06 15:23 ` Duran, Leo
2017-01-07 0:49 ` Jordan Justen
2017-01-07 17:16 ` Duran, Leo
2017-01-09 3:10 ` Gao, Liming
2017-01-09 14:22 ` Duran, Leo
2017-01-09 14:30 ` Duran, Leo
2017-01-09 14:36 ` Duran, Leo
2017-01-09 15:13 ` Laszlo Ersek
2017-01-09 16:37 ` Duran, Leo
2017-01-09 22:41 ` Jordan Justen
2017-01-10 5:33 ` Gao, Liming
2017-01-10 5:48 ` Duran, Leo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox