Add APMT ACPI table support for guest uncore PMU#48
Add APMT ACPI table support for guest uncore PMU#48Arvean-microsoft wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
|
Arvean-microsoft
force-pushed
the
arveanlabib/apmt-support
branch
from
d0990b1 to
0e0d24c
Compare
Add receive and install support for the ARM Performance Monitoring Table (APMT) in the guest UEFI firmware. This enables guest VMs to discover uncore PMU MMIO ranges when the host provides an APMT via the UEFI config blob. Changes: - Add UefiConfigApmt (0x28) enum value and UEFI_CONFIG_APMT struct - Add ARM_ACPI_APMT_TABLE_SIGNATURE definition - Add PcdApmtPtr/PcdApmtSize PCDs (tokens 0x6073/0x6074) - Parse and validate APMT in PEI phase (Config.c) - Install APMT in DXE phase via AcpiInstallApmtTable() - Wire PCDs through both AARCH64 and X64 DSC/INF files The APMT is optional: it is not added to requiredStructures, and the DXE installer gracefully skips when the table is absent. The parsing and installation follow the same pattern as IORT/PPTT/HMAT.
Arvean-microsoft
force-pushed
the
arveanlabib/apmt-support
branch
from
0e0d24c to
64df8e8
Compare
mebersol
left a comment
mebersol
left a comment
There was a problem hiding this comment.
[Copilot] Overall this is a clean change that follows the established IORT/PPTT/HMAT pattern well. One thing to fix: the case UefiConfigApmt: block in Config.c needs braces { } around the body to match the adjacent cases and properly scope variable declarations. The other two comments are minor questions.
| 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; |
There was a problem hiding this comment.
[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.
| gMsvmPkgTokenSpaceGuid.PcdIortSize|0x0|UINT32|0x6071 | ||
|
|
||
| # UEFI_CONFIG_APMT | ||
| gMsvmPkgTokenSpaceGuid.PcdApmtPtr|0x0|UINT64|0x6073 |
There was a problem hiding this comment.
[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.
| gMsvmPkgTokenSpaceGuid.PcdApmtPtr|0x0 | ||
| gMsvmPkgTokenSpaceGuid.PcdApmtSize|0x0 | ||
|
|
There was a problem hiding this comment.
[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?
4 participants
Add receive and install support for the ARM Performance Monitoring Table (APMT) in the guest UEFI firmware. This enables guest VMs to discover uncore PMU MMIO ranges when the host provides an APMT via the UEFI config blob.
Changes:
The APMT is optional: it is not added to requiredStructures, and the DXE installer gracefully skips when the table is absent. The parsing and installation follow the same pattern as IORT/PPTT/HMAT.