Re: [edk2-devel] [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec

2022-07-07 Thread Jon Nettleton
On Wed, Jul 6, 2022 at 11:57 AM Sami Mujawar  wrote:
>
> Bugzilla: 3458 - Add support IORT Rev E.d specification updates
>   (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
>
> The IO Remapping Table, Platform Design Document, Revision E.d,
> Feb 2022 (https://developer.arm.com/documentation/den0049/)
> introduces the following updates, collectively including the
> updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
>   - increments the IORT table revision to 5.
>   - updates the node definition to add an 'Identifier' field.
>   - adds definition of node type 6 - Reserved Memory Range node.
>   - adds definition for Memory Range Descriptors.
>   - adds flag to indicate PRI support for root complexes.
>   - adds flag to indicate if the root complex supports forwarding
> of PASID information on translated transactions to the SMMU.
>   - adds flag to indicate if the root complex supports PASID.
>   - adds flags to define access privilege and attributes for the
> memory ranges.
>
> Therefore, update the IORT header file to reflect these changes,
> and also rename the EFI_ACPI_IO_REMAPPING_TABLE_REVISION macro to
> EFI_ACPI_IO_REMAPPING_TABLE_REV0.
>
> Signed-off-by: Sami Mujawar 
> ---
>
> Notes:
> v4:
>   - Updated patch series to IORT specification revision E.d. [SAMI]
>   - Add flag to indicate if the root complex supports PASID. [SAMI]
>   - Add flags to define access privilege and attributes for  [SAMI]
> the memory ranges.
> v3:
>   - Submit patch series to update platform code to use the   [LIMING]
> EFI_ACPI_IO_REMAPPING_TABLE_REV0 macro.
> Ref: https://edk2.groups.io/g/devel/topic/83618423#76799
>   - Removed definition of EFI_ACPI_IO_REMAPPING_TABLE_REVISION   [SAMI]
> as EFI_ACPI_IO_REMAPPING_TABLE_REV0 has been provided for
> representing Rev 0. Also, a corresponding patch series
> for updating the platforms in edk2-platforms repository
> shall be submitted to the edk2 mailing list.
>   - Include r-b received from v2 series. [SAMI]
> Ref: https://edk2.groups.io/g/devel/topic/83600724#76660
>
> v2:
>   - Set EFI_ACPI_IO_REMAPPING_TABLE_REVISION to Rev 0 as [SAMI]
> setting to Rev 3 will break existing platforms. The
> problem is that existing code would not be populating
> the Identifier field in the nodes. This can lead to
> non-unique values in the Identifier field.
>
>  MdePkg/Include/IndustryStandard/IoRemappingTable.h | 83 ++--
>  1 file changed, 75 insertions(+), 8 deletions(-)
>
> diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h 
> b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> index 
> 79a34678681d45b2982dc8573db6bd447f42e429..07cb7f49dc936fb00cc549113f1e62f988535e5d
>  100644
> --- a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> @@ -1,12 +1,19 @@
>  /** @file
> -  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049D
> -
> -  
> http://infocenter.arm.com/help/topic/com.arm.doc.den0049d/DEN0049D_IO_Remapping_Table.pdf
> +  ACPI IO Remapping Table (IORT) definitions.
>
>Copyright (c) 2017, Linaro Limited. All rights reserved.
> -  Copyright (c) 2018, ARM Limited. All rights reserved.
> +  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.
>
>SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - IO Remapping Table, Platform Design Document, Revision E.d, Feb 2022
> +(https://developer.arm.com/documentation/den0049/)
> +
> +  @par Glossary:
> +  - Ref  : Reference
> +  - Mem  : Memory
> +  - Desc : Descriptor
>  **/
>
>  #ifndef __IO_REMAPPING_TABLE_H__
> @@ -14,7 +21,8 @@
>
>  #include 
>
> -#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION  0x0
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REV0  0x0
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REV5  0x5
>
>  #define EFI_ACPI_IORT_TYPE_ITS_GROUP 0x0
>  #define EFI_ACPI_IORT_TYPE_NAMED_COMP0x1
> @@ -22,6 +30,7 @@
>  #define EFI_ACPI_IORT_TYPE_SMMUv1v2  0x3
>  #define EFI_ACPI_IORT_TYPE_SMMUv30x4
>  #define EFI_ACPI_IORT_TYPE_PMCG  0x5
> +#define EFI_ACPI_IORT_TYPE_RMR   0x6
>
>  #define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA  BIT0
>
> @@ -55,7 +64,29 @@
>  #define EFI_ACPI_IORT_SMMUv3_MODEL_CAVIUM_CN99XX 0x2
>
>  #define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
> -#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED0x1
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTEDBIT0
> +
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PRI_UNSUPPORTED  0x0
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PRI_SUPPORTEDBIT1
> +
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_FWD_UNSUPPORTED  0x0
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_FWD_SUPPORTEDBIT2
> +
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_UNSUPPORTED  0x0
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_SUPPORTED

Re: [edk2-devel] [edk2-platforms PATCH 0/4] ACPI MDIO support for Marvell SoCs

2021-07-12 Thread Jon Nettleton
On Mon, Jul 12, 2021 at 12:52 PM Marcin Wojtas  wrote:
>
> Hi,
>
> wt., 29 cze 2021 o 16:17 Marcin Wojtas  napisał(a):
> >
> > Hi Leif,
> >
> > pon., 14 cze 2021 o 23:55 Leif Lindholm  napisał(a):
> > >
> > > Hi Marcin,
> > >
> > > On Sun, Jun 13, 2021 at 20:16:27 +0200, Marcin Wojtas wrote:
> > > > Hi,
> > > >
> > > > The MDIO ACPI binding has been established and merged to the
> > > > Linux tree,
> > >
> > > Congratulations! :)
> > >
> > > Is FreeBSD expected to follow suit?
> >
> > There's no driver yet, but once it's finally created I will make sure
> > it supports ACPI properly.
> >
> > >
> > > > hence it is now possible to update the ACPI
> > > > description of the platforms that base on the Marvell SoCs.
> > > >
> > > > For convenience, the code is exposed in the public github branch:
> > > > https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/acpi-mdio-r20210613
> > > > There is also MacchiatoBin firmware binary avaialable for testing:
> > > > https://drive.google.com/file/d/1eigP_aeM4wYQpEaLAlQzs3IN_w1-kQr0
> > > >
> > > > I'm looking forward to the comments or remarks.
> > >
> > > The patches themselves look straightforward enough.
> > > I *would* prefer some tested-by, for these sources rather than the
> > > binary, before merging though.
> > >
> >
> > I have some our patches queued, that are blocked by this patchset. In
> > case no time is found for external testers - if this may help to get
> > it pushed through, please see below logs from the next-20210628 tag
> > and unchanged firmware. All network ports of MacchiatoBin and
> > CN913x-DB work properly, with full 1G/10G PHY support via X/MDIO
> > interfaces:
> >
> > MacchiatoBin
> > # uname -a
> > Linux buildroot 5.13.0-rc7-next-20210628 #6 SMP PREEMPT Tue Jun 29
> > 09:14:07 CEST 2021 aarch64 GNU/Linux
> > # dmesg | grep MRVL0101
> > [1.829659] mv88x3340 MRVL0101:00-mii:00: Firmware version 0.3.3.0
> > [1.839622] mv88x3340 MRVL0101:00-mii:08: Firmware version 0.3.3.0
> > [2.748351] mvpp2 MRVL0110:00 eth1: PHY [MRVL0101:00-mii:00] driver
> > [mv88x3340] (irq=POLL)
> > [2.767479] mvpp2 MRVL0110:01 eth2: PHY [MRVL0101:00-mii:08] driver
> > [mv88x3340] (irq=POLL)
> > # dmesg | grep MRVL0100
> > [2.919424] mvpp2 MRVL0110:01 eth3: PHY [MRVL0100:00-mii:00] driver
> > [Marvell 88E1510] (irq=POLL)
> > # dmesg | grep mvpp2
> > [...]
> > [2.748351] mvpp2 MRVL0110:00 eth1: PHY [MRVL0101:00-mii:00] driver
> > [mv88x3340] (irq=POLL)
> > [2.756701] mvpp2 MRVL0110:00 eth1: configuring for phy/10gbase-r link 
> > mode
> > [2.767479] mvpp2 MRVL0110:01 eth2: PHY [MRVL0101:00-mii:08] driver
> > [mv88x3340] (irq=POLL)
> > [2.775834] mvpp2 MRVL0110:01 eth2: configuring for phy/10gbase-r link 
> > mode
> > [2.919424] mvpp2 MRVL0110:01 eth3: PHY [MRVL0100:00-mii:00] driver
> > [Marvell 88E1510] (irq=POLL)
> > [2.928285] mvpp2 MRVL0110:01 eth3: configuring for phy/sgmii link mode
> > [2.936351] mvpp2 MRVL0110:01 eth4: configuring for
> > inband/2500base-x link mode
> > [5.987259] mvpp2 MRVL0110:01 eth3: Link is Up - 1Gbps/Full - flow
> > control off
> > #
> >
> > CN913x-DB
> > # uname -a
> > Linux buildroot 5.13.0-rc7-next-20210628 #6 SMP PREEMPT Tue Jun 29
> > 09:14:07 CEST 2021 aarch64 GNU/Linux
> > # dmesg | grep MRVL0100
> > [2.621201] mvpp2 MRVL0110:00 eth2: PHY [MRVL0100:00-mii:00] driver
> > [Marvell 88E1510] (irq=POLL)
> > [2.741199] mvpp2 MRVL0110:00 eth3: PHY [MRVL0100:00-mii:01] driver
> > [Marvell 88E1510] (irq=POLL)
> > # dmesg | grep mvpp2
> > [...]
> > [2.544917] mvpp2 MRVL0110:00 eth1: configuring for
> > inband/10gbase-r link mode
> > [2.552480] mvpp2 MRVL0110:00 eth1: Link is Up - 10Gbps/Full - flow
> > control rx
> > [2.621201] mvpp2 MRVL0110:00 eth2: PHY [MRVL0100:00-mii:00] driver
> > [Marvell 88E1510] (irq=POLL)
> > [2.630060] mvpp2 MRVL0110:00 eth2: configuring for phy/rgmii-id link 
> > mode
> > [2.741199] mvpp2 MRVL0110:00 eth3: PHY [MRVL0100:00-mii:01] driver
> > [Marvell 88E1510] (irq=POLL)
> > [2.750056] mvpp2 MRVL0110:00 eth3: configuring for phy/rgmii-id link 
> > mode
> > [2.810169] mvpp2 MRVL0110:01 eth4: configuring for
> > inband/10gbase-r link mode
> > [2.817471] mvpp2 MRVL0110:01 eth4: Link is Up - 10Gbps/Full - flow
> > control rx
> > [5.693231] mvpp2 MRVL0110:00 eth2: Link is Up - 1Gbps/Full - flow
> > control off
> > [   10.840942] mvpp2 MRVL0110:00 eth1: Link is Down
> > [   10.864124] mvpp2 MRVL0110:01 eth4: Link is Down
> > #
> >
>
> Both platforms were have been additionally tested by Greg, do you have
> any comments/objections to merging this patchset?
>
> Thanks,
> Marcin

You can add my Tested-by as well.  Finally got time over the weekend
to verify on all my Marvell platforms this effects.

-Jon


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77692): https://edk2.groups.io/g/devel/message/77692
Mute This Topic: https://groups.io/mt/83513508/21656
Group 

Re: [edk2-devel] [edk2-platforms PATCH 0/4] ACPI MDIO support for Marvell SoCs

2021-06-14 Thread Jon Nettleton
On Mon, Jun 14, 2021 at 11:55 PM Leif Lindholm  wrote:
>
> Hi Marcin,
>
> On Sun, Jun 13, 2021 at 20:16:27 +0200, Marcin Wojtas wrote:
> > Hi,
> >
> > The MDIO ACPI binding has been established and merged to the
> > Linux tree,
>
> Congratulations! :)
>
> Is FreeBSD expected to follow suit?
>
> > hence it is now possible to update the ACPI
> > description of the platforms that base on the Marvell SoCs.
> >
> > For convenience, the code is exposed in the public github branch:
> > https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/acpi-mdio-r20210613
> > There is also MacchiatoBin firmware binary avaialable for testing:
> > https://drive.google.com/file/d/1eigP_aeM4wYQpEaLAlQzs3IN_w1-kQr0
> >
> > I'm looking forward to the comments or remarks.
>
> The patches themselves look straightforward enough.
> I *would* prefer some tested-by, for these sources rather than the
> binary, before merging though.

