[edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device exemption
Michael D Kinney
michael.d.kinney at intel.com
Fri Aug 20 00:55:35 UTC 2021
Hi Michael,
Using S/B/D/F is not a good way to provide this information.
It can change based when other PCI devices are enabled/disabled/added/removed.
It is better to use a list of D/F similar to PCI Device Paths or just
use a PCI device path.
Mike
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Monday, August 9, 2021 12:09 PM
> To: devel at edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu at intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Liming Gao
> <gaoliming at byosoft.com.cn>; Dong, Eric <eric.dong at intel.com>; Chris Ruffin <v-cruffin at microsoft.com>; Michael Kubacki
> <michael.kubacki at microsoft.com>
> Subject: [edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device
> exemption
>
> From: Chris Ruffin <v-cruffin at microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3541
>
> Some platforms have devices which do not expose any additional
> risk of DMA attacks but the BME bit cannot be disabled.
>
> To allow MinPlatformPkg consumers to selectively exempt certain
> devices from the PCI bus master test point, this change adds a
> PCD to MinPlatformPkg.dec that allows those packages to specify
> a list of PCI devices by S/B/D/F that should be excluded from
> testing.
>
> Cc: Chasel Chiu <chasel.chiu at intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Chris Ruffin <v-cruffin at microsoft.com>
> Co-authored-by: Michael Kubacki <michael.kubacki at microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki at microsoft.com>
> ---
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckPci.c | 37 ++++++++++++++++++--
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiCheckPci.c | 35 ++++++++++++++++++
> Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec | 4 +++
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf | 1 +
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTestPointCheckLib.inf | 1 +
> 5 files changed, 75 insertions(+), 3 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckPci.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckPci.c
> index 514003944758..95f4fb8b7c7e 100644
> --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckPci.c
> +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckPci.c
> @@ -44,6 +44,13 @@ typedef struct {
> UINT32 Data[48];
> } PCI_CONFIG_SPACE;
>
> +typedef struct {
> + UINT8 Segment;
> + UINT8 Bus;
> + UINT8 Device;
> + UINT8 Function;
> +} EXEMPT_DEVICE;
> +
> #pragma pack()
>
> VOID
> @@ -256,7 +263,7 @@ TestPointCheckPciResource (
> UINT16 MinBus;
> UINT16 MaxBus;
> BOOLEAN IsEnd;
> -
> +
> DEBUG ((DEBUG_INFO, "==== TestPointCheckPciResource - Enter\n"));
> HandleBuf = NULL;
> Status = gBS->LocateHandleBuffer (
> @@ -338,7 +345,7 @@ TestPointCheckPciResource (
> // Device
> DumpPciDevice ((UINT8)Bus, (UINT8)Device, (UINT8)Func, &PciData);
> }
> -
> +
> //
> // If this is not a multi-function device, we can leave the loop
> // to deal with the next device.
> @@ -360,7 +367,7 @@ TestPointCheckPciResource (
> }
> }
> }
> -
> +
> Done:
> if (HandleBuf != NULL) {
> FreePool (HandleBuf);
> @@ -396,6 +403,9 @@ TestPointCheckPciBusMaster (
> UINT8 HeaderType;
> EFI_STATUS Status;
> PCI_SEGMENT_INFO *PciSegmentInfo;
> + EXEMPT_DEVICE *ExemptDevicePcdPtr;
> + BOOLEAN ExemptDeviceFound;
> + UINTN Index;
>
> PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);
> if (PciSegmentInfo == NULL) {
> @@ -407,6 +417,27 @@ TestPointCheckPciBusMaster (
> for (Bus = PciSegmentInfo[Segment].StartBusNumber; Bus <= PciSegmentInfo[Segment].EndBusNumber; Bus++) {
> for (Device = 0; Device <= 0x1F; Device++) {
> for (Function = 0; Function <= 0x7; Function++) {
> + //
> + // Some platforms have devices which do not expose any additional
> + // risk of DMA attacks but are not able to be turned off. Allow
> + // the platform to define these devices and do not record errors
> + // for these devices.
> + //
> + ExemptDevicePcdPtr = (EXEMPT_DEVICE *) PcdGetPtr (PcdTestPointIbvPlatformExemptPciBme);
> + ExemptDeviceFound = FALSE;
> + for (Index = 0; Index < (PcdGetSize (PcdTestPointIbvPlatformExemptPciBme) / sizeof (EXEMPT_DEVICE)); Index++) {
> + if (Segment == ExemptDevicePcdPtr[Index].Segment
> + && Bus == ExemptDevicePcdPtr[Index].Bus
> + && Device == ExemptDevicePcdPtr[Index].Device
> + && Function == ExemptDevicePcdPtr[Index].Function) {
> + ExemptDeviceFound = TRUE;
> + }
> + }
> +
> + if (ExemptDeviceFound) {
> + continue;
> + }
> +
> VendorId = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(PciSegmentInfo[Segment].SegmentNumber, Bus, Device,
> Function, PCI_VENDOR_ID_OFFSET));
> //
> // If VendorId = 0xffff, there does not exist a device at this
> diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiCheckPci.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiCheckPci.c
> index 1061f8ac1c62..25c3caba6eed 100644
> --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiCheckPci.c
> +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiCheckPci.c
> @@ -14,6 +14,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/PciSegmentInfoLib.h>
> #include <IndustryStandard/Pci.h>
>
> +#pragma pack(1)
> +
> + typedef struct EXEMPT_DEVICE_STRUCT {
> + UINT8 Segment;
> + UINT8 Bus;
> + UINT8 Device;
> + UINT8 Function;
> +} EXEMPT_DEVICE;
> +
> +#pragma pack()
> +
> EFI_STATUS
> TestPointCheckPciBusMaster (
> VOID
> @@ -29,6 +40,9 @@ TestPointCheckPciBusMaster (
> UINT8 HeaderType;
> EFI_STATUS Status;
> PCI_SEGMENT_INFO *PciSegmentInfo;
> + EXEMPT_DEVICE *ExemptDevicePcdPtr;
> + BOOLEAN ExemptDeviceFound;
> + UINTN Index;
>
> PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);
> if (PciSegmentInfo == NULL) {
> @@ -40,6 +54,27 @@ TestPointCheckPciBusMaster (
> for (Bus = PciSegmentInfo[Segment].StartBusNumber; Bus <= PciSegmentInfo[Segment].EndBusNumber; Bus++) {
> for (Device = 0; Device <= 0x1F; Device++) {
> for (Function = 0; Function <= 0x7; Function++) {
> + //
> + // Some platforms have devices which do not expose any additional
> + // risk of DMA attacks but are not able to be turned off. Allow
> + // the platform to define these devices and do not record errors
> + // for these devices.
> + //
> + ExemptDevicePcdPtr = (EXEMPT_DEVICE *) PcdGetPtr (PcdTestPointIbvPlatformExemptPciBme);
> + ExemptDeviceFound = FALSE;
> + for (Index = 0; Index < (PcdGetSize (PcdTestPointIbvPlatformExemptPciBme) / sizeof (EXEMPT_DEVICE)); Index++) {
> + if (Segment == ExemptDevicePcdPtr[Index].Segment
> + && Bus == ExemptDevicePcdPtr[Index].Bus
> + && Device == ExemptDevicePcdPtr[Index].Device
> + && Function == ExemptDevicePcdPtr[Index].Function) {
> + ExemptDeviceFound = TRUE;
> + }
> + }
> +
> + if (ExemptDeviceFound) {
> + continue;
> + }
> +
> VendorId = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(PciSegmentInfo[Segment].SegmentNumber, Bus, Device,
> Function, PCI_VENDOR_ID_OFFSET));
> //
> // If VendorId = 0xffff, there does not exist a device at this
> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> index bcb42f0ef9e6..259038dde4df 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> @@ -160,6 +160,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
> # Stage Advanced: {0x03, 0x0F, 0x03, 0x1D, 0x3F, 0x0F, 0x0F, 0x07, 0x03,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
> gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformFeature|{0x03, 0x0F, 0x03, 0x1D, 0x3F, 0x0F, 0x0F, 0x07, 0x03,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x00100302
>
> + # The platform may define a list of devices that are exempt from PCI BME testing.
> + # PCD Format is {SegmentNumber1, BusNumber1, DeviceNumber1, FunctionNumber1, SegmentNumber2, BusNumber2, DeviceNumber2,
> FunctionNumber2, ...}
> + gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformExemptPciBme|{0}|VOID*|0x00100303
> +
> ##
> ## The Flash relevant PCD are ineffective and will be patched basing on FDF definitions during build.
> ## Set all of them to 0 here to prevent from confusion.
> diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
> index 2ae1db4ee483..15779eb9b6de 100644
> --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
> @@ -106,3 +106,4 @@ [Protocols]
>
> [Pcd]
> gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformFeature
> + gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformExemptPciBme
> diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTestPointCheckLib.inf
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTestPointCheckLib.inf
> index 51369fcedc1e..ea6dc6b8ba34 100644
> --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTestPointCheckLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTestPointCheckLib.inf
> @@ -47,6 +47,7 @@ [Sources]
>
> [Pcd]
> gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformFeature
> + gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformExemptPciBme
>
> [Guids]
> gEfiHobMemoryAllocStackGuid
> --
> 2.28.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#78985): https://edk2.groups.io/g/devel/message/78985
> Mute This Topic: https://groups.io/mt/84776712/1643496
> Group Owner: devel+owner at edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney at intel.com]
> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79605): https://edk2.groups.io/g/devel/message/79605
Mute This Topic: https://groups.io/mt/84776712/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list