[AMD Official Use Only - General]
Hi Isaac,
For "INTEL", we can change it to all whitespace as the string would be patched
in runtime using PcdAcpiDefaultOemId (which is also Intel 😊)
For "MSFT", I prefer to keep it as that value is hardcoded in the source code.
We can create a PCD for this as well but this would be not in the migration
scope that claims the same functionality with the one under Intel folder.
Thanks
Abner
> -----Original Message-----
> From: Oram, Isaac W <isaac.w.o...@intel.com>
> Sent: Friday, May 19, 2023 11:28 AM
> To: devel@edk2.groups.io; Chang, Abner <abner.ch...@amd.com>
> Cc: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>; Nickle
> Wang <nick...@nvidia.com>; Tinh Nguyen
> <tinhngu...@os.amperecomputing.com>
> Subject: RE: [edk2-devel] [edk2-platforms][PATCH 1/2]
> ManageabilityPkg/IpmiFrb: IPMI BMC ACPI Driver
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Reviewed-by: Isaac Oram <isaac.w.o...@intel.com>
>
> You could consider changing "INTEL" and ('M', 'S', 'F', 'T') to more generic
> placeholders. But that is also something that could be looked at separately
> and more widely.
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
> Abner via groups.io
> Sent: Saturday, May 13, 2023 8:49 AM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W <isaac.w.o...@intel.com>; Abdul Lateef Attar
> <abdat...@amd.com>; Nickle Wang <nick...@nvidia.com>; Tinh Nguyen
> <tinhngu...@os.amperecomputing.com>
> Subject: [edk2-devel] [edk2-platforms][PATCH 1/2]
> ManageabilityPkg/IpmiFrb: IPMI BMC ACPI Driver
>
> From: Abner Chang <abner.ch...@amd.com>
>
> IpmiBmcAcpi is cloned from
> edk2-platforms/Features/Intel/OutOfBandManagement/
> IpmiFeaturePkg/BmcAcpi in order to consolidate
> edk2 system manageability support in one place.
> Uncustify is applied to C files and no functionalities are changed in this
> patch.
>
> We will still keep the one under IpmiFeaturePkg/BmcAcpi until the reference to
> this instance are removed from platforms.
>
> Signed-off-by: Abner Chang <abner.ch...@amd.com>
> Cc: Isaac Oram <isaac.w.o...@intel.com>
> Cc: Abdul Lateef Attar <abdat...@amd.com>
> Cc: Nickle Wang <nick...@nvidia.com>
> Cc: Tinh Nguyen <tinhngu...@os.amperecomputing.com>
> ---
> .../Universal/IpmiBmcAcpi/BmcAcpi.inf | 47 ++++
> .../Universal/IpmiBmcAcpi/BmcAcpi.c | 264 ++++++++++++++++++
> .../Universal/IpmiBmcAcpi/BmcSsdt/BmcSsdt.asl | 28 ++
> .../IpmiBmcAcpi/BmcSsdt/IpmiOprRegions.asi | 58 ++++
> 4 files changed, 397 insertions(+)
> create mode 100644
> Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
> create mode 100644
> Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
> create mode 100644
> Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/BmcSsdt.asl
> create mode 100644
> Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/IpmiOprRegion
> s.asi
>
> diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
> b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
> new file mode 100644
> index 0000000000..a21c5b9b35
> --- /dev/null
> +++ b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
> @@ -0,0 +1,47 @@
> +### @file
> +# Component description file for BMC ACPI.
> +#
> +# Copyright (c) 2018 - 2019, Intel Corporation. All rights
> +reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = BmcAcpi
> + FILE_GUID = 09E3B4BE-F731-4903-AAE6-BD5D573B6140
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = BmcAcpiEntryPoint
> +
> +[Sources]
> + BmcAcpi.c
> + BmcSsdt/BmcSsdt.asl
> +
> +[Packages]
> + ManageabilityPkg/ManageabilityPkg.dec
> + MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> + BaseMemoryLib
> + DebugLib
> + UefiBootServicesTableLib
> + UefiDriverEntryPoint
> + UefiLib
> +
> +[Protocols]
> + gEfiFirmwareVolume2ProtocolGuid
> + gEfiAcpiTableProtocolGuid
> +
> +[Pcd]
> + gEfiMdePkgTokenSpaceGuid.PcdIpmiKcsIoBaseAddress
> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
> +
> +[Depex]
> + gEfiAcpiTableProtocolGuid
> +
> +[BuildOptions]
> + *_*_*_ASL_FLAGS = -oi
> diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
> b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
> new file mode 100644
> index 0000000000..cf066dd095
> --- /dev/null
> +++ b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
> @@ -0,0 +1,264 @@
> +/** @file
> + IPMI BMC ACPI.
> +
> + Copyright (c) 2018 - 2019, Intel Corporation. All rights
> + reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +//
> +// Statements that include other header files // #include <PiDxe.h>
> +
> +#include <IndustryStandard/Acpi.h>
> +#include <Protocol/AcpiSystemDescriptionTable.h>
> +#include <Protocol/FirmwareVolume2.h>
> +#include <Protocol/AcpiTable.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h> #include
> +<Library/BaseMemoryLib.h> #include <Library/UefiLib.h>
> +
> +#ifndef EFI_ACPI_CREATOR_ID
> +#define EFI_ACPI_CREATOR_ID SIGNATURE_32 ('M', 'S', 'F', 'T') #endif
> +#ifndef EFI_ACPI_CREATOR_REVISION #define EFI_ACPI_CREATOR_REVISION
> +0x0100000D #endif
> +
> +/**
> +
> + Locate the first instance of a protocol. If the protocol requested
> + is an FV protocol, then it will return the first FV that contains the
> + ACPI table storage file.
> +
> + @param [in] Protocol The protocol to find.
> + @param [in] Instance Return pointer to the first instance of the protocol.
> + @param [in] Type The type of protocol to locate.
> +
> + @retval EFI_SUCCESS The function completed successfully.
> + @retval EFI_NOT_FOUND The protocol could not be located.
> + @retval EFI_OUT_OF_RESOURCES There are not enough resources to find
> the protocol.
> +
> +**/
> +EFI_STATUS
> +LocateSupportProtocol (
> + IN EFI_GUID *Protocol,
> + OUT VOID **Instance,
> + IN UINT32 Type
> + )
> +{
> + EFI_STATUS Status;
> + EFI_HANDLE *HandleBuffer;
> + UINTN NumberOfHandles;
> + EFI_FV_FILETYPE FileType;
> + UINT32 FvStatus = 0;
> + EFI_FV_FILE_ATTRIBUTES Attributes;
> + UINTN Size;
> + UINTN Index;
> +
> + Status = gBS->LocateHandleBuffer (ByProtocol, Protocol, NULL,
> + &NumberOfHandles, &HandleBuffer); if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + //
> + // Looking for FV with ACPI storage file // for (Index = 0; Index <
> + NumberOfHandles; Index++) {
> + Status = gBS->HandleProtocol (HandleBuffer[Index], Protocol, Instance);
> + ASSERT (!EFI_ERROR (Status));
> +
> + if (!Type) {
> + //
> + // Not looking for the FV protocol, so find the first instance of the
> + // protocol. There should not be any errors because our handle buffer
> + // should always contain at least one or LocateHandleBuffer would have
> + // returned not found.
> + //
> + break;
> + }
> +
> + //
> + // See if it has the ACPI storage file
> + //
> + Status = ((EFI_FIRMWARE_VOLUME2_PROTOCOL *)(*Instance))->ReadFile
> (
> + *Instance,
> +
> &gEfiCallerIdGuid,
> + NULL,
> + &Size,
> + &FileType,
> + &Attributes,
> + &FvStatus
> + );
> +
> + //
> + // If we found it, then we are done
> + //
> + if (!EFI_ERROR (Status)) {
> + break;
> + }
> + }
> +
> + gBS->FreePool (HandleBuffer);
> + return Status;
> +}
> +
> +/**
> + Update ACPI SSDT for BMC IPMI KCS device
> +
> + @param [in] Table Pointer to ACPI SSDT
> +
> + @retval EFI_SUCCESS SSDT is updated according to PCD settings **/
> +EFI_STATUS UpdateDeviceSsdtTable (
> + IN OUT EFI_ACPI_COMMON_HEADER *Table
> + )
> +{
> + EFI_ACPI_DESCRIPTION_HEADER *TableHeader = NULL;
> + UINT64 TempOemTableId;
> + UINT8 *DataPtr;
> + EFI_ACPI_IO_PORT_DESCRIPTOR *IoRsc;
> +
> + TableHeader = (EFI_ACPI_DESCRIPTION_HEADER *)Table;
> +
> + //
> + // Update the OEMID and OEM Table ID.
> + //
> + CopyMem (&TableHeader->OemId, PcdGetPtr (PcdAcpiDefaultOemId),
> sizeof
> + (TableHeader->OemId)); TempOemTableId = PcdGet64
> + (PcdAcpiDefaultOemTableId); CopyMem (&TableHeader->OemTableId,
> &TempOemTableId, sizeof (UINT64));
> + TableHeader->CreatorId = EFI_ACPI_CREATOR_ID;
> + TableHeader->CreatorRevision = EFI_ACPI_CREATOR_REVISION;
> +
> + //
> + // Update IO(Decode16, 0xCA2, 0xCA2, 0, 2) // DEBUG ((DEBUG_INFO,
> + "UpdateDeviceSsdtTable - IPMI\n")); for (DataPtr = (UINT8 *)(Table +
> + 1);
> + DataPtr < (UINT8 *)((UINT8 *)Table + Table->Length - 4);
> + DataPtr++)
> + {
> + if (CompareMem (DataPtr, "_CRS", 4) == 0) {
> + DataPtr += 4; // Skip _CRS
> + ASSERT (*DataPtr == AML_BUFFER_OP);
> + DataPtr++; // Skip AML_BUFFER_OP
> + ASSERT ((*DataPtr & (BIT7|BIT6)) == 0);
> + DataPtr++; // Skip PkgLength - 0xD
> + ASSERT ((*DataPtr) == AML_BYTE_PREFIX);
> + DataPtr++; // Skip BufferSize OpCode
> + DataPtr++; // Skip BufferSize - 0xA
> + IoRsc = (VOID *)DataPtr;
> + ASSERT (IoRsc->Header.Bits.Type == ACPI_SMALL_ITEM_FLAG);
> + ASSERT (IoRsc->Header.Bits.Name ==
> ACPI_SMALL_IO_PORT_DESCRIPTOR_NAME);
> + ASSERT (IoRsc->Header.Bits.Length == sizeof
> (EFI_ACPI_IO_PORT_DESCRIPTOR) - sizeof
> (ACPI_SMALL_RESOURCE_HEADER));
> + DEBUG ((DEBUG_INFO, "IPMI IO Base in ASL update - 0x%04x <=
> 0x%04x\n", IoRsc->BaseAddressMin, PcdGet16 (PcdIpmiKcsIoBaseAddress)));
> + IoRsc->BaseAddressMin = PcdGet16 (PcdIpmiKcsIoBaseAddress);
> + IoRsc->BaseAddressMax = PcdGet16 (PcdIpmiKcsIoBaseAddress);
> + }
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> +
> + Entry point for Acpi platform driver.
> +
> + @param [in] ImageHandle A handle for the image that is initializing this
> driver.
> + @param [in] SystemTable A pointer to the EFI system table.
> +
> + @retval EFI_SUCCESS Driver initialized successfully.
> + @retval EFI_LOAD_ERROR Failed to Initialize or has been loaded.
> + @retval EFI_OUT_OF_RESOURCES Could not allocate needed resources.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +BmcAcpiEntryPoint (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> + EFI_STATUS AcpiStatus;
> +
> + EFI_FIRMWARE_VOLUME2_PROTOCOL *FwVol;
> + INTN Instance = 0;
> + EFI_ACPI_COMMON_HEADER *CurrentTable = NULL;
> + UINTN TableHandle = 0;
> + UINT32 FvStatus;
> + UINT32 Size;
> +
> + EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
> + UINTN TableSize;
> +
> + //
> + // Find the AcpiTable protocol
> + //
> + Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL, (VOID
> + **)&AcpiTable); if (EFI_ERROR (Status)) {
> + return EFI_ABORTED;
> + }
> +
> + //
> + // Locate the firmware volume protocol // Status =
> + LocateSupportProtocol (&gEfiFirmwareVolume2ProtocolGuid, (VOID
> + **)&FwVol, 1); if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Status = EFI_SUCCESS;
> + Instance = 0;
> +
> + //
> + // Read tables from the storage file.
> + //
> + while (!EFI_ERROR (Status)) {
> + CurrentTable = NULL;
> +
> + Status = FwVol->ReadSection (
> + FwVol,
> + &gEfiCallerIdGuid,
> + EFI_SECTION_RAW,
> + Instance,
> + (VOID **)&CurrentTable,
> + (UINTN *)&Size,
> + &FvStatus
> + );
> + if (!EFI_ERROR (Status)) {
> + //
> + // Perform any table specific updates.
> + //
> + AcpiStatus = UpdateDeviceSsdtTable (CurrentTable);
> + if (!EFI_ERROR (AcpiStatus)) {
> + TableHandle = 0;
> + TableSize = ((EFI_ACPI_DESCRIPTION_HEADER *)CurrentTable)->Length;
> + ASSERT (Size >= TableSize);
> +
> + Status = AcpiTable->InstallAcpiTable (
> + AcpiTable,
> + CurrentTable,
> + TableSize,
> + &TableHandle
> + );
> +
> + ASSERT_EFI_ERROR (Status);
> + }
> +
> + //
> + // Increment the instance
> + //
> + Instance++;
> + }
> + }
> +
> + return EFI_SUCCESS;
> +}
> diff --git
> a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/BmcSsdt.asl
> b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/BmcSsdt.asl
> new file mode 100644
> index 0000000000..8157d744c8
> --- /dev/null
> +++
> b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/BmcSsdt.as
> +++ l
> @@ -0,0 +1,28 @@
> +/** @file
> + BMC ACPI SSDT.
> +
> +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +DefinitionBlock (
> + "BmcSsdt.aml",
> + "SSDT",
> + 0x02, // SSDT revision.
> + // A Revision field value greater than or equal to 2
> signifies that
> integers
> + // declared within the Definition Block are to be
> evaluated as 64-bit
> values
> + "INTEL", // OEM ID (6 byte string), same as PcdAcpiDefaultOemId
> defined in MdeModulePkg.dec
> + "BMCACPI", // OEM table ID (8 byte string)
> + 0x0 // OEM version of DSDT table (4 byte Integer)
> + )
> +{
> +
> + External(\_SB.PC00.LPC0, DeviceObj)
> +
> + Scope (\_SB.PC00.LPC0)
> + {
> + #include "IpmiOprRegions.asi"
> + }
> +
> +}
> diff --git
> a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/IpmiOprRegi
> ons.asi
> b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/IpmiOprRegi
> ons.asi
> new file mode 100644
> index 0000000000..ffd6af3944
> --- /dev/null
> +++
> b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/IpmiOprReg
> +++ ions.asi
> @@ -0,0 +1,58 @@
> +/** @file
> + IPMI ACPI SSDT.
> +
> +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +Device(IPMC)
> +{
> + // Config DWord, modified during POST
> + // Bit definitions are the same as PPMFlags:
> + // [00] = Plug and Play BMC Detection enabled in setup
> + // [31:01] = Reserved = 0
> +
> + Name(ECFL, 0x80000000)
> +
> + // Return the interface specification revision
> + Method(_SRV)
> + {
> + // IPMI Specification Revision v2.0
> + Return(0x0200)
> + }
> +
> + Method(_STA, 0)
> + {
> + //
> + // Assume OK
> + //
> + Store (0xF, Local0)
> +
> + Return(Local0)
> +
> + // Bit 0 - Set if the device is present.
> + // Bit 1 - Set if the device is enabled and decoding its resources.
> + // Bit 2 - Set if the device should be shown in the UI.
> + // Bit 3 - Set if the device is functioning properly (cleared if the
> device
> failed its diagnostics).
> + // Bit 4 - Set if the battery is present.
> + // Bit 5 - Reserved (must be cleared).
> + } // end of _STA
> +
> + // Return the x86 resources consumed by BMC
> + Name(_CRS, ResourceTemplate()
> + {
> + // Uses 8-bit ports 0xCA2-0xCA5
> + IO(Decode16, 0xCA2, 0xCA2, 0, 2)
> + })
> +
> + Name(_HID, "IPI0001") // IPMI device
> + Name(_IFT, 0x1) // KCS system interface type
> + Name(_STR, Unicode("IPMI_KCS"))
> +
> + Name(_UID, 0) // First interface.
> +
> +
> +} // end of Device(IPMC)
> +
> +
> --
> 2.37.1.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105070): https://edk2.groups.io/g/devel/message/105070
Mute This Topic: https://groups.io/mt/98870430/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
