| Message ID | 1460065944-28406-1-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/08/16 05:07, Ni, Ruiyu wrote: > Laszlo, > After I sent the mail to propose changing the CpuIo driver to improve IO performance, > I now saw your patch sent earlier than my last mail. A very interesting feeling. :) Right; after we more or less identified the problem, I dove into the core PciHostBridgeDxe driver, and saw that it relied on the CpuIo2 driver for the IO port access. I figured I could send a patch for that too. > Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com> Thanks! I'll send a new version with the .asm files as well, for the MSFT toolchains' sake, as Jeff (and before him, Jordan) suggested. Cheers Laszlo > > Regards, > Ray > > >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek >> Sent: Friday, April 8, 2016 5:52 AM >> To: edk2-devel-01 <edk2-devel@ml01.01.org> >> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Mark >> <kram321@gmail.com> >> Subject: [edk2] [PATCH] UefiCpuPkg: CpuIo2Dxe: optimize FIFO reads and writes of IO ports >> >> * Short description: >> >> The CpuIoServiceRead() and CpuIoServiceWrite() functions transfer data >> between memory and IO ports with individual Io(Read|Write)(8|16|32) >> function calls, each in an appropriately set up loop. >> >> On the Ia32 and X64 platforms however, FIFO reads and writes can be >> optimized, by coding them in assembly, and delegating the loop to the >> CPU, with the REP prefix. >> >> On KVM virtualization hosts, this difference has a huge performance >> impact: if the loop is open-coded, then the virtual machine traps to the >> hypervisor on every single UINT8 / UINT16 / UINT32 transfer, whereas >> with the REP prefix, KVM can transfer up to a page of data per VM trap. >> This is especially noticeable with IDE PIO transfers, where all the data >> are squeezed through IO ports. >> >> * Long description: >> >> The RootBridgeIoIoRW() function in >> >> PcAtChipsetPkg/PciHostBridgeDxe/PciRootBridgeIo.c >> >> used to have the exact same IO port acces optimization, dating back >> verbatim to commit 1fd376d9792: >> >> PcAtChipsetPkg/PciHostBridgeDxe: Improve KVM FIFO I/O read/write >> performance >> >> OvmfPkg cloned the "PcAtChipsetPkg/PciHostBridgeDxe" driver (for >> unrelated reasons), and inherited the optimization from PcAtChipsetPkg. >> >> The "PcAtChipsetPkg/PciHostBridgeDxe" driver was ultimately removed in >> commit 111d79db47: >> >> PcAtChipsetPkg/PciHostBridge: Remove PciHostBridge driver >> >> and OvmfPkg too was rebased to the new core Pci Host Bridge Driver, in >> commit 4014885ffd: >> >> OvmfPkg: switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe >> >> This caused the optimization to go lost. Namely, the >> RootBridgeIoIoRead() and RootBridgeIoIoWrite() functions in the new core >> Pci Host Bridge Driver delegate IO port accesses to >> EFI_CPU_IO2_PROTOCOL. And, in OvmfPkg (and likely most other Ia32 / X64 >> edk2 platforms), this protocol is provided by "UefiCpuPkg/CpuIo2Dxe", >> which lacks the optimization. >> >> Therefore, this patch ports the C source code logic from commit >> 1fd376d9792 (see above) to "UefiCpuPkg/CpuIo2Dxe", plus it ports the >> NASM-converted assembly helper functions from OvmfPkg commits >> 6026bf460037 and ace1d0517b65: >> >> OvmfPkg PciHostBridgeDxe: Convert Ia32/IoFifo.asm to NASM >> >> OvmfPkg PciHostBridgeDxe: Convert X64/IoFifo.asm to NASM >> >> * Notes about the port: >> >> - The write and read branches from commit 1fd376d9792 are split to the >> separate functions CpuIoServiceWrite() and CpuIoServiceRead(). >> >> - The EfiPciWidthUintXX constants are replaced with EfiCpuIoWidthUintXX. >> >> - The cast expression "(UINTN) Address" is replaced with >> "(UINTN)Address" (i.e., no space), because that's how the receiving >> functions spell it as well. >> >> - The labels in the switch statements are unindented by one level, to >> match the edk2 coding style (and the rest of UefiCpuPkg) better. >> >> * The first signoff belongs to Jordan, because he authored all of >> 1fd376d9792, 6026bf460037 and ace1d0517b65. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> Ref: https://www.redhat.com/archives/vfio-users/2016-April/msg00029.html >> Reported-by: Mark <kram321@gmail.com> >> Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/10424/focus=10432 >> Reported-by: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com> >> Cc: Jeff Fan <jeff.fan@intel.com> >> Cc: Mark <kram321@gmail.com> >> --- >> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf | 7 + >> UefiCpuPkg/CpuIo2Dxe/IoFifo.h | 176 ++++++++++++++++++++ >> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c | 49 ++++++ >> UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm | 136 +++++++++++++++ >> UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm | 125 ++++++++++++++ >> 5 files changed, 493 insertions(+) >> >> diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >> index 8ef8b3d31cff..be79b1b3b992 100644 >> --- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >> +++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >> @@ -30,7 +30,14 @@ [Defines] >> [Sources] >> CpuIo2Dxe.c >> CpuIo2Dxe.h >> + IoFifo.h >> >> +[Sources.IA32] >> + Ia32/IoFifo.nasm >> + >> +[Sources.X64] >> + X64/IoFifo.nasm >> + >> [Packages] >> MdePkg/MdePkg.dec >> >> diff --git a/UefiCpuPkg/CpuIo2Dxe/IoFifo.h b/UefiCpuPkg/CpuIo2Dxe/IoFifo.h >> new file mode 100644 >> index 000000000000..9978f8bfc39a >> --- /dev/null >> +++ b/UefiCpuPkg/CpuIo2Dxe/IoFifo.h >> @@ -0,0 +1,176 @@ >> +/** @file >> + I/O FIFO routines >> + >> + Copyright (c) 2008 - 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. >> + >> +**/ >> + >> +#ifndef _IO_FIFO_H_INCLUDED_ >> +#define _IO_FIFO_H_INCLUDED_ >> + >> +/** >> + 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/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c >> index 3d8a79923025..6ccfc40e10f8 100644 >> --- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c >> +++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c >> @@ -13,6 +13,7 @@ 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 >> @@ -410,6 +411,30 @@ CpuIoServiceRead ( >> InStride = mInStride[Width]; >> OutStride = mOutStride[Width]; >> OperationWidth = (EFI_CPU_IO_PROTOCOL_WIDTH) (Width & 0x03); >> + >> +#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64) >> + if (InStride == 0) { >> + switch (OperationWidth) { >> + case EfiCpuIoWidthUint8: >> + IoReadFifo8 ((UINTN)Address, Count, Buffer); >> + return EFI_SUCCESS; >> + case EfiCpuIoWidthUint16: >> + IoReadFifo16 ((UINTN)Address, Count, Buffer); >> + return EFI_SUCCESS; >> + case EfiCpuIoWidthUint32: >> + IoReadFifo32 ((UINTN)Address, Count, Buffer); >> + return EFI_SUCCESS; >> + default: >> + // >> + // The CpuIoCheckParameter call above will ensure that this >> + // path is not taken. >> + // >> + ASSERT (FALSE); >> + break; >> + } >> + } >> +#endif >> + >> for (Uint8Buffer = Buffer; Count > 0; Address += InStride, Uint8Buffer += OutStride, Count--) { >> if (OperationWidth == EfiCpuIoWidthUint8) { >> *Uint8Buffer = IoRead8 ((UINTN)Address); >> @@ -492,6 +517,30 @@ CpuIoServiceWrite ( >> InStride = mInStride[Width]; >> OutStride = mOutStride[Width]; >> OperationWidth = (EFI_CPU_IO_PROTOCOL_WIDTH) (Width & 0x03); >> + >> +#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64) >> + if (InStride == 0) { >> + switch (OperationWidth) { >> + case EfiCpuIoWidthUint8: >> + IoWriteFifo8 ((UINTN)Address, Count, Buffer); >> + return EFI_SUCCESS; >> + case EfiCpuIoWidthUint16: >> + IoWriteFifo16 ((UINTN)Address, Count, Buffer); >> + return EFI_SUCCESS; >> + case EfiCpuIoWidthUint32: >> + IoWriteFifo32 ((UINTN)Address, Count, Buffer); >> + return EFI_SUCCESS; >> + default: >> + // >> + // The CpuIoCheckParameter call above will ensure that this >> + // path is not taken. >> + // >> + ASSERT (FALSE); >> + break; >> + } >> + } >> +#endif >> + >> for (Uint8Buffer = (UINT8 *)Buffer; Count > 0; Address += InStride, Uint8Buffer += OutStride, Count--) { >> if (OperationWidth == EfiCpuIoWidthUint8) { >> IoWrite8 ((UINTN)Address, *Uint8Buffer); >> diff --git a/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm b/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm >> new file mode 100644 >> index 000000000000..daa90a99af37 >> --- /dev/null >> +++ b/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm >> @@ -0,0 +1,136 @@ >> +;------------------------------------------------------------------------------ >> +; >> +; 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.nasm b/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm >> new file mode 100644 >> index 000000000000..bb3d1da67af5 >> --- /dev/null >> +++ b/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm >> @@ -0,0 +1,125 @@ >> +;------------------------------------------------------------------------------ >> +; >> +; 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.8.3.1 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf index 8ef8b3d31cff..be79b1b3b992 100644 --- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf +++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf @@ -30,7 +30,14 @@ [Defines] [Sources] CpuIo2Dxe.c CpuIo2Dxe.h + IoFifo.h +[Sources.IA32] + Ia32/IoFifo.nasm + +[Sources.X64] + X64/IoFifo.nasm + [Packages] MdePkg/MdePkg.dec diff --git a/UefiCpuPkg/CpuIo2Dxe/IoFifo.h b/UefiCpuPkg/CpuIo2Dxe/IoFifo.h new file mode 100644 index 000000000000..9978f8bfc39a --- /dev/null +++ b/UefiCpuPkg/CpuIo2Dxe/IoFifo.h @@ -0,0 +1,176 @@ +/** @file + I/O FIFO routines + + Copyright (c) 2008 - 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. + +**/ + +#ifndef _IO_FIFO_H_INCLUDED_ +#define _IO_FIFO_H_INCLUDED_ + +/** + 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/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c index 3d8a79923025..6ccfc40e10f8 100644 --- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c +++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c @@ -13,6 +13,7 @@ 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 @@ -410,6 +411,30 @@ CpuIoServiceRead ( InStride = mInStride[Width]; OutStride = mOutStride[Width]; OperationWidth = (EFI_CPU_IO_PROTOCOL_WIDTH) (Width & 0x03); + +#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64) + if (InStride == 0) { + switch (OperationWidth) { + case EfiCpuIoWidthUint8: + IoReadFifo8 ((UINTN)Address, Count, Buffer); + return EFI_SUCCESS; + case EfiCpuIoWidthUint16: + IoReadFifo16 ((UINTN)Address, Count, Buffer); + return EFI_SUCCESS; + case EfiCpuIoWidthUint32: + IoReadFifo32 ((UINTN)Address, Count, Buffer); + return EFI_SUCCESS; + default: + // + // The CpuIoCheckParameter call above will ensure that this + // path is not taken. + // + ASSERT (FALSE); + break; + } + } +#endif + for (Uint8Buffer = Buffer; Count > 0; Address += InStride, Uint8Buffer += OutStride, Count--) { if (OperationWidth == EfiCpuIoWidthUint8) { *Uint8Buffer = IoRead8 ((UINTN)Address); @@ -492,6 +517,30 @@ CpuIoServiceWrite ( InStride = mInStride[Width]; OutStride = mOutStride[Width]; OperationWidth = (EFI_CPU_IO_PROTOCOL_WIDTH) (Width & 0x03); + +#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64) + if (InStride == 0) { + switch (OperationWidth) { + case EfiCpuIoWidthUint8: + IoWriteFifo8 ((UINTN)Address, Count, Buffer); + return EFI_SUCCESS; + case EfiCpuIoWidthUint16: + IoWriteFifo16 ((UINTN)Address, Count, Buffer); + return EFI_SUCCESS; + case EfiCpuIoWidthUint32: + IoWriteFifo32 ((UINTN)Address, Count, Buffer); + return EFI_SUCCESS; + default: + // + // The CpuIoCheckParameter call above will ensure that this + // path is not taken. + // + ASSERT (FALSE); + break; + } + } +#endif + for (Uint8Buffer = (UINT8 *)Buffer; Count > 0; Address += InStride, Uint8Buffer += OutStride, Count--) { if (OperationWidth == EfiCpuIoWidthUint8) { IoWrite8 ((UINTN)Address, *Uint8Buffer); diff --git a/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm b/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm new file mode 100644 index 000000000000..daa90a99af37 --- /dev/null +++ b/UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm @@ -0,0 +1,136 @@ +;------------------------------------------------------------------------------ +; +; 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.nasm b/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm new file mode 100644 index 000000000000..bb3d1da67af5 --- /dev/null +++ b/UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm @@ -0,0 +1,125 @@ +;------------------------------------------------------------------------------ +; +; 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 +
* Short description: The CpuIoServiceRead() and CpuIoServiceWrite() functions transfer data between memory and IO ports with individual Io(Read|Write)(8|16|32) function calls, each in an appropriately set up loop. On the Ia32 and X64 platforms however, FIFO reads and writes can be optimized, by coding them in assembly, and delegating the loop to the CPU, with the REP prefix. On KVM virtualization hosts, this difference has a huge performance impact: if the loop is open-coded, then the virtual machine traps to the hypervisor on every single UINT8 / UINT16 / UINT32 transfer, whereas with the REP prefix, KVM can transfer up to a page of data per VM trap. This is especially noticeable with IDE PIO transfers, where all the data are squeezed through IO ports. * Long description: The RootBridgeIoIoRW() function in PcAtChipsetPkg/PciHostBridgeDxe/PciRootBridgeIo.c used to have the exact same IO port acces optimization, dating back verbatim to commit 1fd376d9792: PcAtChipsetPkg/PciHostBridgeDxe: Improve KVM FIFO I/O read/write performance OvmfPkg cloned the "PcAtChipsetPkg/PciHostBridgeDxe" driver (for unrelated reasons), and inherited the optimization from PcAtChipsetPkg. The "PcAtChipsetPkg/PciHostBridgeDxe" driver was ultimately removed in commit 111d79db47: PcAtChipsetPkg/PciHostBridge: Remove PciHostBridge driver and OvmfPkg too was rebased to the new core Pci Host Bridge Driver, in commit 4014885ffd: OvmfPkg: switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe This caused the optimization to go lost. Namely, the RootBridgeIoIoRead() and RootBridgeIoIoWrite() functions in the new core Pci Host Bridge Driver delegate IO port accesses to EFI_CPU_IO2_PROTOCOL. And, in OvmfPkg (and likely most other Ia32 / X64 edk2 platforms), this protocol is provided by "UefiCpuPkg/CpuIo2Dxe", which lacks the optimization. Therefore, this patch ports the C source code logic from commit 1fd376d9792 (see above) to "UefiCpuPkg/CpuIo2Dxe", plus it ports the NASM-converted assembly helper functions from OvmfPkg commits 6026bf460037 and ace1d0517b65: OvmfPkg PciHostBridgeDxe: Convert Ia32/IoFifo.asm to NASM OvmfPkg PciHostBridgeDxe: Convert X64/IoFifo.asm to NASM * Notes about the port: - The write and read branches from commit 1fd376d9792 are split to the separate functions CpuIoServiceWrite() and CpuIoServiceRead(). - The EfiPciWidthUintXX constants are replaced with EfiCpuIoWidthUintXX. - The cast expression "(UINTN) Address" is replaced with "(UINTN)Address" (i.e., no space), because that's how the receiving functions spell it as well. - The labels in the switch statements are unindented by one level, to match the edk2 coding style (and the rest of UefiCpuPkg) better. * The first signoff belongs to Jordan, because he authored all of 1fd376d9792, 6026bf460037 and ace1d0517b65. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Ref: https://www.redhat.com/archives/vfio-users/2016-April/msg00029.html Reported-by: Mark <kram321@gmail.com> Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/10424/focus=10432 Reported-by: Jordan Justen <jordan.l.justen@intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jeff Fan <jeff.fan@intel.com> Cc: Mark <kram321@gmail.com> --- UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf | 7 + UefiCpuPkg/CpuIo2Dxe/IoFifo.h | 176 ++++++++++++++++++++ UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c | 49 ++++++ UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm | 136 +++++++++++++++ UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm | 125 ++++++++++++++ 5 files changed, 493 insertions(+) -- 1.8.3.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel