Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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
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
4 changes: 4 additions & 0 deletions MsvmPkg/MsvmPkg.dec
Original file line number Diff line number Diff line change
Expand Up @@ -382,3 +382,7 @@
# UEFI_CONFIG_IORT
gMsvmPkgTokenSpaceGuid.PcdIortPtr|0x0|UINT64|0x6070
gMsvmPkgTokenSpaceGuid.PcdIortSize|0x0|UINT32|0x6071

# UEFI_CONFIG_APMT
gMsvmPkgTokenSpaceGuid.PcdApmtPtr|0x0|UINT64|0x6073
Copy link
Copy Markdown
Collaborator

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.

gMsvmPkgTokenSpaceGuid.PcdApmtSize|0x0|UINT32|0x6074
4 changes: 4 additions & 0 deletions MsvmPkg/MsvmPkgAARCH64.dsc
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
4 changes: 4 additions & 0 deletions MsvmPkg/MsvmPkgX64.dsc
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,10 @@
gMsvmPkgTokenSpaceGuid.PcdIortPtr|0x0
gMsvmPkgTokenSpaceGuid.PcdIortSize|0x0

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

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

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?

# UEFI_CONFIG_MEMORY_MAP
gMsvmPkgTokenSpaceGuid.PcdMemoryMapPtr|0x0
gMsvmPkgTokenSpaceGuid.PcdMemoryMapSize|0x0
Expand Down
21 changes: 21 additions & 0 deletions MsvmPkg/PlatformPei/Config.c
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,22 @@ 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 +1653
Copy link
Copy Markdown
Collaborator

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.

}

calculatedConfigSize += header->Length;
Expand Down
2 changes: 2 additions & 0 deletions MsvmPkg/PlatformPei/PlatformPei.inf
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