Skip to content

Add APMT ACPI table support for guest uncore PMU #48

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

Open
Arvean-microsoft wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add APMT ACPI table support for guest uncore PMU #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64df8e8
Add APMT ACPI table support for guest uncore PMU
Arvean-microsoft Mar 13, 2026
0529175
Address APMT review feedback
Arvean-microsoft May 22, 2026
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
65 changes: 65 additions & 0 deletions MsvmPkg/AcpiPlatformDxe/AcpiPlatform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,62 @@ Return Value:
}


EFI_STATUS
AcpiInstallApmtTable(
EFI_ACPI_TABLE_PROTOCOL *AcpiTable
)
/*++

Routine Description:

Retrieves the APMT table from the worker process and installs it.

Arguments:

AcpiTable - A pointer to the ACPI table protocol.

Return Value:

EFI_STATUS.

--*/
{
EFI_STATUS status;
EFI_ACPI_DESCRIPTION_HEADER *table;
UINTN tableHandle;
UINT32 tableSize;

//
// Get the table from the config blob parsed in PEI. It may not be present.
//
tableSize = PcdGet32(PcdApmtSize);

if (tableSize == 0)
{
return EFI_SUCCESS;
}

table = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) PcdGet64(PcdApmtPtr);

if (table == NULL)
{
return EFI_NOT_FOUND;
}

ASSERT(table->Length == tableSize);

//
// Install it into the published tables.
//
status = AcpiTable->InstallAcpiTable(AcpiTable,
table,
table->Length,
&tableHandle);

return status;
}


EFI_STATUS
EFIAPI
AcpiPlatformInitializeAcpiTables(
Expand Down Expand Up @@ -1078,6 +1134,15 @@ Return Value:
goto Cleanup;
}

//
// Add the APMT table if present.
//
status = AcpiInstallApmtTable(acpiTable);
if (EFI_ERROR(status))
{
goto Cleanup;
}

status = EFI_SUCCESS;

Cleanup:
Expand Down
2 changes: 2 additions & 0 deletions MsvmPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@
gMsvmPkgTokenSpaceGuid.PcdSsdtSize ## CONSUMES
gMsvmPkgTokenSpaceGuid.PcdIortPtr ## CONSUMES
gMsvmPkgTokenSpaceGuid.PcdIortSize ## CONSUMES
gMsvmPkgTokenSpaceGuid.PcdApmtPtr ## CONSUMES
gMsvmPkgTokenSpaceGuid.PcdApmtSize ## CONSUMES
gMsvmPkgTokenSpaceGuid.PcdNvdimmCount ## CONSUMES
gMsvmPkgTokenSpaceGuid.PcdPpttPtr ## CONSUMES
gMsvmPkgTokenSpaceGuid.PcdPpttSize ## CONSUMES
Expand Down
1 change: 1 addition & 0 deletions MsvmPkg/Include/AcpiTables.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ typedef struct _VM_ACPI_ENTROPY_TABLE
#define VM_ACPI_ENTROPY_TABLE_SIGNATURE SIGNATURE_32('O','E','M','0')

#define AMD_ACPI_ASPT_TABLE_SIGNATURE SIGNATURE_32('A', 'S', 'P', 'T')
#define ARM_ACPI_APMT_TABLE_SIGNATURE SIGNATURE_32('A', 'P', 'M', 'T')

//
// WDAT table.
Expand Down
7 changes: 7 additions & 0 deletions MsvmPkg/Include/BiosInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ enum UefiStructureType
UefiConfigSsdt = 0x25,
UefiConfigHmat = 0x26,
UefiConfigIort = 0x27,
UefiConfigApmt = 0x28,
};

//
Expand Down Expand Up @@ -879,6 +880,12 @@ typedef struct _UEFI_CONFIG_IORT
UINT8 Iort[];
} UEFI_CONFIG_IORT;

