Re: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent buffer overruns
OK, I got it. Now I have no comment of this patch. I think the description of
'Length' should be 'the whole length of the GT block include GT Block Timer
Structure' or '20+("GT Block Timer Offset" - 20) + n * 40, where n ...' in the
ACPI spec.
Reviewed-by: Zhichao Gao
Thanks,
Zhichao
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Sami Mujawar
> Sent: Monday, August 5, 2019 5:55 PM
> To: devel@edk2.groups.io; Gao, Zhichao ; Krzysztof
> Koch
> Cc: Carsey, Jaben ; Ni, Ray ;
> Matteo Carlini ; nd
> Subject: Re: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent
> buffer overruns
>
> Hi Zhichao,
>
> Please see my response inline.
>
> Regards,
>
> Sami Mujawar
>
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Gao,
> Zhichao via Groups.Io
> Sent: 05 August 2019 08:23 AM
> To: devel@edk2.groups.io; Krzysztof Koch
> Cc: Carsey, Jaben ; Ni, Ray ;
> Sami Mujawar ; Matteo Carlini
> ; nd
> Subject: Re: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent
> buffer overruns
>
> One confusion below:
>
> > -Original Message-
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Krzysztof Koch
> > Sent: Thursday, August 1, 2019 4:44 PM
> > To: devel@edk2.groups.io
> > Cc: Carsey, Jaben ; Ni, Ray
> > ; Gao, Zhichao ;
> > sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com
> > Subject: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent
> > buffer overruns
> >
> > Modify the GTDT table parsing logic to prevent reading past the ACPI
> > buffer lengths provided and to make it consistent with other table
> > parsers. This includes converting the do-while loop in ParseAcpiGtdt() into
> a while loop.
> >
> > Remove a check which ensures that the entire Platform GT Block
> > Structure buffer has been parsed. The ACPI specification does not ban
> > from defining buffers which are larger than the size indicated by the
> > count and sizes of substructures which constitute it.
> >
> > Change the data type of the Length parameter to the DumpGTBlock()
> > function to reflect the width of the respective ACPI structure's field.
> >
> > References:
> > - ACPI 6.3, January 2019, Table 5-124
> >
> > Signed-off-by: Krzysztof Koch
> > ---
> >
> > Notes:
> > v1:
> > - Prevent buffer overruns in GTDT acpiview parser [Krzysztof]
> >
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> > | 147 ++--
> > 1 file changed, 76 insertions(+), 71 deletions(-)
> >
> > diff --git
> >
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> > .c
> >
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> > .c
> > index
> >
> 1e5b5764f50a2d29aa904c889bc89af5bdc3af5c..57174e14c80072f12b90e1996e
> > be8f0002d0c404 100644
> > ---
> >
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> > .c
> > +++
> > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtPars
> > +++ er.c
> > @@ -23,7 +23,6 @@ STATIC CONST UINT8* PlatformTimerType; STATIC
> > CONST UINT16* PlatformTimerLength; STATIC CONST UINT32*
> > GtBlockTimerCount; STATIC CONST UINT32* GtBlockTimerOffset; -STATIC
> > CONST UINT16* GtBlockLength; STATIC
> ACPI_DESCRIPTION_HEADER_INFO
> > AcpiHdrInfo;
> >
> > /**
> > @@ -127,7 +126,7 @@ STATIC CONST ACPI_PARSER
> > GtPlatformTimerHeaderParser[] = { **/ STATIC CONST ACPI_PARSER
> > GtBlockParser[] = {
> >{L"Type", 1, 0, L"%d", NULL, NULL, NULL, NULL},
> > - {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL},
> > + {L"Length", 2, 1, L"%d", NULL, NULL, NULL, NULL},
> >{L"Reserved", 1, 3, L"%x", NULL, NULL, NULL, NULL},
> >{L"Physical address (CntCtlBase)", 8, 4, L"0x%lx", NULL, NULL, NULL,
> > NULL},
> >{L"Timer Count", 4, 12, L"%d", NULL, (VOID**), @@
> > -
> > 168,56 +167,43 @@ STATIC CONST ACPI_PARSER
> SBSAGenericWatchdogParser[]
> > = {
> > /**
> >This function parses the Platform GT Block.
> >
> > - @param [in] Ptr Pointer to the start of the GT Block data.
> > - @param [in] Length Length of the GT Block structure.
> > + @param [in] Ptr Pointer to the start of the GT Block data.
> > + @param [in] LengthLength of the GT Block structure.
> > **/
> > STATIC
> > VOID
> > DumpGTBlock (
> >IN UINT8* Ptr,
> > - IN UINT32 Length
> > + IN UINT16 Length
> >)
> > {
> >UINT32 Index;
> >UINT32 Offset;
> > - UINT32 GTBlockTimerLength;
> >
> > - Offset = ParseAcpi (
> > - TRUE,
> > - 2,
> > - "GT Block",
> > - Ptr,
> > - Length,
> > - PARSER_PARAMS (GtBlockParser)
> > - );
> > - GTBlockTimerLength = (*GtBlockLength - Offset) /
> > (*GtBlockTimerCount);
> > - Length -= Offset;
> > + ParseAcpi (
> > +TRUE,
> > +2,
> > +"GT Block",
> > +Ptr,
> > +Length,
> > +PARSER_PARAMS (GtBlockParser)
> > +);
>
Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support multiple controllers
Hao Wu,
I agree that patches should be cleaned up for
submission to edk2 repo. The work in edk2-staging
may contain bug fixes and design changes in the
patch history that can be consolidated to a smaller
patch set. Also, all feedback to a patch set from
edk2-staging must be addressed just like any other
submission which may include splitting or merging
patches.
Mike
> -Original Message-
> From: Wu, Hao A
> Sent: Monday, August 5, 2019 7:49 PM
> To: Jin, Eric ;
> devel@edk2.groups.io
> Cc: Sean Brogan ; Bret
> Barkelew ; Wang, Jian J
> ; Kinney, Michael D
> ; Gao, Liming
> ; af...@apple.com;
> ler...@redhat.com; leif.lindh...@linaro.org
> Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> Enhance ESRT to support multiple controllers
>
> > -Original Message-
> > From: Jin, Eric
> > Sent: Monday, August 05, 2019 4:03 PM
> > To: devel@edk2.groups.io
> > Cc: Sean Brogan; Bret Barkelew; Wang, Jian J; Wu, Hao
> A; Kinney,
> > Michael D
> > Subject: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance
> ESRT to support
> > multiple controllers
> >
> > REF:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1525
> >
> > The patch is to merge multiple FMP instances into
> single ESRT entry
> > when they have the same GUID.
> >
> > The policy to LastAttemptStatus/LastAttemptVersion of
> ESRT entry is:
> > If all the LastAttemptStatus are
> LAST_ATTEMPT_STATUS_SUCCESS, then
> > LastAttemptVersion should be the smallest of
> LastAttemptVersion. If
> > any of the LastAttemptStatus is not
> LAST_ATTEMPT_STATUS_SUCCESS, then
> > the LastAttemptVersion/LastAttemptStatus should be
> the values of the
> > first FMP instance whose LastAttemptStatus is not
> > LAST_ATTEMPT_STATUS_SUCCESS.
> >
> > To detect possible duplicated GUID/HardwareInstance,
> a table of
> > GUID/HardwareInstance pairs from all the
> > EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP instances
> is built. If a
> > duplicate is found, then generate a DEBUG_ERROR
> message, generate an
> > ASSERT(), and ignore the duplicate
> EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> >
> > Add an internal worker function called
> FmpGetFirmwareImageDescriptor()
> > that retrieves the list of
> EFI_FIRMWARE_IMAGE_DESCRIPTORs from a
> > single FMP instance and returns the descriptors in an
> allocated
> > buffer. This function is used to get the descriptors
> used to build the
> > table of unique GUID/HardwareInstance pairs. It is
> then used again to
> > generate the ESRT Table from all the
> EFI_FIRMWARE_IMAGE_DESCRIPTORs
> > from all the FMP instances. 2 passes are performed so
> the total number
> > of descriptors is known. This allows the correct
> sized buffers to
> > always be allocated.
>
>
> The patch looks good to me.
> Reviewed-by: Hao A Wu
>
> I will wait a little longer to push the patch to see if
> there is additional feedbacks from stewards with regard
> to the approach when merging changes from the edk2-
> staging repo.
>
> Best Regards,
> Hao Wu
>
>
> >
> > Cc: Sean Brogan
> > Cc: Bret Barkelew
> > Cc: Jian J Wang
> > Cc: Hao A Wu
> > Signed-off-by: Michael D Kinney
>
> > Signed-off-by: Eric Jin
> > ---
> > MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394
> +---
> >
> > 1 file changed, 257 insertions(+), 137 deletions(-)
> >
> > diff --git
> a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> > b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> > index 2356da662e..4670349f89 100644
> > --- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> > +++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> > @@ -2,7 +2,7 @@
> >Publishes ESRT table from Firmware Management
> Protocol instances
> >
> >Copyright (c) 2016, Microsoft Corporation
> > - Copyright (c) 2018, Intel Corporation. All rights
> reserved.
> > + Copyright (c) 2018 - 2019, Intel Corporation. All
> rights
> > + reserved.
> >
> >All rights reserved.
> >SPDX-License-Identifier: BSD-2-Clause-Patent @@ -
> 21,6 +21,22 @@
> > #include #include
>
> >
> > +///
> > +/// Structure for array of unique
> GUID/HardwareInstance pairs from
> > +the /// current set of
> EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP
> > Protocols.
> > +///
> > +typedef struct {
> > + ///
> > + /// A unique GUID identifying the firmware image
> type.
> > + ///
> > + EFI_GUID ImageTypeGuid;
> > + ///
> > + /// An optional number to identify the unique
> hardware instance
> > +within
> > the
> > + /// system for devices that may have multiple
> instances whenever
> > possible.
> > + ///
> > + UINT64HardwareInstance;
> > +} GUID_HARDWAREINSTANCE_PAIR;
> > +
> > /**
> > Print ESRT to debug console.
> >
> > @@ -33,11 +49,6 @@ PrintTable (
> >IN EFI_SYSTEM_RESOURCE_TABLE *Table
> >);
> >
> > -//
> > -// Number of ESRT entries to grow by each time we
> run out of room -//
> > -#define GROWTH_STEP 10
> > -
> > /**
> >Install EFI System Resource Table into the UEFI
> Configuration Table
> >
> > @@ -101,90 +112,129 @@ IsSystemFmp (
>
Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support multiple controllers
> -Original Message-
> From: Jin, Eric
> Sent: Monday, August 05, 2019 4:03 PM
> To: devel@edk2.groups.io
> Cc: Sean Brogan; Bret Barkelew; Wang, Jian J; Wu, Hao A; Kinney, Michael D
> Subject: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support
> multiple controllers
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1525
>
> The patch is to merge multiple FMP instances into single ESRT entry
> when they have the same GUID.
>
> The policy to LastAttemptStatus/LastAttemptVersion of ESRT entry is:
> If all the LastAttemptStatus are LAST_ATTEMPT_STATUS_SUCCESS, then
> LastAttemptVersion should be the smallest of LastAttemptVersion. If
> any of the LastAttemptStatus is not LAST_ATTEMPT_STATUS_SUCCESS,
> then the LastAttemptVersion/LastAttemptStatus should be the values
> of the first FMP instance whose LastAttemptStatus is not
> LAST_ATTEMPT_STATUS_SUCCESS.
>
> To detect possible duplicated GUID/HardwareInstance, a table of
> GUID/HardwareInstance pairs from all the
> EFI_FIRMWARE_IMAGE_DESCRIPTORs
> from all FMP instances is built. If a duplicate is found, then generate
> a DEBUG_ERROR message, generate an ASSERT(), and ignore the duplicate
> EFI_FIRMWARE_IMAGE_DESCRIPTOR.
>
> Add an internal worker function called FmpGetFirmwareImageDescriptor()
> that retrieves the list of EFI_FIRMWARE_IMAGE_DESCRIPTORs from a single
> FMP instance and returns the descriptors in an allocated buffer. This
> function is used to get the descriptors used to build the table of
> unique GUID/HardwareInstance pairs. It is then used again to generate
> the ESRT Table from all the EFI_FIRMWARE_IMAGE_DESCRIPTORs from all
> the
> FMP instances. 2 passes are performed so the total number of
> descriptors is known. This allows the correct sized buffers to always
> be allocated.
The patch looks good to me.
Reviewed-by: Hao A Wu
I will wait a little longer to push the patch to see if there is
additional feedbacks from stewards with regard to the approach when
merging changes from the edk2-staging repo.
Best Regards,
Hao Wu
>
> Cc: Sean Brogan
> Cc: Bret Barkelew
> Cc: Jian J Wang
> Cc: Hao A Wu
> Signed-off-by: Michael D Kinney
> Signed-off-by: Eric Jin
> ---
> MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 +---
>
> 1 file changed, 257 insertions(+), 137 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> index 2356da662e..4670349f89 100644
> --- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> +++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> @@ -2,7 +2,7 @@
>Publishes ESRT table from Firmware Management Protocol instances
>
>Copyright (c) 2016, Microsoft Corporation
> - Copyright (c) 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
>
>All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -21,6 +21,22 @@
> #include
> #include
>
> +///
> +/// Structure for array of unique GUID/HardwareInstance pairs from the
> +/// current set of EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP
> Protocols.
> +///
> +typedef struct {
> + ///
> + /// A unique GUID identifying the firmware image type.
> + ///
> + EFI_GUID ImageTypeGuid;
> + ///
> + /// An optional number to identify the unique hardware instance within
> the
> + /// system for devices that may have multiple instances whenever
> possible.
> + ///
> + UINT64HardwareInstance;
> +} GUID_HARDWAREINSTANCE_PAIR;
> +
> /**
> Print ESRT to debug console.
>
> @@ -33,11 +49,6 @@ PrintTable (
>IN EFI_SYSTEM_RESOURCE_TABLE *Table
>);
>
> -//
> -// Number of ESRT entries to grow by each time we run out of room
> -//
> -#define GROWTH_STEP 10
> -
> /**
>Install EFI System Resource Table into the UEFI Configuration Table
>
> @@ -101,90 +112,129 @@ IsSystemFmp (
> }
>
> /**
> - Function to create a single ESRT Entry and add it to the ESRT
> - given a FMP descriptor. If the guid is already in the ESRT it
> - will be ignored. The ESRT will grow if it does not have enough room.
> -
> - @param[in, out] Table On input, pointer to the pointer to the
> ESRT.
> -On output, same as input or pointer to
> the pointer
> -to new enlarged ESRT.
> - @param[in] FmpImageInfoBuf Pointer to the
> EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> - @param[in] FmpVersionFMP Version.
> -
> - @return Status code.
> + Function to create a single ESRT Entry and add it to the ESRT with
> + a given FMP descriptor. If the GUID is already in the ESRT, then the ESRT
> + entry is updated.
> +
> + @param[in,out] TablePointer to the ESRT Table.
> + @param[in,out] HardwareInstancesPointer to the
> GUID_HARDWAREINSTANCE_PAIR.
> + @param[in,out] NumberOfDescriptors The number of
>
[edk2-devel] [PATCH] MdePkg/BaseUefiDecompressLib: Add missing description for parameter
The description of parameter Version is missing in comments. So add the description. Cc: Michael D Kinney Cc: Liming Gao Signed-off-by: Shenglei Zhang --- .../BaseUefiDecompressLib/BaseUefiDecompressLibInternals.h | 1 + 1 file changed, 1 insertion(+) diff --git a/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLibInternals.h b/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLibInternals.h index 0bfb50333777..4df3fa388ddb 100644 --- a/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLibInternals.h +++ b/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLibInternals.h @@ -230,6 +230,7 @@ Decode ( @param Scratch A temporary scratch buffer that is used to perform the decompression. This is an optional parameter that may be NULL if the required scratch buffer size is 0. + @param Version 1 for UEFI Decompress algoruthm, 2 for Tiano Decompess algorithm. @retval RETURN_SUCCESS Decompression completed successfully, and the uncompressed buffer is returned in Destination. -- 2.18.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44920): https://edk2.groups.io/g/devel/message/44920 Mute This Topic: https://groups.io/mt/32737332/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] MdeModulePkg/DxeCapsuleLibFmp: Add missing description for parameter
The description of parameter CapFileName is missing in comments. So add the description. Cc: Jian J Wang Cc: Hao A Wu Signed-off-by: Shenglei Zhang --- MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c index 2cecc87385b3..f2cd0ad3e816 100644 --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c @@ -136,6 +136,7 @@ UINT32 mCapsuleTotalNumber; Caution: This function may receive untrusted input. @param[in] CapsuleHeader Points to a capsule header. + @param[in] CapFileName Capsule file name. @param[out] ResetRequired Indicates whether reset is required or not. @retval EFI_SUCESSProcess Capsule Image successfully. -- 2.18.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44921): https://edk2.groups.io/g/devel/message/44921 Mute This Topic: https://groups.io/mt/32737333/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] UefiCpuPkg/PiSmmCpuDxeSmm: don't free page table pages that are required to handle current page fault
Reviewed-by: Jian J Wang
> -Original Message-
> From: Rusocki, Krzysztof
> Sent: Tuesday, August 06, 2019 5:58 AM
> To: devel@edk2.groups.io
> Cc: Nikodem, Damian ; Dong, Eric
> ; Ni, Ray ; Wang, Jian J
> ; Laszlo Ersek ; Rusocki,
> Krzysztof
> Subject: UefiCpuPkg/PiSmmCpuDxeSmm: don't free page table pages that
> are required to handle current page fault
>
> From: Damian Nikodem
>
> Reclaim may free page table pages that are required to handle current page
> fault. This causes a page leak, and, after sufficent number of specific
> page fault+reclaim pairs, we run out of reclaimable pages and hit:
>
> ASSERT (MinAcc != (UINT64)-1);
>
> To remedy, prevent pages essential to handling current page fault:
> (1) from being considered as reclaim candidates (first reclaim phase)
> (2) from being freed as part of "branch cleanup" (second reclaim phase)
>
> Signed-off-by: Damian Nikodem
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Jian J Wang
> Cc: Laszlo Ersek
> Cc: Krzysztof Rusocki
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index a3b62f7787..f11323f439 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -543,6 +543,11 @@ ReclaimPages (
>UINT64 *ReleasePageAddress;
>IA32_CR4 Cr4;
>BOOLEAN Enable5LevelPaging;
> + UINT64 PFAddress;
> + UINT64 PFAddressPml5Index;
> + UINT64 PFAddressPml4Index;
> + UINT64 PFAddressPdptIndex;
> + UINT64 PFAddressPdtIndex;
>
>Pml4 = NULL;
>Pdpt = NULL;
> @@ -554,6 +559,11 @@ ReclaimPages (
>MinPdt = (UINTN)-1;
>Acc = 0;
>ReleasePageAddress = 0;
> + PFAddress = AsmReadCr2 ();
> + PFAddressPml5Index = BitFieldRead64 (PFAddress, 48, 48 + 8);
> + PFAddressPml4Index = BitFieldRead64 (PFAddress, 39, 39 + 8);
> + PFAddressPdptIndex = BitFieldRead64 (PFAddress, 30, 30 + 8);
> + PFAddressPdtIndex = BitFieldRead64 (PFAddress, 21, 21 + 8);
>
>Cr4.UintN = AsmReadCr4 ();
>Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
> @@ -628,40 +638,46 @@ ReclaimPages (
>// we will find the entry has the smallest access record value
>//
>PDPTEIgnore = TRUE;
> - Acc = GetAndUpdateAccNum (Pdt + PdtIndex);
> + if (PdtIndex != PFAddressPdtIndex || PdptIndex !=
> PFAddressPdptIndex ||
> + Pml4Index != PFAddressPml4Index || Pml5Index !=
> PFAddressPml5Index) {
> +Acc = GetAndUpdateAccNum (Pdt + PdtIndex);
> +if (Acc < MinAcc) {
> + //
> + // If the PD entry has the smallest access record value,
> + // save the Page address to be released
> + //
> + MinAcc = Acc;
> + MinPml5 = Pml5Index;
> + MinPml4 = Pml4Index;
> + MinPdpt = PdptIndex;
> + MinPdt = PdtIndex;
> + ReleasePageAddress = Pdt + PdtIndex;
> +}
> + }
> +}
> + }
> + if (!PDPTEIgnore) {
> +//
> +// If this PDPT entry has no PDT entries pointer to 4 KByte
> pages,
> +// it should only has the entries point to 2 MByte Pages
> +//
> +if (PdptIndex != PFAddressPdptIndex || Pml4Index !=
> PFAddressPml4Index ||
> +Pml5Index != PFAddressPml5Index) {
> + Acc = GetAndUpdateAccNum (Pdpt + PdptIndex);
>if (Acc < MinAcc) {
> //
> -// If the PD entry has the smallest access record value,
> +// If the PDPT entry has the smallest access record value,
> // save the Page address to be released
> //
> MinAcc = Acc;
> MinPml5 = Pml5Index;
> MinPml4 = Pml4Index;
> MinPdpt = PdptIndex;
> -MinPdt = PdtIndex;
> -ReleasePageAddress = Pdt + PdtIndex;
> +MinPdt = (UINTN)-1;
> +ReleasePageAddress = Pdpt + PdptIndex;
>}
> }
>}
> - if (!PDPTEIgnore) {
> -//
> -// If this PDPT entry has no PDT entries pointer to 4 KByte
> pages,
> -// it should only has the entries point to 2 MByte Pages
> -//
> -Acc = GetAndUpdateAccNum (Pdpt + PdptIndex);
> -if (Acc < MinAcc) {
> - //
> - // If the PDPT entry has the smallest access record value,
> - // save the Page address to be released
> - //
> - MinAcc = Acc;
> - MinPml5 = Pml5Index;
> - MinPml4 = Pml4Index;
>
Re: [edk2-devel] [PATCH v2 1/1] CryptoPkg/BaseCryptLib: Wrap OpenSSL HKDF algorithm
Reviewed-by: Jian J Wang > -Original Message- > From: West, Gary > Sent: Wednesday, July 31, 2019 5:54 AM > To: devel@edk2.groups.io > Cc: West, Gary ; West, Gary ; > Wang, Jian J ; Ye, Ting > Subject: [PATCH v2 1/1] CryptoPkg/BaseCryptLib: Wrap OpenSSL HKDF > algorithm > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1928 > > 1. Implement OpenSSL HKDF wrapped function in CryptHkdf.c file. > 2. Implement stub implementation function in CryptHkdfNull.c file. > 3. Add wrapped HKDF function declaration to BaseCryptLib.h file. > 4. Add CryptHkdf.c to module information BaseCryptLib.inf file. > 5. Add CryptHkdfNull.c to module information PeiCryptLib.inf, >RuntimeCryptLib.inf and SmmCryptLib.inf > > Signed-off-by: Gary West > Cc: Jian Wang > Cc: Ting Ye > --- > CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf| 1 + > CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf | 4 +- > CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf | 1 + > CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 1 + > CryptoPkg/Include/Library/BaseCryptLib.h | 33 + > CryptoPkg/Library/BaseCryptLib/Kdf/CryptHkdf.c | 75 > > CryptoPkg/Library/BaseCryptLib/Kdf/CryptHkdfNull.c | 43 +++ > 7 files changed, 155 insertions(+), 3 deletions(-) > > diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > index 020df3c19b3c..8d4988e8c6b4 100644 > --- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > +++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > @@ -37,6 +37,7 @@ [Sources] >Hmac/CryptHmacMd5.c >Hmac/CryptHmacSha1.c >Hmac/CryptHmacSha256.c > + Kdf/CryptHkdf.c >Cipher/CryptAes.c >Cipher/CryptTdes.c >Cipher/CryptArc4.c > diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf > b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf > index 99dbad23ed5d..3da8bd848017 100644 > --- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf > +++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf > @@ -44,10 +44,10 @@ [Sources] >Hmac/CryptHmacMd5Null.c >Hmac/CryptHmacSha1Null.c >Hmac/CryptHmacSha256Null.c > + Kdf/CryptHkdfNull.c >Cipher/CryptAesNull.c >Cipher/CryptTdesNull.c >Cipher/CryptArc4Null.c > - >Pk/CryptRsaBasic.c >Pk/CryptRsaExtNull.c >Pk/CryptPkcs1OaepNull.c > @@ -56,13 +56,11 @@ [Sources] >Pk/CryptPkcs7VerifyCommon.c >Pk/CryptPkcs7VerifyBase.c >Pk/CryptPkcs7VerifyEku.c > - >Pk/CryptDhNull.c >Pk/CryptX509Null.c >Pk/CryptAuthenticodeNull.c >Pk/CryptTsNull.c >Pem/CryptPemNull.c > - >Rand/CryptRandNull.c > >SysCall/CrtWrapper.c > diff --git a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > index 0e58d2b5b0ea..21a481eb7767 100644 > --- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > +++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > @@ -43,6 +43,7 @@ [Sources] >Hmac/CryptHmacMd5Null.c >Hmac/CryptHmacSha1Null.c >Hmac/CryptHmacSha256Null.c > + Kdf/CryptHkdfNull.c >Cipher/CryptAesNull.c >Cipher/CryptTdesNull.c >Cipher/CryptArc4Null.c > diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > index c79f2bf4c6c0..7c187e21b3b9 100644 > --- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > +++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > @@ -43,6 +43,7 @@ [Sources] >Hmac/CryptHmacMd5Null.c >Hmac/CryptHmacSha1Null.c >Hmac/CryptHmacSha256.c > + Kdf/CryptHkdfNull.c >Cipher/CryptAes.c >Cipher/CryptTdesNull.c >Cipher/CryptArc4Null.c > diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h > b/CryptoPkg/Include/Library/BaseCryptLib.h > index 19d1afe3c8c0..da32bb2444fd 100644 > --- a/CryptoPkg/Include/Library/BaseCryptLib.h > +++ b/CryptoPkg/Include/Library/BaseCryptLib.h > @@ -3122,4 +3122,37 @@ RandomBytes ( >IN UINTN Size >); > > +// > = > +//Key Derivation Function Primitive > +// > = > + > +/** > + Derive key data using HMAC-SHA256 based KDF. > + > + @param[in] Key Pointer to the user-supplied key. > + @param[in] KeySize Key size in bytes. > + @param[in] Salt Pointer to the salt(non-secret) value. > + @param[in] SaltSize Salt size in bytes. > + @param[in] Info Pointer to the application specific info. > + @param[in] InfoSize Info size in bytes. > + @param[Out] Out Pointer to buffer to receive hkdf value. > + @param[in] OutSize Size of hkdf bytes to generate. > + > + @retval TRUE Hkdf generated successfully. > + @retval FALSE Hkdf generation failed. > + > +**/ > +BOOLEAN > +EFIAPI > +HkdfSha256ExtractAndExpand ( > + IN CONST UINT8
Re: [edk2-devel] UefiCpuPkg/PiSmmCpuDxeSmm: don't free page table pages that are required to handle current page fault
Reviewed-by: Ray Ni
> -Original Message-
> From: Rusocki, Krzysztof
> Sent: Tuesday, August 6, 2019 5:58 AM
> To: devel@edk2.groups.io
> Cc: Nikodem, Damian ; Dong, Eric
> ; Ni, Ray ; Wang, Jian J
> ; Laszlo Ersek ; Rusocki,
> Krzysztof
> Subject: UefiCpuPkg/PiSmmCpuDxeSmm: don't free page table pages that
> are required to handle current page fault
>
> From: Damian Nikodem
>
> Reclaim may free page table pages that are required to handle current page
> fault. This causes a page leak, and, after sufficent number of specific page
> fault+reclaim pairs, we run out of reclaimable pages and hit:
>
> ASSERT (MinAcc != (UINT64)-1);
>
> To remedy, prevent pages essential to handling current page fault:
> (1) from being considered as reclaim candidates (first reclaim phase)
> (2) from being freed as part of "branch cleanup" (second reclaim phase)
>
> Signed-off-by: Damian Nikodem
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Jian J Wang
> Cc: Laszlo Ersek
> Cc: Krzysztof Rusocki
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index a3b62f7787..f11323f439 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -543,6 +543,11 @@ ReclaimPages (
>UINT64 *ReleasePageAddress;
>IA32_CR4 Cr4;
>BOOLEAN Enable5LevelPaging;
> + UINT64 PFAddress;
> + UINT64 PFAddressPml5Index;
> + UINT64 PFAddressPml4Index;
> + UINT64 PFAddressPdptIndex;
> + UINT64 PFAddressPdtIndex;
>
>Pml4 = NULL;
>Pdpt = NULL;
> @@ -554,6 +559,11 @@ ReclaimPages (
>MinPdt = (UINTN)-1;
>Acc = 0;
>ReleasePageAddress = 0;
> + PFAddress = AsmReadCr2 ();
> + PFAddressPml5Index = BitFieldRead64 (PFAddress, 48, 48 + 8);
> + PFAddressPml4Index = BitFieldRead64 (PFAddress, 39, 39 + 8);
> + PFAddressPdptIndex = BitFieldRead64 (PFAddress, 30, 30 + 8);
> + PFAddressPdtIndex = BitFieldRead64 (PFAddress, 21, 21 + 8);
>
>Cr4.UintN = AsmReadCr4 ();
>Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1); @@ -628,40 +638,46
> @@ ReclaimPages (
>// we will find the entry has the smallest access record value
>//
>PDPTEIgnore = TRUE;
> - Acc = GetAndUpdateAccNum (Pdt + PdtIndex);
> + if (PdtIndex != PFAddressPdtIndex || PdptIndex !=
> PFAddressPdptIndex ||
> + Pml4Index != PFAddressPml4Index || Pml5Index !=
> PFAddressPml5Index) {
> +Acc = GetAndUpdateAccNum (Pdt + PdtIndex);
> +if (Acc < MinAcc) {
> + //
> + // If the PD entry has the smallest access record value,
> + // save the Page address to be released
> + //
> + MinAcc = Acc;
> + MinPml5 = Pml5Index;
> + MinPml4 = Pml4Index;
> + MinPdpt = PdptIndex;
> + MinPdt = PdtIndex;
> + ReleasePageAddress = Pdt + PdtIndex;
> +}
> + }
> +}
> + }
> + if (!PDPTEIgnore) {
> +//
> +// If this PDPT entry has no PDT entries pointer to 4 KByte
> pages,
> +// it should only has the entries point to 2 MByte Pages
> +//
> +if (PdptIndex != PFAddressPdptIndex || Pml4Index !=
> PFAddressPml4Index ||
> +Pml5Index != PFAddressPml5Index) {
> + Acc = GetAndUpdateAccNum (Pdpt + PdptIndex);
>if (Acc < MinAcc) {
> //
> -// If the PD entry has the smallest access record value,
> +// If the PDPT entry has the smallest access record
> + value,
> // save the Page address to be released
> //
> MinAcc = Acc;
> MinPml5 = Pml5Index;
> MinPml4 = Pml4Index;
> MinPdpt = PdptIndex;
> -MinPdt = PdtIndex;
> -ReleasePageAddress = Pdt + PdtIndex;
> +MinPdt = (UINTN)-1;
> +ReleasePageAddress = Pdpt + PdptIndex;
>}
> }
>}
> - if (!PDPTEIgnore) {
> -//
> -// If this PDPT entry has no PDT entries pointer to 4 KByte
> pages,
> -// it should only has the entries point to 2 MByte Pages
> -//
> -Acc = GetAndUpdateAccNum (Pdpt + PdptIndex);
> -if (Acc < MinAcc) {
> - //
> - // If the PDPT entry has the smallest access record value,
> - // save the Page address to be released
> - //
> - MinAcc = Acc;
> - MinPml5 = Pml5Index;
> - MinPml4 = Pml4Index;
> -
Re: [edk2-devel] [Patch 0/4] EmulatorPkg: Fix XCODE5 and VS2015 build failures
Reviewed-by: Ray Ni > -Original Message- > From: Kinney, Michael D > Sent: Friday, August 2, 2019 11:23 AM > To: devel@edk2.groups.io > Cc: Gao, Liming ; Justen, Jordan L > ; Andrew Fish ; Ni, Ray > > Subject: [Patch 0/4] EmulatorPkg: Fix XCODE5 and VS2015 build failures > > https://bugzilla.tianocore.org/show_bug.cgi?id=2045 > https://bugzilla.tianocore.org/show_bug.cgi?id=2046 > > * Fix tool paths for VS2015x86 builds of EmulatorPkg > * Add NetworkPkg to UNIX Host.inf dependencies > * Fix MacOS redefinition of NTOHLL and HTONLL macros > between EDK II and standard includes in UNIX Host.h. > * Fix [BuildOptions] flags for IA32/X64 XCODE5 build issues > * Fix function pointer type mismatch in reverse gasket callback > * Disable #pragma GCC visibility push (hidden) for XCODE5 > > Cc: Liming Gao > Cc: Jordan Justen > Cc: Andrew Fish > Cc: Ray Ni > Signed-off-by: Michael D Kinney > > Michael D Kinney (3): > EmulatorPkg/Unix/Host: Fix NetworkPkg dependencies > EmulatorPkg/Unix/Host: Fix XCODE5 IA32/X64 build failure > MdePkg/X64/ProcessorBind.h: Fix EmulatorPkg X64 XCODE5 > > Michael Kinney (1): > EmulatorPkg: Fix VS2015 build when VS2017 also installed > > EmulatorPkg/Unix/Host/EmuThunk.c | 4 ++-- > EmulatorPkg/Unix/Host/Gasket.h | 4 ++-- > EmulatorPkg/Unix/Host/Host.h | 6 +- > EmulatorPkg/Unix/Host/Host.inf | 9 + > EmulatorPkg/Unix/Host/Pthreads.c | 4 ++-- > EmulatorPkg/Win/Host/WinHost.inf | 22 -- > MdePkg/Include/X64/ProcessorBind.h | 4 ++-- > 7 files changed, 26 insertions(+), 27 deletions(-) > > -- > 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44916): https://edk2.groups.io/g/devel/message/44916 Mute This Topic: https://groups.io/mt/32687215/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch 3/3] BaseTools/PatchCheck: Disable text conversion in 'git show'
Laszlo, The context of this change is only to the PatchCheck.py tool. and how that tool uses git show. I agree with the summary of very flexible capabilities in git to help developers review different types of files. All of those settings that were added to support UNI files in UTF-16 file format were very valuable when we had to review the text changes to those binary files. We should be using UTF-8 now, and we can even extend PatchCheck.py to flag an error if a UNI file is in UTF-16 format. I still prefer we make this change only to PatchCheck.py to prevent false positives and print lines of text that can not be found in a developer's working directory. I prefer this one time change to PatchCheck.py instead of having to update .gitattributes whenever the git show features are extended to convert more binary files to text files. My expectation is that EDK II Maintainers need to review if a binary file is ok or not for a repo. I would also be ok with adding general rules to PatchCheck.py to generate an error if a binary file is added/updated in one of the text only repos (edk2, edk2-platforms) and not generate an error if a binary file is added/updated in a repo that supports binaries (edk-non-osi). Best regards, Mike > -Original Message- > From: devel@edk2.groups.io > [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek > Sent: Friday, August 2, 2019 4:58 PM > To: Justen, Jordan L ; > Kinney, Michael D ; > devel@edk2.groups.io > Cc: Feng, Bob C ; Gao, Liming > > Subject: Re: [edk2-devel] [Patch 3/3] > BaseTools/PatchCheck: Disable text conversion in 'git > show' > > On 08/02/19 20:29, Jordan Justen wrote: > > First, I hope we don't add lots of large .pdf files > into the source > > tree. I see two duplicated > 200k .pdf files in edk2, > which seems like > > a waste of space in the edk2 tree. > > > > BaseTools/Source/C/BrotliCompress/docs/brotli- > comparison-study-2015-09 > > -22.pdf > > > MdeModulePkg/Library/BrotliCustomDecompressLib/docs/bro > tli-comparison- > > study-2015-09-22.pdf > > > > Second, I'm not too sure about this change. It seems > like it might > > have unintended consequences. > > > > One thought I had is that it might break UTF-16 > unicode files diffs, > > but luckily I think we've pretty much gotten rid of > UTF-16 files at > > this point. > > > > I also wonder if adding a .pdf attribute like Laszlo > recommends for > > .efi might be an alternative solution. > > > > > https://github.com/tianocore/tianocore.github.io/wiki/L > aszlo's-unkempt > > -git-guide-for-edk2-contributors-and- > maintainers#contrib-09 > > > > We could actually add these setting in the git tree > in a > > .gitattributes file, right? Has this been suggested? > I think Laszlo > > documented this before we had converted edk2 to git. > > This is a complex question. > > There are files which are binary for all intents and > purposes, so you never want to see a "textualized" > version of them. Git can attempt recognizing these > files, but it can take hints too. > > For these files, once they are safely classified as > "binary", it is a separate question whether you want a > changeset shown (or > email-formatted) for these files to contain base64-like > binary patches, or just a statement that "binary files > differ". > > Then there are files that are binary, but can be > "textualized" for the purposes of diffing and even > merging. Automated UCS2<->UTF8 conversion is an > example, and I used that while edk2 still had UCS2- > encoded UNI files. > > Some examples (not claiming this is going to be an > exhaustive list -- for that, see gitattributes(5)), to > be placed in .git/info/attributes (local clone only) or > .gitattributes (tracked in the repo): > > *.efi -diff > > This classifies *.efi as binary (turns off the > heuristics). > > Consider gitattributes(5) -- excerpt: > >diff >Unset >A path to which the diff attribute is > unset will >generate Binary files differ (or a > binary patch, if >binary patches are enabled). >Unspecified >A path to which the diff attribute is > unspecified first >gets its contents inspected, and if it > looks like text >and is smaller than > core.bigFileThreshold, it is >treated as text. Otherwise it would > generate Binary >files differ. > > git-show will only report "binary files differ", unless > we pass --binary to it; in which case it will offer a > base64-like binary patch. > git-format-patch will produce that binary patch by > default, unless we pass --no-binary to it. > > > Another example, in .git/info/attributes or > .gitattributes: > > *.uni diff=uni merge=uni > > The manual says: > >diff >String >Diff is shown using the specified diff > driver. Each >driver may specify one or more options, > as described in >
Re: [edk2-devel] [Patch v3 0/2] ShellPkg: Fix IA32 build failure in acpiview
Reviewed-by: Jaben Carsey Thanks -Jaben > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Sami Mujawar > Sent: Monday, August 05, 2019 3:48 AM > To: devel@edk2.groups.io; Kinney, Michael D > Cc: Carsey, Jaben ; Ni, Ray ; > Gao, Zhichao > Subject: Re: [edk2-devel] [Patch v3 0/2] ShellPkg: Fix IA32 build failure in > acpiview > > For this patch series. > > Tested-by: Sami Mujawar > Reviewed-by: Sami Mujawar > > Regards, > > Sami Mujawar > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Michael > D Kinney via Groups.Io > Sent: 03 August 2019 02:47 AM > To: devel@edk2.groups.io > Cc: Jaben Carsey ; Ray Ni ; > Zhichao Gao ; Sami Mujawar > > Subject: [edk2-devel] [Patch v3 0/2] ShellPkg: Fix IA32 build failure in > acpiview > > Use BaseLib MultuU64x64() to prevent intrinsics being added on IA32 builds. > Add acpiview to a version of the Shell that is build with ShellPkg.dsc to > catch > this type of issue in ShellPkg builds. > > Cc: Jaben Carsey > Cc: Ray Ni > Cc: Zhichao Gao > Cc: Sami Mujawar > Signed-off-by: Michael D Kinney > > Michael D Kinney (2): > ShellPkg/AcpiView: Fix IA32 link error > ShellPkg: Add shell with all commands integrated > > .../Parsers/Slit/SlitParser.c | 2 +- > ShellPkg/ShellPkg.dsc | 22 ++- > 2 files changed, 22 insertions(+), 2 deletions(-) > > -- > 2.21.0.windows.1 > > > > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, > please notify the sender immediately and do not disclose the contents to any > other person, use it for any purpose, or store or copy the information in any > medium. Thank you. > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44914): https://edk2.groups.io/g/devel/message/44914 Mute This Topic: https://groups.io/mt/32697114/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [tianocore-docs EDK_II_Secure_Coding_Guide PATCH] Add Appendix: Threat Mode for EDK II.
This looks good.
Helps address some of the concerns brought up in evaluating secure coding
guidelines for alternate feature implementations, too.
Reviewed-by: Vincent Zimmer
-Original Message-
From: Yao, Jiewen
Sent: Monday, August 5, 2019 12:48 AM
To: devel@edk2.groups.io
Cc: Zimmer, Vincent
Subject: [tianocore-docs EDK_II_Secure_Coding_Guide PATCH] Add Appendix: Threat
Mode for EDK II.
This patch adds "Threat model for EDK II" as the appendix section
of "EDK II secure coding guide" document.
The threat model discussed here is a general guide and serves as the baseline of
the EDK II firmware. For each specific feature in EDK II firmware, there might
be
additional feature-based threat models in addition to the general threat model.
The full gitbook can be also avaiable at
https://github.com/jyao1/EDK_II_Secure_Coding_Guide/tree/Threat_model.
Cc: Vincent Zimmer
Signed-off-by: Jiewen Yao
Reviewed-by: Vincent Zimmer
---
SUMMARY.md| 6 ++
appendix_threat_model_for_edk_ii/README.md| 70 +++
.../asset_boot_flow.md| 63 +
.../asset_build_tool.md | 39 +++
.../asset_flash_content.md| 59
.../asset_management_mode.md | 58 +++
.../asset_s3_resume.md| 61
7 files changed, 356 insertions(+)
create mode 100644 appendix_threat_model_for_edk_ii/README.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_boot_flow.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_build_tool.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_flash_content.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_management_mode.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_s3_resume.md
diff --git a/SUMMARY.md b/SUMMARY.md
index dc22f1e..d56dee3 100644
--- a/SUMMARY.md
+++ b/SUMMARY.md
@@ -38,6 +38,12 @@
* [SMM](secure_coding_guidelines_intel_platforms/smm.md)
* [Intel® Boot
Guard](secure_coding_guidelines_intel_platforms/intel_boot_guard.md)
* [Intel® Bios
Guard](secure_coding_guidelines_intel_platforms/intel_bios_guard.md)
+* [Appendix - Threat Model for EDK
II](appendix_threat_model_for_edk_ii/README.md)
+ * [Asset: Flash
Content](appendix_threat_model_for_edk_ii/asset_flash_content.md)
+ * [Asset: Boot Flow](appendix_threat_model_for_edk_ii/asset_boot_flow.md)
+ * [Asset: S3 Resume](appendix_threat_model_for_edk_ii/asset_s3_resume.md)
+ * [Asset: Management
Mode](appendix_threat_model_for_edk_ii/asset_management_mode.md)
+ * [Asset: Build Tool](appendix_threat_model_for_edk_ii/asset_build_tool.md)
* [References](references.md)
* [Books and Papers ](references.md#books-and-papers)
* [Web ](references.md#web)
diff --git a/appendix_threat_model_for_edk_ii/README.md
b/appendix_threat_model_for_edk_ii/README.md
new file mode 100644
index 000..6c4ac16
--- /dev/null
+++ b/appendix_threat_model_for_edk_ii/README.md
@@ -0,0 +1,70 @@
+
+
+# Appendix:Threat Model for EDK II {#appendix-threat-model-for-edk-ii}
+This chapter provides the basic assumptions for the threat model of EDK II
firmware. The threat model discussed here is a general guide and serves as the
baseline of the EDK II firmware. For each specific feature in EDK II firmware,
there might be additional feature-based threat models in addition to the
general threat model.
+
+In [UEFI Threat
Model](https://uefi.org/sites/default/files/resources/Intel-UEFI-ThreatModel.pdf),
we discussed the asset, threat and mitigation. Here we will revisit these
items and based upon [STRIDE](https://en.wikipedia.org/wiki/STRIDE_(security)).
+
+| Threat | Desired Property |
+| --- | --- |
+| Spoofing | Authentication |
+| Tampering | Integrity |
+| Repudiation | Non-Repudiation |
+| Information Disclosure | Confidentiality |
+| Denial of Service | Availability |
+| Elevation of Privilege | Authorization |
+
+In EDK II firmware, the denial of service can be temporary in the current
boot, or permanent in which case the system never boot again. The latter is
more serious and it is named as permanent denial of service (PDoS).
+
+We will consider the below adversary for the EDK II firmware:
+
+| Adversary | Capability |
+| --- | --- |
+| Network Attacker | The attacker may connect to the system by network in
order to eavesdrop, intercept, masquerade, or modify the network packet. |
+| Unprivileged Software Attacker | The attacker may run ring-3 software in an
OS application layer. The attacker may perform a software based side channel
attack (such as using cache timing). |
+| System Software Attacker | The attacker may run ring-0 software in the OS
kernel or hypervisor, or run 3rd party firmware code in firmware boot phase.
The attacker may perform the software based side channel attack (such as using
cache timing,
Re: [edk2-devel] [edk2-platforms: PATCH] Marvell/Drivers: XenonDxe: Explicitly disable HS400
Hi Marcin, On Thu, Jul 11, 2019 at 09:45:00AM +0100, Leif Lindholm wrote: > > > > On Wed, Jun 26, 2019 at 09:04:14AM +0200, Marcin Wojtas wrote: > > > > > Ensure that in case of SlowMode or 3.3V operation, > > > > > also the HS400 capability will be disabled in the > > > > > SdMmc driver. > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > > > > Well done on keeping this tag. But I'm thinking we need to do that > > > > relicensing sooner rather than later, and drop the tag. > > > > > > I left it, as this file is still not 2-clause SPDX tagged. > > > > > > > > > > > > > > > However, can you clarify what problem this solves? > > > > > > > > > > On another SoC revision, the capability register marks HS400 support > > > as enabled. However the interface itself is powered with 3.3V and it > > > turned out that my flags in SdMmcOverride driver did not cover this > > > case, which resulted in an unsuccessful EmmcSwitchToHS400 () execution > > > - it shouldn't be done at all. > > > > > > > Did you have a chance to see my explanation? Should I repost? > > Sorry, yes. Explanation is fine. If you can update the commit message > and drop the Contributed-under, I will push this once we get the > licenses updated. Licenses have now been updated. Can you please resubmit with the updated commit message and dropped Contributed-under? Best Regards, Leif -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44912): https://edk2.groups.io/g/devel/message/44912 Mute This Topic: https://groups.io/mt/32212561/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch v3 0/2] ShellPkg: Fix IA32 build failure in acpiview
For this patch series. Tested-by: Sami Mujawar Reviewed-by: Sami Mujawar Regards, Sami Mujawar -Original Message- From: devel@edk2.groups.io On Behalf Of Michael D Kinney via Groups.Io Sent: 03 August 2019 02:47 AM To: devel@edk2.groups.io Cc: Jaben Carsey ; Ray Ni ; Zhichao Gao ; Sami Mujawar Subject: [edk2-devel] [Patch v3 0/2] ShellPkg: Fix IA32 build failure in acpiview Use BaseLib MultuU64x64() to prevent intrinsics being added on IA32 builds. Add acpiview to a version of the Shell that is build with ShellPkg.dsc to catch this type of issue in ShellPkg builds. Cc: Jaben Carsey Cc: Ray Ni Cc: Zhichao Gao Cc: Sami Mujawar Signed-off-by: Michael D Kinney Michael D Kinney (2): ShellPkg/AcpiView: Fix IA32 link error ShellPkg: Add shell with all commands integrated .../Parsers/Slit/SlitParser.c | 2 +- ShellPkg/ShellPkg.dsc | 22 ++- 2 files changed, 22 insertions(+), 2 deletions(-) -- 2.21.0.windows.1 IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44911): https://edk2.groups.io/g/devel/message/44911 Mute This Topic: https://groups.io/mt/32697114/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] static data in dxe_runtime modules
On Sat, Aug 03, 2019 at 04:03:04AM +0200, Laszlo Ersek via Groups.Io wrote: > On 08/01/19 21:16, Roman Kagan wrote: > This is a serious bug. Thank you for reporting and analyzing it. Can you > file it in the TianoCore Bugzilla too, please? https://bugzilla.tianocore.org/show_bug.cgi?id=2053 > I wonder how far in OpenSSL history this issue goes back. Is this a > regression from the latest OpenSSL rebase in edk2? This is certainly not a recent regression. We've initially caught it with Virtuozzo OVMF based on the one from RHEL-7.6, based in turn on EDKII as of May 2018. However, the infrastructure that causes the problem is there for much longer. > > What would be the best way to fix it? > > > > One option is to audit OpenSSL and make sure it either doesn't put > > pointers into static storage or properly cleans them up (and call the > > cleanup function in RuntimeCryptLibAddressChangeEvent); then assume the > > rest of EDKII code is safe in this regard. > > > > Another is to assume that no static data in dxe_runtime modules should > > survive SetVirtualAddressMap, and thus make > > PeCoffLoaderRelocateImageForRuntime reinitialize the modules from > > scratch instead of re-applying the relocations only. > > > > I must admit I don't yet quite understand the full consequences of > > either option. Perhaps there are better ones. > > Any suggestion is appreciated. > > If the runtime driver remembers pointer *values* from before > SetVirtualAddressMap() -- i.e. it saves pointer values into some > storage, for de-referencing at OS runtime --, those stored values have > to be converted in the virtual address change event notification function. [...] > The "thread_local_storage" array from > "CryptoPkg/Library/OpensslLib/openssl/crypto/threads_none.c" has to be > exposed to RuntimeCryptLibAddressChangeEvent() somehow. > > Perhaps OpenSSL should allow edk2 to bring its own CRYPTO_THREAD_* APIs. I think this would be too awkward, as edk2 has no reason to have any visibility into it. I'd rather make use of the observation that in reality there's no data in OpenSSL that needs to survive SetVirtualAddressMap(). At first I started to cook up a fix that involved calling OPENSSL_cleanup() from RuntimeCryptLibAddressChangeEvent(), but it soon turned out it didn't clean things up to the pristine state so it didn't address the problem. Moreover I think it's overoptimistic to expect from OpenSSL developers the mindset that their code should work seamlessly across relocations at runtime. I don't see what would stop them from introducing another pointer variable with global storage further down the road, and nothing would allow to even timely spot this new problem. That's why I think the most reliable solution would be to just reload the module completely. If this can't be done for all runtime modules then I'd do it for the one(s) linking to OpenSSL. Roman. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44910): https://edk2.groups.io/g/devel/message/44910 Mute This Topic: https://groups.io/mt/32686575/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent buffer overruns
Hi Zhichao,
Please see my response inline.
Regards,
Sami Mujawar
-Original Message-
From: devel@edk2.groups.io On Behalf Of Gao, Zhichao via
Groups.Io
Sent: 05 August 2019 08:23 AM
To: devel@edk2.groups.io; Krzysztof Koch
Cc: Carsey, Jaben ; Ni, Ray ; Sami
Mujawar ; Matteo Carlini ; nd
Subject: Re: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent
buffer overruns
One confusion below:
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Krzysztof Koch
> Sent: Thursday, August 1, 2019 4:44 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray
> ; Gao, Zhichao ;
> sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com
> Subject: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent
> buffer overruns
>
> Modify the GTDT table parsing logic to prevent reading past the ACPI
> buffer lengths provided and to make it consistent with other table
> parsers. This includes converting the do-while loop in ParseAcpiGtdt() into a
> while loop.
>
> Remove a check which ensures that the entire Platform GT Block
> Structure buffer has been parsed. The ACPI specification does not ban
> from defining buffers which are larger than the size indicated by the
> count and sizes of substructures which constitute it.
>
> Change the data type of the Length parameter to the DumpGTBlock()
> function to reflect the width of the respective ACPI structure's field.
>
> References:
> - ACPI 6.3, January 2019, Table 5-124
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Notes:
> v1:
> - Prevent buffer overruns in GTDT acpiview parser [Krzysztof]
>
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> | 147 ++--
> 1 file changed, 76 insertions(+), 71 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> index
> 1e5b5764f50a2d29aa904c889bc89af5bdc3af5c..57174e14c80072f12b90e1996e
> be8f0002d0c404 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtPars
> +++ er.c
> @@ -23,7 +23,6 @@ STATIC CONST UINT8* PlatformTimerType; STATIC
> CONST UINT16* PlatformTimerLength; STATIC CONST UINT32*
> GtBlockTimerCount; STATIC CONST UINT32* GtBlockTimerOffset; -STATIC
> CONST UINT16* GtBlockLength; STATIC ACPI_DESCRIPTION_HEADER_INFO
> AcpiHdrInfo;
>
> /**
> @@ -127,7 +126,7 @@ STATIC CONST ACPI_PARSER
> GtPlatformTimerHeaderParser[] = { **/ STATIC CONST ACPI_PARSER
> GtBlockParser[] = {
>{L"Type", 1, 0, L"%d", NULL, NULL, NULL, NULL},
> - {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL},
> + {L"Length", 2, 1, L"%d", NULL, NULL, NULL, NULL},
>{L"Reserved", 1, 3, L"%x", NULL, NULL, NULL, NULL},
>{L"Physical address (CntCtlBase)", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL},
>{L"Timer Count", 4, 12, L"%d", NULL, (VOID**), @@
> -
> 168,56 +167,43 @@ STATIC CONST ACPI_PARSER SBSAGenericWatchdogParser[]
> = {
> /**
>This function parses the Platform GT Block.
>
> - @param [in] Ptr Pointer to the start of the GT Block data.
> - @param [in] Length Length of the GT Block structure.
> + @param [in] Ptr Pointer to the start of the GT Block data.
> + @param [in] LengthLength of the GT Block structure.
> **/
> STATIC
> VOID
> DumpGTBlock (
>IN UINT8* Ptr,
> - IN UINT32 Length
> + IN UINT16 Length
>)
> {
>UINT32 Index;
>UINT32 Offset;
> - UINT32 GTBlockTimerLength;
>
> - Offset = ParseAcpi (
> - TRUE,
> - 2,
> - "GT Block",
> - Ptr,
> - Length,
> - PARSER_PARAMS (GtBlockParser)
> - );
> - GTBlockTimerLength = (*GtBlockLength - Offset) /
> (*GtBlockTimerCount);
> - Length -= Offset;
> + ParseAcpi (
> +TRUE,
> +2,
> +"GT Block",
> +Ptr,
> +Length,
> +PARSER_PARAMS (GtBlockParser)
> +);
>
> - if (*GtBlockTimerCount != 0) {
> -Ptr += (*GtBlockTimerOffset);
> -Index = 0;
> -while ((Index < (*GtBlockTimerCount)) && (Length >=
> GTBlockTimerLength)) {
> - Offset = ParseAcpi (
> - TRUE,
> - 2,
> - "GT Block Timer",
> - Ptr,
> - GTBlockTimerLength,
> - PARSER_PARAMS (GtBlockTimerParser)
> - );
> - // Increment by GT Block Timer structure size
> - Ptr += Offset;
> - Length -= Offset;
> - Index++;
> -}
> + Offset = *GtBlockTimerOffset;
> + Index = 0;
>
> -if (Length != 0) {
> - IncrementErrorCount ();
> - Print (
> -L"ERROR:GT Block Timer length mismatch. Unparsed %d bytes.\n",
> -Length
> -);
> -}
> + // Parse the specified number of GT Block Timer Structures or the
> + GT Block //
Re: [edk2-devel] [PATCH v6 1/1] Platform: Intel: Add Stratix 10 platform support
Hi Leif,
OK noted, I'll submit a new patch on top of the commit.
I'll also try to ping Micheal.
Thanks
Tien Hock
> -Original Message-
> From: Leif Lindholm
> Sent: Monday, August 5, 2019 5:26 PM
> To: Loh, Tien Hock ; Kinney, Michael D
>
> Cc: devel@edk2.groups.io; thlo...@gmail.com; Ard Biesheuvel
>
> Subject: Re: [PATCH v6 1/1] Platform: Intel: Add Stratix 10 platform support
>
> Hi Tien Hock,
>
> I have already given my reviewed-by to v5 of this patch.
> So can you please resubmit the changes since then as a separate patch?
>
> But I am still waiting for a response from Mike to
> https://edk2.groups.io/g/devel/message/44042
> before I am able to push the platform support.
>
> Best Regards,
>
> Leif
>
> On Mon, Aug 05, 2019 at 03:34:14AM +, Loh, Tien Hock wrote:
> > Hi Leif, Ard, Micheal,
> >
> > Any comments on this patch?
> >
> > Thanks
> > Tien Hock
> >
> > > -Original Message-
> > > From: Loh, Tien Hock
> > > Sent: Thursday, August 1, 2019 6:32 PM
> > > To: devel@edk2.groups.io; thlo...@gmail.com
> > > Cc: Loh, Tien Hock ; Ard Biesheuvel
> > > ; Leif Lindholm ;
> > > Kinney, Michael D
> > > Subject: [PATCH v6 1/1] Platform: Intel: Add Stratix 10 platform support
> > >
> > > From: "Tien Hock, Loh"
> > >
> > > Adds support for Intel Stratix 10 Platform.
> > >
> > > Signed-off-by: "Tien Hock, Loh"
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Cc: Ard Biesheuvel
> > > Cc: Leif Lindholm
> > > Cc: Michael D Kinney
> > >
> > > ---
> > > v5:
> > > Remove hardcoded UART clock
> > > ---
> > > Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
> |
> > > 44 ++
> > > Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
> |
> > > 48 ++
> > >
> > >
> Platform/Intel/Stratix10/Library/IntelPlatformLib/AArch64/ArmPlatformHelp
> > > er.S | 51 ++
> > > Platform/Intel/Stratix10/Library/IntelPlatformLib/IntelPlatformLib.inf
> |
> > > 54 +++
> > > Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10Mmu.c
> |
> > > 155 ++
> > > Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10PlatformLib.c
> |
> > > 153 ++
> > > Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.c
> |
> > > 43 ++
> > > Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf
> > > | 40 ++
> > > Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.c
> > > | 121 +
> > > Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.h
> > > | 40 ++
> > > Platform/Intel/Stratix10/Readme.md
> > > | 61 +++
> > > Platform/Intel/Stratix10/Stratix10SoCPkg.dec
> > > | 22 +
> > > Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
> > > | 501
> > >
> > > Platform/Intel/Stratix10/Stratix10SoCPkg.fdf
> > > | 253
> > > ++
> > > Readme.md
> > > | 3 +
> > > 15 files changed, 1589 insertions(+)
> > >
> > > diff --git
> > > a/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
> > > b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
> > > new file mode 100644
> > > index ..a801f12bb59e
> > > --- /dev/null
> > > +++
> b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
> > > @@ -0,0 +1,44 @@
> > > +/** @file
> > > +*
> > > +* Copyright (c) 2019, Intel All rights reserved.
> > > +*
> > > +* This program and the accompanying materials
> > > +* are licensed and made available under the terms and conditions of
> the
> > > BSD License
> > > +* which accompanies this distribution. The full text of the license may
> be
> > > found at
> > > +* http://opensource.org/licenses/bsd-license.php
> > > +*
> > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> > > BASIS,
> > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > > EXPRESS OR IMPLIED.
> > > +*
> > > +**/
> > > +
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#include "../../Library/S10ClockManager/S10ClockManager.h"
> > > +EFI_STATUS
> > > +EFIAPI
> > > +IntelPlatformDxeEntryPoint (
> > > + IN EFI_HANDLE ImageHandle,
> > > + IN EFI_SYSTEM_TABLE *SystemTable
> > > + )
> > > +{
> > > + EFI_STATUSStatus = 0;
> > > +
> > > + return Status;
> > > +}
> > > +
> > > diff --git
> > > a/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
> > > b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
> > > new file mode 100644
> > > index ..64b398969f1e
> > > --- /dev/null
> > > +++
> > >
Re: [edk2-devel] [PATCH v6 1/1] Platform: Intel: Add Stratix 10 platform support
Hi Tien Hock,
I have already given my reviewed-by to v5 of this patch.
So can you please resubmit the changes since then as a separate patch?
But I am still waiting for a response from Mike to
https://edk2.groups.io/g/devel/message/44042
before I am able to push the platform support.
Best Regards,
Leif
On Mon, Aug 05, 2019 at 03:34:14AM +, Loh, Tien Hock wrote:
> Hi Leif, Ard, Micheal,
>
> Any comments on this patch?
>
> Thanks
> Tien Hock
>
> > -Original Message-
> > From: Loh, Tien Hock
> > Sent: Thursday, August 1, 2019 6:32 PM
> > To: devel@edk2.groups.io; thlo...@gmail.com
> > Cc: Loh, Tien Hock ; Ard Biesheuvel
> > ; Leif Lindholm ;
> > Kinney, Michael D
> > Subject: [PATCH v6 1/1] Platform: Intel: Add Stratix 10 platform support
> >
> > From: "Tien Hock, Loh"
> >
> > Adds support for Intel Stratix 10 Platform.
> >
> > Signed-off-by: "Tien Hock, Loh"
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Cc: Ard Biesheuvel
> > Cc: Leif Lindholm
> > Cc: Michael D Kinney
> >
> > ---
> > v5:
> > Remove hardcoded UART clock
> > ---
> > Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
> >|
> > 44 ++
> > Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
> >|
> > 48 ++
> >
> > Platform/Intel/Stratix10/Library/IntelPlatformLib/AArch64/ArmPlatformHelp
> > er.S | 51 ++
> > Platform/Intel/Stratix10/Library/IntelPlatformLib/IntelPlatformLib.inf
> >|
> > 54 +++
> > Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10Mmu.c
> >|
> > 155 ++
> > Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10PlatformLib.c
> >|
> > 153 ++
> > Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.c
> >|
> > 43 ++
> > Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf
> > | 40 ++
> > Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.c
> > | 121 +
> > Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.h
> > | 40 ++
> > Platform/Intel/Stratix10/Readme.md
> >| 61 +++
> > Platform/Intel/Stratix10/Stratix10SoCPkg.dec
> >| 22 +
> > Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
> >| 501
> >
> > Platform/Intel/Stratix10/Stratix10SoCPkg.fdf
> >| 253
> > ++
> > Readme.md
> >| 3 +
> > 15 files changed, 1589 insertions(+)
> >
> > diff --git
> > a/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
> > b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
> > new file mode 100644
> > index ..a801f12bb59e
> > --- /dev/null
> > +++ b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
> > @@ -0,0 +1,44 @@
> > +/** @file
> > +*
> > +* Copyright (c) 2019, Intel All rights reserved.
> > +*
> > +* This program and the accompanying materials
> > +* are licensed and made available under the terms and conditions of the
> > BSD License
> > +* which accompanies this distribution. The full text of the license may
> > be
> > found at
> > +* http://opensource.org/licenses/bsd-license.php
> > +*
> > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS,
> > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "../../Library/S10ClockManager/S10ClockManager.h"
> > +EFI_STATUS
> > +EFIAPI
> > +IntelPlatformDxeEntryPoint (
> > + IN EFI_HANDLE ImageHandle,
> > + IN EFI_SYSTEM_TABLE *SystemTable
> > + )
> > +{
> > + EFI_STATUSStatus = 0;
> > +
> > + return Status;
> > +}
> > +
> > diff --git
> > a/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
> > b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
> > new file mode 100644
> > index ..64b398969f1e
> > --- /dev/null
> > +++
> > b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
> > @@ -0,0 +1,48 @@
> > +#/** @file
> > +#
> > +# Copyright (c) 2019, Intel All rights reserved.
> > +#
> > +# This program and the accompanying materials
> > +# are licensed and made available under the terms and conditions of the
> > BSD License
> > +# which accompanies this distribution. The full text of the license may
> > be
> > found at
> > +# http://opensource.org/licenses/bsd-license.php
> > +#
> > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS,
> > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
Re: [edk2-devel] [Patch v2 0/2] UefiCpuPkg: Remove debug message.
Reviewed-by: Ray Ni > -Original Message- > From: Dong, Eric > Sent: Monday, August 5, 2019 2:44 PM > To: devel@edk2.groups.io > Cc: Ni, Ray ; Laszlo Ersek > Subject: [Patch v2 0/2] UefiCpuPkg: Remove debug message. > > This debug message may be called by BSP and APs. It may caused ASSERT > when APs call this debug code. > > In order to avoid system boot assert, Remove this debug message. > > Cc: Ray Ni > Cc: Laszlo Ersek > > Eric Dong (2): > UefiCpuPkg/RegisterCpuFeaturesLib: Remove debug message. > UefiCpuPkg/PiSmmCpuDxeSmm: Remove debug message. > > .../CpuFeaturesInitialize.c | 22 --- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 22 +-- > 2 files changed, 1 insertion(+), 43 deletions(-) > > -- > 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44906): https://edk2.groups.io/g/devel/message/44906 Mute This Topic: https://groups.io/mt/32723168/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/1] SecurityPkg/TpmCommLib: Remove TpmCommLib
Reviewed-by: Jian J Wang
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Zhang, Shenglei
> Sent: Monday, August 05, 2019 1:55 PM
> To: devel@edk2.groups.io
> Cc: Zhang, Shenglei ; Yao, Jiewen
> ; Wang, Jian J ; Zhang,
> Chao B
> Subject: [edk2-devel] [PATCH 1/1] SecurityPkg/TpmCommLib: Remove
> TpmCommLib
>
> From: shenglei
>
> TpmCommonLib is no longer used by TcgPei/TcgDxe/Tcg2ConfigPei
> modules. So TpmCommLib can be removed.
>
> Cc: Jiewen Yao
> Cc: Jian Wang
> Cc: Chao Zhang
> Signed-off-by: Shenglei Zhang
> ---
> SecurityPkg/Library/TpmCommLib/TisPc.c| 177 ---
> SecurityPkg/Library/TpmCommLib/TpmComm.c | 44 ---
> SecurityPkg/Include/Library/TpmCommLib.h | 281 --
> SecurityPkg/Library/TpmCommLib/CommonHeader.h | 23 --
> SecurityPkg/Library/TpmCommLib/TpmCommLib.inf | 45 ---
> SecurityPkg/Library/TpmCommLib/TpmCommLib.uni | 17 --
> SecurityPkg/SecurityPkg.dec | 4 -
> SecurityPkg/SecurityPkg.dsc | 2 -
> 8 files changed, 593 deletions(-)
> delete mode 100644 SecurityPkg/Library/TpmCommLib/TisPc.c
> delete mode 100644 SecurityPkg/Library/TpmCommLib/TpmComm.c
> delete mode 100644 SecurityPkg/Include/Library/TpmCommLib.h
> delete mode 100644 SecurityPkg/Library/TpmCommLib/CommonHeader.h
> delete mode 100644 SecurityPkg/Library/TpmCommLib/TpmCommLib.inf
> delete mode 100644 SecurityPkg/Library/TpmCommLib/TpmCommLib.uni
>
> diff --git a/SecurityPkg/Library/TpmCommLib/TisPc.c
> b/SecurityPkg/Library/TpmCommLib/TisPc.c
> deleted file mode 100644
> index 162e883d2170..
> --- a/SecurityPkg/Library/TpmCommLib/TisPc.c
> +++ /dev/null
> @@ -1,177 +0,0 @@
> -/** @file
> - Basic TIS (TPM Interface Specification) functions.
> -
> -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#include "CommonHeader.h"
> -
> -/**
> - Check whether TPM chip exist.
> -
> - @param[in] TisReg Pointer to TIS register.
> -
> - @retvalTRUETPM chip exists.
> - @retvalFALSE TPM chip is not found.
> -**/
> -BOOLEAN
> -TisPcPresenceCheck (
> - IN TIS_PC_REGISTERS_PTR TisReg
> - )
> -{
> - UINT8 RegRead;
> -
> - RegRead = MmioRead8 ((UINTN)>Access);
> - return (BOOLEAN)(RegRead != (UINT8)-1);
> -}
> -
> -/**
> - Check whether the value of a TPM chip register satisfies the input BIT
> setting.
> -
> - @param[in] Register Address port of register to be checked.
> - @param[in] BitSet Check these data bits are set.
> - @param[in] BitClear Check these data bits are clear.
> - @param[in] TimeOut The max wait time (unit MicroSecond) when
> checking register.
> -
> - @retval EFI_SUCCESS The register satisfies the check bit.
> - @retval EFI_TIMEOUT The register can't run into the expected status in
> time.
> -**/
> -EFI_STATUS
> -EFIAPI
> -TisPcWaitRegisterBits (
> - IN UINT8 *Register,
> - IN UINT8 BitSet,
> - IN UINT8 BitClear,
> - IN UINT32TimeOut
> - )
> -{
> - UINT8 RegRead;
> - UINT32WaitTime;
> -
> - for (WaitTime = 0; WaitTime < TimeOut; WaitTime += 30){
> -RegRead = MmioRead8 ((UINTN)Register);
> -if ((RegRead & BitSet) == BitSet && (RegRead & BitClear) == 0)
> - return EFI_SUCCESS;
> -MicroSecondDelay (30);
> - }
> - return EFI_TIMEOUT;
> -}
> -
> -/**
> - Get BurstCount by reading the burstCount field of a TIS regiger
> - in the time of default TIS_TIMEOUT_D.
> -
> - @param[in] TisRegPointer to TIS register.
> - @param[out] BurstCountPointer to a buffer to store the got
> BurstConut.
> -
> - @retval EFI_SUCCESS Get BurstCount.
> - @retval EFI_INVALID_PARAMETER TisReg is NULL or BurstCount is NULL.
> - @retval EFI_TIMEOUT BurstCount can't be got in time.
> -**/
> -EFI_STATUS
> -EFIAPI
> -TisPcReadBurstCount (
> - IN TIS_PC_REGISTERS_PTR TisReg,
> - OUT UINT16*BurstCount
> - )
> -{
> - UINT32WaitTime;
> - UINT8 DataByte0;
> - UINT8 DataByte1;
> -
> - if (BurstCount == NULL || TisReg == NULL) {
> -return EFI_INVALID_PARAMETER;
> - }
> -
> - WaitTime = 0;
> - do {
> -//
> -// TIS_PC_REGISTERS_PTR->burstCount is UINT16, but it is not 2bytes
> aligned,
> -// so it needs to use MmioRead8 to read two times
> -//
> -DataByte0 = MmioRead8 ((UINTN)>BurstCount);
> -DataByte1 = MmioRead8 ((UINTN)>BurstCount + 1);
> -*BurstCount = (UINT16)((DataByte1 << 8) + DataByte0);
> -if (*BurstCount != 0) {
> - return EFI_SUCCESS;
> -}
> -MicroSecondDelay (30);
> -WaitTime +=
Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
Hi Zhichao,
The reason why processing of the Debug Device Information Structure is split
into:
1. loading the header
2. dumping the entire structure
Is because we want to let the users control how much of the structure is
dumped. This is important for backward compatibility of the acpiview tool with
the ACPI specification (and other specs).
New ACPI table fields are appended at the end of structures/tables. If, for
example, we are asked to parse an old version of Debug Device Information
Structure, the 'Length' field will tell us to ignore some of the newly added
fields. These fields do not make sense in the context of an old version of the
corresponding spec.
The following code in Dbg2Parser.c:
// Make sure the Debug Device Information structure lies inside the table.
if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {
IncrementErrorCount ();
Print (
L"ERROR: Invalid Debug Device Information structure length. " \
L"DbgDevInfoLen = %d. RemainingTableBufferLength = %d. " \
L"DBG2 parsing aborted.\n",
*DbgDevInfoLen,
AcpiTableLength - Offset
);
return;
}
Makes sure that the user-provided structure length won't result in a buffer
overrun with respect to the DBG2 table buffer. This way we allow users to
specify how much of the structure they want to parse while still preventing
buffer overruns.
In short, I'm not sure if getting rid of DbgDevInfoHeaderParser would work as
you assume that the remaining table buffer length should be passed to
ParseAcpi() as an argument, not the length of the Debug Device Information
Structure. What do you think?
Kind regards,
Krzysztof
-Original Message-
From: Gao, Zhichao
Sent: Monday, August 5, 2019 7:48
To: Krzysztof Koch ; devel@edk2.groups.io
Cc: Carsey, Jaben ; Ni, Ray ; Sami
Mujawar ; Matteo Carlini ; nd
Subject: RE: [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
About DbgDevInfoHeaderParser and DbgDevInfoParser.
This patch would parse same DbgDevInfo twice, one for getting length, the other
for dumping structure info.
How about the following?
Add one parameter for DumpDbgDeviceInfo
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
IN UINT8* Ptr,
OUT UINT32* Length
)
==>
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
IN UINT8* Ptr,
IN UINT32* Length// remain length of acpi struct to
parse to make sure all operation is in a valid scope
OUT UINT16* DbgDevInfoLength // return pointer dbgdevinfo length
)
Then we would not need an anditional DbgDevInfoHeaderParser and the header
would be parsed for only once.
Any better comments, please let me know.
Thanks,
Zhichao
> -Original Message-
> From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> Sent: Thursday, August 1, 2019 4:44 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray
> ; Gao, Zhichao ;
> sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com
> Subject: [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer
> overruns
>
> Modify the DBG2 table parsing logic to prevent reading past the ACPI
> buffer lengths provided.
>
> Modify the signature of the DumpDbgDeviceInfo() function to make it
> consistent with the ACPI structure processing functions in other
> acpiview parsers. Now, the length of the Debug Device Information
> Structure is read before the entire structure is dumped.
>
> This refactoring change makes it easier to stop reading beyond the
> DBG2 table buffer if the Debug Device Information Structure Buffer
> does not fit in the DBG2 buffer.
>
> For processing the first two fields of the Debug Device Information
> Structure (to get the length) a new ACPI_PARSER array is defined.
>
> References:
> - Microsoft Debug Port Table 2 (DBG2), December 10, 2015
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Notes:
> v1:
> - Prevent buffer overruns in DBG2 acpiview parser [Krzysztof]
>
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> | 141 +---
> 1 file changed, 92 insertions(+), 49 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> index
> c6929695a1032c57761ef85002d6c51b7800ce23..869e700b9beda4886bf7bc5ae
> 4ced3ab9a59efa3 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Pars
> +++ er.c
> @@ -64,10 +64,17 @@ STATIC CONST ACPI_PARSER Dbg2Parser[] = {
> (VOID**), NULL, NULL} };
>
> +/// An ACPI_PARSER array describing the debug device information
> +structure /// header.
> +STATIC CONST ACPI_PARSER DbgDevInfoHeaderParser[] = {
> + {L"Revision", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL}
> +};
> +
> /// An ACPI_PARSER array describing the debug device
Re: [edk2-devel] [PATCH 0/3] Enhance ESRT to support multiple controllers
Hao, Thank you for the suggestion. I have sent the patch v2 out and merge series to one patch. Let's wait one day to follow the possible comments from stewards. Thanks. Best Regards Eric -Original Message- From: Wu, Hao A Sent: Monday, August 5, 2019 2:20 PM To: devel@edk2.groups.io; Jin, Eric Cc: Gao, Liming ; Kinney, Michael D ; af...@apple.com; ler...@redhat.com; leif.lindh...@linaro.org Subject: RE: [edk2-devel] [PATCH 0/3] Enhance ESRT to support multiple controllers > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Eric Jin > Sent: Wednesday, July 31, 2019 10:59 PM > To: devel@edk2.groups.io > Subject: [edk2-devel] [PATCH 0/3] Enhance ESRT to support multiple > controllers > > Multiple Controllers Support solution > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1525 > > The patch set is to makes enhancement to the ESRT when multiple same > controllers exist in one system. > > Eric Jin (3): > MdeModulePkg/EsrtFmpDxe: Merge multiple FMP into ESRT > MdeModulePkg/EsrtFmpDxe: Correct LastAttemptVersion algorithm in ESRT > MdeModulePkg/EsrtFmpDxe: Detect duplicate GUID/HardwareInstance Hello Eric, After looking into the series, I found that patches 2/3 & 3/3 are actually addressing issues introduced by the 1/3 patch. It looks to me that these 3 patches should be squashed into 1 commit, since they all focus on adding a feature for ESRT to support multiple controllers. However, originally, the 3 separate commits come from the edk2-staging repository, so I am not very sure what approach should be adopted when merging them back to the edk2 repo master branch. (Includes the stewards here for suggestions.) My personal preference is to merge them together as one patch. Best Regards, Hao Wu > > MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 +--- > > 1 file changed, 257 insertions(+), 137 deletions(-) > > -- > 2.20.1.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44903): https://edk2.groups.io/g/devel/message/44903 Mute This Topic: https://groups.io/mt/32667728/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 0/3] Enhance ESRT to support multiple controllers
Hi Laszlo, Thank you for the suggestion. Will set them in git. Best Regards Eric -Original Message- From: Laszlo Ersek Sent: Thursday, August 1, 2019 5:03 AM To: Jin, Eric Cc: devel@edk2.groups.io Subject: Re: [edk2-devel] [PATCH 0/3] Enhance ESRT to support multiple controllers On 07/31/19 16:59, Eric Jin wrote: > Multiple Controllers Support solution > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1525 > > The patch set is to makes enhancement to the ESRT when multiple same > controllers exist in one system. > > Eric Jin (3): > MdeModulePkg/EsrtFmpDxe: Merge multiple FMP into ESRT > MdeModulePkg/EsrtFmpDxe: Correct LastAttemptVersion algorithm in ESRT > MdeModulePkg/EsrtFmpDxe: Detect duplicate GUID/HardwareInstance > > MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 > +--- > 1 file changed, 257 insertions(+), 137 deletions(-) > Not a comment on the patches themselves, just a workflow suggestion for *future* postings: please enable shallow threading in git. git config sendemail.thread true git config sendemail.chainreplyto false Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44902): https://edk2.groups.io/g/devel/message/44902 Mute This Topic: https://groups.io/mt/32667728/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support multiple controllers
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1525
The patch is to merge multiple FMP instances into single ESRT entry
when they have the same GUID.
The policy to LastAttemptStatus/LastAttemptVersion of ESRT entry is:
If all the LastAttemptStatus are LAST_ATTEMPT_STATUS_SUCCESS, then
LastAttemptVersion should be the smallest of LastAttemptVersion. If
any of the LastAttemptStatus is not LAST_ATTEMPT_STATUS_SUCCESS,
then the LastAttemptVersion/LastAttemptStatus should be the values
of the first FMP instance whose LastAttemptStatus is not
LAST_ATTEMPT_STATUS_SUCCESS.
To detect possible duplicated GUID/HardwareInstance, a table of
GUID/HardwareInstance pairs from all the EFI_FIRMWARE_IMAGE_DESCRIPTORs
from all FMP instances is built. If a duplicate is found, then generate
a DEBUG_ERROR message, generate an ASSERT(), and ignore the duplicate
EFI_FIRMWARE_IMAGE_DESCRIPTOR.
Add an internal worker function called FmpGetFirmwareImageDescriptor()
that retrieves the list of EFI_FIRMWARE_IMAGE_DESCRIPTORs from a single
FMP instance and returns the descriptors in an allocated buffer. This
function is used to get the descriptors used to build the table of
unique GUID/HardwareInstance pairs. It is then used again to generate
the ESRT Table from all the EFI_FIRMWARE_IMAGE_DESCRIPTORs from all the
FMP instances. 2 passes are performed so the total number of
descriptors is known. This allows the correct sized buffers to always
be allocated.
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jian J Wang
Cc: Hao A Wu
Signed-off-by: Michael D Kinney
Signed-off-by: Eric Jin
---
MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 +---
1 file changed, 257 insertions(+), 137 deletions(-)
diff --git a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
index 2356da662e..4670349f89 100644
--- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
+++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
@@ -2,7 +2,7 @@
Publishes ESRT table from Firmware Management Protocol instances
Copyright (c) 2016, Microsoft Corporation
- Copyright (c) 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -21,6 +21,22 @@
#include
#include
+///
+/// Structure for array of unique GUID/HardwareInstance pairs from the
+/// current set of EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP Protocols.
+///
+typedef struct {
+ ///
+ /// A unique GUID identifying the firmware image type.
+ ///
+ EFI_GUID ImageTypeGuid;
+ ///
+ /// An optional number to identify the unique hardware instance within the
+ /// system for devices that may have multiple instances whenever possible.
+ ///
+ UINT64HardwareInstance;
+} GUID_HARDWAREINSTANCE_PAIR;
+
/**
Print ESRT to debug console.
@@ -33,11 +49,6 @@ PrintTable (
IN EFI_SYSTEM_RESOURCE_TABLE *Table
);
-//
-// Number of ESRT entries to grow by each time we run out of room
-//
-#define GROWTH_STEP 10
-
/**
Install EFI System Resource Table into the UEFI Configuration Table
@@ -101,90 +112,129 @@ IsSystemFmp (
}
/**
- Function to create a single ESRT Entry and add it to the ESRT
- given a FMP descriptor. If the guid is already in the ESRT it
- will be ignored. The ESRT will grow if it does not have enough room.
-
- @param[in, out] Table On input, pointer to the pointer to the
ESRT.
-On output, same as input or pointer to the
pointer
-to new enlarged ESRT.
- @param[in] FmpImageInfoBuf Pointer to the
EFI_FIRMWARE_IMAGE_DESCRIPTOR.
- @param[in] FmpVersionFMP Version.
-
- @return Status code.
+ Function to create a single ESRT Entry and add it to the ESRT with
+ a given FMP descriptor. If the GUID is already in the ESRT, then the ESRT
+ entry is updated.
+
+ @param[in,out] TablePointer to the ESRT Table.
+ @param[in,out] HardwareInstancesPointer to the
GUID_HARDWAREINSTANCE_PAIR.
+ @param[in,out] NumberOfDescriptors The number of
EFI_FIRMWARE_IMAGE_DESCRIPTORs.
+ @param[in] FmpImageInfoBuf Pointer to the
EFI_FIRMWARE_IMAGE_DESCRIPTOR.
+ @param[in] FmpVersion FMP Version.
+
+ @retval EFI_SUCCESS FmpImageInfoBuf was use to fill in a new ESRT entry
+ in Table.
+ @retval EFI_SUCCESS The ImageTypeId GUID in FmpImageInfoBuf matches an
+ existing ESRT entry in Table, and the information
+ from FmpImageInfoBuf was merged into the the
existing
+ ESRT entry.
+ @retval EFI_UNSPOORTED The GUID/HardareInstance in FmpImageInfoBuf has is a
+ duplicate.
**/
EFI_STATUS
CreateEsrtEntry (
- IN OUT EFI_SYSTEM_RESOURCE_TABLE **Table,
- IN
[edk2-devel] [tianocore-docs EDK_II_Secure_Coding_Guide PATCH] Add Appendix: Threat Mode for EDK II.
This patch adds "Threat model for EDK II" as the appendix section
of "EDK II secure coding guide" document.
The threat model discussed here is a general guide and serves as the baseline of
the EDK II firmware. For each specific feature in EDK II firmware, there might
be
additional feature-based threat models in addition to the general threat model.
The full gitbook can be also avaiable at
https://github.com/jyao1/EDK_II_Secure_Coding_Guide/tree/Threat_model.
Cc: Vincent Zimmer
Signed-off-by: Jiewen Yao
Reviewed-by: Vincent Zimmer
---
SUMMARY.md| 6 ++
appendix_threat_model_for_edk_ii/README.md| 70 +++
.../asset_boot_flow.md| 63 +
.../asset_build_tool.md | 39 +++
.../asset_flash_content.md| 59
.../asset_management_mode.md | 58 +++
.../asset_s3_resume.md| 61
7 files changed, 356 insertions(+)
create mode 100644 appendix_threat_model_for_edk_ii/README.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_boot_flow.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_build_tool.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_flash_content.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_management_mode.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_s3_resume.md
diff --git a/SUMMARY.md b/SUMMARY.md
index dc22f1e..d56dee3 100644
--- a/SUMMARY.md
+++ b/SUMMARY.md
@@ -38,6 +38,12 @@
* [SMM](secure_coding_guidelines_intel_platforms/smm.md)
* [Intel® Boot
Guard](secure_coding_guidelines_intel_platforms/intel_boot_guard.md)
* [Intel® Bios
Guard](secure_coding_guidelines_intel_platforms/intel_bios_guard.md)
+* [Appendix - Threat Model for EDK
II](appendix_threat_model_for_edk_ii/README.md)
+ * [Asset: Flash
Content](appendix_threat_model_for_edk_ii/asset_flash_content.md)
+ * [Asset: Boot Flow](appendix_threat_model_for_edk_ii/asset_boot_flow.md)
+ * [Asset: S3 Resume](appendix_threat_model_for_edk_ii/asset_s3_resume.md)
+ * [Asset: Management
Mode](appendix_threat_model_for_edk_ii/asset_management_mode.md)
+ * [Asset: Build Tool](appendix_threat_model_for_edk_ii/asset_build_tool.md)
* [References](references.md)
* [Books and Papers ](references.md#books-and-papers)
* [Web ](references.md#web)
diff --git a/appendix_threat_model_for_edk_ii/README.md
b/appendix_threat_model_for_edk_ii/README.md
new file mode 100644
index 000..6c4ac16
--- /dev/null
+++ b/appendix_threat_model_for_edk_ii/README.md
@@ -0,0 +1,70 @@
+
+
+# Appendix:Threat Model for EDK II {#appendix-threat-model-for-edk-ii}
+This chapter provides the basic assumptions for the threat model of EDK II
firmware. The threat model discussed here is a general guide and serves as the
baseline of the EDK II firmware. For each specific feature in EDK II firmware,
there might be additional feature-based threat models in addition to the
general threat model.
+
+In [UEFI Threat
Model](https://uefi.org/sites/default/files/resources/Intel-UEFI-ThreatModel.pdf),
we discussed the asset, threat and mitigation. Here we will revisit these
items and based upon [STRIDE](https://en.wikipedia.org/wiki/STRIDE_(security)).
+
+| Threat | Desired Property |
+| --- | --- |
+| Spoofing | Authentication |
+| Tampering | Integrity |
+| Repudiation | Non-Repudiation |
+| Information Disclosure | Confidentiality |
+| Denial of Service | Availability |
+| Elevation of Privilege | Authorization |
+
+In EDK II firmware, the denial of service can be temporary in the current
boot, or permanent in which case the system never boot again. The latter is
more serious and it is named as permanent denial of service (PDoS).
+
+We will consider the below adversary for the EDK II firmware:
+
+| Adversary | Capability |
+| --- | --- |
+| Network Attacker | The attacker may connect to the system by network in
order to eavesdrop, intercept, masquerade, or modify the network packet. |
+| Unprivileged Software Attacker | The attacker may run ring-3 software in an
OS application layer. The attacker may perform a software based side channel
attack (such as using cache timing). |
+| System Software Attacker | The attacker may run ring-0 software in the OS
kernel or hypervisor, or run 3rd party firmware code in firmware boot phase.
The attacker may perform the software based side channel attack (such as using
cache timing, performance counters, branch information, or power status). |
+| Simple Hardware Attacker | The attacker may touch the platform hardware
(such as power button or jumper) and attach/remove a simple malicious device
(such as hardware debugger, PCI Leach to the external port, PCIE card to the
PCIE slot, memory DIMM, NIC cable, hard drive, keyboard, USB device, Bluetooth
device). The attacker may hijack the
Re: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent buffer overruns
One confusion below:
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Krzysztof Koch
> Sent: Thursday, August 1, 2019 4:44 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray ;
> Gao, Zhichao ; sami.muja...@arm.com;
> matteo.carl...@arm.com; n...@arm.com
> Subject: [edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: GTDT: Prevent
> buffer overruns
>
> Modify the GTDT table parsing logic to prevent reading past the ACPI buffer
> lengths provided and to make it consistent with other table parsers. This
> includes converting the do-while loop in ParseAcpiGtdt() into a while loop.
>
> Remove a check which ensures that the entire Platform GT Block Structure
> buffer has been parsed. The ACPI specification does not ban from defining
> buffers which are larger than the size indicated by the count and sizes of
> substructures which constitute it.
>
> Change the data type of the Length parameter to the DumpGTBlock()
> function to reflect the width of the respective ACPI structure's field.
>
> References:
> - ACPI 6.3, January 2019, Table 5-124
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Notes:
> v1:
> - Prevent buffer overruns in GTDT acpiview parser [Krzysztof]
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> | 147 ++--
> 1 file changed, 76 insertions(+), 71 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> index
> 1e5b5764f50a2d29aa904c889bc89af5bdc3af5c..57174e14c80072f12b90e1996e
> be8f0002d0c404 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtPars
> +++ er.c
> @@ -23,7 +23,6 @@ STATIC CONST UINT8* PlatformTimerType; STATIC
> CONST UINT16* PlatformTimerLength; STATIC CONST UINT32*
> GtBlockTimerCount; STATIC CONST UINT32* GtBlockTimerOffset; -STATIC
> CONST UINT16* GtBlockLength; STATIC ACPI_DESCRIPTION_HEADER_INFO
> AcpiHdrInfo;
>
> /**
> @@ -127,7 +126,7 @@ STATIC CONST ACPI_PARSER
> GtPlatformTimerHeaderParser[] = { **/ STATIC CONST ACPI_PARSER
> GtBlockParser[] = {
>{L"Type", 1, 0, L"%d", NULL, NULL, NULL, NULL},
> - {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL},
> + {L"Length", 2, 1, L"%d", NULL, NULL, NULL, NULL},
>{L"Reserved", 1, 3, L"%x", NULL, NULL, NULL, NULL},
>{L"Physical address (CntCtlBase)", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL},
>{L"Timer Count", 4, 12, L"%d", NULL, (VOID**), @@ -
> 168,56 +167,43 @@ STATIC CONST ACPI_PARSER
> SBSAGenericWatchdogParser[] = {
> /**
>This function parses the Platform GT Block.
>
> - @param [in] Ptr Pointer to the start of the GT Block data.
> - @param [in] Length Length of the GT Block structure.
> + @param [in] Ptr Pointer to the start of the GT Block data.
> + @param [in] LengthLength of the GT Block structure.
> **/
> STATIC
> VOID
> DumpGTBlock (
>IN UINT8* Ptr,
> - IN UINT32 Length
> + IN UINT16 Length
>)
> {
>UINT32 Index;
>UINT32 Offset;
> - UINT32 GTBlockTimerLength;
>
> - Offset = ParseAcpi (
> - TRUE,
> - 2,
> - "GT Block",
> - Ptr,
> - Length,
> - PARSER_PARAMS (GtBlockParser)
> - );
> - GTBlockTimerLength = (*GtBlockLength - Offset) / (*GtBlockTimerCount);
> - Length -= Offset;
> + ParseAcpi (
> +TRUE,
> +2,
> +"GT Block",
> +Ptr,
> +Length,
> +PARSER_PARAMS (GtBlockParser)
> +);
>
> - if (*GtBlockTimerCount != 0) {
> -Ptr += (*GtBlockTimerOffset);
> -Index = 0;
> -while ((Index < (*GtBlockTimerCount)) && (Length >=
> GTBlockTimerLength)) {
> - Offset = ParseAcpi (
> - TRUE,
> - 2,
> - "GT Block Timer",
> - Ptr,
> - GTBlockTimerLength,
> - PARSER_PARAMS (GtBlockTimerParser)
> - );
> - // Increment by GT Block Timer structure size
> - Ptr += Offset;
> - Length -= Offset;
> - Index++;
> -}
> + Offset = *GtBlockTimerOffset;
> + Index = 0;
>
> -if (Length != 0) {
> - IncrementErrorCount ();
> - Print (
> -L"ERROR:GT Block Timer length mismatch. Unparsed %d bytes.\n",
> -Length
> -);
> -}
> + // Parse the specified number of GT Block Timer Structures or the GT
> + Block // Structure buffer length. Whichever is minimum.
> + while ((Index++ < *GtBlockTimerCount) &&
> + (Offset < Length)) {
> +Offset += ParseAcpi (
> +TRUE,
> +2,
> +"GT Block Timer",
> +Ptr + Offset,
> +Length - Offset,
> +PARSER_PARAMS (GtBlockTimerParser)
> +);
The above confuse me a lot. ACPI Spec 6.3
Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
About DbgDevInfoHeaderParser and DbgDevInfoParser.
This patch would parse same DbgDevInfo twice, one for getting length, the other
for dumping structure info.
How about the following?
Add one parameter for DumpDbgDeviceInfo
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
IN UINT8* Ptr,
OUT UINT32* Length
)
==>
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
IN UINT8* Ptr,
IN UINT32* Length// remain length of acpi struct to
parse to make sure all operation is in a valid scope
OUT UINT16* DbgDevInfoLength // return pointer dbgdevinfo length
)
Then we would not need an anditional DbgDevInfoHeaderParser and the header
would be parsed for only once.
Any better comments, please let me know.
Thanks,
Zhichao
> -Original Message-
> From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> Sent: Thursday, August 1, 2019 4:44 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray ;
> Gao, Zhichao ; sami.muja...@arm.com;
> matteo.carl...@arm.com; n...@arm.com
> Subject: [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
>
> Modify the DBG2 table parsing logic to prevent reading past the ACPI buffer
> lengths provided.
>
> Modify the signature of the DumpDbgDeviceInfo() function to make it
> consistent with the ACPI structure processing functions in other acpiview
> parsers. Now, the length of the Debug Device Information Structure is read
> before the entire structure is dumped.
>
> This refactoring change makes it easier to stop reading beyond the
> DBG2 table buffer if the Debug Device Information Structure Buffer does not
> fit in the DBG2 buffer.
>
> For processing the first two fields of the Debug Device Information Structure
> (to get the length) a new ACPI_PARSER array is defined.
>
> References:
> - Microsoft Debug Port Table 2 (DBG2), December 10, 2015
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Notes:
> v1:
> - Prevent buffer overruns in DBG2 acpiview parser [Krzysztof]
>
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> | 141 +---
> 1 file changed, 92 insertions(+), 49 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> index
> c6929695a1032c57761ef85002d6c51b7800ce23..869e700b9beda4886bf7bc5ae
> 4ced3ab9a59efa3 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Pars
> +++ er.c
> @@ -64,10 +64,17 @@ STATIC CONST ACPI_PARSER Dbg2Parser[] = {
> (VOID**), NULL, NULL} };
>
> +/// An ACPI_PARSER array describing the debug device information
> +structure /// header.
> +STATIC CONST ACPI_PARSER DbgDevInfoHeaderParser[] = {
> + {L"Revision", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL} };
> +
> /// An ACPI_PARSER array describing the debug device information.
> STATIC CONST ACPI_PARSER DbgDevInfoParser[] = {
>{L"Revision", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
> - {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL},
> + {L"Length", 2, 1, L"%d", NULL, NULL, NULL, NULL},
>
>{L"Generic Address Registers Count", 1, 3, L"0x%x", NULL,
> (VOID**), NULL, NULL},
> @@ -93,76 +100,91 @@ STATIC CONST ACPI_PARSER DbgDevInfoParser[] = {
> /**
>This function parses the debug device information structure.
>
> - @param [in] Ptr Pointer to the start of the buffer.
> - @param [out] Length Pointer in which the length of the debug
> - device information is returned.
> + @param [in] Ptr Pointer to the start of the buffer.
> + @param [in] Length Length of the debug device information structure.
> **/
> STATIC
> VOID
> EFIAPI
> DumpDbgDeviceInfo (
> - IN UINT8* Ptr,
> - OUT UINT32* Length
> + IN UINT8* Ptr,
> + IN UINT16 Length
>)
> {
>UINT16 Index;
> - UINT8* DataPtr;
> - UINT32* AddrSize;
> -
> - // Parse the debug device info to get the Length
> - ParseAcpi (
> -FALSE,
> -0,
> -"Debug Device Info",
> -Ptr,
> -3, // Length is 2 bytes starting at offset 1
> -PARSER_PARAMS (DbgDevInfoParser)
> -);
> + UINT16 Offset;
>
>ParseAcpi (
> TRUE,
> 2,
> "Debug Device Info",
> Ptr,
> -*DbgDevInfoLen,
> +Length,
> PARSER_PARAMS (DbgDevInfoParser)
> );
>
> - // GAS and Address Size
> + // GAS
>Index = 0;
> - DataPtr = Ptr + (*BaseAddrRegOffset);
> - AddrSize = (UINT32*)(Ptr + (*AddrSizeOffset));
> - while (Index < (*GasCount)) {
> + Offset = *BaseAddrRegOffset;
> + while ((Index++ < *GasCount) &&
> + (Offset < Length)) {
> PrintFieldName (4, L"BaseAddressRegister");
> -DumpGasStruct (DataPtr, 4, GAS_LENGTH);
> +Offset += (UINT16)DumpGasStruct (
> +Ptr + Offset,
> +4,
> +
[edk2-devel] [Patch v2 0/2] UefiCpuPkg: Remove debug message.
This debug message may be called by BSP and APs. It may caused ASSERT when APs call this debug code. In order to avoid system boot assert, Remove this debug message. Cc: Ray Ni Cc: Laszlo Ersek Eric Dong (2): UefiCpuPkg/RegisterCpuFeaturesLib: Remove debug message. UefiCpuPkg/PiSmmCpuDxeSmm: Remove debug message. .../CpuFeaturesInitialize.c | 22 --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 22 +-- 2 files changed, 1 insertion(+), 43 deletions(-) -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44895): https://edk2.groups.io/g/devel/message/44895 Mute This Topic: https://groups.io/mt/32723168/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v2 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Remove debug message.
This debug message may be called by BSP and APs. It may
caused ASSERT when APs call this debug code.
In order to avoid system boot assert, Remove this debug
message.
Signed-off-by: Eric Dong
Cc: Ray Ni
Cc: Laszlo Ersek
---
.../CpuFeaturesInitialize.c | 22 ---
1 file changed, 22 deletions(-)
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 4e97e863c7..fb0535edd6 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -9,7 +9,6 @@
#include "RegisterCpuFeatures.h"
CHAR16 *mDependTypeStr[] = {L"None", L"Thread", L"Core", L"Package",
L"Invalid" };
-CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR", L"MMIO", L"CACHE", L"SEMAP",
L"INVALID" };
/**
Worker function to save PcdCpuFeaturesCapability.
@@ -772,7 +771,6 @@ ProgramProcessorRegister (
UINT32PackageThreadsCount;
UINT32CurrentThread;
UINTN ProcessorIndex;
- UINTN ThreadIndex;
UINTN ValidThreadCount;
UINT32*ValidCoreCountPerPackage;
@@ -785,26 +783,6 @@ ProgramProcessorRegister (
RegisterTableEntry = [Index];
-DEBUG_CODE_BEGIN ();
- //
- // Wait for the AP to release the MSR spin lock.
- //
- while (!AcquireSpinLockOrFail (>ConsoleLogLock)) {
-CpuPause ();
- }
- ThreadIndex = ApLocation->Package * CpuStatus->MaxCoreCount *
CpuStatus->MaxThreadCount +
- ApLocation->Core * CpuStatus->MaxThreadCount +
- ApLocation->Thread;
- DEBUG ((
-DEBUG_INFO,
-"Processor = %08lu, Index %08lu, Type = %s!\n",
-(UINT64)ThreadIndex,
-(UINT64)Index,
-mRegisterTypeStr[MIN ((REGISTER_TYPE)RegisterTableEntry->RegisterType,
InvalidReg)]
-));
- ReleaseSpinLock (>ConsoleLogLock);
-DEBUG_CODE_END ();
-
//
// Check the type of specified register
//
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44896): https://edk2.groups.io/g/devel/message/44896
Mute This Topic: https://groups.io/mt/32723169/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Remove debug message.
This debug message may be called by BSP and APs. It may
caused ASSERT when APs call this debug code.
In order to avoid system boot assert, Remove this debug
message.
Signed-off-by: Eric Dong
Cc: Ray Ni
Cc: Laszlo Ersek
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 22 +-
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 2cfc61b2c6..d20bc4aae6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -1,7 +1,7 @@
/** @file
Code for Processor S3 restoration
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -90,8 +90,6 @@ UINT8mApHltLoopCodeTemplate[] = {
0xEB, 0xFC // jmp $-2
};
-CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR", L"MMIO", L"CACHE", L"SEMAP",
L"INVALID" };
-
/**
Sync up the MTRR values for all processors.
@@ -189,7 +187,6 @@ ProgramProcessorRegister (
UINT32PackageThreadsCount;
UINT32CurrentThread;
UINTN ProcessorIndex;
- UINTN ThreadIndex;
UINTN ValidThreadCount;
UINT32*ValidCoreCountPerPackage;
@@ -202,23 +199,6 @@ ProgramProcessorRegister (
RegisterTableEntry = [Index];
-DEBUG_CODE_BEGIN ();
- if (ApLocation != NULL) {
-AcquireSpinLock (>ConsoleLogLock);
-ThreadIndex = ApLocation->Package * CpuStatus->MaxCoreCount *
CpuStatus->MaxThreadCount +
- ApLocation->Core * CpuStatus->MaxThreadCount +
- ApLocation->Thread;
-DEBUG ((
- DEBUG_INFO,
- "Processor = %lu, Entry Index %lu, Type = %s!\n",
- (UINT64)ThreadIndex,
- (UINT64)Index,
- mRegisterTypeStr[MIN
((REGISTER_TYPE)RegisterTableEntry->RegisterType, InvalidReg)]
- ));
-ReleaseSpinLock (>ConsoleLogLock);
- }
-DEBUG_CODE_END ();
-
//
// Check the type of specified register
//
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44897): https://edk2.groups.io/g/devel/message/44897
Mute This Topic: https://groups.io/mt/32723170/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/1] SecurityPkg/TpmCommLib: Remove TpmCommLib
Good. Reviewed-by: jiewen@intel.com
> -Original Message-
> From: Zhang, Shenglei
> Sent: Monday, August 5, 2019 1:55 PM
> To: devel@edk2.groups.io
> Cc: Zhang, Shenglei ; Yao, Jiewen
> ; Wang, Jian J ; Zhang,
> Chao B
> Subject: [PATCH 1/1] SecurityPkg/TpmCommLib: Remove TpmCommLib
>
> From: shenglei
>
> TpmCommonLib is no longer used by TcgPei/TcgDxe/Tcg2ConfigPei
> modules. So TpmCommLib can be removed.
>
> Cc: Jiewen Yao
> Cc: Jian Wang
> Cc: Chao Zhang
> Signed-off-by: Shenglei Zhang
> ---
> SecurityPkg/Library/TpmCommLib/TisPc.c| 177 ---
> SecurityPkg/Library/TpmCommLib/TpmComm.c | 44 ---
> SecurityPkg/Include/Library/TpmCommLib.h | 281 --
> SecurityPkg/Library/TpmCommLib/CommonHeader.h | 23 --
> SecurityPkg/Library/TpmCommLib/TpmCommLib.inf | 45 ---
> SecurityPkg/Library/TpmCommLib/TpmCommLib.uni | 17 --
> SecurityPkg/SecurityPkg.dec | 4 -
> SecurityPkg/SecurityPkg.dsc | 2 -
> 8 files changed, 593 deletions(-)
> delete mode 100644 SecurityPkg/Library/TpmCommLib/TisPc.c
> delete mode 100644 SecurityPkg/Library/TpmCommLib/TpmComm.c
> delete mode 100644 SecurityPkg/Include/Library/TpmCommLib.h
> delete mode 100644 SecurityPkg/Library/TpmCommLib/CommonHeader.h
> delete mode 100644 SecurityPkg/Library/TpmCommLib/TpmCommLib.inf
> delete mode 100644 SecurityPkg/Library/TpmCommLib/TpmCommLib.uni
>
> diff --git a/SecurityPkg/Library/TpmCommLib/TisPc.c
> b/SecurityPkg/Library/TpmCommLib/TisPc.c
> deleted file mode 100644
> index 162e883d2170..
> --- a/SecurityPkg/Library/TpmCommLib/TisPc.c
> +++ /dev/null
> @@ -1,177 +0,0 @@
> -/** @file
> - Basic TIS (TPM Interface Specification) functions.
> -
> -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#include "CommonHeader.h"
> -
> -/**
> - Check whether TPM chip exist.
> -
> - @param[in] TisReg Pointer to TIS register.
> -
> - @retvalTRUETPM chip exists.
> - @retvalFALSE TPM chip is not found.
> -**/
> -BOOLEAN
> -TisPcPresenceCheck (
> - IN TIS_PC_REGISTERS_PTR TisReg
> - )
> -{
> - UINT8 RegRead;
> -
> - RegRead = MmioRead8 ((UINTN)>Access);
> - return (BOOLEAN)(RegRead != (UINT8)-1);
> -}
> -
> -/**
> - Check whether the value of a TPM chip register satisfies the input BIT
> setting.
> -
> - @param[in] Register Address port of register to be checked.
> - @param[in] BitSet Check these data bits are set.
> - @param[in] BitClear Check these data bits are clear.
> - @param[in] TimeOut The max wait time (unit MicroSecond)
> when checking register.
> -
> - @retval EFI_SUCCESS The register satisfies the check bit.
> - @retval EFI_TIMEOUT The register can't run into the expected
> status in time.
> -**/
> -EFI_STATUS
> -EFIAPI
> -TisPcWaitRegisterBits (
> - IN UINT8 *Register,
> - IN UINT8 BitSet,
> - IN UINT8 BitClear,
> - IN UINT32TimeOut
> - )
> -{
> - UINT8 RegRead;
> - UINT32WaitTime;
> -
> - for (WaitTime = 0; WaitTime < TimeOut; WaitTime += 30){
> -RegRead = MmioRead8 ((UINTN)Register);
> -if ((RegRead & BitSet) == BitSet && (RegRead & BitClear) == 0)
> - return EFI_SUCCESS;
> -MicroSecondDelay (30);
> - }
> - return EFI_TIMEOUT;
> -}
> -
> -/**
> - Get BurstCount by reading the burstCount field of a TIS regiger
> - in the time of default TIS_TIMEOUT_D.
> -
> - @param[in] TisRegPointer to TIS register.
> - @param[out] BurstCountPointer to a buffer to store the
> got BurstConut.
> -
> - @retval EFI_SUCCESS Get BurstCount.
> - @retval EFI_INVALID_PARAMETER TisReg is NULL or BurstCount is
> NULL.
> - @retval EFI_TIMEOUT BurstCount can't be got in time.
> -**/
> -EFI_STATUS
> -EFIAPI
> -TisPcReadBurstCount (
> - IN TIS_PC_REGISTERS_PTR TisReg,
> - OUT UINT16*BurstCount
> - )
> -{
> - UINT32WaitTime;
> - UINT8 DataByte0;
> - UINT8 DataByte1;
> -
> - if (BurstCount == NULL || TisReg == NULL) {
> -return EFI_INVALID_PARAMETER;
> - }
> -
> - WaitTime = 0;
> - do {
> -//
> -// TIS_PC_REGISTERS_PTR->burstCount is UINT16, but it is not 2bytes
> aligned,
> -// so it needs to use MmioRead8 to read two times
> -//
> -DataByte0 = MmioRead8 ((UINTN)>BurstCount);
> -DataByte1 = MmioRead8 ((UINTN)>BurstCount + 1);
> -*BurstCount = (UINT16)((DataByte1 << 8) + DataByte0);
> -if (*BurstCount != 0) {
> - return EFI_SUCCESS;
> -}
> -MicroSecondDelay (30);
> -WaitTime += 30;
> - } while (WaitTime < TIS_TIMEOUT_D);
> -
> - return
Re: [edk2-devel] [PATCH 0/3] Enhance ESRT to support multiple controllers
> -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Eric Jin > Sent: Wednesday, July 31, 2019 10:59 PM > To: devel@edk2.groups.io > Subject: [edk2-devel] [PATCH 0/3] Enhance ESRT to support multiple > controllers > > Multiple Controllers Support solution > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1525 > > The patch set is to makes enhancement to the ESRT when multiple > same controllers exist in one system. > > Eric Jin (3): > MdeModulePkg/EsrtFmpDxe: Merge multiple FMP into ESRT > MdeModulePkg/EsrtFmpDxe: Correct LastAttemptVersion algorithm in ESRT > MdeModulePkg/EsrtFmpDxe: Detect duplicate GUID/HardwareInstance Hello Eric, After looking into the series, I found that patches 2/3 & 3/3 are actually addressing issues introduced by the 1/3 patch. It looks to me that these 3 patches should be squashed into 1 commit, since they all focus on adding a feature for ESRT to support multiple controllers. However, originally, the 3 separate commits come from the edk2-staging repository, so I am not very sure what approach should be adopted when merging them back to the edk2 repo master branch. (Includes the stewards here for suggestions.) My personal preference is to merge them together as one patch. Best Regards, Hao Wu > > MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 +--- > > 1 file changed, 257 insertions(+), 137 deletions(-) > > -- > 2.20.1.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44893): https://edk2.groups.io/g/devel/message/44893 Mute This Topic: https://groups.io/mt/32667728/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