I will get the ACPI changes from Marcin and put a Tested-By on
from SolidRun's side.

Jon

>
> Best Regards,
>
> Leif
>
> > Best regards,
> > Marcin
> >
> > Marcin Wojtas (4):
> >   SolidRun/Armada80x0McBin: Add ACPI MDIO description
> >   Marvell/Cn913xDb: Add ACPI MDIO description
> >   Marvell/Armada70x0Db: Add ACPI MDIO description
> >   Marvell/Armada80x0Db: Add ACPI MDIO description
> >
> >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl| 24 
> > +
> >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl| 38 
> > ++
> >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl | 53 
> > 
> >  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl |  1 +
> >  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl | 24 
> > +
> >  5 files changed, 140 insertions(+)
> >
> > --
> > 2.29.0
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76498): https://edk2.groups.io/g/devel/message/76498
Mute This Topic: https://groups.io/mt/83513508/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] SR-IOV setup in edk2

2021-03-25 Thread Jon Nettleton
On Mon, Mar 22, 2021 at 8:04 PM Laszlo Ersek  wrote:
>
> On 03/21/21 13:46, Jon Nettleton wrote:
> > I am looking for some example code, or direction in how SR-IOV
> > functions are expected to be picked up by the platform pcie host
> > library so it can setup required bits like LUTs.  Currently the
> > HostLib is getting the gEfiPciIoProtocolGuid event and then setting
> > the controller up based on the BDF.  I see that PciScanBus is
> > detecting the VF's and calling PciAllocateBusNumber().
> >
> > PCI-IOV ScanBus - SubBusNumber - 0x2
> > PciBus: Discovered PPB @ [00|00|00]
> >
> > However my setup function is only ever triggered with the real device
> > BDF's...
>
> What do you mean by "setup function"?

Well currently we followed the methodology of most the other
PCI Host Bus drivers and as I said above get the Io event and then
finish the completion of the device setup.  This includes setting up
the Luts, adding the device to the SMMU tables, and a few other
operations.