typedef struct _UEFI_CONFIG_APMT
{
UEFI_CONFIG_HEADER Header;
UINT8 Apmt[];
} UEFI_CONFIG_APMT;

//
// UEFI configuration information for direct parsing of IGVM parameters.
//
Expand Down
5 changes: 5 additions & 0 deletions MsvmPkg/MsvmPkg.dec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,3 +382,8 @@
# UEFI_CONFIG_IORT
gMsvmPkgTokenSpaceGuid.PcdIortPtr|0x0|UINT64|0x6070
gMsvmPkgTokenSpaceGuid.PcdIortSize|0x0|UINT32|0x6071

# UEFI_CONFIG_APMT
# 0x6072 is used by PcdAziHsmEnabled above.
gMsvmPkgTokenSpaceGuid.PcdApmtPtr|0x0|UINT64|0x6073
Copy link
Copy Markdown
Collaborator

@mebersol mebersol Apr 16, 2026

Choose a reason for hiding this comment

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

[Copilot] Nit: IORT uses tokens 0x6070/0x6071. APMT jumps to 0x6073/0x6074, skipping 0x6072. Is this intentional (reserved for something else) or a typo? If intentional, a comment noting the reservation would help future readers.

Copy link
Copy Markdown
Author

@Arvean-microsoft Arvean-microsoft May 22, 2026

Choose a reason for hiding this comment

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

Kept 0x6073/0x6074 because 0x6072 is already assigned to PcdAziHsmEnabled, and added a DEC comment to make that explicit.

gMsvmPkgTokenSpaceGuid.PcdApmtSize|0x0|UINT32|0x6074
4 changes: 4 additions & 0 deletions MsvmPkg/MsvmPkgAARCH64.dsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,10 @@
gMsvmPkgTokenSpaceGuid.PcdIortPtr|0x0
gMsvmPkgTokenSpaceGuid.PcdIortSize|0x0

# UEFI_CONFIG_APMT
gMsvmPkgTokenSpaceGuid.PcdApmtPtr|0x0
gMsvmPkgTokenSpaceGuid.PcdApmtSize|0x0

# UEFI_CONFIG_MEMORY_MAP
gMsvmPkgTokenSpaceGuid.PcdMemoryMapPtr|0x0
gMsvmPkgTokenSpaceGuid.PcdMemoryMapSize|0x0
Expand Down
6 changes: 6 additions & 0 deletions MsvmPkg/MsvmPkgX64.dsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,12 @@
gMsvmPkgTokenSpaceGuid.PcdIortPtr|0x0
gMsvmPkgTokenSpaceGuid.PcdIortSize|0x0

# UEFI_CONFIG_APMT
# AcpiPlatformDxe and PlatformPei are shared modules and consume these
# optional PCDs in X64 builds even though APMT is only supplied on ARM hosts.
gMsvmPkgTokenSpaceGuid.PcdApmtPtr|0x0
gMsvmPkgTokenSpaceGuid.PcdApmtSize|0x0

Comment on lines +562 to +564
Copy link
Copy Markdown
Collaborator

@mebersol mebersol Apr 16, 2026

Choose a reason for hiding this comment

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

[Copilot] APMT is ARM-specific (ARM Performance Monitoring Table). Adding it to the X64 DSC is harmless since the host will never provide one and the installer gracefully skips when absent — but is this intentional for symmetry, or should the PCDs only be wired in the AARCH64 DSC?

Copy link
Copy Markdown
Author

@Arvean-microsoft Arvean-microsoft May 22, 2026

Choose a reason for hiding this comment

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

Left the optional APMT PCD defaults in X64 because PlatformPei and AcpiPlatformDxe are shared modules that consume them in both builds; added a DSC comment clarifying that APMT is only supplied on ARM hosts.

