Is there an existing issue for this?
Current Behavior
The "End Of DXE - Third Party Option ROM dispatched" test point relies upon TestPointCheckNon3rdPartyImage() in MinPlatformPkg/Test/Library/TestPointCheckLib/TestPointImageDump.c:
|
EFI_STATUS |
|
TestPointCheckNon3rdPartyImage ( |
|
IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage, |
|
IN EFI_DEVICE_PATH_PROTOCOL *DevicePath, |
|
IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath |
|
) |
|
{ |
|
if (LoadedImageDevicePath != NULL) { |
|
// LoadedImageDevicePath should be Fv()/FvFile() |
|
if ((DevicePathType (LoadedImageDevicePath) == MEDIA_DEVICE_PATH) && |
|
(DevicePathSubType (LoadedImageDevicePath) == MEDIA_PIWG_FW_VOL_DP)) |
|
{ |
|
return EFI_SUCCESS; |
|
} |
|
} else { |
|
if (LoadedImage->FilePath != NULL) { |
|
// LoadedImage->FilePath should be FvFile() |
|
if ((DevicePathType (LoadedImage->FilePath) == MEDIA_DEVICE_PATH) && |
|
(DevicePathSubType (LoadedImage->FilePath) == MEDIA_PIWG_FW_FILE_DP)) |
|
{ |
|
return EFI_SUCCESS; |
|
} |
|
} |
|
|
|
if (DevicePath != NULL) { |
|
// DevicePath should be Fv() |
|
if ((DevicePathType (DevicePath) == MEDIA_DEVICE_PATH) && |
|
(DevicePathSubType (DevicePath) == MEDIA_PIWG_FW_VOL_DP)) |
|
{ |
|
return EFI_SUCCESS; |
|
} |
|
} |
|
} |
|
|
|
return EFI_INVALID_PARAMETER; |
|
} |
This function accepts a loaded image device path (LoadedImageDevicePath of type EFI_DEVICE_PATH_PROTOCOL) or a LoadedImage (EFI_LOADED_IMAGE_PROTOCOL) + a device path (EFI_DEVICE_PATH_PROTOCOL).
The function prefers LoadedImageDevicePath and will use that if provided (not NULL).
The actual device path used for a given firmware volume is based on whether an extended header is present on the firmware volume.
This results in paths like the following when the FV extended header is present.
Those with LoadedImageDevicePath:
LoadedImageDevicePath=Fv(FF91464D-588E-4949-B89A-494474F8ED18)/FvFile(85B299E8-8105-4E85-B987-4CB06209F6C0)
Those with the loaded image protocol + device path:
FilePath=FvFile(D6A2CB7F-6A18-4E2F-B43B-9920A733700A) DevicePath=Fv(11EB1176-CEC4-4A8B-81F8-34190C647579)
Due to the way the test point logic is written, a loaded image protocol + device path for a memory mapped path like this will pass:
FilePath=FvFile(68A10D85-6858-4402-B070-028B3EA21747) DevicePath=MemoryMapped(0xB,0x94ECB000,0x94FA41F7)
But a loaded image device path like this will fail:
LoadedImageDevicePath=MemoryMapped(0xB,0x94ECB000,0x94FA41F7)/FvFile(D38FC876-0B17-4D95-A7F8-A022ECA1CA42)
Even though, they're both memory mapped file paths from a firmware volume.
This is because the loaded image device path logic will always fail. It only returns EFI_SUCCESS if LoadedImageDevicePath is strictly Fv()/FvFile():
if (LoadedImageDevicePath != NULL) {
// LoadedImageDevicePath should be Fv()/FvFile()
if ((DevicePathType (LoadedImageDevicePath) == MEDIA_DEVICE_PATH) &&
(DevicePathSubType (LoadedImageDevicePath) == MEDIA_PIWG_FW_VOL_DP))
{
return EFI_SUCCESS;
}
}
The loaded image + device path logic will return EFI_SUCCESS since LoadedImage->FilePath is FvFile().
if (LoadedImage->FilePath != NULL) {
// LoadedImage->FilePath should be FvFile()
if ((DevicePathType (LoadedImage->FilePath) == MEDIA_DEVICE_PATH) &&
(DevicePathSubType (LoadedImage->FilePath) == MEDIA_PIWG_FW_FILE_DP))
{
return EFI_SUCCESS;
}
}
The second device set of conditions are not checked:
if (DevicePath != NULL) {
// DevicePath should be Fv()
if ((DevicePathType (DevicePath) == MEDIA_DEVICE_PATH) &&
(DevicePathSubType (DevicePath) == MEDIA_PIWG_FW_VOL_DP))
{
return EFI_SUCCESS;
}
}
There is logical inconsistency in addition to a somewhat arbitrary result based on the way image device paths are constructed.
Expected Behavior
The test should be effective and have consistent logic.
Steps To Reproduce
Review the code linked above.
Build Environment
Version Information
Urgency
Low
Are you going to fix this?
Someone else needs to fix it
Do you need maintainer feedback?
No maintainer feedback needed
Anything else?
No response
Is there an existing issue for this?
Current Behavior
The "End Of DXE - Third Party Option ROM dispatched" test point relies upon
TestPointCheckNon3rdPartyImage()in MinPlatformPkg/Test/Library/TestPointCheckLib/TestPointImageDump.c:mu_common_intel_min_platform/MinPlatformPkg/Test/Library/TestPointCheckLib/TestPointImageDump.c
Lines 142 to 177 in 804bbe6
This function accepts a loaded image device path (
LoadedImageDevicePathof typeEFI_DEVICE_PATH_PROTOCOL) or aLoadedImage(EFI_LOADED_IMAGE_PROTOCOL) + a device path (EFI_DEVICE_PATH_PROTOCOL).The function prefers
LoadedImageDevicePathand will use that if provided (notNULL).The actual device path used for a given firmware volume is based on whether an extended header is present on the firmware volume.
FwVolHeader->ExtHeaderOffset == 0), thenFV_MEMMAP_DEVICE_PATHis used.FV_PIWG_DEVICE_PATHis used.This results in paths like the following when the FV extended header is present.
Those with
LoadedImageDevicePath:Those with the loaded image protocol + device path:
Due to the way the test point logic is written, a loaded image protocol + device path for a memory mapped path like this will pass:
But a loaded image device path like this will fail:
Even though, they're both memory mapped file paths from a firmware volume.
This is because the loaded image device path logic will always fail. It only returns
EFI_SUCCESSifLoadedImageDevicePathis strictlyFv()/FvFile():The loaded image + device path logic will return
EFI_SUCCESSsinceLoadedImage->FilePathisFvFile().The second device set of conditions are not checked:
There is logical inconsistency in addition to a somewhat arbitrary result based on the way image device paths are constructed.
Expected Behavior
The test should be effective and have consistent logic.
Steps To Reproduce
Review the code linked above.
Build Environment
Version Information
Urgency
Low
Are you going to fix this?
Someone else needs to fix it
Do you need maintainer feedback?
No maintainer feedback needed
Anything else?
No response