The VFs for SR-IOV are different as they are treated as hot
pluggable devices in both edk2 and linux, however there is still
setup that needs to be done to the host controller to support the
VFs on the bus.  The PciScanBus is obviously partially wired to
do some enumeration for PCI-IOV but I am unclear where the
platform code should hook into this for the additional proving
and setup.

I have now implemented a driver connected to gEfiPciPlatformProtocolGuid
and I can get events passed in for the platform setup hooks, is this where
we should be doing the bus specific setup, rather than catching
gEfiPciIoProtocolGuid?

>
> If you have a UEFI driver that follows the UEFI driver model, i.e. it
> installs at least one instance of the Driver Binding protocol, then it's
> up to platform BDS to call ConnectController() on those devices that
> should be connected per platform policy.
>
> > Is it up to me to check for the VFs if SR-IOV is enabled and
> > set them up manually?  Is there another event I should be listening to
> > that will be triggered for VF's vs PF's?
>
> My guess is that platform BDS does not try to connect the PciIo
> instances that stand for VFs to any drivers.
>
> In the UEFI shell, try running
>
>   dh -d -v -p PciIo
>
> Pick a handle ID that appears to stand for a VF, then run

No handles are created for the VFs as they aren't setup yet.

Thanks,
Jon

>
>   connect 
>
> Thanks
> Laszlo
>
> >
> > Any pointers would be very helpful.
> >
> > -Jon
> >
> >
> > 
> >
> >
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73262): https://edk2.groups.io/g/devel/message/73262
Mute This Topic: https://groups.io/mt/81498733/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] SR-IOV setup in edk2

2021-03-21 Thread Jon Nettleton
I am looking for some example code, or direction in how SR-IOV
functions are expected to be picked up by the platform pcie host
library so it can setup required bits like LUTs.  Currently the
HostLib is getting the gEfiPciIoProtocolGuid event and then setting
the controller up based on the BDF.  I see that PciScanBus is
detecting the VF's and calling PciAllocateBusNumber().

PCI-IOV ScanBus - SubBusNumber - 0x2
PciBus: Discovered PPB @ [00|00|00]

However my setup function is only ever triggered with the real device
BDF's...Is it up to me to check for the VFs if SR-IOV is enabled and
set them up manually?  Is there another event I should be listening to
that will be triggered for VF's vs PF's?

Any pointers would be very helpful.

-Jon


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73056): https://edk2.groups.io/g/devel/message/73056
Mute This Topic: https://groups.io/mt/81498733/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/VariableRuntimeDxe: avoid double VA conversion of FVB protocol

2021-03-13 Thread Jon Nettleton
; +EfiConvertPointer (0x0, (VOID **));
> +EfiConvertPointer (0x0, (VOID **));
> +EfiConvertPointer (0x0, (VOID **));
> +EfiConvertPointer (0x0, (VOID **));
> +EfiConvertPointer (0x0, (VOID **));
> +EfiConvertPointer (0x0, (VOID **));
> +EfiConvertPointer (0x0, (VOID **));
>  EfiConvertPointer (0x0, (VOID **) >FvbInstance);
>}
>EfiConvertPointer (0x0, (VOID **) 
> >PlatformLangCodes);
> @@ -454,6 +483,13 @@ FtwNotificationEvent (
>}
>mVariableModuleGlobal->FvbInstance = FvbProtocol;
>
> +  //
> +  // Store the boot time values of the function pointers so we can compare
> +  // them later. This is needed to avoid double conversion during the call
> +  // to SetVirtualAddressMap.
> +  //
> +  CopyMem (, FvbProtocol, sizeof mFvbProtocolShadow);
> +
>//
>// Mark the variable storage region of the FLASH as RUNTIME.
>//
> --
> 2.30.1
>
You can add my

Tested-By: Jon Nettleton 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72740): https://edk2.groups.io/g/devel/message/72740
Mute This Topic: https://groups.io/mt/81292874/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] Conflicting virtual addresses causing Runtime Services issues

2021-03-11 Thread Jon Nettleton
On Fri, Mar 12, 2021 at 4:02 AM Jon Nettleton via groups.io
 wrote:
>
> On Thu, Mar 11, 2021 at 11:39 PM Ard Biesheuvel  wrote:
> >
> > On Thu, 11 Mar 2021 at 23:25, Laszlo Ersek  wrote:
> > >
> > > Adding Ard and Leif, comments below:
> > >
> > > On 03/11/21 15:50, Laszlo Ersek wrote:
> > > > On 03/11/21 10:48, Jon Nettleton wrote:
> > >
> > > [...]
> > >
> > > >> And this is where the pointer gets remapped again and into the MMIO
> > > >> space of the nor flash.  If I remove the calls to ConvertPointer for
> > > >> the FvbProtocol I am still seeing those addresses getting remapped
> > > >> but only once and runtime works as expected.
> > > >>
> > > >> I am seeing that in
> > > >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> > > >> >FvbInstance->* are all being converted.  It
> > > >> is possible this is a long standing bug and it just so happens that
> > > >> our configuration has caused a conflict and exposed it.
> > > >
> > > > Yes, this is curious, I noticed it too yesterday, trying to see where
> > > > the FVB protocol member function pointers were converted. I found that
> > > > OVMF's flash driver (OvmfPkg/QemuFlashFvbServicesRuntimeDxe) didn't do
> > > > it, but MdeModulePkg/Universal/Variable/RuntimeDxe did. That was
> > > > certainly strange, as the variable driver is a consumer of the
> > > > protocol (not the producer thereof), so I'd say it has no business
> > > > poking new values into the protocol interface structure.
> > >
> > > [...]
> > >
> > > > ... Strangely, the other flash (FVB) driver in edk2,
> > > > ArmPlatformPkg/Drivers/NorFlashDxe, *does* perform the conversion
> > > > itself! See NorFlashVirtualNotifyEvent().
> > > >
> > > > I don't understand that. Is it possible that, with
> > > > "ArmPlatformPkg/Drivers/NorFlashDxe" too, the conversion happens
> > > > *twice*, but (at least) one of those mappings is "identity"?
> > >
> > > Confirmed.
> > >
> > > I had to write some elaborate debug patches for determining this,
> > > because in ArmVirtQemu, I cannot produce DEBUG output from the
> > > SetVirtualAddressMap() notification functions. So here's the approach I
> > > took:
> > >
> > > (1) Introduce a new GUID-ed HOB structure in MdeModulePkg. The structure
> > > itself lives in reserved memory, but its address is exposed in a GUID-ed
> > > HOB. The structure is named FVB_ADDRESS_LIST, and it has the following
> > > fields:
> > >
> > >   - signature ("FVBADRLS" -- FVB Address List)
> > >   - 16 entries of:
> > > - owner signature [what driver set this entry]
> > > - address
> > >   - number of entries used (aka next entry to fill)
> > >
> > > (2) In PlatformPei, allocate and initialize this structure (in reserved
> > > memory), and expose its address via the GUID-ed HOB. Furthermore,
> > > produce a log message with the allocation address.
> > >
> > > (3) In NorFlashDxe, look up the structure via the GUID-ed HOB, in the
> > > entry point function; remember the address in a global variable. In the
> > > SetVirtualAddressMap() handler function, treat the conversion of the
> > > "GetPhysicalAddress" FVB member function specially: via the global
> > > variable pointer to FVB_ADDRESS_LIST in reserved memory, save both the
> > > physical (original) and the virtual (converted) address of the
> > > "GetPhysicalAddress" FVB member function, in new entries. As owner
> > > signature in both entries, use "NORFLASH".
> > >
> > > (4) In the runtime DXE variable driver, do the exact same thing, just
> > > use a different "owner signature" -- "VARIABLE".
> > >
> > > (5) Once the guest is up and running, run "efibootmgr --delete-timeout"
> > > at a root prompt in the guest, deleting the existent "Timeout" UEFI
> > > non-volatile variable, for verifying that the runtime variable (write)
> > > service is functional.
> > >
> > > (6) Using the log message from point (2):
> > >
> > > > PlatformPeim: FvbAddressList @ 13FEC9000
> > >
> > > hexdump the guest memory containing the FVB_ADDRESS_LIST, as follows:
> > >
> > >

