* [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device exemption
@ 2021-08-09 19:08 Michael Kubacki
2021-08-20 0:55 ` [edk2-devel] " Michael D Kinney
0 siblings, 1 reply; 3+ messages in thread
From: Michael Kubacki @ 2021-08-09 19:08 UTC (permalink / raw)
To: devel
Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong, Chris Ruffin,
Michael Kubacki
From: Chris Ruffin <v-cruffin@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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Chris Ruffin <v-cruffin@microsoft.com>
Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device exemption
2021-08-09 19:08 [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device exemption Michael Kubacki
@ 2021-08-20 0:55 ` Michael D Kinney
2021-08-20 1:25 ` Michael Kubacki
0 siblings, 1 reply; 3+ messages in thread
From: Michael D Kinney @ 2021-08-20 0:55 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com,
Kinney, Michael D
Cc: Chiu, Chasel, Desimone, Nathaniel L, Liming Gao, Dong, Eric,
Chris Ruffin, Michael Kubacki
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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Monday, August 9, 2021 12:09 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Chris Ruffin <v-cruffin@microsoft.com>; Michael Kubacki
> <michael.kubacki@microsoft.com>
> Subject: [edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device
> exemption
>
> From: Chris Ruffin <v-cruffin@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@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Chris Ruffin <v-cruffin@microsoft.com>
> Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@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@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device exemption
2021-08-20 0:55 ` [edk2-devel] " Michael D Kinney
@ 2021-08-20 1:25 ` Michael Kubacki
0 siblings, 0 replies; 3+ messages in thread
From: Michael Kubacki @ 2021-08-20 1:25 UTC (permalink / raw)
To: devel, michael.d.kinney
Cc: Chiu, Chasel, Desimone, Nathaniel L, Liming Gao, Dong, Eric,
Chris Ruffin, Michael Kubacki
That's a good point. We were mostly considering this for on-chip PCI
devices. I will rework this in v2.
Thanks,
Michael
On 8/19/2021 8:55 PM, Michael D Kinney wrote:
> 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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
>> Sent: Monday, August 9, 2021 12:09 PM
>> To: devel@edk2.groups.io
>> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Liming Gao
>> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Chris Ruffin <v-cruffin@microsoft.com>; Michael Kubacki
>> <michael.kubacki@microsoft.com>
>> Subject: [edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device
>> exemption
>>
>> From: Chris Ruffin <v-cruffin@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@intel.com>
>> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Chris Ruffin <v-cruffin@microsoft.com>
>> Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@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@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
>> -=-=-=-=-=-=
>>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-20 1:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-09 19:08 [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device exemption Michael Kubacki
2021-08-20 0:55 ` [edk2-devel] " Michael D Kinney
2021-08-20 1:25 ` Michael Kubacki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox