Re: [edk2] [PATCH edk2-platforms 34/39] Silicon/NXP: Implement EFI_CPU_IO2_PROTOCOL
>-Original Message- >From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >Sent: Tuesday, April 24, 2018 6:04 PM >To: Vabhav Sharma <vabhav.sha...@nxp.com> >Cc: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; Leif Lindholm ><leif.lindh...@linaro.org>; Kinney, Michael D <michael.d.kin...@intel.com>; >edk2-devel@lists.01.org; Udit Kumar <udit.ku...@nxp.com>; Varun Sethi ><v.se...@nxp.com> >Subject: Re: [PATCH edk2-platforms 34/39] Silicon/NXP: Implement >EFI_CPU_IO2_PROTOCOL > >On 24 April 2018 at 14:26, Vabhav Sharma <vabhav.sha...@nxp.com> wrote: >> >> >>>-Original Message- >>>From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>>Sent: Friday, April 20, 2018 2:11 PM >>>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com> >>>Cc: Leif Lindholm <leif.lindh...@linaro.org>; Kinney, Michael D >>><michael.d.kin...@intel.com>; edk2-devel@lists.01.org; Udit Kumar >>><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma >>><vabhav.sha...@nxp.com> >>>Subject: Re: [PATCH edk2-platforms 34/39] Silicon/NXP: Implement >>>EFI_CPU_IO2_PROTOCOL >>> >>>On 16 February 2018 at 09:50, Meenakshi <meenakshi.aggar...@nxp.com> >>>wrote: >>>> From: Vabhav <vabhav.sha...@nxp.com> >>>> >>>> NXP SOC has mutiple PCIe RCs,Adding respective implementation of >>>> EFI_CPU_IO2_PROTOCOL to provide Memory Space Read/Write functions >>>> used by generic Host Bridge Driver including correct value for the >>>> translation offset during MMIO accesses >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com> >>>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com> >>> >>>This driver looks completely wrong to me: MMIO access is memory >>>mapped, and given that you don't implement PCI to CPU translation of >>>MMIO accesses, the memory read and write functions should not perform >>>any translation at all, and just relay the accesses. On the other >>>hand, the I/O accessors are not implemented at all, and these are the >>>ones that require translation, given that the I/O port addresses in the CPU >space need translation to MMIO addressess. >> >> On NXP SoC, Mapping between CPU view and PCIe view is not 1:1 and require >CPU view translation for MMIO regions access, Accordingly translation is added >during memory read/write services. >> Bus driver relays the address range where PCIe device Bar region is split >> from, >Translation is required for relaying it to correct PCIe controller cpu view >address. >> > >You cannot implement this only in the EFI_CPU_IO2_PROTOCOL driver. >That way, EFI_PCI_IO_PROTOCOL.GetBarAttributes() will return resource >descriptors with untranslated addresses, breaking drivers that rely on this >information. > >If your PCIe implementation relies on MMIO translation, please refer to the >recently merged code in EDK2 and edk2-platforms implementing this for >Socionext SynQuacer. Ok, PciIo->GetBarAttributes is expected to return CPU view address. Are you referring to edk2 commit 74d0a33(Address translation support added to generic PciHostBridge driver)? Submitted patch development is done prior to this commit. I will refer the commits for Socionext SynQuacer in edk2-platforms for MMIO translation. > > >>> >>>Also, you don't seem to be using the PcdPciExp?BaseAddr PCDs anywhere, >>>so you can drop them from the .dsc >> No, It's used for checking the access to MMIO32 region and CPU view >> base address varies between different NXP SoCs >>> >>>> --- >>>> Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c | 529 >>>++ >>>> Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf | 48 ++ >>>> 2 files changed, 577 insertions(+) >>>> create mode 100644 Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c >>>> create mode 100644 >>>> Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf >>>> >>>> diff --git a/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c >>>> b/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c >>>> new file mode 100644 >>>> index 000..b5fb72c >>>> --- /dev/null >>>> +++ b/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c >>>> @@ -0,0 +1,529 @@ >>>> +/** @file >>>> + Produces the CPU I/O 2 Protocol. &
Re: [edk2] [PATCH edk2-platforms 39/39] Platform/NXP:PCIe enablement for LS2088A RDB
>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Friday, April 20, 2018 9:06 PM
>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar
><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma
><vabhav.sha...@nxp.com>
>Subject: Re: [PATCH edk2-platforms 39/39] Platform/NXP:PCIe enablement for
>LS2088A RDB
>
>On Fri, Feb 16, 2018 at 02:20:35PM +0530, Meenakshi wrote:
>> From: Vabhav <vabhav.sha...@nxp.com>
>>
>> Compilation: Update the fdf, dsc and dec files.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com>
>> ---
>> Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc| 17
>+
>> Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf| 9 +
>> .../Library/PlatformLib/ArmPlatformLib.inf | 2 ++
>> .../LS2088aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c | 6 ++
>> Silicon/NXP/LS2088A/LS2088A.dsc | 3 +++
>> 5 files changed, 37 insertions(+)
>>
>> diff --git a/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc
>> b/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc
>> index 4d32ea5..1ae55d4 100755
>> --- a/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc
>> +++ b/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc
>> @@ -43,6 +43,8 @@
>>BoardLib|Platform/NXP/LS2088aRdbPkg/Library/BoardLib/BoardLib.inf
>>FpgaLib|Platform/NXP/LS2088aRdbPkg/Library/FpgaLib/FpgaLib.inf
>>NorFlashLib|Silicon/NXP/Library/NorFlashLib/NorFlashLib.inf
>> + PciSegmentLib|Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
>> +
>> + PciHostBridgeLib|Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeL
>> + ib.inf
>>
>> [PcdsFixedAtBuild.common]
>>
>> @@ -97,6 +99,13 @@
>>gNxpQoriqLsTokenSpaceGuid.PcdFlashDeviceBase64|0x58000
>>gNxpQoriqLsTokenSpaceGuid.PcdFlashReservedRegionBase64|0x58030
>>
>> + #
>> + # PCI PCDs.
>> + #
>> + gNxpQoriqLsTokenSpaceGuid.PcdPciDebug|FALSE
>> + gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x8
>> + gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x407FC
>> +
>>
>>
>##
>
>> ##
>> #
>> # Components Section - list of all EDK II Modules needed by this
>> Platform @@ -115,3 +124,11 @@
>>Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
>>Silicon/NXP/Drivers/NorFlashDxe/NorFlashDxe.inf
>>Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
>> + Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
>> + MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
>> +
>> + gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8010004F
>> + }
>> + MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> +
>> + ##
>> diff --git a/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf
>> b/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf
>> index 8688d85..35a79bd 100644
>> --- a/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf
>> +++ b/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf
>> @@ -127,6 +127,13 @@ READ_LOCK_STATUS = TRUE
>>INF Silicon/NXP/Drivers/NorFlashDxe/NorFlashDxe.inf
>>
>>#
>> + # PCI
>> + #
>> + INF Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
>> + INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
>> + INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> +
>> + #
>># Network modules
>>#
>>INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
>> @@ -153,6 +160,8 @@ READ_LOCK_STATUS = TRUE
>>
>>INF
>>
>MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDev
>> iceDxe.inf
>>
>> + INF
>> +
>ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>> +
>
>Same comment as previously platforms: please conditionalise and mention in
>commit message.
>(Please add some detail to commit message in general about what is being
>enabled.)
>
>/
>Leif
Ok , Updated in header
I will update in commit message
Thanks.
>
>>#
>># USB Support
>>#
>> diff --git
>> a/Platform/NXP/LS2088aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
>> b/Platform/NXP/LS2088aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
>> index f5e5abd..0b836a8 100644
>> ---
>> a/Platform/NXP/LS2088aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
>> +++ b/Platform/NXP/LS2088aRdbPkg/Library/PlatformLi
Re: [edk2] [PATCH edk2-platforms 38/39] Platform/NXP:PCIe enablement for LS1046A RDB
>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Friday, April 20, 2018 9:03 PM
>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar
><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma
><vabhav.sha...@nxp.com>
>Subject: Re: [PATCH edk2-platforms 38/39] Platform/NXP:PCIe enablement for
>LS1046A RDB
>
>On Fri, Feb 16, 2018 at 02:20:34PM +0530, Meenakshi wrote:
>> From: Vabhav <vabhav.sha...@nxp.com>
>>
>> Compilation: Update the fdf, dsc and dec files.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com>
>> ---
>> Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc | 15
>+++
>> Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf | 9 +
>> .../LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf | 2 ++
>> .../NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c | 6 ++
>> Silicon/NXP/LS1046A/LS1046A.dsc | 3 +++
>> 5 files changed, 35 insertions(+)
>>
>> diff --git a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> index 36002d5..231207d 100644
>> --- a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> +++ b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> @@ -41,6 +41,8 @@
>>IfcLib|Silicon/NXP/Library/IfcLib/IfcLib.inf
>>BoardLib|Platform/NXP/LS1046aRdbPkg/Library/BoardLib/BoardLib.inf
>>FpgaLib|Platform/NXP/LS1046aRdbPkg/Library/FpgaLib/FpgaLib.inf
>> + PciSegmentLib|Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
>> +
>> + PciHostBridgeLib|Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeL
>> + ib.inf
>>
>> [PcdsFixedAtBuild.common]
>>
>> @@ -65,6 +67,7 @@
>>gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian|TRUE
>>gNxpQoriqLsTokenSpaceGuid.PcdWdogBigEndian|TRUE
>>gNxpQoriqLsTokenSpaceGuid.PcdIfcBigEndian|TRUE
>> + gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|TRUE
>>
>>#
>># I2C controller Pcds
>> @@ -77,6 +80,12 @@
>>gNxpQoriqLsTokenSpaceGuid.PcdI2cSlaveAddress|0x51
>>gNxpQoriqLsTokenSpaceGuid.PcdI2cSpeed|10
>>
>> + #
>> + # PCI PCDs.
>> + #
>> + gNxpQoriqLsTokenSpaceGuid.PcdPciDebug|FALSE
>> + gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x8
>> + gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x407FC
>>
>>
>##
>
>> ##
>> #
>> # Components Section - list of all EDK II Modules needed by this
>> Platform @@ -90,5 +99,11 @@
>>
>>Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf
>>Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
>> + Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
>> + MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
>> +
>> + gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8010004F
>> + }
>> + MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>>
>> ##
>> diff --git a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
>> b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
>> index 834e3a4..3351a06 100644
>> --- a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
>> +++ b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
>> @@ -123,6 +123,13 @@ READ_LOCK_STATUS = TRUE
>>INF EmbeddedPkg/SimpleTextInOutSerial/SimpleTextInOutSerial.inf
>>
>>#
>> + # PCI
>> + #
>> + INF Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
>> + INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
>> + INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> +
>> + #
>># Network modules
>>#
>>INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
>> @@ -147,6 +154,8 @@ READ_LOCK_STATUS = TRUE
>>INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>> !endif
>>
>> + INF
>> +
>ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>> +
>
>Same comment as for previous platform(s): please conditionalise and mention in
>commit message.
>
>/
>Leif
Ok, I will update it.
>
>>#
>># FAT filesystem + GPT/MBR partitioning
>>#
>> diff --git
>> a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
>> b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
>> index 49b57fc..5e09757 100644
>> ---
>> a/Platform/NXP/LS1046aRdbPkg/Librar
Re: [edk2] [PATCH edk2-platforms 35/39] Compilation: Update the fdf, dsc and dec files.
>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Friday, April 20, 2018 8:53 PM
>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar
><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma
><vabhav.sha...@nxp.com>
>Subject: Re: [PATCH edk2-platforms 35/39] Compilation: Update the fdf, dsc and
>dec files.
>
>On Fri, Feb 16, 2018 at 02:20:31PM +0530, Meenakshi wrote:
>> From: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>>
>> LS1043A PCIe compilation and update firmware device, description and
>> declaration files.Defining Embedded Package PCD which should be at
>> least 20 for 64K PCIe IO size required for CPU hob during PEI phase to
>> Add IO space post PEI phase.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com>
>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>> ---
>> Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc | 16
>
>> Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf | 9 +
>> .../LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf | 2 ++
>> .../LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c| 6 ++
>> Platform/NXP/NxpQoriqLs.dsc | 7 +++
>> Silicon/NXP/LS1043A/LS1043A.dsc | 4
>> Silicon/NXP/NxpQoriqLs.dec | 10 ++
>> 7 files changed, 54 insertions(+)
>>
>> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
>> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
>> index b2b514e..8cbaf88 100644
>> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
>> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
>> @@ -42,6 +42,8 @@
>>BoardLib|Platform/NXP/LS1043aRdbPkg/Library/BoardLib/BoardLib.inf
>>FpgaLib|Platform/NXP/LS1043aRdbPkg/Library/FpgaLib/FpgaLib.inf
>>NorFlashLib|Silicon/NXP/Library/NorFlashLib/NorFlashLib.inf
>> + PciSegmentLib|Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
>> +
>> + PciHostBridgeLib|Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeL
>> + ib.inf
>>
>> [PcdsFixedAtBuild.common]
>>
>> @@ -79,6 +81,13 @@
>>gNxpQoriqLsTokenSpaceGuid.PcdFlashDeviceBase64|0x06000
>>gNxpQoriqLsTokenSpaceGuid.PcdFlashReservedRegionBase64|0x6030
>>
>> + #
>> + # PCI PCDs.
>> + #
>> + gNxpQoriqLsTokenSpaceGuid.PcdPciDebug|FALSE
>> + gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x1
>> + gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x7FC
>> +
>>
>>
>##
>
>> ##
>> #
>> # Components Section - list of all EDK II Modules needed by this
>> Platform @@ -99,4 +108,11 @@
>>Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
>>Silicon/NXP/Drivers/NorFlashDxe/NorFlashDxe.inf
>>
>> + Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
>> + MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
>> +
>> + gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8010004F
>> + }
>> + MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> +
>> ##
>> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
>> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
>> index 6b5b63f..7993bf1 100644
>> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
>> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
>> @@ -130,6 +130,13 @@ READ_LOCK_STATUS = TRUE
>>INF Silicon/NXP/Drivers/NorFlashDxe/NorFlashDxe.inf
>>
>>#
>> + # PCI
>> + #
>> + INF Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
>> + INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
>> + INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> +
>> + #
>># Network modules
>>#
>>INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
>> @@ -154,6 +161,8 @@ READ_LOCK_STATUS = TRUE
>>INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>> !endif
>>
>> + INF
>> +
>ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>> +
>
>I'm pretty OK with most of these random updates squashed into one file, but the
>TftpDynamicCommand is something I generally don't like to see included by
>default.
>
>Other platforms put this inside a conditional statement:
>
>!ifdef $(INCLUDE_TFTP_COMMAND)
> INF
>ShellPkg/Dy
Re: [edk2] [PATCH edk2-platforms 34/39] Silicon/NXP: Implement EFI_CPU_IO2_PROTOCOL
>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Friday, April 20, 2018 8:46 PM
>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar
><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma
><vabhav.sha...@nxp.com>
>Subject: Re: [PATCH edk2-platforms 34/39] Silicon/NXP: Implement
>EFI_CPU_IO2_PROTOCOL
>
>On Fri, Feb 16, 2018 at 02:20:30PM +0530, Meenakshi wrote:
>> From: Vabhav <vabhav.sha...@nxp.com>
>>
>> NXP SOC has mutiple PCIe RCs,Adding respective implementation of
>> EFI_CPU_IO2_PROTOCOL to provide Memory Space Read/Write functions used
>> by generic Host Bridge Driver including correct value for the
>> translation offset during MMIO accesses
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com>
>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>> ---
>> Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c | 529
>++
>> Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf | 48 ++
>> 2 files changed, 577 insertions(+)
>> create mode 100644 Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c
>> create mode 100644 Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
>>
>> diff --git a/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c
>> b/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c
>> new file mode 100644
>> index 000..b5fb72c
>> --- /dev/null
>> +++ b/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c
>> @@ -0,0 +1,529 @@
>> +/** @file
>> + Produces the CPU I/O 2 Protocol.
>> +
>> + Copyright (c) 2009 - 2012, Intel Corporation. All rights
>> + reserved. Copyright (c) 2016, Linaro Ltd. All rights
>> + reserved. Copyright 2018 NXP
>> +
>> + 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
>> +
>> + https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
>> + ensource.org%2Flicenses%2Fbsd-license.php=02%7C01%7Cvabhav.shar
>> +
>ma%40nxp.com%7C42500fab2cd1447bbe6308d5a6d19d8b%7C686ea1d3bc2b4c
>6fa9
>> +
>2cd99c5c301635%7C0%7C0%7C636598341623135085=Zo6s2LhxPSElw4F
>XsV
>> + 7%2Bx3Veb5yptglf1UQiA%2FNRRc4%3D=0
>> +
>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define MAX_IO_PORT_ADDRESS PCI_SEG2_PORTIO_MAX
>> +
>> +//
>> +// Handle for the CPU I/O 2 Protocol
>> +//
>> +STATIC EFI_HANDLE mHandle;
>> +
>> +//
>> +// Lookup table for increment values based on transfer widths //
>> +STATIC CONST UINT8 mInStride[] = {
>> + 1, // EfiCpuIoWidthUint8
>> + 2, // EfiCpuIoWidthUint16
>> + 4, // EfiCpuIoWidthUint32
>> + 8, // EfiCpuIoWidthUint64
>> + 0, // EfiCpuIoWidthFifoUint8
>> + 0, // EfiCpuIoWidthFifoUint16
>> + 0, // EfiCpuIoWidthFifoUint32
>> + 0, // EfiCpuIoWidthFifoUint64
>> + 1, // EfiCpuIoWidthFillUint8
>> + 2, // EfiCpuIoWidthFillUint16
>> + 4, // EfiCpuIoWidthFillUint32
>> + 8 // EfiCpuIoWidthFillUint64
>> +};
>> +
>> +//
>> +// Lookup table for increment values based on transfer widths //
>> +STATIC CONST UINT8 mOutStride[] = {
>> + 1, // EfiCpuIoWidthUint8
>> + 2, // EfiCpuIoWidthUint16
>> + 4, // EfiCpuIoWidthUint32
>> + 8, // EfiCpuIoWidthUint64
>> + 1, // EfiCpuIoWidthFifoUint8
>> + 2, // EfiCpuIoWidthFifoUint16
>> + 4, // EfiCpuIoWidthFifoUint32
>> + 8, // EfiCpuIoWidthFifoUint64
>> + 0, // EfiCpuIoWidthFillUint8
>> + 0, // EfiCpuIoWidthFillUint16
>> + 0, // EfiCpuIoWidthFillUint32
>> + 0 // EfiCpuIoWidthFillUint64
>> +};
>> +
>> +/**
>> + Check parameters to a CPU I/O 2 Protocol service request.
>> +
>> + The I/O operations are carried out exactly as requested. The caller
>> + is responsible for satisfying any alignment and I/O width
>> + restrictions that a PI System on a platform might require. For
>> + example on some platforms, width requests of
>> + EfiCpuIoWidthUint64 do not work.
>> +
>> +
Re: [edk2] [PATCH edk2-platforms 33/39] Silicon/NXP: Implement PciHostBridgeLib support
>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Friday, April 20, 2018 8:24 PM
>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar
><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma
><vabhav.sha...@nxp.com>
>Subject: Re: [PATCH edk2-platforms 33/39] Silicon/NXP: Implement
>PciHostBridgeLib support
>
>My only comment in addition to Ard's comments/questions:
>Use same endianness handling here as for preceding patches?
>
>/
>Leif
On NXP SoCs PCIe IP is LE except link state register access for which
endianness is taken care.
>
>On Fri, Feb 16, 2018 at 02:20:29PM +0530, Meenakshi wrote:
>> From: Vabhav <vabhav.sha...@nxp.com>
>>
>> Implement the library that exposes the PCIe root complexes to the
>> generic PCI host bridge driver,Putting SoC Specific low level init
>> code for the RCs.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com>
>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>> ---
>> .../Library/PciHostBridgeLib/PciHostBridgeLib.c| 618
>+
>> .../Library/PciHostBridgeLib/PciHostBridgeLib.inf | 50 ++
>> 2 files changed, 668 insertions(+)
>> create mode 100644
>> Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> create mode 100644
>> Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>>
>> diff --git a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> new file mode 100644
>> index 000..e6f9b7c
>> --- /dev/null
>> +++ b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> @@ -0,0 +1,618 @@
>> +/** @file
>> + PCI Host Bridge Library instance for NXP SoCs
>> +
>> + Copyright 2018 NXP
>> +
>> + 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
>https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou
>rce.org%2Flicenses%2Fbsd-
>license.php=02%7C01%7Cvabhav.sharma%40nxp.com%7C90628d666bad4
>6bf6dde08d5a6ce8fe0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
>636598328488287967=6Jwkzr%2BkvDURRF%2FQ8rvSN1OBggJNg14NGh
>W5s3WkuFM%3D=0.
>> +
>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include #include
>> +#include #include #include
>> +
>> +#include
>> +
>> +#pragma pack(1)
>> +typedef struct {
>> + ACPI_HID_DEVICE_PATH AcpiDevicePath;
>> + EFI_DEVICE_PATH_PROTOCOL EndDevicePath; }
>> +EFI_PCI_ROOT_BRIDGE_DEVICE_PATH; #pragma pack ()
>> +
>> +STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH
>> +mEfiPciRootBridgeDevicePath[] = {
>> + {
>> +{
>> + {
>> +ACPI_DEVICE_PATH,
>> +ACPI_DP,
>> +{
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH)),
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH) >> 8)
>> +}
>> + },
>> + EISA_PNP_ID (0x0A08), // PCI Express
>> + PCI_SEG0_NUM
>> +},
>> +
>> +{
>> + END_DEVICE_PATH_TYPE,
>> + END_ENTIRE_DEVICE_PATH_SUBTYPE,
>> + {
>> +END_DEVICE_PATH_LENGTH,
>> +0
>> + }
>> +}
>> + },
>> + {
>> +{
>> + {
>> +ACPI_DEVICE_PATH,
>> +ACPI_DP,
>> +{
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH)),
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH) >> 8)
>> +}
>> + },
>> + EISA_PNP_ID (0x0A08), // PCI Express
>> + PCI_SEG1_NUM
>> +},
>> +
>> +{
>> + END_DEVICE_PATH_TYPE,
>> + END_ENTIRE_DEVICE_PATH_SUBTYPE,
>> + {
>> +END_DEVICE_PATH_LENGTH,
>> +0
>> + }
>> +}
>> + },
>> + {
>> +{
>> + {
>> +ACPI_DEVICE_PATH,
>> +ACPI_DP,
>> +{
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH)),
&g
Re: [edk2] [PATCH edk2-platforms 32/39] Silicon/NXP: Implement PciSegmentLib to support multiple RCs
>-Original Message- >From: Leif Lindholm [mailto:leif.lindh...@linaro.org] >Sent: Friday, April 20, 2018 6:12 PM >To: Vabhav Sharma <vabhav.sha...@nxp.com> >Cc: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; >ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar ><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com> >Subject: Re: [PATCH edk2-platforms 32/39] Silicon/NXP: Implement >PciSegmentLib to support multiple RCs > >On Fri, Apr 20, 2018 at 06:40:27AM +, Vabhav Sharma wrote: >> >> >> >-Original Message- >> >From: Leif Lindholm [mailto:leif.lindh...@linaro.org] >> >Sent: Friday, April 20, 2018 12:57 AM >> >To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com> >> >Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar >> ><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma >> ><vabhav.sha...@nxp.com> >> >Subject: Re: [PATCH edk2-platforms 32/39] Silicon/NXP: Implement >> >PciSegmentLib to support multiple RCs >> > >> >On Fri, Feb 16, 2018 at 02:20:28PM +0530, Meenakshi wrote: >> >> From: Vabhav <vabhav.sha...@nxp.com> >> >> >> >> Multiple root complex support is not provided by standard library >> >> PciLib/PciExpressLib/PciSegmentLib, Reimplementing it and provide >> >> function for reading/writing into PCIe configuration Space. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> Signed-off-by: Vabhav <vabhav.sha...@nxp.com> >> >> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com> >> >> --- >> >> Silicon/NXP/Include/Pcie.h | 143 + >> >> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c | 604 >> >+ >> >> .../NXP/Library/PciSegmentLib/PciSegmentLib.inf| 41 ++ >> >> 3 files changed, 788 insertions(+) create mode 100644 >> >> Silicon/NXP/Include/Pcie.h create mode 100644 >> >> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c >> >> create mode 100644 >> >> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf >> >> >> >> diff --git a/Silicon/NXP/Include/Pcie.h >> >> b/Silicon/NXP/Include/Pcie.h new file mode 100644 index >> >> 000..a7e6f9b >> >> --- /dev/null >> >> +++ b/Silicon/NXP/Include/Pcie.h >> >> @@ -0,0 +1,143 @@ >> >> +/** @file >> >> + PCI memory configuration for NXP >> >> + >> >> + Copyright 2018 NXP >> >> + >> >> + 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 >> >https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope >> >nsou >> >rce.org%2Flicenses%2Fbsd- >> >>license.php=02%7C01%7Cvabhav.sharma%40nxp.com%7C081a2ebc43af4 >a >> >>daaf8e08d5a62b9921%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C >6 >> >>36597628604269440=kt661HfkUD2E3sYswy0I4vVFTV3%2Fq%2FiM%2Bi >W >> >egCISP%2BU%3D=0. >> >> + >> >> + 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 __PCI_H__ >> >> +#define __PCI_H__ >> > >> >I'm not super happy about reusing such a generic name for the include >> >guard - or really even the filename. (MdePkg/Include/Pci.h has >> >_PCI_H_.) >> > >> >Could you rename this header NxpPcie.h and change the include guard >> >to _NXP_PCIE_H_? >> I see, Sure. >> > >> >> + >> >> +// Segment 0 >> >> +#define PCI_SEG0_NUM 0 >> >> + >> >> +#define PCI_SEG0_BUSNUM_MIN 0x0 >> >> +#define PCI_SEG0_BUSNUM_MAX 0xff >> >> + >> >> +#define PCI_SEG0_PORTIO_MIN 0x0 >> >> +#define PCI_SEG0_PORTIO_MAX 0x >> >> + >> >> +#define PCI_SEG0_MMIO32_MIN 0x4000 >> >> +#define PCI_SEG0_MMIO32_MAX 0x4fff >> >> +#define PCI_SEG0_MMIO64_MIN PCI_SEG0_MMIO_MEMBASE + >> >SEG_MEM_SIZE >> &
Re: [edk2] [PATCH edk2-platforms 34/39] Silicon/NXP: Implement EFI_CPU_IO2_PROTOCOL
>-Original Message-
>From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>Sent: Friday, April 20, 2018 2:11 PM
>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>Cc: Leif Lindholm <leif.lindh...@linaro.org>; Kinney, Michael D
><michael.d.kin...@intel.com>; edk2-devel@lists.01.org; Udit Kumar
><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma
><vabhav.sha...@nxp.com>
>Subject: Re: [PATCH edk2-platforms 34/39] Silicon/NXP: Implement
>EFI_CPU_IO2_PROTOCOL
>
>On 16 February 2018 at 09:50, Meenakshi <meenakshi.aggar...@nxp.com>
>wrote:
>> From: Vabhav <vabhav.sha...@nxp.com>
>>
>> NXP SOC has mutiple PCIe RCs,Adding respective implementation of
>> EFI_CPU_IO2_PROTOCOL to provide Memory Space Read/Write functions used
>> by generic Host Bridge Driver including correct value for the
>> translation offset during MMIO accesses
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com>
>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>
>This driver looks completely wrong to me: MMIO access is memory mapped, and
>given that you don't implement PCI to CPU translation of MMIO accesses, the
>memory read and write functions should not perform any translation at all, and
>just relay the accesses. On the other hand, the I/O accessors are not
>implemented at all, and these are the ones that require translation, given
>that the
>I/O port addresses in the CPU space need translation to MMIO addressess.
On NXP SoC, Mapping between CPU view and PCIe view is not 1:1 and require CPU
view translation for MMIO regions access, Accordingly translation is added
during memory read/write services.
Bus driver relays the address range where PCIe device Bar region is split from,
Translation is required for relaying it to correct PCIe controller cpu view
address.
>
>Also, you don't seem to be using the PcdPciExp?BaseAddr PCDs anywhere, so you
>can drop them from the .dsc
No, It's used for checking the access to MMIO32 region and CPU view base
address varies between different NXP SoCs
>
>> ---
>> Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c | 529
>++
>> Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf | 48 ++
>> 2 files changed, 577 insertions(+)
>> create mode 100644 Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c
>> create mode 100644 Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
>>
>> diff --git a/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c
>> b/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c
>> new file mode 100644
>> index 000..b5fb72c
>> --- /dev/null
>> +++ b/Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.c
>> @@ -0,0 +1,529 @@
>> +/** @file
>> + Produces the CPU I/O 2 Protocol.
>> +
>> + Copyright (c) 2009 - 2012, Intel Corporation. All rights
>> + reserved. Copyright (c) 2016, Linaro Ltd. All rights
>> + reserved. Copyright 2018 NXP
>> +
>> + 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
>> +
>> + https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
>> + ensource.org%2Flicenses%2Fbsd-license.php=02%7C01%7Cvabhav.shar
>> +
>ma%40nxp.com%7C4507c0726b244ad6476d08d5a69a64ee%7C686ea1d3bc2b4c
>6fa9
>> +
>2cd99c5c301635%7C0%7C0%7C636598104414046440=IFSU0%2FeTdWrw
>gg0f
>> + 0Lq2qSVGgogG68tYZevrRmC%2BkV8%3D=0
>> +
>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define MAX_IO_PORT_ADDRESS PCI_SEG2_PORTIO_MAX
>> +
>> +//
>> +// Handle for the CPU I/O 2 Protocol
>> +//
>> +STATIC EFI_HANDLE mHandle;
>> +
>> +//
>> +// Lookup table for increment values based on transfer widths //
>> +STATIC CONST UINT8 mInStride[] = {
>> + 1, // EfiCpuIoWidthUint8
>> + 2, // EfiCpuIoWidthUint16
>> + 4, // EfiCpuIoWidthUint32
>> + 8, // EfiCpuIoWidthUint64
>> + 0, // EfiCpuIoWidthFifoUint8
>> + 0, // EfiCpuIoWidthFifoUint16
>> + 0, // EfiCpuIoWidthFifoUint32
>> + 0, // EfiCpuIoWidthFifoUint64
>> + 1, // EfiCpuIoWidthFillUint8
>> + 2, // EfiCpuIoWidthFillUint16
Re: [edk2] [PATCH edk2-platforms 33/39] Silicon/NXP: Implement PciHostBridgeLib support
>-Original Message-
>From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>Sent: Friday, April 20, 2018 2:05 PM
>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>Cc: Leif Lindholm <leif.lindh...@linaro.org>; Kinney, Michael D
><michael.d.kin...@intel.com>; edk2-devel@lists.01.org; Udit Kumar
><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma
><vabhav.sha...@nxp.com>
>Subject: Re: [PATCH edk2-platforms 33/39] Silicon/NXP: Implement
>PciHostBridgeLib support
>
>On 16 February 2018 at 09:50, Meenakshi <meenakshi.aggar...@nxp.com>
>wrote:
>> From: Vabhav <vabhav.sha...@nxp.com>
>>
>> Implement the library that exposes the PCIe root complexes to the
>> generic PCI host bridge driver,Putting SoC Specific low level init
>> code for the RCs.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com>
>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>> ---
>> .../Library/PciHostBridgeLib/PciHostBridgeLib.c| 618
>+
>> .../Library/PciHostBridgeLib/PciHostBridgeLib.inf | 50 ++
>> 2 files changed, 668 insertions(+)
>> create mode 100644
>> Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> create mode 100644
>> Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>>
>> diff --git a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> new file mode 100644
>> index 000..e6f9b7c
>> --- /dev/null
>> +++ b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> @@ -0,0 +1,618 @@
>> +/** @file
>> + PCI Host Bridge Library instance for NXP SoCs
>> +
>> + Copyright 2018 NXP
>> +
>> + 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
>https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou
>rce.org%2Flicenses%2Fbsd-
>license.php=02%7C01%7Cvabhav.sharma%40nxp.com%7C44d0e1dcd1014
>e3dc13308d5a6999342%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
>636598100906238234=sX%2BPAAKHQN41oqF8BnYLdIVKYYLgIMqWBNqPs
>9D3NdE%3D=0.
>> +
>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include #include
>> +#include #include #include
>> +
>> +#include
>> +
>> +#pragma pack(1)
>> +typedef struct {
>> + ACPI_HID_DEVICE_PATH AcpiDevicePath;
>> + EFI_DEVICE_PATH_PROTOCOL EndDevicePath; }
>> +EFI_PCI_ROOT_BRIDGE_DEVICE_PATH; #pragma pack ()
>> +
>> +STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH
>> +mEfiPciRootBridgeDevicePath[] = {
>> + {
>> +{
>> + {
>> +ACPI_DEVICE_PATH,
>> +ACPI_DP,
>> +{
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH)),
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH) >> 8)
>> +}
>> + },
>> + EISA_PNP_ID (0x0A08), // PCI Express
>> + PCI_SEG0_NUM
>> +},
>> +
>> +{
>> + END_DEVICE_PATH_TYPE,
>> + END_ENTIRE_DEVICE_PATH_SUBTYPE,
>> + {
>> +END_DEVICE_PATH_LENGTH,
>> +0
>> + }
>> +}
>> + },
>> + {
>> +{
>> + {
>> +ACPI_DEVICE_PATH,
>> +ACPI_DP,
>> +{
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH)),
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH) >> 8)
>> +}
>> + },
>> + EISA_PNP_ID (0x0A08), // PCI Express
>> + PCI_SEG1_NUM
>> +},
>> +
>> +{
>> + END_DEVICE_PATH_TYPE,
>> + END_ENTIRE_DEVICE_PATH_SUBTYPE,
>> + {
>> +END_DEVICE_PATH_LENGTH,
>> +0
>> + }
>> +}
>> + },
>> + {
>> +{
>> + {
>> +ACPI_DEVICE_PATH,
>> +ACPI_DP,
>> +{
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH)),
>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH) >> 8)
>> +}
>> + },
>> + EISA_PNP_ID (0x0A08), // PCI Express
Re: [edk2] [PATCH edk2-platforms 32/39] Silicon/NXP: Implement PciSegmentLib to support multiple RCs
>-Original Message- >From: Leif Lindholm [mailto:leif.lindh...@linaro.org] >Sent: Friday, April 20, 2018 12:57 AM >To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com> >Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar ><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma ><vabhav.sha...@nxp.com> >Subject: Re: [PATCH edk2-platforms 32/39] Silicon/NXP: Implement >PciSegmentLib to support multiple RCs > >On Fri, Feb 16, 2018 at 02:20:28PM +0530, Meenakshi wrote: >> From: Vabhav <vabhav.sha...@nxp.com> >> >> Multiple root complex support is not provided by standard library >> PciLib/PciExpressLib/PciSegmentLib, Reimplementing it and provide >> function for reading/writing into PCIe configuration Space. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Vabhav <vabhav.sha...@nxp.com> >> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com> >> --- >> Silicon/NXP/Include/Pcie.h | 143 + >> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c | 604 >+ >> .../NXP/Library/PciSegmentLib/PciSegmentLib.inf| 41 ++ >> 3 files changed, 788 insertions(+) >> create mode 100644 Silicon/NXP/Include/Pcie.h create mode 100644 >> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c >> create mode 100644 >> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf >> >> diff --git a/Silicon/NXP/Include/Pcie.h b/Silicon/NXP/Include/Pcie.h >> new file mode 100644 index 000..a7e6f9b >> --- /dev/null >> +++ b/Silicon/NXP/Include/Pcie.h >> @@ -0,0 +1,143 @@ >> +/** @file >> + PCI memory configuration for NXP >> + >> + Copyright 2018 NXP >> + >> + 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 >https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou >rce.org%2Flicenses%2Fbsd- >license.php=02%7C01%7Cvabhav.sharma%40nxp.com%7C081a2ebc43af4a >daaf8e08d5a62b9921%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 >36597628604269440=kt661HfkUD2E3sYswy0I4vVFTV3%2Fq%2FiM%2BiW >egCISP%2BU%3D=0. >> + >> + 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 __PCI_H__ >> +#define __PCI_H__ > >I'm not super happy about reusing such a generic name for the include guard - >or >really even the filename. (MdePkg/Include/Pci.h has >_PCI_H_.) > >Could you rename this header NxpPcie.h and change the include guard to >_NXP_PCIE_H_? I see, Sure. > >> + >> +// Segment 0 >> +#define PCI_SEG0_NUM 0 >> + >> +#define PCI_SEG0_BUSNUM_MIN 0x0 >> +#define PCI_SEG0_BUSNUM_MAX 0xff >> + >> +#define PCI_SEG0_PORTIO_MIN 0x0 >> +#define PCI_SEG0_PORTIO_MAX 0x >> + >> +#define PCI_SEG0_MMIO32_MIN 0x4000 >> +#define PCI_SEG0_MMIO32_MAX 0x4fff >> +#define PCI_SEG0_MMIO64_MIN PCI_SEG0_MMIO_MEMBASE + >SEG_MEM_SIZE >> +#define PCI_SEG0_MMIO64_MAX PCI_SEG0_MMIO_MEMBASE + >SEG_MEM_LIMIT >> +#define PCI_SEG0_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp1BaseAddr) >> + >> +#define PCI_SEG0_DBI_BASE 0x0340 >> + >> +// Segment 1 >> +#define PCI_SEG1_NUM 1 >> + >> +#define PCI_SEG1_BUSNUM_MIN 0x0 >> +#define PCI_SEG1_BUSNUM_MAX 0xff >> + >> +#define PCI_SEG1_PORTIO_MIN 0x1 >> +#define PCI_SEG1_PORTIO_MAX 0x1 >> + >> +#define PCI_SEG1_MMIO32_MIN 0x5000 >> +#define PCI_SEG1_MMIO32_MAX 0x5fff >> +#define PCI_SEG1_MMIO64_MIN PCI_SEG1_MMIO_MEMBASE + >SEG_MEM_SIZE >> +#define PCI_SEG1_MMIO64_MAX PCI_SEG1_MMIO_MEMBASE + >SEG_MEM_LIMIT >> +#define PCI_SEG1_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp2BaseAddr) >> + >> +#define PCI_SEG1_DBI_BASE 0x0350 >> + >> +// Segment 2 >> +#define PCI_SEG2_NUM 2 >> + >> +#define PCI_SEG2_BUSNUM_MIN 0x0 >> +#define PCI_SEG2_BUSNUM_MAX 0xff >> + >> +#define PCI_SEG2_PORTIO_MIN 0x2 >> +#define PCI_SEG2_PORTIO_MAX 0x2 >> + >> +#define PCI_SEG2_MMIO32_MIN 0x6000 >> +#define PCI_SEG2_MMIO32_MAX 0x6f
Re: [edk2] [PATCH edk2-platforms 0/3] Platform/NXP-Added NXP PCI Host Bridge Driver
>-Original Message- >From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >Sent: Friday, December 22, 2017 9:04 PM >To: Vabhav Sharma <vabhav.sha...@nxp.com> >Cc: Leif Lindholm <leif.lindh...@linaro.org>; Kinney, Michael D ><michael.d.kin...@intel.com>; edk2-devel@lists.01.org; Udit Kumar ><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com> >Subject: Re: [PATCH edk2-platforms 0/3] Platform/NXP-Added NXP PCI Host >Bridge Driver > >On 21 December 2017 at 18:48, Vabhav <vabhav.sha...@nxp.com> wrote: >> Following patches will add support of NXP PCI Host Bridge Driver in edk2- >platforms directory 'edk2-platforms/Platform/NXP' >> > >Why do you need a new PciHostBridgeDxe driver? Can't you use the one in >MdeModulePkg instead? Using PciHostbridge dxe driver with changes for multiple(three) host bridge instances with 1:1 mapping for HostBridge:Root bridge(Hb:Rb), I will evaluate MdeModulePkg for Multiple host bridge support > >> Updated Directory structure for added folders in 'edk2- >platforms/Platform/NXP' will be: >> >> Platform/NXP/Drivers/PciHostBridgeDxe/ >> |-- PciHostBridgeDxe.c >> |-- PciHostBridgeDxe.inf >> `-- PciRootBridgeIo.c >> >> Platform/NXP/Library/PciHostBridgeLib/ >> |-- PciCntrl.c >> |-- PciHostBridgeLib.inf >> `-- PciRbLib.c >> > >Please put these in Silicon/NXP, not Platform/NXP Reference is taken from ARM/Hisilicon directory structure , We plan to put only chassis specific code in Silicon/NXP and Drivers, Library in Platform/NXP. Please suggest if there is any specific reason for putting them in Silicon/NXP? > >> In Platform/NXP/Library >> PciHostBridgeLib librady is added >> >> In Platform/NXP/Drivers: >> PciHostBridgeDxe driver is added >> >> Please review and look forward for your support in upstreaming the patches in >edk2-platforms. >> >> Vabhav (3): >> Platform/NXP : Add PCI Host Bridge Libary >> Platform/NXP : Add PCI Host Bridge Driver >> Compilation:Modify dsc,fdf files >> >> .../Drivers/PciHostBridgeDxe/PciHostBridgeDxe.c| 967 >> .../Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf | 61 + >> .../NXP/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c | 1193 > >> Platform/NXP/Include/PciCntrlLib.h | 323 ++ >> Platform/NXP/Include/PciHostBridge.h | 466 >> Platform/NXP/Include/PciLib.h | 414 +++ >> Platform/NXP/Include/PciRootBridge.h | 674 +++ >> Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc | 31 + >> Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf |6 + >> Platform/NXP/Library/PciHostBridgeLib/PciCntrl.c | 628 +++ >> .../Library/PciHostBridgeLib/PciHostBridgeLib.inf | 49 + >> Platform/NXP/Library/PciHostBridgeLib/PciRbLib.c | 331 ++ >> Silicon/NXP/Chassis/Chassis.c | 11 + >> Silicon/NXP/Chassis/Chassis2/SerDes.h | 11 + >> Silicon/NXP/LS1043A/LS1043A.dsc|1 + >> 15 files changed, 5166 insertions(+) >> create mode 100644 >> Platform/NXP/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.c >> create mode 100644 >> Platform/NXP/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf >> create mode 100644 >> Platform/NXP/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c >> create mode 100644 Platform/NXP/Include/PciCntrlLib.h >> create mode 100644 Platform/NXP/Include/PciHostBridge.h >> create mode 100644 Platform/NXP/Include/PciLib.h create mode 100644 >> Platform/NXP/Include/PciRootBridge.h >> create mode 100644 Platform/NXP/Library/PciHostBridgeLib/PciCntrl.c >> create mode 100644 >> Platform/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> create mode 100644 Platform/NXP/Library/PciHostBridgeLib/PciRbLib.c >> >> -- >> 1.9.1 >> ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] SError Exception and HCR_EL2 Usage
Dear Experts, I am facing SError exception during UEFI bring-up. At boot , secure f/w starts in EL3 and loads UEFI image to DDR. After this secure f/w passes control to UEFI in EL2. I debugged and manifest the problem by adding below lines in UEFI PrePi entry point(ModuleEntryPoint.S) ASM_FUNC(_ModuleEntryPoint) +msr daifclr,#4 +isb +mrs x0, hcr_el2 +ldr x1, =0x0800 +orr x0, x0, x1 +msr hcr_el2, x0 +isb Once exception occurs than ELR_EL2 point to 'isb' instruction and ESR_EL2 is SError Exception syndrome. Could you please suggest if this is UEFI problem or Secure f/w issue? Additionally, TGE bit is set in hcr_el2 three times during PrePei phase(ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c),DxeMain(),ArmCpuDxe. Please explain the purpose of setting it or require to be fixed? Regards, Vabhav ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] MTFTP file transfer timeout error
Hi Fu Siyuan,
Thank you for explanation and hope you enjoyed vacation.
I am using 32K block size for file transfer.
IP layer divides the 32K block size into 23 number of fragments(with last
fragment of less size indicating no more fragments)
Using tcpdump and debug code the flow is:
NIC interface receives 23 fragments and pass it to upper layer.
Mtftp4 Client receives the data block 'x' with expected block number 'x+1'
Mtftp4 Client Sends ACK for received data block 'x'
After receiving approximate 900x block, NIC interface receive 22 fragments
instead of 23(One packet drop),At this point function MnpReceivePacket() didn't
receive more packets with status as 'EFI_NOT_READY' with
<>TFTP Server keeps on sending data(23 fragments) block 'x+1'(900) expecting
ACK for data block 'x+1'(900),Whereas
<>Mtftp4 Client retransmit acknowledgement six times for data block 'x'(899)
before going to timeout expecting next data block 'x+1'(900).
Added debug logs in the below code and Mtft4 client didn't receive data once
MnpReceivePacket() status is set to 'EFI_NOT_READY' causing timeout.
Is this expected if one frame is dropped at Ethernet Layer than whole packet
will be discarded at IP layer?
Regards,
Vabhav
-Original Message-
From: Fu, Siyuan [mailto:siyuan...@intel.com]
Sent: Monday, October 09, 2017 7:28 AM
To: Vabhav Sharma <vabhav.sha...@nxp.com>; edk2-devel@lists.01.org
Subject: RE: MTFTP file transfer timeout error
Hi, Vabhav
Sorry for the late response, I just came back from the vacation.
The default retry count for tftp shell command is 6, which means the ACK packet
will be retransmitted 6 times before the code goes to the
Mtftp4CleanOperation() you marked below. The MTFTP driver always saves the last
transmitted packet in Instance->LastPacket, and the Mtftp4Retransmit() will try
to retransmit it if not reach the max retry count.
As you mentioned 1K block size is always success, I guess it's may because the
IP fragment. A MTFTP(UDP) packet which is larger than 1.5K (the default MTU)
will be fragmented to several IP frame. During the transmit, the whole UDP
packet will be discarded by the IP layer if any of the IP fragment is lost in
the network. As a result, using large MTFTP block size will increase the
possibility of timeout in bad network connection.
I suggest you add some debug in Mtftp4RrqInput() in below lines to confirm if
the EFI client can receive the MTFTP packet correctly, also you could use
Wireshark in your server to check whether it receives the ACK from client.
switch (Opcode) {
case EFI_MTFTP4_OPCODE_DATA:
if ((Len > (UINT32) (MTFTP4_DATA_HEAD_LEN + Instance->BlkSize)) ||
(Len < (UINT32) MTFTP4_DATA_HEAD_LEN)) {
goto ON_EXIT;
}
Status = Mtftp4RrqHandleData (Instance, Packet, Len, Multicast,
);
break;
BestRegards
Fu Siyuan
-----Original Message-
From: Vabhav Sharma [mailto:vabhav.sha...@nxp.com]
Sent: Friday, October 6, 2017 1:26 AM
To: Fu, Siyuan <siyuan...@intel.com>; edk2-devel@lists.01.org
Subject: RE: MTFTP file transfer timeout error
Dear Experts,
I traced that timeout error for different block size during file transfer using
tftp(rrq opcode) is returned from function Mtftp4OnTimerTick() TFTP layer in
UEFI network stack.
Mtftp4OnTimerTick()
//
// Retransmit the packet if haven't reach the maxmium retry count,
// otherwise exit the transfer.
//
if (++Instance->CurRetry < Instance->MaxRetry) {
Mtftp4Retransmit (Instance);
Mtftp4SetTimeout (Instance);
} else {
Mtftp4CleanOperation (Instance, EFI_TIMEOUT);//Timeout is set
continue;
}
Once this is set, It gets populated to downloadfile() function in tftp shellpkg
library.
There seems to be issue(corruption) with tfp client state machine -Not sending
ACK for received data blocks -Sending duplicate ACK
If server don't receive ACK, It stops sending data packets and timeout occurs
after maximum retry.
Please suggest if this is new or known issue to be fixed?
Regards,
Vabhav
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Vabhav
Sharma
Sent: Thursday, September 28, 2017 5:04 PM
To: siyuan...@intel.com
Cc: edk2-devel@lists.01.org; edk2-devel <edk2-devel-boun...@lists.01.org>
Subject: Re: [edk2] MTFTP file transfer timeout error
[This sender failed our fraud detection checks and may not be who they appear
to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
Hello Fu Siyuan,
I see that blocksize option with tftp command is introduced with commit
2be45bfe2779043bc3566e879e7ec279412012dc.
Could you please help me clarify with the timeout error behavior observed in
previous mail
Please note the behavior varies for different file type(Attached sheet)
Regards, Vabhav
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists
Re: [edk2] MTFTP file transfer timeout error
Dear Experts,
I traced that timeout error for different block size during file transfer using
tftp(rrq opcode) is returned from function Mtftp4OnTimerTick() TFTP layer in
UEFI network stack.
Mtftp4OnTimerTick()
//
// Retransmit the packet if haven't reach the maxmium retry count,
// otherwise exit the transfer.
//
if (++Instance->CurRetry < Instance->MaxRetry) {
Mtftp4Retransmit (Instance);
Mtftp4SetTimeout (Instance);
} else {
Mtftp4CleanOperation (Instance, EFI_TIMEOUT);//Timeout is set
continue;
}
Once this is set, It gets populated to downloadfile() function in tftp shellpkg
library.
There seems to be issue(corruption) with tfp client state machine
-Not sending ACK for received data blocks
-Sending duplicate ACK
If server don't receive ACK, It stops sending data packets and timeout occurs
after maximum retry.
Please suggest if this is new or known issue to be fixed?
Regards,
Vabhav
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Vabhav
Sharma
Sent: Thursday, September 28, 2017 5:04 PM
To: siyuan...@intel.com
Cc: edk2-devel@lists.01.org; edk2-devel <edk2-devel-boun...@lists.01.org>
Subject: Re: [edk2] MTFTP file transfer timeout error
[This sender failed our fraud detection checks and may not be who they appear
to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
Hello Fu Siyuan,
I see that blocksize option with tftp command is introduced with commit
2be45bfe2779043bc3566e879e7ec279412012dc.
Could you please help me clarify with the timeout error behavior observed in
previous mail
Please note the behavior varies for different file type(Attached sheet)
Regards, Vabhav
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Vabhav
Sharma
Sent: Saturday, September 23, 2017 4:21 PM
To: edk2-devel@lists.01.org; edk2-devel <edk2-devel-boun...@lists.01.org>
Subject: [edk2] MTFTP file transfer timeout error
[This sender failed our fraud detection checks and may not be who they appear
to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
Hi All,
I am facing timeout error with file transfer using tftp on UEFI shell with ARM
based SoCs Command used: tftp -s -i
File transfer with file size greater 50 or 60 MB is returning timeout(Also
depends on type of file like data file, ASCII file, boot sector)
I verified by playing around with blocksize from 32K to 42K for different file
size(100MB,200MB,500MB) and identify that increasing the block size for large
file size helps with successful transfer.
File transfer is always successful with 1K blocksize but file transfer time is
increased.
Please suggest if there is any link between block size with file size or anyone
faced such issue? I assume expectation is to use any blocksize from
512(default) to 64K.
Regards,
Vabhav
___
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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Clarification about InitializeCpuExceptionHandlers() and TGE bit in hcr_el2
Andrew Fish, Yes catching the exception is very critical for debugging. Looks like, This is the model where edk2 starts from cold boot As per ARM Platform documentation(ArmPlatformPkg\Documentation) edk2 starts from either cold boot(SEC Phase) or 2nd stage bootloader(Prepi Phase where Platform ROM code is 1st stage bootloader) and I don't see handlers are installed before dxemain. Does same model(edk2 from 2nd stage bootloader) exist for x86 as well? Regards, Vabhav From: af...@apple.com [mailto:af...@apple.com] Sent: Thursday, October 05, 2017 10:32 PM To: Vabhav Sharma <vabhav.sha...@nxp.com> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; edk2-devel@lists.01.org Subject: Re: [edk2] Clarification about InitializeCpuExceptionHandlers() and TGE bit in hcr_el2 On Oct 5, 2017, at 9:53 AM, Vabhav Sharma <vabhav.sha...@nxp.com<mailto:vabhav.sha...@nxp.com>> wrote: Thanks Andrew Fish, I understand. In PEI Phase, No handlers are installed and there might be pending exception. ExceptionHandlers() can be installed during PEI phase like after initializing the MMU to catch unhandled exception. Please suggest? Vabhav, It looks like for x86 InitializeCpuExceptionHandlers() is called in SEC and then in CPU PEIM calls InitializeCpuExceptionHandlers(). ~/work/src/edk2(master)>git grep InitializeCpuExceptionHandlers ArmPkg/Drivers/CpuDxe/Exception.c:37: InitializeCpuExceptionHandlers(VectorInfo); ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.c:94:InitializeCpuExceptionHandlers( ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.c:242:NOTE: This function should be invoked after InitializeCpuExceptionHandlers() or MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:261: Status = InitializeCpuExceptionHandlers (VectorInfoList); MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h:40:InitializeCpuExceptionHandlers ( MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h:73: NOTE: This function should be invoked after InitializeCpuExceptionHandlers() or MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c:35:InitializeCpuExceptionHandlers ( MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c:74: NOTE: This function should be invoked after InitializeCpuExceptionHandlers() or MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/IA32/SetIdtEntry.c:44: Status = InitializeCpuExceptionHandlers (NULL); MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c:155: Status = InitializeCpuExceptionHandlers (NULL); MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c:249: Status = InitializeCpuExceptionHandlers (NULL); UefiCpuPkg/CpuMpPei/CpuMpPei.c:447: Status = InitializeCpuExceptionHandlers (VectorInfo); UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h:158:InitializeCpuExceptionHandlersWorker ( UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:62:InitializeCpuExceptionHandlers ( UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:69: return InitializeCpuExceptionHandlersWorker (VectorInfo, ); UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:175: NOTE: This function should be invoked after InitializeCpuExceptionHandlers() or UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c:88:InitializeCpuExceptionHandlers ( UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c:105: Status = InitializeCpuExceptionHandlersWorker (VectorInfo, ExceptionHandlerData); UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c:156: NOTE: This function should be invoked after InitializeCpuExceptionHandlers() or UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:203:InitializeCpuExceptionHandlersWorker ( UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c:64:InitializeCpuExceptionHandlers ( UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c:155: NOTE: This function should be invoked after InitializeCpuExceptionHandlers() or UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c:62:InitializeCpuExceptionHandlers ( UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c:69: return InitializeCpuExceptionHandlersWorker (VectorInfo, ); UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c:104: NOTE: This function should be invoked after InitializeCpuExceptionHandlers() or UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c:646:Status = InitializeCpuExceptionHandlers (NULL); UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c:157: Status = InitializeCpuExceptionHandlers (NULL); UefiCpuPkg/SecCore/SecMain.c:184: Status = InitializeCpuExceptionHandlers (NULL); At least on x86 the exceptions don't tend to pend, they just happen. So for example an ASSERT() macro can map to an INT 3 (Breakpoint) on x86, you can get GP faults for accessing a non-cononical address, etc. So catching the exception and printing out the PC, and stack trace if possible is very useful for debugging. Thanks, Andrew Fish Dear Arm Folks, I request you to comment on hcr_el2 usage
Re: [edk2] Clarification about InitializeCpuExceptionHandlers() and TGE bit in hcr_el2
Thanks Andrew Fish, I understand. In PEI Phase, No handlers are installed and there might be pending exception. ExceptionHandlers() can be installed during PEI phase like after initializing the MMU to catch unhandled exception. Please suggest? Dear Arm Folks, I request you to comment on hcr_el2 usage mentioned in below email I understand that Enabling TGE bit will route the EL1 exception to EL2.Is there any EL1 code during UEFI execution? Regards, Vabhav -Original Message- From: af...@apple.com [mailto:af...@apple.com] Sent: Thursday, September 28, 2017 7:31 PM To: Vabhav Sharma <vabhav.sha...@nxp.com> Cc: edk2-devel@lists.01.org; edk2-devel <edk2-devel-boun...@lists.01.org> Subject: Re: [edk2] Clarification about InitializeCpuExceptionHandlers() and TGE bit in hcr_el2 > On Sep 28, 2017, at 4:23 AM, Vabhav Sharma <vabhav.sha...@nxp.com> wrote: > > Hi All, > > I see that InitializeCpuExceptionHandlers() is called from DxeMain to take > over exception handlers and later from ArmCpuDxe. > Is there any specific purpose to call it from two places during dxe phase? > Vabhav, DxeMain is the DXE Core and that is like (micro) kernel and it is platform agnostic code. InitializeCpuExceptionHandlers() exists in that location to catch unhandled exceptions, especially in the case when no debugger stub is linked in. The CPU Dxe driver abstracts CPU specifics from the DXE Core and it adds supports for interrupts, cachability, etc. and the DXE Core uses services from this driver to abstract CPU implementation. To make things even more complex on some platforms PEI and DXE run in entirely different modes. For example on x86 is is common for PEI to be 32-bit and and DXE to be 64-bit. This is mostly due to how complex it is to turn on memory, and the fact that there is no good place to put the page tables prior to memory init. I'll let the ARM folks comment on hcr_el2 usage. Thanks, Andrew Fish > Additionally we are setting TGE bit three times in hcr_el2 during PrePei > phase(ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c) > and Twice in Dxe phase: dxemain(),ArmCpuDxe > > Please help to clarify or required to be fixed? > > Regards, > Vabhav > ___ > 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] FW: MTFTP file transfer timeout error
Hello Fu Siyuan, I see that blocksize option with tftp command is introduced with commit 2be45bfe2779043bc3566e879e7ec279412012dc. Could you please help me clarify with the timeout error behavior observed in previous mail Please note the behavior varies for different file type(Attached sheet). Regards, Vabhav -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Vabhav Sharma Sent: Saturday, September 23, 2017 4:21 PM To: edk2-devel@lists.01.org; edk2-devel <edk2-devel-boun...@lists.01.org> Subject: [edk2] MTFTP file transfer timeout error [This sender failed our fraud detection checks and may not be who they appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing] Hi All, I am facing timeout error with file transfer using tftp on UEFI shell with ARM based SoCs Command used: tftp -s -i File transfer with file size greater 50 or 60 MB is returning timeout(Also depends on type of file like data file, ASCII file, boot sector) I verified by playing around with blocksize from 32K to 42K for different file size(100MB,200MB,500MB) and identify that increasing the block size for large file size helps with successful transfer. File transfer is always successful with 1K blocksize but file transfer time is increased. Please suggest if there is any link between block size with file size or anyone faced such issue? I assume expectation is to use any blocksize from 512(default) to 64K. Regards, Vabhav ___ 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] Recall: MTFTP file transfer timeout error
Vabhav Sharma would like to recall the message, "MTFTP file transfer timeout error". ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] MTFTP file transfer timeout error
Hello Fu Siyuan, I see that blocksize option with tftp command is introduced with commit 2be45bfe2779043bc3566e879e7ec279412012dc. Could you please help me clarify with the timeout error behavior observed in previous mail Please note the behavior varies for different file type(Attached sheet) Regards, Vabhav -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Vabhav Sharma Sent: Saturday, September 23, 2017 4:21 PM To: edk2-devel@lists.01.org; edk2-devel <edk2-devel-boun...@lists.01.org> Subject: [edk2] MTFTP file transfer timeout error [This sender failed our fraud detection checks and may not be who they appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing] Hi All, I am facing timeout error with file transfer using tftp on UEFI shell with ARM based SoCs Command used: tftp -s -i File transfer with file size greater 50 or 60 MB is returning timeout(Also depends on type of file like data file, ASCII file, boot sector) I verified by playing around with blocksize from 32K to 42K for different file size(100MB,200MB,500MB) and identify that increasing the block size for large file size helps with successful transfer. File transfer is always successful with 1K blocksize but file transfer time is increased. Please suggest if there is any link between block size with file size or anyone faced such issue? I assume expectation is to use any blocksize from 512(default) to 64K. Regards, Vabhav ___ 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] Clarification about InitializeCpuExceptionHandlers() and TGE bit in hcr_el2
Hi All, I see that InitializeCpuExceptionHandlers() is called from DxeMain to take over exception handlers and later from ArmCpuDxe. Is there any specific purpose to call it from two places during dxe phase? Additionally we are setting TGE bit three times in hcr_el2 during PrePei phase(ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c) and Twice in Dxe phase: dxemain(),ArmCpuDxe Please help to clarify or required to be fixed? Regards, Vabhav ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] MTFTP file transfer timeout error
Hi All, I am facing timeout error with file transfer using tftp on UEFI shell with ARM based SoCs Command used: tftp -s -i File transfer with file size greater 50 or 60 MB is returning timeout(Also depends on type of file like data file, ASCII file, boot sector) I verified by playing around with blocksize from 32K to 42K for different file size(100MB,200MB,500MB) and identify that increasing the block size for large file size helps with successful transfer. File transfer is always successful with 1K blocksize but file transfer time is increased. Please suggest if there is any link between block size with file size or anyone faced such issue? I assume expectation is to use any blocksize from 512(default) to 64K. Regards, Vabhav ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] MTFTP file transfer timeout error
Hi All, I am facing timeout error with file transfer using tftp on UEFI shell with ARM based SoCs Command used: tftp -s -i File transfer with file size greater 50 or 60 MB is returning timeout(Also depends on type of file like data file, ASCII file, boot sector) I verified by playing around with blocksize from 32K to 42K for different file size(100MB,200MB,500MB) and identify that increasing the block size for large file size helps with successful transfer. File transfer is always successful with 1K blocksize but file transfer time is increased. Please suggest if there is any link between block size with file size or anyone faced such issue? I assume expectation is to use any blocksize from 512(default) to 64K. Regards, Vabhav ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