Re: [edk2-devel] Conflicting virtual addresses causing Runtime Services issues

2021-03-11 Thread Jon Nettleton
On Thu, Mar 11, 2021 at 11:39 PM Ard Biesheuvel  wrote:
>
> On Thu, 11 Mar 2021 at 23:25, Laszlo Ersek  wrote:
> >
> > Adding Ard and Leif, comments below:
> >
> > On 03/11/21 15:50, Laszlo Ersek wrote:
> > > On 03/11/21 10:48, Jon Nettleton wrote:
> >
> > [...]
> >
> > >> And this is where the pointer gets remapped again and into the MMIO
> > >> space of the nor flash.  If I remove the calls to ConvertPointer for
> > >> the FvbProtocol I am still seeing those addresses getting remapped
> > >> but only once and runtime works as expected.
> > >>
> > >> I am seeing that in
> > >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> > >> >FvbInstance->* are all being converted.  It
> > >> is possible this is a long standing bug and it just so happens that
> > >> our configuration has caused a conflict and exposed it.
> > >
> > > Yes, this is curious, I noticed it too yesterday, trying to see where
> > > the FVB protocol member function pointers were converted. I found that
> > > OVMF's flash driver (OvmfPkg/QemuFlashFvbServicesRuntimeDxe) didn't do
> > > it, but MdeModulePkg/Universal/Variable/RuntimeDxe did. That was
> > > certainly strange, as the variable driver is a consumer of the
> > > protocol (not the producer thereof), so I'd say it has no business
> > > poking new values into the protocol interface structure.
> >
> > [...]
> >
> > > ... Strangely, the other flash (FVB) driver in edk2,
> > > ArmPlatformPkg/Drivers/NorFlashDxe, *does* perform the conversion
> > > itself! See NorFlashVirtualNotifyEvent().
> > >
> > > I don't understand that. Is it possible that, with
> > > "ArmPlatformPkg/Drivers/NorFlashDxe" too, the conversion happens
> > > *twice*, but (at least) one of those mappings is "identity"?
> >
> > Confirmed.
> >
> > I had to write some elaborate debug patches for determining this,
> > because in ArmVirtQemu, I cannot produce DEBUG output from the
> > SetVirtualAddressMap() notification functions. So here's the approach I
> > took:
> >
> > (1) Introduce a new GUID-ed HOB structure in MdeModulePkg. The structure
> > itself lives in reserved memory, but its address is exposed in a GUID-ed
> > HOB. The structure is named FVB_ADDRESS_LIST, and it has the following
> > fields:
> >
> >   - signature ("FVBADRLS" -- FVB Address List)
> >   - 16 entries of:
> > - owner signature [what driver set this entry]
> > - address
> >   - number of entries used (aka next entry to fill)
> >
> > (2) In PlatformPei, allocate and initialize this structure (in reserved
> > memory), and expose its address via the GUID-ed HOB. Furthermore,
> > produce a log message with the allocation address.
> >
> > (3) In NorFlashDxe, look up the structure via the GUID-ed HOB, in the
> > entry point function; remember the address in a global variable. In the
> > SetVirtualAddressMap() handler function, treat the conversion of the
> > "GetPhysicalAddress" FVB member function specially: via the global
> > variable pointer to FVB_ADDRESS_LIST in reserved memory, save both the
> > physical (original) and the virtual (converted) address of the
> > "GetPhysicalAddress" FVB member function, in new entries. As owner
> > signature in both entries, use "NORFLASH".
> >
> > (4) In the runtime DXE variable driver, do the exact same thing, just
> > use a different "owner signature" -- "VARIABLE".
> >
> > (5) Once the guest is up and running, run "efibootmgr --delete-timeout"
> > at a root prompt in the guest, deleting the existent "Timeout" UEFI
> > non-volatile variable, for verifying that the runtime variable (write)
> > service is functional.
> >
> > (6) Using the log message from point (2):
> >
> > > PlatformPeim: FvbAddressList @ 13FEC9000
> >
> > hexdump the guest memory containing the FVB_ADDRESS_LIST, as follows:
> >
> > > $ virsh qemu-monitor-command aavmf.rhel7.registered --hmp xp  /268cb 
> > > 0x13FEC9000
> >
> > Ccomments to the right of the hexdump:
> >
> > > 00013fec9000: 'F' 'V' 'B' 'A' 'D' 'R' 'L' 'S' 
> > > <- structure signature: FVBADRLS
> > > 00013fec9008: 'N' 'O' 'R' 'F' 'L' 'A' 'S' 'H' 
> > > <- entry[0], signature: NORFLASH
> > > 00013fec9010: 'T' ' ' '

Re: [edk2-devel] Conflicting virtual addresses causing Runtime Services issues

