|
Re: [PATCH] MdeModulePkg/SetupBrowserDxe: Fix IsZeroGuid() ASSERT.
Liming Gao
Nickle:
toggle quoted messageShow quoted text
Is this a real issue found in production? Thanks Liming
-----Original Message-----
|
|
|
|
Re: [PATCHv2 0/4] MdeModulePkg/SdMmcPciHcDxe: Refactor command processing
Albecki, Mateusz
Hi,
toggle quoted messageShow quoted text
Github with test code: https://github.com/malbecki/edk2/tree/test_code_for_async This test code was used as is on Platform2 and it worked if I have only scheduled one request. On platform 1 I had to rebuild it a couple of times to make it read instead of writing when testing after reboot. Regarding the push - I am fine with this change making it to master after the stable tag. Thanks, Mateusz
-----Original Message------------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
|
|
|
|
[PATCH edk2-platforms v2 1/1] Platform: Add SerialUartClockLib for all platforms
Pankaj Bansal <pankaj.bansal@...>
From: Pankaj Bansal <pankaj.bansal@...>
BaseSerialPortLib16550 has been modified to use SerialUartClockLib. Therefore, add the default implementation of SerialUartClockLib for all the platforms using BaseSerialPortLib16550. Signed-off-by: Pankaj Bansal <pankaj.bansal@...> --- Notes: V2: - removed RaspberryPi3 and RaspberryPi4 platforms from patch as these platforms no longer use BaseSerialPortLib16550.inf Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc | 1 + Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc | 1 + Platform/Intel/QuarkPlatformPkg/Quark.dsc | 1 + Platform/Intel/QuarkPlatformPkg/QuarkMin.dsc | 1 + Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 6 ++++++ Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 6 ++++++ Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 1 + Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc | 1 + Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dsc | 1 + Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 1 + 10 files changed, 20 insertions(+) diff --git a/Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc b/Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc index b83e72b48c47..90cd72aff0d5 100644 --- a/Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc +++ b/Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc @@ -74,6 +74,7 @@ [LibraryClasses.common] PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf SortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc index 23ab53dbfe12..172ba6822419 100644 --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc @@ -20,6 +20,7 @@ [LibraryClasses.common] DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf diff --git a/Platform/Intel/QuarkPlatformPkg/Quark.dsc b/Platform/Intel/QuarkPlatformPkg/Quark.dsc index a02adb64e695..b275298a5ddb 100644 --- a/Platform/Intel/QuarkPlatformPkg/Quark.dsc +++ b/Platform/Intel/QuarkPlatformPkg/Quark.dsc @@ -213,6 +213,7 @@ [LibraryClasses] # IohLib|QuarkSocPkg/QuarkSouthCluster/Library/IohLib/IohLib.inf I2cLib|QuarkSocPkg/QuarkSouthCluster/Library/I2cLib/I2cLib.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf diff --git a/Platform/Intel/QuarkPlatformPkg/QuarkMin.dsc b/Platform/Intel/QuarkPlatformPkg/QuarkMin.dsc index 3dbf616c664d..cffd777a6721 100644 --- a/Platform/Intel/QuarkPlatformPkg/QuarkMin.dsc +++ b/Platform/Intel/QuarkPlatformPkg/QuarkMin.dsc @@ -185,6 +185,7 @@ [LibraryClasses] # IohLib|QuarkSocPkg/QuarkSouthCluster/Library/IohLib/IohLib.inf I2cLib|QuarkSocPkg/QuarkSouthCluster/Library/I2cLib/I2cLib.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc index 463b952e6582..91b2754ea509 100644 --- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc +++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc @@ -184,6 +184,7 @@ [LibraryClasses.common] SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf !else DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf !endif @@ -200,6 +201,7 @@ [LibraryClasses.common] !if $(SOURCE_DEBUG_ENABLE) == TRUE PeCoffExtraActionLib|SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLibDebug.inf DebugCommunicationLib|SourceLevelDebugPkg/Library/DebugCommunicationLibSerialPort/DebugCommunicationLibSerialPort.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf !else PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf @@ -280,6 +282,7 @@ [LibraryClasses.IA32.PEIM, LibraryClasses.IA32.PEI_CORE, LibraryClasses.IA32.SEC SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf !else DebugLib|MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf !endif @@ -779,6 +782,7 @@ [Components.IA32] PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf } !endif @@ -933,6 +937,7 @@ [Components.IA32] MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf { <LibraryClasses> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf } Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmm.inf @@ -1223,6 +1228,7 @@ [Components.IA32] <LibraryClasses> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf } !endif diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc index ee18b45c9712..749a2dd4948b 100644 --- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc +++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc @@ -186,6 +186,7 @@ [LibraryClasses.common] SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf !else DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf !endif @@ -202,6 +203,7 @@ [LibraryClasses.common] !if $(SOURCE_DEBUG_ENABLE) == TRUE PeCoffExtraActionLib|SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLibDebug.inf DebugCommunicationLib|SourceLevelDebugPkg/Library/DebugCommunicationLibSerialPort/DebugCommunicationLibSerialPort.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf !else PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf @@ -282,6 +284,7 @@ [LibraryClasses.IA32.PEIM, LibraryClasses.IA32.PEI_CORE, LibraryClasses.IA32.SEC SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf !else DebugLib|MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf !endif @@ -781,6 +784,7 @@ [Components.IA32] PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf } !endif @@ -948,6 +952,7 @@ [Components.X64] MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf { <LibraryClasses> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf } Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmm.inf @@ -1238,6 +1243,7 @@ [Components.X64] <LibraryClasses> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf } !endif diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc index 9f8cb68cdd26..281de3197d0c 100644 --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc @@ -32,6 +32,7 @@ [LibraryClasses] !if $(DEBUG_ON_UART1) == FALSE SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf !else + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf diff --git a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc index 00f6d4496377..ef3ed0f20911 100644 --- a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc +++ b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc @@ -35,6 +35,7 @@ [LibraryClasses] StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf diff --git a/Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dsc b/Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dsc index e743a5e272e4..2e4bf8c1259c 100644 --- a/Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dsc +++ b/Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dsc @@ -105,6 +105,7 @@ [LibraryClasses] # Quark South Cluster # IohLib|QuarkSocPkg/QuarkSouthCluster/Library/IohLib/IohLib.inf + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf # diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc index 42f8bbba926c..4cf6429d7476 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc @@ -87,6 +87,7 @@ [LibraryClasses.common] UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf # Serial port libraries + SerialUartClockLib|MdeModulePkg/Library/BaseSerialUartClockLib/BaseSerialUartClockLib.inf SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf -- 2.17.1
|
|
|
|
Re: [EXT] RE: [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
Gaurav Jain
toggle quoted messageShow quoted text
-----Original Message-----Done the suggested changes and sent v2. Please review.
|
|
|
|
[PATCH v2 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
Gaurav Jain
ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
Conformance Test. SCT Test expect return as Invalid Parameter or Unsupported. Added Checks for Function Parameters. return Invalid or Unsupported if Check fails. Added Checks in PciIoPollIo(), PciIoIoRead() PciIoIoWrite() Signed-off-by: Gaurav Jain <gaurav.jain@...> --- Notes: v2 - Reverted ASSERT(FALSE) code. - Added Checks for Width, BarIndex, Buffer, Address range in PciIoIoRead, PciIoIoWrite. - Added Checks for Width, BarIndex, Result, Address range in PciIoPollIo, PciIoPollMem, PciIoCopyMem. - Added Checks for Attributes, BarIndex, Address range in PciIoSetBarAttributes. .../NonDiscoverablePciDeviceIo.c | 180 ++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c index 2d55c9699322..4dd804356021 100644 --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c @@ -93,6 +93,35 @@ PciIoPollMem ( OUT UINT64 *Result ) { + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + UINTN Count; + EFI_STATUS Status; + + if ((UINT32)Width > EfiPciIoWidthUint64) { + return EFI_INVALID_PARAMETER; + } + + if (BarIndex >= PCI_MAX_BAR) { + return EFI_UNSUPPORTED; + } + + if (Result == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + Count = 1; + + Status = GetBarResource (Dev, BarIndex, &Desc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { + return EFI_UNSUPPORTED; + } + ASSERT (FALSE); return EFI_UNSUPPORTED; } @@ -126,6 +155,35 @@ PciIoPollIo ( OUT UINT64 *Result ) { + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + UINTN Count; + EFI_STATUS Status; + + if ((UINT32)Width > EfiPciIoWidthUint64) { + return EFI_INVALID_PARAMETER; + } + + if (BarIndex >= PCI_MAX_BAR) { + return EFI_UNSUPPORTED; + } + + if (Result == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + Count = 1; + + Status = GetBarResource (Dev, BarIndex, &Desc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { + return EFI_UNSUPPORTED; + } + ASSERT (FALSE); return EFI_UNSUPPORTED; } @@ -396,6 +454,33 @@ PciIoIoRead ( IN OUT VOID *Buffer ) { + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + EFI_STATUS Status; + + if ((UINT32)Width > EfiPciIoWidthUint64) { + return EFI_INVALID_PARAMETER; + } + + if (BarIndex >= PCI_MAX_BAR) { + return EFI_UNSUPPORTED; + } + + if (Buffer == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + + Status = GetBarResource (Dev, BarIndex, &Desc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { + return EFI_UNSUPPORTED; + } + ASSERT (FALSE); return EFI_UNSUPPORTED; } @@ -425,6 +510,33 @@ PciIoIoWrite ( IN OUT VOID *Buffer ) { + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + EFI_STATUS Status; + + if ((UINT32)Width > EfiPciIoWidthUint64) { + return EFI_INVALID_PARAMETER; + } + + if (BarIndex >= PCI_MAX_BAR) { + return EFI_UNSUPPORTED; + } + + if (Buffer == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + + Status = GetBarResource (Dev, BarIndex, &Desc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { + return EFI_UNSUPPORTED; + } + ASSERT (FALSE); return EFI_UNSUPPORTED; } @@ -556,6 +668,40 @@ PciIoCopyMem ( IN UINTN Count ) { + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *DestDesc; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *SrcDesc; + EFI_STATUS Status; + + if ((UINT32)Width > EfiPciIoWidthUint64) { + return EFI_INVALID_PARAMETER; + } + + if (DestBarIndex >= PCI_MAX_BAR || + SrcBarIndex >= PCI_MAX_BAR) { + return EFI_UNSUPPORTED; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + + Status = GetBarResource (Dev, DestBarIndex, &DestDesc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (DestOffset + (Count << (Width & 0x3)) > DestDesc->AddrLen) { + return EFI_UNSUPPORTED; + } + + Status = GetBarResource (Dev, SrcBarIndex, &SrcDesc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (SrcOffset + (Count << (Width & 0x3)) > SrcDesc->AddrLen) { + return EFI_UNSUPPORTED; + } + ASSERT (FALSE); return EFI_UNSUPPORTED; } @@ -1414,6 +1560,40 @@ PciIoSetBarAttributes ( IN OUT UINT64 *Length ) { + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + EFI_PCI_IO_PROTOCOL_WIDTH Width; + UINTN Count; + EFI_STATUS Status; + + #define DEV_SUPPORTED_ATTRIBUTES \ + (EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) + + if ((Attributes & (~DEV_SUPPORTED_ATTRIBUTES)) != 0) { + return EFI_UNSUPPORTED; + } + + if (BarIndex >= PCI_MAX_BAR) { + return EFI_UNSUPPORTED; + } + + if (Offset == NULL || Length == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + Width = EfiPciIoWidthUint8; + Count = (UINT32) *Length; + + Status = GetBarResource(Dev, BarIndex, &Desc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (*Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { + return EFI_UNSUPPORTED; + } + ASSERT (FALSE); return EFI_UNSUPPORTED; } -- 2.17.1
|
|
|
|
Re: [PATCH v3 0/1] Add PCD to disable safe string constraint assertions
Marvin Häuser <mhaeuser@...>
Hey Andrew,
Thanks once again for your comments, mine are inline. Best regards, Marvin Am 20.02.2020 um 00:55 schrieb Andrew Fish: Sorry, we were refering to the UEFI Multi-Processor services, so threadsOn Feb 17, 2020, at 12:26 AM, Marvin Häuser <mhaeuser@...Well EFI does not have Threads but it does have Events. I agree if an in a physical sense. I think very similar concerns apply regarding these services as with software multithreading in this context, please correct me if I'm wrong. Event handler changes the constraint handler it would need to restore itThis is true, but yet all edge-cases should be covered (event "interruption", actual (processor) interrupts, Multi-Processor execution), so developers do not run into unexpected (seemingly) undeterministic behaviour. However, I agree it is easier to ensure for something like UEFI than for a full-fledged OS of course. So far, I prefer this idea as the only requirement is that functionIt is an interesting idea to add granularity, but at the end of the contracts are well-designed (i.e. reflect environmental vs constraint errors by using sensical return values). It's definitely always a good idea to stick to standards and conventions. It appears my proposal is not conventional at all. However, I also believe there sometimes is a more pragmatic approach to things without imposing significant disadvantages, especially by limiting the scope. To be honest, I'm now interested for which cases the C11 path is advantagous over my approach, except that setting the constraint handler is a one-time thing if it remains consistent throughout execution. I'd imagine for some more complex handling logic I cannot think of an example of (all we really want to do is ASSERT conditionally afterall), and I hope this is not something edk2 would ever run into - keep it simple in my opinion. Fully agreed with the last sentence. That was one of the approaches I mentioned before my proposal, but I included it only for completeness' sake so we have a full overview. This requires prototype adaption for literally every function that may be used in such a way (which scopes beyond just BaseLib), ugly macros and more. Please don't do that. :) I'm afraid that's what most platforms will end up with. Our main concern was we may not allow such constraint violations to ASSERT, that is our main objective. However, if it can be made advantageous to future coding practice, it would be nice to have a decent solution for the caller to have some freedom. More about that below. Yes, agreed. We want predictable behaviour of "not ASSERTing" for untrusted input, because we *know* it is untrusted and the function *can* (and should be able to) handle it - it is simply not a precondition. A lot of Si and core (DxeCore, PeiCore, related libraries) code seems to ASSERT (frequently *only* ASSERT with no actual handling, especially for AllocatePool() == NULL) when there are valid error situations possible, so I hope this approach would be a handy tool to satisfy their needs without this absolutely terrible practice. :) I think I understood all your comment individually, but I'm afraid I'm not sure what your suggested solution is right now. You commented semi-positively on my proposal, you did not revoke your C11 path, and you furthermore introduced CHECK_* - this is brainstorming, and I think it's a great thing, but I'm simply not sure what your exact goal or prefered route is right now. My proposal is a bit unconventional and I agree it sounds hacky, but so far I'm not convinced it would be bad practice at all. It boils down to sensefully using the information a function returns. This requires nothing but the function to return sensical information, which every function should anyway. Your proposal is close to the C standard, and that is a good thing first of all. However, with the rest of edk2 not being very compliant, I don't think it's an instant win as in "we just comply to standards" because this choice does not suddenly make porting existing code significantly easier compared to the total lack of standard library support for anything but Shell apps. Pragmatically, I think it solves the same problem equally well, but at a higher cost (ensuring the implementation is safe and, strictly speaking, an ever-so-slight runtime overhead). Agreed.However, CONSTRAINT_ASSERT might actually be too specific. Maybe we wantYes it the new .inc DSC file could be overridden by the platform DSC Sure, thanks for taking time to respond!It would be nice if you had comments regarding every aspect I justSorry for the delay
|
|
|
|
Re: [edk2-platforms] [PATCH V2 2/2] Enable build for CometlakeOpenBoardPkg
Chaganty, Rangasai V
Reviewed-by: Sai Chaganty <rangasai.v.chaganty@...>
toggle quoted messageShow quoted text
-----Original Message-----
From: Esakkithevar, Kathappan <kathappan.esakkithevar@...> Sent: Wednesday, February 19, 2020 3:56 AM To: devel@edk2.groups.io Cc: Chaganty, Rangasai V <rangasai.v.chaganty@...>; Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Kethi Reddy, Deepika <deepika.kethi.reddy@...>; Agyeman, Prince <prince.agyeman@...> Subject: [edk2-platforms] [PATCH V2 2/2] Enable build for CometlakeOpenBoardPkg REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2280 This change adds the configuration to enable build for CometlakeURvp. Also it updates Cometlake U Rvp details to the Readme.md. Signed-off-by: Kathappan Esakkithevar <kathappan.esakkithevar@...> Cc: Sai Chaganty <rangasai.v.chaganty@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...> Cc: Prince Agyeman <prince.agyeman@...> --- Platform/Intel/Readme.md | 25 ++++++++++++++++++------- Platform/Intel/build.cfg | 3 ++- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/Platform/Intel/Readme.md b/Platform/Intel/Readme.md index 02d9517d19..2e66b4ce72 100644 --- a/Platform/Intel/Readme.md +++ b/Platform/Intel/Readme.md @@ -19,7 +19,7 @@ package. ## Board Naming Convention The board packages supported by Intel follow the naming convention \<xxx\>OpenBoardPkg where xxx refers to the encompassing platform name for a particular platform generation. For example, the [`KabylakeOpenBoardPkg`](https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/KabylakeOpenBoardPkg) contains the -board code for Intel Kaby Lake reference systems. Intel uses the moniker "OpenBoardPkg" to indicate that this package +board code for Intel KabyLake reference systems. Intel uses the moniker +"OpenBoardPkg" to indicate that this package is the open source board code. A closed source counterpart may exist which simply uses "BoardPkg". Both directly use the MinPlatformPkg from edk2-platforms. @@ -53,9 +53,10 @@ A UEFI firmware implementation using MinPlatformPkg is constructed using the fol ## Board Support -* The `KabylakeOpenBoardPkg` contains board implementations for Kaby Lake systems. +* The `KabylakeOpenBoardPkg` contains board implementations for KabyLake systems. * The `SimicsOpenBoardPkg` contains board implementations for the Simics hardware simulator. -* The `WhiskeylakeOpenBoardPkg` contains board implementations for Whiskey Lake systems. +* The `WhiskeylakeOpenBoardPkg` contains board implementations for WhiskeyLake systems. +* The `CometlakeOpenBoardPkg` contains board implementations for CometLake systems. ### **Supported Hardware** @@ -65,8 +66,9 @@ A UEFI firmware implementation using MinPlatformPkg is constructed using the fol | Machine Name | Supported Chipsets | BoardPkg | Board Name | ----------------------------------------|--------------------------------------------|------------------------------|--------------------| -| RVP 3 | Sky Lake, Kaby Lake, Kaby Lake Refresh | KabylakeOpenBoardPkg | KabylakeRvp3 | -| WHL-U DDR4 RVP | Whiskey Lake | WhiskeylakeOpenBoardPkg | WhiskeylakeURvp | +| RVP 3 | SkyLake, KabyLake, KabyLake Refresh | KabylakeOpenBoardPkg | KabylakeRvp3 | +| WHL-U DDR4 RVP | WhiskeyLake | WhiskeylakeOpenBoardPkg | WhiskeylakeURvp | +| CML-U LPDDR3 RVP | CometLake V1 | CometlakeOpenBoardPkg | CometlakeURvp | *Note: RVP = Reference and Validation Platform* @@ -82,8 +84,8 @@ A UEFI firmware implementation using MinPlatformPkg is constructed using the fol | Machine Name | Supported Chipsets | BoardPkg | Board Name | ----------------------------------------|--------------------------------------------|------------------------------|--------------------| -| galp2 | Kaby Lake | KabylakeOpenBoardPkg | GalagoPro3 | -| galp3 & galp3-b | Kaby Lake Refresh | KabylakeOpenBoardPkg | GalagoPro3 | +| galp2 | KabyLake | KabylakeOpenBoardPkg | GalagoPro3 | +| galp3 & galp3-b | KabyLake Refresh | KabylakeOpenBoardPkg | GalagoPro3 | ## Board Package Organization The board package follows the standard EDK II package structure with the following additional elements and guidelines: @@ -237,6 +239,11 @@ return back to the minimum platform caller. | | | |---build_config.cfg: WhiskeylakeURvp specific build | | | settings environment variables. | | | + | | |------CometlakeOpenBoardPkg + | | | |------CometlakeURvp + | | | |---build_config.cfg: CometlakeURvp specific build + | | | settings environment variables. + | | | |------FSP </pre> @@ -257,6 +264,10 @@ return back to the minimum platform caller. 1. This firmware project has only been tested booting to Microsoft Windows 10 x64 with AHCI mode and Integrated Graphic Device. +**CometlakeOpenBoardPkg** +1. This firmware project has been tested booting to Microsoft Windows 10 x64 with AHCI mode and External Graphic Device. +2. This firmware project has been also tested booting to Ubuntu 17.10 with AHCI mode and Integrated Graphic Device. + ### **Package Builds** In some cases, such as BoardModulePkg, a package may provide a set of functionality that is included in other diff --git a/Platform/Intel/build.cfg b/Platform/Intel/build.cfg index 86a9115021..5bc1dea43c 100644 --- a/Platform/Intel/build.cfg +++ b/Platform/Intel/build.cfg @@ -1,7 +1,7 @@ # @ build.cfg # This is the main/default build configuration file # -# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2019 - 2020 Intel Corporation. All rights reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -58,3 +58,4 @@ BoardX58Ich10 = SimicsOpenBoardPkg/BoardX58Ich10/build_config.cfg GalagoPro3 = KabylakeOpenBoardPkg/GalagoPro3/build_config.cfg KabylakeRvp3 = KabylakeOpenBoardPkg/KabylakeRvp3/build_config.cfg WhiskeylakeURvp = WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/build_config.cfg +CometlakeURvp = CometlakeOpenBoardPkg/CometlakeURvp/build_config.cfg -- 2.16.2.windows.1
|
|
|
|
Re: [edk2-platforms] [PATCH V2 2/2] Enable build for CometlakeOpenBoardPkg
Chiu, Chasel
Reviewed-by: Chasel Chiu <chasel.chiu@...>
toggle quoted messageShow quoted text
-----Original Message-----
|
|
|
|
Re: [PATCH] MdeModulePkg/SetupBrowserDxe: Fix IsZeroGuid() ASSERT.
Dandan Bi
Thanks Nickle for the fix.
toggle quoted messageShow quoted text
One minor comment is that please pay attention to the format of commit message. Refer to https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format, Line length of commit message should be less than 76 characters when possible. Please address it when submit the patch. Reviewed-by: Dandan Bi <dandan.bi@...> Thanks, Dandan
-----Original Message-----
|
|
|
|
Re: Patch List for 202002 stable tag
Vitaly Cheptsov
Liming, no problem from our side. The patch is now reviewed and I believe I provided all the necessarily material regarding its status. In case Ray would rather postpone it, I give no objection to this without prior notice. There is no problem from our side if EDK II team wants to prioritise other issues, we can always merge it right after the stable tag lands. Best wishes, Vitaly
|
|
|
|
[PATCH] MdeModulePkg/Application: Overflowed Array Index
GuoMinJ
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2272
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2289 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2290 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2287 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2288 Index should be off-by one than size of array, so when check array, the max index should less than size of array. Signed-off-by: GuoMinJ <newexplorerj@...> --- .../SmiHandlerProfileInfo/SmiHandlerProfileInfo.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c index 0f7163160b..4f195b16ce 100644 --- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c +++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c @@ -382,7 +382,7 @@ SxTypeToString ( IN EFI_SLEEP_TYPE Type ) { - if (Type >= 0 && Type <= ARRAY_SIZE(mSxTypeString)) { + if (Type >= 0 && Type < ARRAY_SIZE(mSxTypeString)) { return mSxTypeString[Type]; } else { AsciiSPrint (mNameString, sizeof(mNameString), "0x%x", Type); @@ -407,7 +407,7 @@ SxPhaseToString ( IN EFI_SLEEP_PHASE Phase ) { - if (Phase >= 0 && Phase <= ARRAY_SIZE(mSxPhaseString)) { + if (Phase >= 0 && Phase < ARRAY_SIZE(mSxPhaseString)) { return mSxPhaseString[Phase]; } else { AsciiSPrint (mNameString, sizeof(mNameString), "0x%x", Phase); @@ -457,7 +457,7 @@ StandbyButtonPhaseToString ( IN EFI_STANDBY_BUTTON_PHASE Phase ) { - if (Phase >= 0 && Phase <= ARRAY_SIZE(mStandbyButtonPhaseString)) { + if (Phase >= 0 && Phase < ARRAY_SIZE(mStandbyButtonPhaseString)) { return mStandbyButtonPhaseString[Phase]; } else { AsciiSPrint (mNameString, sizeof(mNameString), "0x%x", Phase); @@ -483,7 +483,7 @@ IoTrapTypeToString ( IN EFI_SMM_IO_TRAP_DISPATCH_TYPE Type ) { - if (Type >= 0 && Type <= ARRAY_SIZE(mIoTrapTypeString)) { + if (Type >= 0 && Type < ARRAY_SIZE(mIoTrapTypeString)) { return mIoTrapTypeString[Type]; } else { AsciiSPrint (mNameString, sizeof(mNameString), "0x%x", Type); @@ -508,7 +508,7 @@ UsbTypeToString ( IN EFI_USB_SMI_TYPE Type ) { - if (Type >= 0 && Type <= ARRAY_SIZE(mUsbTypeString)) { + if (Type >= 0 && Type < ARRAY_SIZE(mUsbTypeString)) { return mUsbTypeString[Type]; } else { AsciiSPrint (mNameString, sizeof(mNameString), "0x%x", Type); -- 2.17.1
|
|
|
|
Re: Patch List for 202002 stable tag
Liming Gao
Ray, Zhichao and Vitaly: Thanks. BZ is the good place to catch all discussion. Because we are close to edk2 stable tag 202002, can you make the agreement soon for BZ 2510?
Thanks Liming
From: Ni, Ray <ray.ni@...>
Sent: Thursday, February 20, 2020 11:13 AM To: Gao, Liming <liming.gao@...>; vit9696 <vit9696@...> Cc: Kinney, Michael D <michael.d.kinney@...>; Laszlo Ersek <lersek@...>; Guptha, Soumya K <soumya.k.guptha@...>; leif@...; afish@...; devel@edk2.groups.io; Marvin Häuser <marvin.haeuser@...>; Gao, Zhichao <zhichao.gao@...> Subject: RE: Patch List for 202002 stable tag
Liming, I provided my comments in the BZ.
From: Gao, Liming <liming.gao@...>
Vitaly: I add my comments.
Zhichao and Ray: Can you give your opinion for BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2510? Is it a bug fix or feature enhancement?
Thanks Liming From: vit9696 <vit9696@...>
Liming,
Thanks for pinging me about this!
With the PCD[1][2] I fully agree. The fact that it did not manage to land is mainly due to a sudden discussion that arose after complete silence for almost half a year, which was sort of unexpected. I will use this message as a suggestion to include this change as one of the primary goals for 202005 and kindly ask others to help to agree on the actual implementation. This bug strongly concerns us and we believe the fact that it does not (yet) cause issues to everyone is mainly coincidence.
[Liming] You can also present the topic in Tiano Design meeting to collect the feedback. Ni, Ray is the meeting host. You can send the topic to him.
With the Shell patch, the fact that I cannot enter upper case letters or use hotkeys in the editor sounds like a bug to me. The way the actual commit message is written reflects the change of the internal logic in the codebase (it adds support of specific behaviour handling on the target). In my opinion, it should not necessarily include the word «Fix» to be qualified as a bugfix, this is what bugzilla is for.
[Liming] If this fix is the bug, I agree it follows the process to catch this stable tag. I include ShellPkg maintainers (Ray Ni and Zhichao Gao) to give the opinion for the bug or not.
I am personally ok with deferring it to a next stable tag, but if the reasoning for this is «Feature planning freeze» dates, they do not strictly apply due to the reasons I stated above. So far the patch received only one review comment, which in fact was due to a minor misinterpretation. We also did some fairly extensive testing on our side before the submission (that’s why it actually took us a few more days). Unless the team has a lot of important work for the release, we can postpone the merge, otherwise I think it should be safe to merge this.
Best wishes, Vitaly
|
|
|
|
[PATCH] MdeModulePkg/Application: Overflowed Array Index
GuoMinJ
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2272
Index should be off-by one than size of array, so when check mUsbTypeString, the max index should less than size of array. Signed-off-by: GuoMinJ <newexplorerj@...> --- .../Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c index 0f7163160b..f8afcd6f96 100644 --- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c +++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c @@ -508,7 +508,7 @@ UsbTypeToString ( IN EFI_USB_SMI_TYPE Type ) { - if (Type >= 0 && Type <= ARRAY_SIZE(mUsbTypeString)) { + if (Type >= 0 && Type < ARRAY_SIZE(mUsbTypeString)) { return mUsbTypeString[Type]; } else { AsciiSPrint (mNameString, sizeof(mNameString), "0x%x", Type); -- 2.17.1
|
|
|
|
Re: [PATCH v4 1/1] NetworkPkg/ArpDxe: Recycle invalid ARP packets (CVE-2019-14559)
Liming Gao
This patch is for BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2031
toggle quoted messageShow quoted text
So, please add this line into the commit message. This can be done when submit this patch. Don't need to send the patch again. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2031 Thanks Liming
-----Original Message-----
|
|
|
|
Re: [PATCH v2 1/1] ShellPkg: acpiview: Prevent infinite loop if structure length is 0
Gao, Zhichao
Since there is no change for the V2 patch. It is a good behavior to add the R-B from V1 to V2. That would make the reviewer aware that there is no code change and review requirement for this patch. I would keep the R-B in V1 and help to merge the patch.
toggle quoted messageShow quoted text
Thanks, Zhichao
-----Original Message-----
|
|
|
|
Re: [EXT] RE: [edk2-devel] [PATCH v2 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.
Liming Gao
Gaurav:
toggle quoted messageShow quoted text
Got it. Lefi and Ard: Can you review this patch for stable tag 202002? Thanks Liming
-----Original Message-----
|
|
|
|
Re: [EXT] RE: [edk2-devel] [PATCH v2 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.
Gaurav Jain
Gaurav:Yes Thanks Gaurav -----Original Message-----
|
|
|
|
Re: Patch List for 202002 stable tag
Ni, Ray
Liming, I provided my comments in the BZ.
From: Gao, Liming <liming.gao@...>
Sent: Thursday, February 20, 2020 9:17 AM To: vit9696 <vit9696@...> Cc: Kinney, Michael D <michael.d.kinney@...>; Laszlo Ersek <lersek@...>; Guptha, Soumya K <soumya.k.guptha@...>; leif@...; afish@...; devel@edk2.groups.io; Marvin Häuser <marvin.haeuser@...>; Ni, Ray <ray.ni@...>; Gao, Zhichao <zhichao.gao@...> Subject: RE: Patch List for 202002 stable tag
Vitaly: I add my comments.
Zhichao and Ray: Can you give your opinion for BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2510? Is it a bug fix or feature enhancement?
Thanks Liming From: vit9696 <vit9696@...>
Liming,
Thanks for pinging me about this!
With the PCD[1][2] I fully agree. The fact that it did not manage to land is mainly due to a sudden discussion that arose after complete silence for almost half a year, which was sort of unexpected. I will use this message as a suggestion to include this change as one of the primary goals for 202005 and kindly ask others to help to agree on the actual implementation. This bug strongly concerns us and we believe the fact that it does not (yet) cause issues to everyone is mainly coincidence.
[Liming] You can also present the topic in Tiano Design meeting to collect the feedback. Ni, Ray is the meeting host. You can send the topic to him.
With the Shell patch, the fact that I cannot enter upper case letters or use hotkeys in the editor sounds like a bug to me. The way the actual commit message is written reflects the change of the internal logic in the codebase (it adds support of specific behaviour handling on the target). In my opinion, it should not necessarily include the word «Fix» to be qualified as a bugfix, this is what bugzilla is for.
[Liming] If this fix is the bug, I agree it follows the process to catch this stable tag. I include ShellPkg maintainers (Ray Ni and Zhichao Gao) to give the opinion for the bug or not.
I am personally ok with deferring it to a next stable tag, but if the reasoning for this is «Feature planning freeze» dates, they do not strictly apply due to the reasons I stated above. So far the patch received only one review comment, which in fact was due to a minor misinterpretation. We also did some fairly extensive testing on our side before the submission (that’s why it actually took us a few more days). Unless the team has a lot of important work for the release, we can postpone the merge, otherwise I think it should be safe to merge this.
Best wishes, Vitaly
|
|
|
|
Re: [EXTERNAL] [edk2-devel] [PATCH] UnitTestFrameworkPkg: Suspicious check for pointer Suite
Bret Barkelew
Reviewed-by: Bret Barkelew <bret.barkelew@...>
- Bret
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of GuoMinJ via Groups.Io <newexplorerj@...>
Sent: Tuesday, February 18, 2020 6:34:49 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: GuoMinJ <newexplorerj@...> Subject: [EXTERNAL] [edk2-devel] [PATCH] UnitTestFrameworkPkg: Suspicious check for pointer Suite REF:
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2530&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc3d0da979cc14c97e3ab08d7b4e4b9d0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637176766775311376&sdata=eRhhSwb6DcYF9%2B%2BTnCe0wzBPvRHTgl0Ue%2BRZ7LCYtxU%3D&reserved=0
The Suite pointer is used before check if it is valid, correct it to check the validation before use. Signed-off-by: GuoMinJ <newexplorerj@...> --- UnitTestFrameworkPkg/Library/UnitTestLib/RunTests.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/UnitTestFrameworkPkg/Library/UnitTestLib/RunTests.c b/UnitTestFrameworkPkg/Library/UnitTestLib/RunTests.c index fb247c59e7..b053e04959 100644 --- a/UnitTestFrameworkPkg/Library/UnitTestLib/RunTests.c +++ b/UnitTestFrameworkPkg/Library/UnitTestLib/RunTests.c @@ -33,13 +33,13 @@ RunTestSuite ( UNIT_TEST *Test; UNIT_TEST_FRAMEWORK *ParentFramework; - TestEntry = NULL; - ParentFramework = (UNIT_TEST_FRAMEWORK *)Suite->ParentFramework; - if (Suite == NULL) { return EFI_INVALID_PARAMETER; } + TestEntry = NULL; + ParentFramework = (UNIT_TEST_FRAMEWORK *)Suite->ParentFramework; + DEBUG ((DEBUG_VERBOSE, "---------------------------------------------------------\n")); DEBUG ((DEBUG_VERBOSE, "RUNNING TEST SUITE: %a\n", Suite->Title)); DEBUG ((DEBUG_VERBOSE, "---------------------------------------------------------\n")); -- 2.17.1
|
|
|
|
Re: [EXTERNAL] Re: [edk2-devel] [Patch 1/2] UnitTestFrameworkPkg: Disable EBC in DSC file
Bret Barkelew
Reviewed-by: Bret Barkelew <bret.barkelew@...>
- Bret
From: Laszlo Ersek <lersek@...>
Sent: Tuesday, February 11, 2020 1:02:17 AM To: devel@edk2.groups.io <devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@...> Cc: Sean Brogan <sean.brogan@...>; Bret Barkelew <Bret.Barkelew@...> Subject: [EXTERNAL] Re: [edk2-devel] [Patch 1/2] UnitTestFrameworkPkg: Disable EBC in DSC file On 02/10/20 21:24, Michael D Kinney wrote:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2514&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cabce6dc4e8984e69faae08d7aed12330%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637170085571112419&sdata=J40%2BxMF1CTlCdsZMK4%2Fjtp2jKkSdunF1b07tU34BIk4%3D&reserved=0 > > Remove EBC as one of the supported architectures > in the UnitTestFrameworkPkg DSC file. The EBC > compiler does not support cararg macros and the small typo: s/cararg/vararg/ Thanks Laszlo > UnitTestLib class uses this feature. > > Cc: Sean Brogan <sean.brogan@...> > Cc: Bret Barkelew <Bret.Barkelew@...> > Signed-off-by: Michael D Kinney <michael.d.kinney@...> > --- > UnitTestFrameworkPkg/UnitTestFrameworkPkg.dsc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkg.dsc b/UnitTestFrameworkPkg/UnitTestFrameworkPkg.dsc > index 0ad4482273..53d8f52754 100644 > --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkg.dsc > +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkg.dsc > @@ -13,7 +13,7 @@ [Defines] > PLATFORM_VERSION = 1.00 > DSC_SPECIFICATION = 0x00010005 > OUTPUT_DIRECTORY = Build/UnitTestFrameworkPkg > - SUPPORTED_ARCHITECTURES = IA32|X64|EBC|ARM|AARCH64 > + SUPPORTED_ARCHITECTURES = IA32|X64|ARM|AARCH64 > BUILD_TARGETS = DEBUG|RELEASE|NOOPT > SKUID_IDENTIFIER = DEFAULT > >
|
|
|