From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.78972.1629422713463971782 for ; Thu, 19 Aug 2021 18:25:13 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=ftm9XxRP; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [10.0.0.19] (c-73-27-179-174.hsd1.fl.comcast.net [73.27.179.174]) by linux.microsoft.com (Postfix) with ESMTPSA id 097B320C33BF; Thu, 19 Aug 2021 18:25:11 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 097B320C33BF DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1629422712; bh=ZZ1cfhtww0hT0b9basOALhmzlwCzPdKnijmCjQNTLwY=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=ftm9XxRPM7wHlGZCawwGpFYtGWvqhecTYJWOh7jmoa8iv1exGn/gaZUxwmifES3L7 RvCC4ZmMFpW/sKwITIYUGmMgU5sB64qkfdhx+ag85qa1s6WzCG3wvebeHcwuThws4r MDAsFJg+BR9kSfoOKUarmq6Z21m5zERcQM9ZAeOU= Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device exemption To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: "Chiu, Chasel" , "Desimone, Nathaniel L" , Liming Gao , "Dong, Eric" , Chris Ruffin , Michael Kubacki References: <20210809190854.3043-1-mikuback@linux.microsoft.com> From: "Michael Kubacki" Message-ID: <3ec701b6-84c4-7567-6cd6-99a8b6ebebac@linux.microsoft.com> Date: Thu, 19 Aug 2021 21:25:11 -0400 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 On Behalf Of Michael Kubacki >> Sent: Monday, August 9, 2021 12:09 PM >> To: devel@edk2.groups.io >> Cc: Chiu, Chasel ; Desimone, Nathaniel L ; Liming Gao >> ; Dong, Eric ; Chris Ruffin ; Michael Kubacki >> >> Subject: [edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device >> exemption >> >> From: Chris Ruffin >> >> 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 >> Cc: Nate DeSimone >> Cc: Liming Gao >> Cc: Eric Dong >> Cc: Chris Ruffin >> Co-authored-by: Michael Kubacki >> Signed-off-by: Michael Kubacki >> --- >> 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 >> #include >> >> +#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] >> -=-=-=-=-=-= >> > > > > > >