2021-03-11 Thread Jon Nettleton
On Thu, Mar 11, 2021 at 3:51 PM Laszlo Ersek  wrote:
>
> On 03/11/21 10:48, Jon Nettleton wrote:
> > On Thu, Mar 11, 2021 at 7:54 AM Jon Nettleton via groups.io
> >  wrote:
> >>
> >> On Wed, Mar 10, 2021 at 3:52 PM Laszlo Ersek  wrote:
> >>>
> >>> On 03/10/21 09:04, Jon Nettleton wrote:
> >>>> I am debugging a failure that I am seeing while using the HoneyComb's
> >>>> spi-nor flash for runtime variable storage.  I am hoping someone on
> >>>> the list can give me some insight as what may be the problem.
> >>>>
> >>>> The problem showed up when we switched the MMIO region for the fspi
> >>>> flash device to be marked as non executable.  reading variables is
> >>>> fine, however writes began throwing an error.
> >>>>
> >>>> [  556.709828] Unable to handle kernel execute from non-executable
> >>>> memory at virtual address 206a3968
> >>>>
> >>>> I have patched the kernel and removed the X86 requirement and enabled
> >>>> the sysfs runtime mappings kernel config so I can get an easy view of
> >>>> the mappings the kernel carries for runtime services.  I then track
> >>>> that virtual address to the MMIO region of nor flash, which makes
> >>>> sense that region is marked as non executable.  The question is why is
> >>>> code being executed from this address range
> >>>>
> >>>> attribute
> >>>> ::
> >>>> 0x8001
> >>>> ::
> >>>> num_pages
> >>>> ::
> >>>> 0x40
> >>>> ::
> >>>> phys_addr
> >>>> ::
> >>>> 0x2050
> >>>> ::
> >>>> type
> >>>> ::
> >>>> 0xb
> >>>> ::
> >>>> virt_addr
> >>>> ::
> >>>> 0x2068
> >>>>
> >>>> So then I patched the PL011 serial driver to be able to log to the
> >>>> console in runtime and I track down the access to Status =
> >>>> Fvb->GetPhysicalAddress(Fvb, ); in UpdateVariableStore().
> >>>> What I don't understand is why EfiConvertPointer is mapping that
> >>>> pointer into the Virtual address space occupied by the runtime mmio of
> >>>> the flash.  The pointer is being properly remapped.  Here are the
> >>>> pointer addresses in EFI and Kernel Runtime
> >>>>
> >>>> EFI:
> >>>> UpdateVariableStore:156 ECE33968
> >>>> FvbGetPhysicalAddress(BaseAddress=0x2000)
> >>>>
> >>>> KERNEL:
> >>>> UpdateVariableStore:156 206A3968
> >>>> [  556.709828] Unable to handle kernel execute from non-executable
> >>>> memory at virtual address 206a3968
> >>>>
> >>>> Any insight that anyone could provide would be much appreciated.
> >>>
> >>> Your platform appears misconfigured -- the flash MMIO range appears to
> >>> overlap runtime services code even before SetVirtualAddressMap. The
> >>> virtual address conflict is likely the result of the original physical
> >>> address conflict.
> >>
> >> There is no physical address conflict.  Running in physical mode the 
> >> pointer
> >> for GetPhysicalAddress is at 0xECE33968 and the MMIO physical
> >> region is 0x2000 - 0x2FFF.  Shown as the BaseAddress above.
> >> Obviously I don't use that full MMIO region because our flash is not
> >> 256MB.
> >>
> >>>
> >>> After the virtual address updates, the EfiMemoryMappedIO (0xb) type
> >>> range is mapped at virtual address range [0x2068, 0x206C). The
> >>> GetPhysicalAddress function seems to be located at offset 0x23968 in
> >>> that range (with 0x1C698 bytes to go in the range). That's inexplicable.
> >>
> >> Yes, exactly why I am reaching out.
> >>
> >>>
> >>> What is the physical base address of the flash? What are the PCDs used
> >>> in "ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c"?
> >>
> >> We don't use the ArmPlatformPkg
> >>
> >> The code is available here.
> >> https://github.com/SolidRun/edk2-platforms/tree/LX2160_UEFI_ACPI_EAR3-lx2160cex7/S

Re: [edk2-devel] Conflicting virtual addresses causing Runtime Services issues

2021-03-11 Thread Jon Nettleton
On Thu, Mar 11, 2021 at 7:54 AM Jon Nettleton via groups.io
 wrote:
>
> On Wed, Mar 10, 2021 at 3:52 PM Laszlo Ersek  wrote:
> >
> > On 03/10/21 09:04, Jon Nettleton wrote:
> > > I am debugging a failure that I am seeing while using the HoneyComb's
> > > spi-nor flash for runtime variable storage.  I am hoping someone on
> > > the list can give me some insight as what may be the problem.
> > >
> > > The problem showed up when we switched the MMIO region for the fspi
> > > flash device to be marked as non executable.  reading variables is
> > > fine, however writes began throwing an error.
> > >
> > > [  556.709828] Unable to handle kernel execute from non-executable
> > > memory at virtual address 206a3968
> > >
> > > I have patched the kernel and removed the X86 requirement and enabled
> > > the sysfs runtime mappings kernel config so I can get an easy view of
> > > the mappings the kernel carries for runtime services.  I then track
> > > that virtual address to the MMIO region of nor flash, which makes
> > > sense that region is marked as non executable.  The question is why is
> > > code being executed from this address range
> > >
> > > attribute
> > > ::
> > > 0x8001
> > > ::
> > > num_pages
> > > ::
> > > 0x40
> > > ::
> > > phys_addr
> > > ::
> > > 0x2050
> > > ::
> > > type
> > > ::
> > > 0xb
> > > ::
> > > virt_addr
> > > ::
> > > 0x2068
> > >
> > > So then I patched the PL011 serial driver to be able to log to the
> > > console in runtime and I track down the access to Status =
> > > Fvb->GetPhysicalAddress(Fvb, ); in UpdateVariableStore().
> > > What I don't understand is why EfiConvertPointer is mapping that
> > > pointer into the Virtual address space occupied by the runtime mmio of
> > > the flash.  The pointer is being properly remapped.  Here are the
> > > pointer addresses in EFI and Kernel Runtime
> > >
> > > EFI:
> > > UpdateVariableStore:156 ECE33968
> > > FvbGetPhysicalAddress(BaseAddress=0x2000)
> > >
> > > KERNEL:
> > > UpdateVariableStore:156 206A3968
> > > [  556.709828] Unable to handle kernel execute from non-executable
> > > memory at virtual address 206a3968
> > >
> > > Any insight that anyone could provide would be much appreciated.
> >
> > Your platform appears misconfigured -- the flash MMIO range appears to
> > overlap runtime services code even before SetVirtualAddressMap. The
> > virtual address conflict is likely the result of the original physical
> > address conflict.
>
> There is no physical address conflict.  Running in physical mode the pointer
> for GetPhysicalAddress is at 0xECE33968 and the MMIO physical
> region is 0x2000 - 0x2FFF.  Shown as the BaseAddress above.
> Obviously I don't use that full MMIO region because our flash is not
> 256MB.
>
> >
> > After the virtual address updates, the EfiMemoryMappedIO (0xb) type
> > range is mapped at virtual address range [0x2068, 0x206C). The
> > GetPhysicalAddress function seems to be located at offset 0x23968 in
> > that range (with 0x1C698 bytes to go in the range). That's inexplicable.
>
> Yes, exactly why I am reaching out.
>
> >
> > What is the physical base address of the flash? What are the PCDs used
> > in "ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c"?
>
> We don't use the ArmPlatformPkg
>
> The code is available here.
> https://github.com/SolidRun/edk2-platforms/tree/LX2160_UEFI_ACPI_EAR3-lx2160cex7/Silicon/NXP/Drivers/SpiNorFlashDxe
>
> Thanks
> >

Found the root of my problem and fixed it.  As used from other devices
they are relocating the individual pointer for each function in FvbProtocol
of the DXE.  This is done in ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c,
as well as lots of vendor drivers.  I was able to dump the addresses being used
in RuntimeDriverConvertPointer and could locate the pointer to
GetPhysicalAddress.

Convert 0xEC8C1648:0xEC8B:0x203D
New virtual address is then 203E1648 which is fine.

However then later down in the remapping I found
Convert 0x203E1648:0x2000:0x2080

And this is where the pointer gets remapped again and into the
MMIO space of the nor flash.  If I remove the calls to
ConvertPointer for the FvbProtocol I am still seeing those addresses
getting remapped but only once and runtime works as expected.

I am seeing that in MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>FvbInstance->* are all being converted.  It is
possible this is a long standing bug and it just so happens that our
configuration
has caused a conflict and exposed it.

-Jon


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72665): https://edk2.groups.io/g/devel/message/72665
Mute This Topic: https://groups.io/mt/81222488/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] Conflicting virtual addresses causing Runtime Services issues

