[edk2] [PATCH edk2-platforms 2/2] Silicon/SynQuacerI2cDxe: remove special runtime treatment of DEBUG()s
It is no longer necessary to inhibit DEBUG() calls at runtime, due to the fact that we switched to a DebugLib implementation that takes care of this. Since a platform could theoretically wire up DXE_RUNTIME_DRIVER modules to a DebugLib/SerialPortLib implementation combo that does remap the UART MMIO region for runtime and produces UEFI debug output on a UART that is not owned by the OS, it is better not to handle this in the driver directly. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c index 5e70f9d921c3..415e3f0804ed 100644 --- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c +++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c @@ -13,8 +13,6 @@ #include "SynQuacerI2cDxe.h" -#define BOOTTIME_DEBUG(x) do { if (!EfiAtRuntime()) DEBUG (x); } while (0) - // // We cannot use Stall () or timer events at runtime, so we need to busy-wait // for the controller to signal the completion interrupts. This value was @@ -168,27 +166,27 @@ SynQuacerI2cMasterStart ( MmioWrite8 (I2c->MmioBase + F_I2C_REG_DAR, SlaveAddress << 1); } - BOOTTIME_DEBUG ((DEBUG_INFO, "%a: slave:0x%02x\n", __FUNCTION__, + DEBUG ((DEBUG_INFO, "%a: slave:0x%02x\n", __FUNCTION__, SlaveAddress)); Bsr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BSR); Bcr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BCR); if ((Bsr & F_I2C_BSR_BB) && !(Bcr & F_I2C_BCR_MSS)) { -BOOTTIME_DEBUG ((DEBUG_INFO, "%a: bus is busy\n", __FUNCTION__)); +DEBUG ((DEBUG_INFO, "%a: bus is busy\n", __FUNCTION__)); return EFI_ALREADY_STARTED; } if (Bsr & F_I2C_BSR_BB) { // Bus is busy -BOOTTIME_DEBUG ((DEBUG_INFO, "%a: Continuous Start\n", __FUNCTION__)); +DEBUG ((DEBUG_INFO, "%a: Continuous Start\n", __FUNCTION__)); MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR, Bcr | F_I2C_BCR_SCC); } else { if (Bcr & F_I2C_BCR_MSS) { - BOOTTIME_DEBUG ((DEBUG_WARN, + DEBUG ((DEBUG_WARN, "%a: is not in master mode\n", __FUNCTION__)); return EFI_DEVICE_ERROR; } -BOOTTIME_DEBUG ((DEBUG_INFO, "%a: Start Condition\n", __FUNCTION__)); +DEBUG ((DEBUG_INFO, "%a: Start Condition\n", __FUNCTION__)); MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR, Bcr | F_I2C_BCR_MSS | F_I2C_BCR_INTE | F_I2C_BCR_BEIE); } @@ -329,13 +327,13 @@ SynQuacerI2cStartRequest ( Status = WaitForInterrupt (I2c); if (EFI_ERROR (Status)) { -BOOTTIME_DEBUG ((DEBUG_WARN, "%a: Timeout waiting for interrupt - %r\n", +DEBUG ((DEBUG_WARN, "%a: Timeout waiting for interrupt - %r\n", __FUNCTION__, Status)); break; } if (MmioRead8 (I2c->MmioBase + F_I2C_REG_BSR) & F_I2C_BSR_LRB) { - BOOTTIME_DEBUG ((DEBUG_WARN, "%a: No ack received\n", __FUNCTION__)); + DEBUG ((DEBUG_WARN, "%a: No ack received\n", __FUNCTION__)); Status = EFI_DEVICE_ERROR; break; } @@ -346,13 +344,13 @@ SynQuacerI2cStartRequest ( Bcr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BCR); if (Bcr & F_I2C_BCR_BER) { -BOOTTIME_DEBUG ((DEBUG_WARN, "%a: Bus error detected\n", __FUNCTION__)); +DEBUG ((DEBUG_WARN, "%a: Bus error detected\n", __FUNCTION__)); Status = EFI_DEVICE_ERROR; break; } if ((Bsr & F_I2C_BSR_AL) || !(Bcr & F_I2C_BCR_MSS)) { -BOOTTIME_DEBUG ((DEBUG_WARN, "%a: Arbitration lost\n", __FUNCTION__)); +DEBUG ((DEBUG_WARN, "%a: Arbitration lost\n", __FUNCTION__)); Status = EFI_DEVICE_ERROR; break; } @@ -368,7 +366,7 @@ SynQuacerI2cStartRequest ( Status = WaitForInterrupt (I2c); if (EFI_ERROR (Status)) { - BOOTTIME_DEBUG ((DEBUG_WARN, + DEBUG ((DEBUG_WARN, "%a: Timeout waiting for interrupt - %r\n", __FUNCTION__, Status)); break; } @@ -383,13 +381,13 @@ SynQuacerI2cStartRequest ( Status = WaitForInterrupt (I2c); if (EFI_ERROR (Status)) { - BOOTTIME_DEBUG ((DEBUG_WARN, + DEBUG ((DEBUG_WARN, "%a: Timeout waiting for interrupt - %r\n", __FUNCTION__, Status)); break; } if (MmioRead8 (I2c->MmioBase + F_I2C_REG_BSR) & F_I2C_BSR_LRB) { - BOOTTIME_DEBUG ((DEBUG_WARN, "%a: No ack received\n", __FUNCTION__)); + DEBUG ((DEBUG_WARN, "%a: No ack received\n", __FUNCTION__)); Status = EFI_DEVICE_ERROR; break; } @@ -486,7 +484,7 @@ SynQuacerI2cInit ( Dev->Resources[0].AddrLen, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); if (EFI_ERROR (Status)) { -
[edk2] [PATCH edk2-platforms 1/2] Platform, Silicon: use DxeRuntimeDebugLibSerialPort for runtime DXE drivers
Commit 4bf95a9f361e ("MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message") broke the DEBUG build for all platforms that rely on MMIO mapped UART devices, since it introduces a DEBUG() print that may trigger at runtime, at which such UART devices are usually not mapped, resulting in an OS crash. Given that this mostly only affects ARM and AARCH64, it is not unlikely that similar inadvertent breakage will occur again in the future, so let's fix this once and for all by switching affected platforms to the new DxeRuntimeDebugLibSerialPort DebugLib implementation that takes care not to touch the UART hardware after ExitBootServices(). Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Platform/AMD/OverdriveBoard/OverdriveBoard.dsc | 3 +++ Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 +++ Platform/LeMaker/CelloBoard/CelloBoard.dsc | 3 +++ Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 3 +++ Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +++ Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc | 3 +++ Silicon/Hisilicon/Hisilicon.dsc.inc | 3 +++ Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc| 3 +++ 8 files changed, 24 insertions(+) diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc index 7d85b78642da..348828e18d44 100644 --- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc +++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc @@ -227,6 +227,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf +!if $(TARGET) != RELEASE + DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf +!endif [LibraryClasses.common.UEFI_APPLICATION] UefiDecompressLib|IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.inf diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc index b026ce3a420a..7cb47937329e 100644 --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc @@ -227,6 +227,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] !if $(SECURE_BOOT_ENABLE) == TRUE BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf !endif +!if $(TARGET) != RELEASE + DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf +!endif [LibraryClasses.ARM, LibraryClasses.AARCH64] # diff --git a/Platform/LeMaker/CelloBoard/CelloBoard.dsc b/Platform/LeMaker/CelloBoard/CelloBoard.dsc index 2468583c0daa..569e444f6b26 100644 --- a/Platform/LeMaker/CelloBoard/CelloBoard.dsc +++ b/Platform/LeMaker/CelloBoard/CelloBoard.dsc @@ -211,6 +211,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf +!if $(TARGET) != RELEASE + DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf +!endif [LibraryClasses.common.UEFI_APPLICATION] UefiDecompressLib|IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.inf diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc index 8450d7800e43..925ce36d278b 100644 --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc @@ -208,6 +208,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf +!if $(TARGET) != RELEASE + DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf +!endif # diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc index 45ab2afc4069..2d5a94ed1dab 100644 --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc @@ -203,6 +203,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf +!if
Re: [edk2] [PATCH v4 0/2] Create UART DebugLib implementation for runtime drivers
On 23 February 2018 at 22:36, Kinney, Michael D wrote: > Ard, > > Just a few comments: > * Please pre-initialize global mEfiAtRuntime = FALSE; > * The UNI file in the patch is identical to BaseDebugLibSerialPort. > Please update UNI file header and strings to describe this as the > runtime version of the DebugLib as described in the .inf and .C files. > * gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue should be > SOMETIMES_CONSUMES in the INF file. > > With those changes: > > Reviewed-by: Michael D Kinney > Thanks all Pushed as 8bdb0221152c..ebfca258f5d7 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] BaseTools:Override the MAKE_FLAGS by BuildOptions in DSC
The issue that *_*_*_MAKE_FLAGS doesn't work in DSC [BuildOptions] section. It means MAKE flags can't be set in platform DSC file. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yonghong Zhu --- BaseTools/Source/Python/AutoGen/AutoGen.py | 7 +++ 1 file changed, 7 insertions(+) diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py index 405bfa1..1787dec 100644 --- a/BaseTools/Source/Python/AutoGen/AutoGen.py +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py @@ -1905,10 +1905,17 @@ class PlatformAutoGen(AutoGen): self._BuildCommand += SplitOption(self.ToolDefinition["MAKE"]["PATH"]) if "FLAGS" in self.ToolDefinition["MAKE"]: NewOption = self.ToolDefinition["MAKE"]["FLAGS"].strip() if NewOption != '': self._BuildCommand += SplitOption(NewOption) +if "MAKE" in self.EdkIIBuildOption: +if "FLAGS" in self.EdkIIBuildOption["MAKE"]: +Flags = self.EdkIIBuildOption["MAKE"]["FLAGS"] +if Flags.startswith('='): +self._BuildCommand = [self._BuildCommand[0]] + [Flags[1:]] +else: +self._BuildCommand += [Flags] return self._BuildCommand ## Get tool chain definition # # Get each tool defition for given tool chain from tools_def.txt and platform -- 2.6.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg SmmLockBox: Return updated Length for EFI_BUFFER_TOO_SMALL
The updated Length value should be returned for EFI_BUFFER_TOO_SMALL case. Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c index 2dc8815f2eb5..d4183ca4b057 100644 --- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c +++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c @@ -9,7 +9,7 @@ SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(), SmmLockBoxSave() will receive untrusted input and do basic validation. -Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. +Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions @@ -217,6 +217,9 @@ SmmLockBoxRestore ( (VOID *)(UINTN)TempLockBoxParameterRestore.Buffer, (UINTN *)&TempLockBoxParameterRestore.Length ); +if (Status == EFI_BUFFER_TOO_SMALL) { + LockBoxParameterRestore->Length = TempLockBoxParameterRestore.Length; +} } LockBoxParameterRestore->Header.ReturnStatus = (UINT64)Status; return ; -- 2.7.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [RFC v1 0/1] Add PcdUsbBootIoBlocks to UsbMassBoot
Adding PcdUsbBootIoBlocks to UsbMassBoot to fix booting failed from USB issue in some platforms. Ming Huang (1): MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h | 7 +-- MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf | 4 MdeModulePkg/MdeModulePkg.dec| 4 MdeModulePkg/MdeModulePkg.uni| 4 4 files changed, 17 insertions(+), 2 deletions(-) -- 1.9.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
Booting from USB may fail while the macro USB_BOOT_IO_BLOCKS set to 128 because 128 blocks is exceeded the maximun blocks of some USB devices,like some virtual CD-ROM from BMC. So, give a chance to set the value of USB_BOOT_IO_BLOCKS by adding a Pcd. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ming Huang --- MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h | 7 +-- MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf | 4 MdeModulePkg/MdeModulePkg.dec| 4 MdeModulePkg/MdeModulePkg.uni| 4 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h index 5ee50ac52a21..ca9240adbd5f 100644 --- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h +++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h @@ -16,6 +16,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #ifndef _EFI_USB_MASS_BOOT_H_ #define _EFI_USB_MASS_BOOT_H_ +#include + // // The opcodes of various USB boot commands: // INQUIRY/REQUEST_SENSE are "No Timeout Commands" as specified @@ -66,9 +68,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #define USB_PDT_SIMPLE_DIRECT 0x0E ///< Simplified direct access device // -// Other parameters, Max carried size is 512B * 128 = 64KB +// Other parameters, Max carried size is depanded on Pcd. +// The default of PcdUsbBootIoBlocks is 128. 512B * 128 = 64KB // -#define USB_BOOT_IO_BLOCKS 128 +#define USB_BOOT_IO_BLOCKS (FixedPcdGet32 (PcdUsbBootIoBlocks)) // // Retry mass command times, set by experience diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf index 26d15c7679bf..40426512f884 100644 --- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf +++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf @@ -60,6 +60,7 @@ [Sources] UsbMassDiskInfo.c [Packages] + MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec [LibraryClasses] @@ -83,5 +84,8 @@ [Protocols] # EVENT_TYPE_RELATIVE_TIMER## CONSUMES # +[FixedPcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks + [UserExtensions.TianoCore."ExtraFiles"] UsbMassStorageDxeExtra.uni diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 455979386e3f..fc40745315a0 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -999,6 +999,10 @@ [PcdsFixedAtBuild] # @Prompt Enable UEFI Stack Guard. gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055 +## The Max blocks of usb transfer. The default is 128. +# @Prompt The Max blocks of usb transfer +gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|UINT32|0x010B + [PcdsFixedAtBuild, PcdsPatchableInModule] ## Dynamic type PCD can be registered callback function for Pcd setting action. # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index f3fa616438b0..c996d6b4ebe0 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1243,3 +1243,7 @@ #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEdkiiFpdtStringRecordEnableOnly_HELP #language en-US "Control which FPDT record format will be used to store the performance entry.\n" "On TRUE, the string FPDT record will be used to store every performance entry.\n" "On FALSE, the different FPDT record will be used to store the different performance entries." + +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_PROMPT #language en-US "The Max blocks of usb transfer." + +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_HELP #language en-US "The Max blocks of usb transfer. The default is 128." -- 1.9.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v3 1/3] MdeModulePkg/PciHostBridgeDxe: Add support for address translation
On Sat, Feb 24, 2018 at 04:30:42PM +0800, Ni, Ruiyu wrote: > On 2/24/2018 11:59 AM, Guo Heyi wrote: > >On Sat, Feb 24, 2018 at 11:11:35AM +0800, Ni, Ruiyu wrote: > >>On 2/24/2018 9:51 AM, Guo Heyi wrote: > >>>On Fri, Feb 23, 2018 at 04:05:17PM +0100, Laszlo Ersek wrote: > Thanks for writing up the details! > > Comments: > > On 02/23/18 09:53, Heyi Guo wrote: > >PCI address translation is necessary for some non-x86 platforms. On > >such platforms, address value (denoted as "device address" or "address > >in PCI view") set to PCI BAR registers in configuration space might be > >different from the address which is used by CPU to access the > >registers in memory BAR or IO BAR spaces (denoted as "host address" or > >"address in CPU view"). The difference between the two addresses is > >called "Address Translation Offset" or simply "translation", and can > >be represented by "Address Translation Offset" in ACPI QWORD Address > >Space Descriptor (Offset 0x1E). However UEFI and ACPI differs on the > >definitions of QWORD Address Space Descriptor, and we will follow UEFI > >definition on UEFI protocols, such as PCI root bridge IO protocol and > >PCI IO protocol. In UEFI 2.7, "Address Translation Offset" is "Offset > >to apply to the Starting address to convert it to a PCI address". This > >means: > > > >1. Translation = device address - host address. > > OK, this looks like a sensible choice to me. It means that the "apply" > term used in the UEFI spec means "add", which (I think) is the "natural" > interpretation. > > >2. PciRootBridgeIo->Configuration should return CPU view address, as > >well as PciIo->GetBarAttributes. > > OK. > > > > >Summary of addresses used: > > > >1. Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address, for > >it is easy to check whether the address is below 4G or above 4G. > > I agree that PciHostBridgeLib instances that do not set a nonzero > Translation need not care about the difference. > > (1) Still, can you confirm this is a "natural" choice? Were the > DmaAbove4G, MemAbove4G and PMemAbove4G fields originally introduced with > the device (PCI) view in mind? > > ... I also meant to raise a concern about the InitializePciHostBridge() > function where we call AddIoSpace() and AddMemoryMappedIoSpace() -- > which end up manipulating GCD -- with data from > PCI_ROOT_BRIDGE_APERTURE. I can see you modify those call sites in the > patch, so that's good. (I do have more comments on the actual > implementation.) > >>DmaAbove4G in PCI_ROOT_BRIDGE_APERTURE indicates the capability of > >>the root bridge HW, whether it's capable to do DMA on device address > >>above 4GB. > >> > > > > >2. Address returned by > >EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL::GetProposedResources > >is device address, or else PCI bus driver cannot set correct address > >into PCI BAR registers. > > > >3. Address returned by PciRootBridgeIo->Configuration is host address > >per UEFI 2.7 definition. > > > >4. Addresses used in GCD manipulation are host address. > > > >5. Addresses in ResAllocNode of PCI_ROOT_BRIDGE_INSTANCE are host > >address, for they are allocated from GCD. > > > >6. Address passed to PciHostBridgeResourceConflict is host address, > >for it comes from ResAllocNode. > > > >Contributed-under: TianoCore Contribution Agreement 1.1 > >Signed-off-by: Heyi Guo > >Cc: Ruiyu Ni > >Cc: Ard Biesheuvel > >Cc: Star Zeng > >Cc: Eric Dong > >Cc: Laszlo Ersek > >Cc: Michael D Kinney > >--- > > .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 74 +++--- > > .../Bus/Pci/PciHostBridgeDxe/PciHostResource.h | 2 + > > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 112 > > ++--- > > MdeModulePkg/Include/Library/PciHostBridgeLib.h| 8 ++ > > 4 files changed, 167 insertions(+), 29 deletions(-) > > > >diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c > >b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c > >index 1494848..e8979eb 100644 > >--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c > >+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c > >@@ -32,6 +32,29 @@ EDKII_IOMMU_PROTOCOL*mIoMmuProtocol; > > EFI_EVENT mIoMmuEvent; > > VOID*mIoMmuRegistration; > > > >+STATIC > >+UINT64 > >+GetTranslationByResourceType ( > >+ IN PCI_ROOT_BRIDGE_INSTANCE *RootBridge, > >+ IN PCI_RESOURCE_TYPEResourceType > >+ ) > >+{ > >+ switch (ResourceType) { > >+case TypeIo: > >+ return RootBridge->Io.Translation; > >+case TypeMem3
Re: [edk2] MdeModulePkg/UefiBootManagerLib: recursive call of BmRepairAllControllers
Sure. On Sat, Feb 24, 2018 at 08:40:20AM +, Ni, Ruiyu wrote: > Will you submit a patch for this change? > > Thanks/Ray > > > -Original Message- > > From: Guo Heyi [mailto:heyi@linaro.org] > > Sent: Saturday, February 24, 2018 4:29 PM > > To: Ni, Ruiyu > > Cc: Guo Heyi ; edk2-devel@lists.01.org; Zeng, Star > > ; Dong, Eric > > Subject: Re: MdeModulePkg/UefiBootManagerLib: recursive call of > > BmRepairAllControllers > > > > On Sat, Feb 24, 2018 at 04:20:52PM +0800, Ni, Ruiyu wrote: > > > On 2/24/2018 2:23 PM, Guo Heyi wrote: > > > >Hi folks, > > > > > > > >In BmDriverHealth.c, function BmRepairAllControllers may recursively > > > >call itself if some driver health protocol returns > > EfiDriverHealthStatusReconnectRequired. > > > >However, if there is something wrong in some 3rd party driver (e.g. > > > >PCI oprom), the driver health protocol of that driver may always > > > >return such status even after one and another reconnect. The endless > > > >iteration will cause stack overflow and then system exception, and it > > > >may be not easy to find that the exception is actually caused by stack > > overflow. > > > > > > yes. I agree. > > > If a buggy PCI oprom always return ReconnectRequired, the stack > > > overflow will happen. > > > > > > How about we choose maximum recursive call depth as 10 and continue to > > > boot if call depth reaches 11? > > > > That's fine for me :) > > > > Thanks, > > > > Gary (Heyi Guo) > > > > > > > > > > > > >So does it make sense to set a maximum count of this recursive call > > > >to avoid whole system hang even there is a buggy 3rd party driver? > > > > > > > >Thanks, > > > > > > > >Gary (Heyi Guo) > > > > > > > > > > > > > -- > > > Thanks, > > > Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] MdeModulePkg/UefiBootManagerLib: recursive call of BmRepairAllControllers
Will you submit a patch for this change? Thanks/Ray > -Original Message- > From: Guo Heyi [mailto:heyi@linaro.org] > Sent: Saturday, February 24, 2018 4:29 PM > To: Ni, Ruiyu > Cc: Guo Heyi ; edk2-devel@lists.01.org; Zeng, Star > ; Dong, Eric > Subject: Re: MdeModulePkg/UefiBootManagerLib: recursive call of > BmRepairAllControllers > > On Sat, Feb 24, 2018 at 04:20:52PM +0800, Ni, Ruiyu wrote: > > On 2/24/2018 2:23 PM, Guo Heyi wrote: > > >Hi folks, > > > > > >In BmDriverHealth.c, function BmRepairAllControllers may recursively > > >call itself if some driver health protocol returns > EfiDriverHealthStatusReconnectRequired. > > >However, if there is something wrong in some 3rd party driver (e.g. > > >PCI oprom), the driver health protocol of that driver may always > > >return such status even after one and another reconnect. The endless > > >iteration will cause stack overflow and then system exception, and it > > >may be not easy to find that the exception is actually caused by stack > overflow. > > > > yes. I agree. > > If a buggy PCI oprom always return ReconnectRequired, the stack > > overflow will happen. > > > > How about we choose maximum recursive call depth as 10 and continue to > > boot if call depth reaches 11? > > That's fine for me :) > > Thanks, > > Gary (Heyi Guo) > > > > > > > > >So does it make sense to set a maximum count of this recursive call > > >to avoid whole system hang even there is a buggy 3rd party driver? > > > > > >Thanks, > > > > > >Gary (Heyi Guo) > > > > > > > > > -- > > Thanks, > > Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v3 1/3] MdeModulePkg/PciHostBridgeDxe: Add support for address translation
On 2/24/2018 11:59 AM, Guo Heyi wrote: On Sat, Feb 24, 2018 at 11:11:35AM +0800, Ni, Ruiyu wrote: On 2/24/2018 9:51 AM, Guo Heyi wrote: On Fri, Feb 23, 2018 at 04:05:17PM +0100, Laszlo Ersek wrote: Thanks for writing up the details! Comments: On 02/23/18 09:53, Heyi Guo wrote: PCI address translation is necessary for some non-x86 platforms. On such platforms, address value (denoted as "device address" or "address in PCI view") set to PCI BAR registers in configuration space might be different from the address which is used by CPU to access the registers in memory BAR or IO BAR spaces (denoted as "host address" or "address in CPU view"). The difference between the two addresses is called "Address Translation Offset" or simply "translation", and can be represented by "Address Translation Offset" in ACPI QWORD Address Space Descriptor (Offset 0x1E). However UEFI and ACPI differs on the definitions of QWORD Address Space Descriptor, and we will follow UEFI definition on UEFI protocols, such as PCI root bridge IO protocol and PCI IO protocol. In UEFI 2.7, "Address Translation Offset" is "Offset to apply to the Starting address to convert it to a PCI address". This means: 1. Translation = device address - host address. OK, this looks like a sensible choice to me. It means that the "apply" term used in the UEFI spec means "add", which (I think) is the "natural" interpretation. 2. PciRootBridgeIo->Configuration should return CPU view address, as well as PciIo->GetBarAttributes. OK. Summary of addresses used: 1. Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address, for it is easy to check whether the address is below 4G or above 4G. I agree that PciHostBridgeLib instances that do not set a nonzero Translation need not care about the difference. (1) Still, can you confirm this is a "natural" choice? Were the DmaAbove4G, MemAbove4G and PMemAbove4G fields originally introduced with the device (PCI) view in mind? ... I also meant to raise a concern about the InitializePciHostBridge() function where we call AddIoSpace() and AddMemoryMappedIoSpace() -- which end up manipulating GCD -- with data from PCI_ROOT_BRIDGE_APERTURE. I can see you modify those call sites in the patch, so that's good. (I do have more comments on the actual implementation.) DmaAbove4G in PCI_ROOT_BRIDGE_APERTURE indicates the capability of the root bridge HW, whether it's capable to do DMA on device address above 4GB. 2. Address returned by EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL::GetProposedResources is device address, or else PCI bus driver cannot set correct address into PCI BAR registers. 3. Address returned by PciRootBridgeIo->Configuration is host address per UEFI 2.7 definition. 4. Addresses used in GCD manipulation are host address. 5. Addresses in ResAllocNode of PCI_ROOT_BRIDGE_INSTANCE are host address, for they are allocated from GCD. 6. Address passed to PciHostBridgeResourceConflict is host address, for it comes from ResAllocNode. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Heyi Guo Cc: Ruiyu Ni Cc: Ard Biesheuvel Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Michael D Kinney --- .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 74 +++--- .../Bus/Pci/PciHostBridgeDxe/PciHostResource.h | 2 + .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 112 ++--- MdeModulePkg/Include/Library/PciHostBridgeLib.h| 8 ++ 4 files changed, 167 insertions(+), 29 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c index 1494848..e8979eb 100644 --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c @@ -32,6 +32,29 @@ EDKII_IOMMU_PROTOCOL*mIoMmuProtocol; EFI_EVENT mIoMmuEvent; VOID*mIoMmuRegistration; +STATIC +UINT64 +GetTranslationByResourceType ( + IN PCI_ROOT_BRIDGE_INSTANCE *RootBridge, + IN PCI_RESOURCE_TYPEResourceType + ) +{ + switch (ResourceType) { +case TypeIo: + return RootBridge->Io.Translation; +case TypeMem32: + return RootBridge->Mem.Translation; +case TypePMem32: + return RootBridge->PMem.Translation; +case TypeMem64: + return RootBridge->MemAbove4G.Translation; +case TypePMem64: + return RootBridge->PMemAbove4G.Translation; +default: (2) How about we return zero for TypeBus, explicitly, and then ASSERT(FALSE) and return zero for "default"? + return 0; + } +} + /** Ensure the compatibility of an IO space descriptor with the IO aperture. @@ -411,8 +434,12 @@ InitializePciHostBridge ( } if (RootBridges[Index].Io.Base <= RootBridges[Index].Io.Limit) { + // Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address. + // According to UEFI 2.7, device address = host address + Translation. +
Re: [edk2] MdeModulePkg/UefiBootManagerLib: recursive call of BmRepairAllControllers
On Sat, Feb 24, 2018 at 04:20:52PM +0800, Ni, Ruiyu wrote: > On 2/24/2018 2:23 PM, Guo Heyi wrote: > >Hi folks, > > > >In BmDriverHealth.c, function BmRepairAllControllers may recursively call > >itself > >if some driver health protocol returns > >EfiDriverHealthStatusReconnectRequired. > >However, if there is something wrong in some 3rd party driver (e.g. PCI > >oprom), > >the driver health protocol of that driver may always return such status even > >after one and another reconnect. The endless iteration will cause stack > >overflow > >and then system exception, and it may be not easy to find that the exception > >is > >actually caused by stack overflow. > > yes. I agree. > If a buggy PCI oprom always return ReconnectRequired, the stack overflow > will happen. > > How about we choose maximum recursive call depth as 10 and continue to > boot if call depth reaches 11? That's fine for me :) Thanks, Gary (Heyi Guo) > > > > >So does it make sense to set a maximum count of this recursive call to avoid > >whole system hang even there is a buggy 3rd party driver? > > > >Thanks, > > > >Gary (Heyi Guo) > > > > > -- > Thanks, > Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] MdeModulePkg/UefiBootManagerLib: recursive call of BmRepairAllControllers
On 2/24/2018 2:23 PM, Guo Heyi wrote: Hi folks, In BmDriverHealth.c, function BmRepairAllControllers may recursively call itself if some driver health protocol returns EfiDriverHealthStatusReconnectRequired. However, if there is something wrong in some 3rd party driver (e.g. PCI oprom), the driver health protocol of that driver may always return such status even after one and another reconnect. The endless iteration will cause stack overflow and then system exception, and it may be not easy to find that the exception is actually caused by stack overflow. yes. I agree. If a buggy PCI oprom always return ReconnectRequired, the stack overflow will happen. How about we choose maximum recursive call depth as 10 and continue to boot if call depth reaches 11? So does it make sense to set a maximum count of this recursive call to avoid whole system hang even there is a buggy 3rd party driver? Thanks, Gary (Heyi Guo) -- Thanks, Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v3 1/3] MdeModulePkg/PciHostBridgeDxe: Add support for address translation
On 24 February 2018 at 03:11, Ni, Ruiyu wrote: ... >>> (6) For all of these, I believe that we have to think about the corner >>> case when the Translation value is not a multiple of (Alignment + 1). >>> >>> "Alignment" comes from the PCI BAR in question, whereas Base comes from >>> the platform PciHostBridgeLib. I think these are fairly independent (you >>> can plug a 3rd party, discrete PCI card into a board). I find it >>> plausible that the card has such a huge MMIO BAR (and alignment) that >>> the platform's Translation offset is not a multiple thereof. >>> >>> So, which "view" has to be aligned here? The PCI (device) view, or the >>> host (CPU) view? >> >> >> I believe the alignment requirement is from PCI view, not the CPU view. >> This >> also implies alignment in allocating GCD resources becomes meaningless. > > I agree. It's an issue we need to think about. > > All address spaces in GCD are added by gDS->AddXXX(). > So it means for a given range of address [HostBase, HostLimit) in GCD, > the corresponding device address is > [HostBase + Offset, HostLimit + Offset). > E.g.: GCD entry = [0xFFD, 0x3FFD), Offset = 0x3 > The corresponding device address range is [0x1000, 0x4000) > If we want to allocate 3 page-aligned pages of MMIO from GCD, > current AllocateResource() implementation will fail to allocate. > > What will the Offset be in real world? > If it's quite small (smaller than device required alignment), > thing becomes complicated. I even cannot think of any solution. > If it's quite large (larger than device required alignment), > thing becomes easy. Today's implementation can handle it well. > I see two typical use cases for this address translation feature: - adding an address offset to TypeTranslation I/O range, i.e., two RCs both using 0x0-0x on the PCI side, but requiring non-overlapping memory ranges on the CPU side (this is a limitation in the ACPI spec, where an I/O range cannot be subject to address translation and type translation at the same time) - mapping a MMIO32 range high in the CPU address space In both cases, I think it is reasonable as an implementation requirement that the translation is sufficiently aligned, maybe even to the size of the entire window. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/Terminal: ReadKeyStrokeEx always return key state
Reviewed-by: Star Zeng Thanks, Star -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni Sent: Saturday, February 24, 2018 3:14 PM To: edk2-devel@lists.01.org Cc: Zeng, Star Subject: [edk2] [PATCH] MdeModulePkg/Terminal: ReadKeyStrokeEx always return key state Because terminal doesn't support shift and toggle key state, ReadKeyStrokeEx just sets the two states to 0. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Star Zeng --- MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c index 3c1102dac4..99316376cb 100644 --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c @@ -2,7 +2,7 @@ Implementation for EFI_SIMPLE_TEXT_INPUT_PROTOCOL protocol. (C) Copyright 2014 Hewlett-Packard Development Company, L.P. -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved. +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. Copyright (C) 2016 Silicon Graphics, Inc. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -41,13 +41,12 @@ ReadKeyStrokeWorker ( return EFI_INVALID_PARAMETER; } - if (!EfiKeyFiFoRemoveOneKey (TerminalDevice, &KeyData->Key)) { -return EFI_NOT_READY; - } - KeyData->KeyState.KeyShiftState = 0; KeyData->KeyState.KeyToggleState = 0; + if (!EfiKeyFiFoRemoveOneKey (TerminalDevice, &KeyData->Key)) { +return EFI_NOT_READY; + } return EFI_SUCCESS; -- 2.16.1.windows.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
Re: [edk2] [PATCH] MdeModulePkg/Logo.idf: Remove incorrect comments.
Reviewed-by: Star Zeng Thanks, Star -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni Sent: Saturday, February 24, 2018 4:05 PM To: edk2-devel@lists.01.org Cc: Zeng, Star Subject: [edk2] [PATCH] MdeModulePkg/Logo.idf: Remove incorrect comments. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Star Zeng --- MdeModulePkg/Logo/Logo.idf | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/MdeModulePkg/Logo/Logo.idf b/MdeModulePkg/Logo/Logo.idf index f4c39b78c4..bad878399f 100644 --- a/MdeModulePkg/Logo/Logo.idf +++ b/MdeModulePkg/Logo/Logo.idf @@ -1,10 +1,7 @@ // /** @file // Platform Logo image definition file. // -// Console Platfrom DXE Driver that specifies whether device can be used as console -// input/output device or error output device and update global variables accordingly. -// -// Copyright (c) 2016, Intel Corporation. All rights reserved. +// Copyright (c) 2016 - 2018, Intel Corporation. All rights +reserved. // // This program and the accompanying materials // are licensed and made available under the terms and conditions of the BSD License -- 2.16.1.windows.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
[edk2] [PATCH] MdeModulePkg/Logo.idf: Remove incorrect comments.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Star Zeng --- MdeModulePkg/Logo/Logo.idf | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/MdeModulePkg/Logo/Logo.idf b/MdeModulePkg/Logo/Logo.idf index f4c39b78c4..bad878399f 100644 --- a/MdeModulePkg/Logo/Logo.idf +++ b/MdeModulePkg/Logo/Logo.idf @@ -1,10 +1,7 @@ // /** @file // Platform Logo image definition file. // -// Console Platfrom DXE Driver that specifies whether device can be used as console -// input/output device or error output device and update global variables accordingly. -// -// Copyright (c) 2016, Intel Corporation. All rights reserved. +// Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved. // // This program and the accompanying materials // are licensed and made available under the terms and conditions of the BSD License -- 2.16.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/PlatformLogo.h: Correct function header comments
Reviewed-by: Star Zeng Thanks, Star -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni Sent: Saturday, February 24, 2018 3:47 PM To: edk2-devel@lists.01.org Cc: Zeng, Star Subject: [edk2] [PATCH] MdeModulePkg/PlatformLogo.h: Correct function header comments Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Star Zeng --- MdeModulePkg/Include/Protocol/PlatformLogo.h | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/MdeModulePkg/Include/Protocol/PlatformLogo.h b/MdeModulePkg/Include/Protocol/PlatformLogo.h index 4773173889..c25e7b04cb 100644 --- a/MdeModulePkg/Include/Protocol/PlatformLogo.h +++ b/MdeModulePkg/Include/Protocol/PlatformLogo.h @@ -2,7 +2,7 @@ The Platform Logo Protocol defines the interface to get the Platform logo image with the display attribute. -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. +Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved. 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 @@ -39,22 +39,17 @@ typedef enum { } EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE; /** - Load a platform logo image and return its data and attributes. @param This The pointer to this protocol instance. @param Instance The visible image instance is found. - @param FormatThe format of the image. Examples: BMP, JPEG. - @param ImageData The image data for the badge file. Currently only - supports the .bmp file format. - @param ImageSize The size of the image returned. + @param Image Points to the image. @param Attribute The display attributes of the image returned. @param OffsetX The X offset of the image regarding the Attribute. @param OffsetY The Y offset of the image regarding the Attribute. @retval EFI_SUCCESS The image was fetched successfully. @retval EFI_NOT_FOUNDThe specified image could not be found. - **/ typedef EFI_STATUS -- 2.16.1.windows.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