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.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Liming Gao > <gaolim...@byosoft.com.cn>; Dong, Eric <eric.d...@intel.com>; Chris Ruffin > <v-cruf...@microsoft.com>; Michael Kubacki > <michael.kuba...@microsoft.com> > Subject: [edk2-devel] [edk2-platforms][PATCH v1 1/1] > MinPlatformPkg/TestPointCheckLib: Add support for BME device > exemption > > From: Chris Ruffin <v-cruf...@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.c...@intel.com> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Chris Ruffin <v-cruf...@microsoft.com> > Co-authored-by: Michael Kubacki <michael.kuba...@microsoft.com> > Signed-off-by: Michael Kubacki <michael.kuba...@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+ow...@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kin...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