2021-03-10 Thread Jon Nettleton
On Wed, Mar 10, 2021 at 3:52 PM Laszlo Ersek  wrote:
>
> On 03/10/21 09:04, Jon Nettleton wrote:
> > I am debugging a failure that I am seeing while using the HoneyComb's
> > spi-nor flash for runtime variable storage.  I am hoping someone on
> > the list can give me some insight as what may be the problem.
> >
> > The problem showed up when we switched the MMIO region for the fspi
> > flash device to be marked as non executable.  reading variables is
> > fine, however writes began throwing an error.
> >
> > [  556.709828] Unable to handle kernel execute from non-executable
> > memory at virtual address 206a3968
> >
> > I have patched the kernel and removed the X86 requirement and enabled
> > the sysfs runtime mappings kernel config so I can get an easy view of
> > the mappings the kernel carries for runtime services.  I then track
> > that virtual address to the MMIO region of nor flash, which makes
> > sense that region is marked as non executable.  The question is why is
> > code being executed from this address range
> >
> > attribute
> > ::
> > 0x8001
> > ::
> > num_pages
> > ::
> > 0x40
> > ::
> > phys_addr
> > ::
> > 0x2050
> > ::
> > type
> > ::
> > 0xb
> > ::
> > virt_addr
> > ::
> > 0x2068
> >
> > So then I patched the PL011 serial driver to be able to log to the
> > console in runtime and I track down the access to Status =
> > Fvb->GetPhysicalAddress(Fvb, ); in UpdateVariableStore().
> > What I don't understand is why EfiConvertPointer is mapping that
> > pointer into the Virtual address space occupied by the runtime mmio of
> > the flash.  The pointer is being properly remapped.  Here are the
> > pointer addresses in EFI and Kernel Runtime
> >
> > EFI:
> > UpdateVariableStore:156 ECE33968
> > FvbGetPhysicalAddress(BaseAddress=0x2000)
> >
> > KERNEL:
> > UpdateVariableStore:156 206A3968
> > [  556.709828] Unable to handle kernel execute from non-executable
> > memory at virtual address 206a3968
> >
> > Any insight that anyone could provide would be much appreciated.
>
> Your platform appears misconfigured -- the flash MMIO range appears to
> overlap runtime services code even before SetVirtualAddressMap. The
> virtual address conflict is likely the result of the original physical
> address conflict.

There is no physical address conflict.  Running in physical mode the pointer
for GetPhysicalAddress is at 0xECE33968 and the MMIO physical
region is 0x2000 - 0x2FFF.  Shown as the BaseAddress above.
Obviously I don't use that full MMIO region because our flash is not
256MB.

>
> After the virtual address updates, the EfiMemoryMappedIO (0xb) type
> range is mapped at virtual address range [0x2068, 0x206C). The
> GetPhysicalAddress function seems to be located at offset 0x23968 in
> that range (with 0x1C698 bytes to go in the range). That's inexplicable.

Yes, exactly why I am reaching out.

>
> What is the physical base address of the flash? What are the PCDs used
> in "ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c"?

We don't use the ArmPlatformPkg

The code is available here.
https://github.com/SolidRun/edk2-platforms/tree/LX2160_UEFI_ACPI_EAR3-lx2160cex7/Silicon/NXP/Drivers/SpiNorFlashDxe

Thanks
>
> Laszlo
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72662): https://edk2.groups.io/g/devel/message/72662
Mute This Topic: https://groups.io/mt/81222488/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] Conflicting virtual addresses causing Runtime Services issues

2021-03-10 Thread Jon Nettleton
I am debugging a failure that I am seeing while using the HoneyComb's
spi-nor flash for runtime variable storage.  I am hoping someone on
the list can give me some insight as what may be the problem.

The problem showed up when we switched the MMIO region for the fspi
flash device to be marked as non executable.  reading variables is
fine, however writes began throwing an error.

[  556.709828] Unable to handle kernel execute from non-executable
memory at virtual address 206a3968

I have patched the kernel and removed the X86 requirement and enabled
the sysfs runtime mappings kernel config so I can get an easy view of
the mappings the kernel carries for runtime services.  I then track
that virtual address to the MMIO region of nor flash, which makes
sense that region is marked as non executable.  The question is why is
code being executed from this address range

attribute
::
0x8001
::
num_pages
::
0x40
::
phys_addr
::
0x2050
::
type
::
0xb
::
virt_addr
::
0x2068

So then I patched the PL011 serial driver to be able to log to the
console in runtime and I track down the access to Status =
Fvb->GetPhysicalAddress(Fvb, ); in UpdateVariableStore().
What I don't understand is why EfiConvertPointer is mapping that
pointer into the Virtual address space occupied by the runtime mmio of
the flash.  The pointer is being properly remapped.  Here are the
pointer addresses in EFI and Kernel Runtime

EFI:
UpdateVariableStore:156 ECE33968
FvbGetPhysicalAddress(BaseAddress=0x2000)

KERNEL:
UpdateVariableStore:156 206A3968
[  556.709828] Unable to handle kernel execute from non-executable
memory at virtual address 206a3968

Any insight that anyone could provide would be much appreciated.

Thanks,
Jon


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72609): https://edk2.groups.io/g/devel/message/72609
Mute This Topic: https://groups.io/mt/81222488/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH edk2-platforms 09/16] Silicon/NXP: Implement PciSegmentLib for PCIe Layerscape Controller

