Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DynamicTablesPkg: Support for generic DBG2 devices #5779

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

DynamicTablesPkg: Support for generic DBG2 devices #5779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions DynamicTablesPkg/Include/ArmNameSpaceObjects.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/** @file

Copyright (c) 2017 - 2024, Arm Limited. All rights reserved.<BR>
Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. <BR>

SPDX-License-Identifier: BSD-2-Clause-Patent

Expand All @@ -13,6 +14,8 @@
#ifndef ARM_NAMESPACE_OBJECTS_H_
#define ARM_NAMESPACE_OBJECTS_H_

#include <IndustryStandard/AcpiAml.h>
#include <IndustryStandard/Pci.h>
#include <AcpiObjects.h>
#include <StandardNameSpaceObjects.h>

Expand Down Expand Up @@ -73,6 +76,7 @@ typedef enum ArmObjectID {
EArmObjPccSubspaceType5Info, ///< 48 - Pcc Subspace Type 5 Info
EArmObjEtInfo, ///< 49 - Embedded Trace Extension/Module Info
EArmObjPsdInfo, ///< 50 - P-State Dependency (PSD) Info
EArmObjDbg2DeviceInfo, ///< 51 - Generic DBG2 devices
EArmObjMax
} EARM_OBJECT_ID;

Expand Down Expand Up @@ -1345,6 +1349,34 @@ typedef struct CmArmEtInfo {
*/
typedef AML_PSD_INFO CM_ARM_PSD_INFO;

/** A structure that describes a generic device to add a DBG2 device node from.

ID: EArmObjDbg2DeviceInfo,
*/
typedef struct CmArmDbg2DeviceInfo {
/// The number of addresses in the device
UINT8 NumberOfAddresses;

/// The physical base address array for the device
UINT64 BaseAddress[PCI_MAX_BAR];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the max number of addresses should be bound to PCI_MAX_BAR and to the pci code.

In order to re-use the CM_ARM_SERIAL_PORT_INFO struct, would it be possible to do this instead ?

  • Add the following fields to CM_ARM_SERIAL_PORT_INFO:
  ///
  /// Reference Token for a CM_ARM_ADDRESS_RANGE_INFO,
  /// describing other addresses if the port needs multiple.
  /// Token identifying a CM_ARM_OBJ_REF structure.
  /// Only used in DBG2.
  ///
  CM_OBJECT_TOKEN    AdditionnalAddressesToken;

  ///
  /// The port type.
  /// Only used in DBG2.
  ///
  UINT16    PortType;

  ///
  /// ASCII Null terminated string.
  /// Only used in DBG2.
  ///
  CHAR8     ObjectName[AML_NAME_SEG_SIZE + 1];
  • Create the following object:
typedef struct CmArmAddressRangeInfo {
  /// Base Address
  UINT64    BaseAddress;

  /// Address length
  UINT8     AddressLength;
} CM_ARM_ADDRESS_RANGE_INFO;

This would allow the Dbg2Generator to only have to handle one type of serial port description.
If AdditionnalAddressesToken is not the NULL token, then other addresses need to be fetched.
You can check how the CM_ARM_PCI_ADDRESS_MAP_INFO object / CM_ARM_PCI_CONFIG_SPACE_INFO.AddressMapToken field are used at
Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c

Also, would it be possible to make the changes on top of this branch:
https://github.com/pierregondois/edk2/commits/pg/dyntables_libraries_reorg/
The objects/code has been shuffled quite a lot, so it would avoid both of us some work if the changes were targeting this branch instead.
(this means that CM_ARM_ADDRESS_RANGE_INFO should go to the ArchCommon name space)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that sounds good. will work on getting a patch in both places, our platforms to test this uses current edk2 (Do you have ETA for when the reorg will hit tiancore?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I don't have a date to give, but hopefully this might happen next week (@samimujawar )


/// The Base address length array
UINT64 BaseAddressLength[PCI_MAX_BAR];

/// The DBG2 port type
UINT16 PortType;

/// The DBG2 port subtype
UINT16 PortSubtype;

/// Access Size
UINT8 AccessSize;

/** ASCII Null terminated string that will be appended to \_SB_. for the full path.
*/
CHAR8 ObjectName[AML_NAME_SEG_SIZE + 1];
} CM_ARM_DBG2_DEVICE_INFO;

#pragma pack()

#endif // ARM_NAMESPACE_OBJECTS_H_
1 change: 1 addition & 0 deletions DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg2LibArm.inf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
[LibraryClasses]
BaseLib
PL011UartLib
PrintLib
SsdtSerialPortFixupLib

[FixedPcd]
Expand Down
Loading
Loading