# UEFI_CONFIG_MEMORY_MAP
gMsvmPkgTokenSpaceGuid.PcdMemoryMapPtr|0x0
gMsvmPkgTokenSpaceGuid.PcdMemoryMapSize|0x0
Expand Down
23 changes: 23 additions & 0 deletions MsvmPkg/PlatformPei/Config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,10 @@ DebugDumpUefiConfigStruct(
DebugDumpHmat(hmat->Hmat);
break;

case UefiConfigApmt:
DEBUG((DEBUG_VERBOSE, "\tAPMT table found.\n"));
break;

case UefiConfigMemoryMap:
UEFI_CONFIG_MEMORY_MAP *memMap = (UEFI_CONFIG_MEMORY_MAP*) Header;
DebugDumpMemoryMap(memMap->MemoryMap, Header->Length - sizeof(UEFI_CONFIG_HEADER), PcdGetBool(PcdLegacyMemoryMap));
Expand Down Expand Up @@ -1065,6 +1069,7 @@ Return Value:
0, //UefiConfigSsdt
0, //UefiConfigHmat
0, //UefiConfigIort
0, //UefiConfigApmt
};

//
Expand Down Expand Up @@ -1630,6 +1635,24 @@ Return Value:
PEI_FAIL_FAST_IF_FAILED(PcdSet64S(PcdIortPtr, (UINT64)iortStructure->Iort));
PEI_FAIL_FAST_IF_FAILED(PcdSet32S(PcdIortSize, iortHdr->Length));
break;

case UefiConfigApmt:
{
UEFI_CONFIG_APMT *apmtStructure = (UEFI_CONFIG_APMT*) header;
EFI_ACPI_DESCRIPTION_HEADER *apmtHdr = (EFI_ACPI_DESCRIPTION_HEADER*) apmtStructure->Apmt;

if (apmtStructure->Header.Length < (sizeof(UEFI_CONFIG_HEADER) + sizeof(EFI_ACPI_DESCRIPTION_HEADER)) ||
apmtHdr->Signature != ARM_ACPI_APMT_TABLE_SIGNATURE ||
apmtHdr->Length > (apmtStructure->Header.Length - sizeof(UEFI_CONFIG_HEADER)))
{
DEBUG((DEBUG_ERROR, "*** Malformed APMT\n"));
FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR();
}

PEI_FAIL_FAST_IF_FAILED(PcdSet64S(PcdApmtPtr, (UINT64)apmtStructure->Apmt));
PEI_FAIL_FAST_IF_FAILED(PcdSet32S(PcdApmtSize, apmtHdr->Length));
break;
Comment on lines +1639 to +1654
Copy link
Copy Markdown
Collaborator

@mebersol mebersol Apr 16, 2026

Choose a reason for hiding this comment

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

[Copilot] The adjacent UefiConfigIort case (and others that declare local variables) wraps the body in { }:

case UefiConfigIort:
{
    UEFI_CONFIG_IORT *iortStructure = ...;
    ...
    break;
}

This case declares apmtStructure and apmtHdr without a compound statement. Please add braces for consistency and to properly scope the variable declarations.

Copy link
Copy Markdown
Author

@Arvean-microsoft Arvean-microsoft May 22, 2026

Choose a reason for hiding this comment

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

Fixed in 0529175 by wrapping the UefiConfigApmt case body in braces so the local declarations are scoped consistently with the neighboring cases.

}
}

calculatedConfigSize += header->Length;
Expand Down
2 changes: 2 additions & 0 deletions MsvmPkg/PlatformPei/PlatformPei.inf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@
gMsvmPkgTokenSpaceGuid.PcdLegacyMemoryMap
gMsvmPkgTokenSpaceGuid.PcdIortPtr
gMsvmPkgTokenSpaceGuid.PcdIortSize
gMsvmPkgTokenSpaceGuid.PcdApmtPtr
gMsvmPkgTokenSpaceGuid.PcdApmtSize
gMsvmPkgTokenSpaceGuid.PcdMadtPtr
gMsvmPkgTokenSpaceGuid.PcdMadtSize
gMsvmPkgTokenSpaceGuid.PcdMcfgPtr
Expand Down