2020-05-24 Thread Jon Nettleton
On Sun, May 24, 2020 at 8:32 PM Wasim Khan (OSS)  wrote:
>
>
>
> > -Original Message-
> > From: Ard Biesheuvel 
> > Sent: Friday, May 22, 2020 3:00 PM
> > To: Wasim Khan (OSS) ; devel@edk2.groups.io;
> > Meenakshi Aggarwal ; Vabhav Sharma
> > ; Varun Sethi ;
> > l...@nuviainc.com; j...@solid-run.com
> > Cc: Wasim Khan 
> > Subject: Re: [PATCH edk2-platforms 09/16] Silicon/NXP: Implement
> > PciSegmentLib for PCIe Layerscape Controller
> >
> > On 5/22/20 1:02 AM, Wasim Khan wrote:
> > > From: Wasim Khan 
> > >
> > > We have different PCI config space region for bus 0 (Controller space)
> > > and bus[0x1-0xff] on NXP SoCs with PCIe LS controller.
> > > Add PciSegmentLib for PCIe LS controller.
> > >
> > > For config transactions for Bus0:
> > >- Config transaction address = PCIe controller address + offset
> > >
> > > For config transactions for Bus[0x1-0xff]:
> > >- PCIe IP requires target BDF to be written at bit[31:16] of PCIe
> > >  type0/type1 outbound window.
> > >- Config transaction address = PCIe config space address + offset
> > >
> > > Signed-off-by: Vabhav Sharma 
> > > Signed-off-by: Wasim Khan 
> > > ---
> > >   .../NXP/Library/PciSegmentLib/PciSegmentLib.inf|  32 ++
> > >   Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c  | 612
> > +
> > >   2 files changed, 644 insertions(+)
> > >   create mode 100755 Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
> > >   create mode 100755 Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> > >
> > > diff --git a/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
> > > b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
> > > new file mode 100755
> > > index ..a36e79239b33
> > > --- /dev/null
> > > +++ b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
> > > @@ -0,0 +1,32 @@
> > > +## @file
> > > +#  PCI Segment Library for NXP SoCs with multiple RCs # #  Copyright
> > > +2018-2020 NXP # #  SPDX-License-Identifier: BSD-2-Clause-Patent ##
> > > +
> > > +[Defines]
> > > +  INF_VERSION= 0x0001001A
> > > +  BASE_NAME  = PciSegmentLib
> > > +  FILE_GUID  = c9f59261-5a60-4a4c-82f6-1f520442e100
> > > +  MODULE_TYPE= DXE_DRIVER
> >
> > Can this be BASE ?
> >
>
> No, we need constructor function PciSegLibInit() which requires it to be 
> DXE_DRIVER.
>
> > > +  VERSION_STRING = 1.0
> > > +  LIBRARY_CLASS  = PciSegmentLib|DXE_DRIVER
> > > +  CONSTRUCTOR= PciSegLibInit
> > > +
> > > +[Sources]
> > > +  PciSegmentLib.c
> > > +
> > > +[Packages]
> > > +  MdePkg/MdePkg.dec
> > > +  Silicon/NXP/NxpQoriqLs.dec
> > > +
> > > +[LibraryClasses]
> > > +  BaseLib
> > > +  DebugLib
> > > +  IoLib
> > > +  PcdLib
> > > +
> > > +[FixedPcd]
> > > +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr
> > > diff --git a/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> > > b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> > > new file mode 100755
> > > index ..ecd36971b753
> > > --- /dev/null
> > > +++ b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> > > @@ -0,0 +1,612 @@
> > > +/** @file
> > > +  PCI Segment Library for NXP SoCs with multiple RCs
> > > +
> > > +  Copyright 2018-2020 NXP
> > > +
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include 
> > > +#include  #include  #include
> > > + #include  #include
> > > + #include  #include
> > > + #include  #include
> > > +
> > > +
> > > +typedef enum {
> > > +  PciCfgWidthUint8  = 0,
> > > +  PciCfgWidthUint16,
> > > +  PciCfgWidthUint32,
> > > +  PciCfgWidthMax
> > > +} PCI_CFG_WIDTH;
> > > +
> > > +/**
> > > +  Assert the validity of a PCI Segment address.
> > > +  A valid PCI Segment address should not contain 1's in bits 28..31
> > > +and 48..63
> > > +
> > > +  @param  A The address to validate.
> > > +  @param  M Additional bits to assert to be zero.
> > > +
> > > +**/
> > > +#define ASSERT_INVALID_PCI_SEGMENT_ADDRESS(A,M) \
> > > +  ASSERT (((A) & (0xf000ULL | (M))) == 0)
> > > +
> > > +STATIC
> > > +UINT64
> > > +PciLsCfgTarget (
> > > +  IN  EFI_PHYSICAL_ADDRESS Dbi,
> > > +  IN  UINT64   Address,
> > > +  IN  UINT16   Segment,
> > > +  IN  UINT8Bus,
> > > +  IN  UINT16   Offset
> > > +  )
> > > +{
> > > +  UINT32 Target;
> > > +
> > > +  Target = Address >> 20) & 0xFF) << 24) |
> > > +   (((Address >> 15) & 0x1F) << 19) |
> > > +   (((Address >> 12) & 0x7) << 16));
> >
> > You can drop the outer () here
> >
>
> OK
>
> > > +
> > > +  if (Bus > 1) {
> > > +MmioWrite32 ((UINTN)Dbi + IATU_VIEWPORT_OFF,
> > > + IATU_VIEWPORT_OUTBOUND | IATU_REGION_INDEX1);  } else {
> > > +MmioWrite32 ((UINTN)Dbi + IATU_VIEWPORT_OFF,
> > > + IATU_VIEWPORT_OUTBOUND | IATU_REGION_INDEX0);  }
> > > +
> > > +  MmioWrite32 ((UINTN)Dbi +
> > IATU_LWR_TARGET_ADDR_OFF_OUTBOUND_0,
> > > + Target);
> > > +
> > > +  if (Bus > 1) {
> 

Re: [edk2-devel] Question regarding MMIO address space exposed from GetMemoryMap

2019-11-01 Thread Jon Nettleton
On Fri, Nov 1, 2019 at 6:57 AM Jon Nettleton via Groups.Io
 wrote:
>
> On Thu, Oct 31, 2019 at 5:44 PM Andrew Fish via Groups.Io
>  wrote:
> >
> > Jon,
> >
> > Its a little confusing but gBS->GetMemoryMap () only returns information 
> > about DRAM and any address that requires a kernel virtual address mapping 
> > in EFI. The OS calls EFI Runtime Services from a kernel virtual mapping so 
> > the memory map is also involved in the hand shake to communicate the 
> > virtual address mappings to EFI. Thus any MMIO region in the EFI Memory Map 
> > should always have  the EFI_MEMORY_RUNTIME attribute set. The most common 
> > MMIO region to be mapped is the NOR FLASH to support the EFI Runtime 
> > Variable services.
> >
> > On a UEFI system MMIO regions are described to the OS via ACPI. Processing 
> > the ACPI tables requires an interpreter, and source to an interpreter is 
> > available at the ACPI CA site [1].
> >
> > The PI (Platform Initialization) Spec has a concept called the Global 
> > Coherency Domain (GCD) [2]. The GCD is the map for who owns the CPU address 
> > and IO (x86 in/out instructions not MMIO) space. You can dump the GCD info 
> > via gDS->GetMemorySpaceMap(). The idea is the DRAM controller would carve 
> > out a EfiGcdMemoryTypeSystemMemory range, and the PCI Host Bridge would 
> > carve out a EfiGcdMemoryTypeMemoryMappedIo range, etc.
> >
> > Note: A UEFI implementation is not required to be constructed out of PI 
> > Spec components, but the EDKII UEFI implementation is constructed using the 
> > PI Specification.
> >
> > [1] https://acpica.org/downloads/uefi-support
> > [2] Sorry I could not make up a better name at the time.
> >
> > Thanks,
> >
> > Andrew Fish
> >
> > > On Oct 31, 2019, at 1:32 AM, Jon Nettleton  wrote:
> > >
> > > I am working on sorting out a failure on test 605 of the SBSA test.
> > > The test is "Where a memory access is to an unpopulated part of the
> > > addressable memory space, accesses must be terminated in a manner that
> > > is presented to the PE as either a precise Data Abort or that causes a
> > > system error interrupt or an SPI or LPI interrupt to be delivered to
> > > the GIC."  The issue is that the random test address that was chosen
> > > 0x0420 falls directly in the middle of the Qoriq configuration,
> > > control, and status register (CCSR) address space.  This is an area in
> > > the memory space that provides CPU access to the device registers.
> > >
> > > An entry is added for this region in the VirtualMemoryMap, and
> > > registered with the attribute, ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.
> > > However only a handful of devices are being registered so only a
> > > couple of ranges are showing up in memmap as MMIO.
> > >
> > > The SBSA test in question gets the memorymap and starts at the address
> > > mentioned above and looks for the first chunk of memory that is free
> > > and then attempts to access the memory and is looking for a Data
> > > Abort.  Of course a data abort is not generated because 0x0420 is
> > > a valid address.  If you offset this to 0x4200 then a DA is
> > > generated as expected and the test passes.
> > >
> > > There is a very old thread started by Ard, "MMIO regions in
> > > GetMemoryMap ()", in which he questions if all the MMIO regions should
> > > be reported by GetMemoryMap() or only the ones being used by
> > > initialized devices, which is what the current implementation does.
> > > The end result of the thread was the current implementation is
> > > correct.  That leaves me with the question, "What is the proper
> > > solution for the current implementation that I am working with?"
> > >
> > > To me it seems like I need a special Library that on boot goes through
> > > and claims and maps every valid MMIO slot in this region, and then
> > > have the drivers use this library for proxying requests to
> > > gDS->AddMemorySpace (), gDS->SetMemorySpaceAttributes () etc.   If
> > > there is a standardized way to deal with this configuration I would
> > > much rather follow that.
> > >
> > > Thanks for any input,
> > > Jon
> > >
> > >
> > >
> >
> >
> >
> >
>
> Andrew,
>
> Thanks for clearing this up.  I definitely understand the
> relationships much better now.  I am still uncertain of the proper way
> to fix this issue r

Re: [edk2-devel] Question regarding MMIO address space exposed from GetMemoryMap

2019-10-31 Thread Jon Nettleton
On Thu, Oct 31, 2019 at 5:44 PM Andrew Fish via Groups.Io
 wrote:
>
> Jon,
>
> Its a little confusing but gBS->GetMemoryMap () only returns information 
> about DRAM and any address that requires a kernel virtual address mapping in 
> EFI. The OS calls EFI Runtime Services from a kernel virtual mapping so the 
> memory map is also involved in the hand shake to communicate the virtual 
> address mappings to EFI. Thus any MMIO region in the EFI Memory Map should 
> always have  the EFI_MEMORY_RUNTIME attribute set. The most common MMIO 
> region to be mapped is the NOR FLASH to support the EFI Runtime Variable 
> services.
>
> On a UEFI system MMIO regions are described to the OS via ACPI. Processing 
> the ACPI tables requires an interpreter, and source to an interpreter is 
> available at the ACPI CA site [1].
>
> The PI (Platform Initialization) Spec has a concept called the Global 
> Coherency Domain (GCD) [2]. The GCD is the map for who owns the CPU address 
> and IO (x86 in/out instructions not MMIO) space. You can dump the GCD info 
> via gDS->GetMemorySpaceMap(). The idea is the DRAM controller would carve out 
> a EfiGcdMemoryTypeSystemMemory range, and the PCI Host Bridge would carve out 
> a EfiGcdMemoryTypeMemoryMappedIo range, etc.
>
> Note: A UEFI implementation is not required to be constructed out of PI Spec 
> components, but the EDKII UEFI implementation is constructed using the PI 
> Specification.
>
> [1] https://acpica.org/downloads/uefi-support
> [2] Sorry I could not make up a better name at the time.
>
> Thanks,
>
> Andrew Fish
>
> > On Oct 31, 2019, at 1:32 AM, Jon Nettleton  wrote:
> >
> > I am working on sorting out a failure on test 605 of the SBSA test.
> > The test is "Where a memory access is to an unpopulated part of the
> > addressable memory space, accesses must be terminated in a manner that
> > is presented to the PE as either a precise Data Abort or that causes a
> > system error interrupt or an SPI or LPI interrupt to be delivered to
> > the GIC."  The issue is that the random test address that was chosen
> > 0x0420 falls directly in the middle of the Qoriq configuration,
> > control, and status register (CCSR) address space.  This is an area in
> > the memory space that provides CPU access to the device registers.
> >
> > An entry is added for this region in the VirtualMemoryMap, and
> > registered with the attribute, ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.
> > However only a handful of devices are being registered so only a
> > couple of ranges are showing up in memmap as MMIO.
> >
> > The SBSA test in question gets the memorymap and starts at the address
> > mentioned above and looks for the first chunk of memory that is free
> > and then attempts to access the memory and is looking for a Data
> > Abort.  Of course a data abort is not generated because 0x0420 is
> > a valid address.  If you offset this to 0x4200 then a DA is
> > generated as expected and the test passes.
> >
> > There is a very old thread started by Ard, "MMIO regions in
> > GetMemoryMap ()", in which he questions if all the MMIO regions should
> > be reported by GetMemoryMap() or only the ones being used by
> > initialized devices, which is what the current implementation does.
> > The end result of the thread was the current implementation is
> > correct.  That leaves me with the question, "What is the proper
> > solution for the current implementation that I am working with?"
> >
> > To me it seems like I need a special Library that on boot goes through
> > and claims and maps every valid MMIO slot in this region, and then
> > have the drivers use this library for proxying requests to
> > gDS->AddMemorySpace (), gDS->SetMemorySpaceAttributes () etc.   If
> > there is a standardized way to deal with this configuration I would
> > much rather follow that.
> >
> > Thanks for any input,
> > Jon
> >
> >
> >
>
>
> 
>

Andrew,

Thanks for clearing this up.  I definitely understand the
relationships much better now.  I am still uncertain of the proper way
to fix this issue regarding the SBSA peripheral test.  The test is
using gBS->GetMemoryMap in order to look for available memory
segments, which according to the earlier thread and your confirmation
"gBS->GetMemoryMap () only returns information about DRAM and any
address that requires a kernel virtual address mapping in EFI.", which
means that this test will never work on this platform because the
address that was randomly chosen just happens to fall right into the
address range for the device addresses that a

[edk2-devel] Question regarding MMIO address space exposed from GetMemoryMap

2019-10-31 Thread Jon Nettleton
I am working on sorting out a failure on test 605 of the SBSA test.
The test is "Where a memory access is to an unpopulated part of the
addressable memory space, accesses must be terminated in a manner that
is presented to the PE as either a precise Data Abort or that causes a
system error interrupt or an SPI or LPI interrupt to be delivered to
the GIC."  The issue is that the random test address that was chosen
0x0420 falls directly in the middle of the Qoriq configuration,
control, and status register (CCSR) address space.  This is an area in
the memory space that provides CPU access to the device registers.

An entry is added for this region in the VirtualMemoryMap, and
registered with the attribute, ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.
However only a handful of devices are being registered so only a
couple of ranges are showing up in memmap as MMIO.

The SBSA test in question gets the memorymap and starts at the address
mentioned above and looks for the first chunk of memory that is free
and then attempts to access the memory and is looking for a Data
Abort.  Of course a data abort is not generated because 0x0420 is
a valid address.  If you offset this to 0x4200 then a DA is
generated as expected and the test passes.

There is a very old thread started by Ard, "MMIO regions in
GetMemoryMap ()", in which he questions if all the MMIO regions should
be reported by GetMemoryMap() or only the ones being used by
initialized devices, which is what the current implementation does.
The end result of the thread was the current implementation is
correct.  That leaves me with the question, "What is the proper
solution for the current implementation that I am working with?"

To me it seems like I need a special Library that on boot goes through
and claims and maps every valid MMIO slot in this region, and then
have the drivers use this library for proxying requests to
gDS->AddMemorySpace (), gDS->SetMemorySpaceAttributes () etc.   If
there is a standardized way to deal with this configuration I would
much rather follow that.

Thanks for any input,
Jon

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49748): https://edk2.groups.io/g/devel/message/49748
Mute This Topic: https://groups.io/mt/40108740/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-