From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=supreeth.venkatesh@arm.com; receiver=edk2-devel@lists.01.org Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id BBC1F21BADAB2 for ; Tue, 16 Oct 2018 02:48:38 -0700 (PDT) Received: by usa-sjc-mx-foss1.foss.arm.com (Postfix, from userid 105) id 9083B36D4; Tue, 16 Oct 2018 03:33:31 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E5B2B35A8; Mon, 15 Oct 2018 05:51:12 -0700 (PDT) Received: from [10.6.43.238] (bc-c3-3-14.eu.iaas.arm.com [10.6.43.238]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B9B3B3F5B1; Mon, 15 Oct 2018 05:50:51 -0700 (PDT) From: Supreeth Venkatesh To: Eric Jin , edk2-devel@lists.01.org Cc: Jiaxin Wu References: <20181013154252.1260-1-eric.jin@intel.com> <51d0b297-b4ec-83a4-4db2-9c02c4a082bb@arm.com> Message-ID: <82eed9af-77fc-df2d-47ea-9ba71f522e35@arm.com> Date: Mon, 15 Oct 2018 13:50:49 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <51d0b297-b4ec-83a4-4db2-9c02c4a082bb@arm.com> Subject: Re: [PATCH] uefi-sct/SctPkg:Fix the flaw in BBTestCreateEventEx_Func_Sub3 on certain situation. Besides AllocatePages(), CreateEventEx may cause the memorymap change itself. Enhance the test to filter the side effect from CreateEventEx() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Oct 2018 09:48:39 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit FYI On 10/15/2018 02:33 AM, Supreeth Venkatesh wrote: > Please use a commit message less than 80 Cols. > > > On 10/13/2018 04:42 PM, Eric Jin wrote: >> Cc: Supreeth Venkatesh >> Cc: Jiaxin Wu >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Eric Jin >> --- >>   ...rTaskPriorityServicesBBTestCreateEventEx.c | 26 +++++++++++-------- >>   .../BlackBoxTest/Support.c                    | 19 +++++++++++++- >>   2 files changed, 33 insertions(+), 12 deletions(-) >> >> diff --git >> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c >> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c >> >> index e2e173ab..25d1ed97 100644 >> --- >> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c >> +++ >> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c >> @@ -1,7 +1,7 @@ >>   /** @file >>       Copyright 2006 - 2016 Unified EFI, Inc.
>> -  Copyright (c) 2010 - 2016, Intel Corporation. All rights >> reserved.
>> +  Copyright (c) 2010 - 2018, Intel Corporation. All rights >> reserved.
>>       This program and the accompanying materials >>     are licensed and made available under the terms and conditions of >> the BSD License >> @@ -192,6 +192,10 @@ BBTestCreateEventEx_Func ( >>     BBTestCreateEventEx_Func_Sub2 (StandardLib); >>   #endif >>   +  // >> +  // The test for the EFI_EVENT_GROUP_MEMORY_MAP_CHANGE >> +  // This event group is notified by the system when the memory map >> has changed. >> +  // >>     BBTestCreateEventEx_Func_Sub3 (StandardLib); >>       // >> @@ -599,12 +603,12 @@ BBTestCreateEventEx_Func_Sub1 ( >>     UINTN               Buffer[MAX_TEST_EVENT_NUM + >> MAX_TEST_EVENT_NUM*2]; >>       // >> -  // Initialize Buffer >> +  // Initialize Buffer and the 0xAA is only for the Sub3 test >>     // >>     for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) { > Strange Logic here. Needs re-look. >>       Buffer[Index] = Index; >> -    Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1); >> -    Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1); >> +    Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA); > Magic Number 0xAA >> +    Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA); > Magic Number 0xAA. Please define Macro or const. >>     } >>       // >> @@ -755,12 +759,12 @@ BBTestCreateEventEx_Func_Sub2 ( >>     UINTN               Buffer[MAX_TEST_EVENT_NUM + >> MAX_TEST_EVENT_NUM*2]; >>       // >> -  // Initialize Buffer >> +  // Initialize Buffer and the 0xAA is only for the Sub3 test >>     // >>     for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) { >>       Buffer[Index] = Index; >> -    Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1); >> -    Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1); >> +    Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA); > Magic Number 0xAA. Please define Macro or const. >> +    Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA); > Magic Number 0xAA. Please define Macro or const. >>     } >>       // >> @@ -914,12 +918,12 @@ BBTestCreateEventEx_Func_Sub3 ( >>     UINTN               Buffer[MAX_TEST_EVENT_NUM + >> MAX_TEST_EVENT_NUM*2]; >>       // >> -  // Initialize Buffer >> +  // Initialize Buffer and the trick to initial it as 0xAA >>     // >>     for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) { >>       Buffer[Index] = Index; >> -    Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1); >> -    Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1); >> +    Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA); > Strange Logic here. Needs re-look. Also, Magic Number AA. >> +    Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA); > Strange Logic here. Needs re-look. Also, Magic Number AA. >>     } >>       // >> @@ -974,7 +978,7 @@ BBTestCreateEventEx_Func_Sub3 ( >>     } >>         // >> -  // Install a configuration table at TPL_NOTIFY >> +  // Call AllocatePage to change the memorymap >>     // >>     OldTpl = gtBS->RaiseTPL (TPL_NOTIFY); >>     diff --git >> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c >> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c >> >> index aa02383e..823e16ab 100644 >> --- >> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c >> +++ >> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c >> @@ -1,7 +1,7 @@ >>   /** @file >>       Copyright 2006 - 2010 Unified EFI, Inc.
>> -  Copyright (c) 2010, Intel Corporation. All rights reserved.
>> +  Copyright (c) 2010 - 2018, Intel Corporation. All rights >> reserved.
>>       This program and the accompanying materials >>     are licensed and made available under the terms and conditions of >> the BSD License >> @@ -64,6 +64,23 @@ NotifyFunctionTplEx( >>         EventIndex = Buffer[0]; >>   +    // >> +    // The special code for the sub3 >> +    // To block possible enter triggered by CreateEventEx >> +    // >> +    if (EventIndex != 2 && Buffer[4] == (UINTN)(0xAA)) > Magic Numbers 2, 4 and OxAA. >> +      return; >> + >> +    // >> +    // The special code for the sub3 >> +    // To initial the Buffer and block the possible enter caused by >> the CloseEvent >> +    // >> +    if (EventIndex == 2 && Buffer[1] == (UINTN)(0xAA)) { > Magic Numbers 2, 1 and OxAA. >> +      for (Index=1; Index<7; Index++) { > Magic Numbers 1, 7. >> +        Buffer[Index] = (UINTN)(-1); > Magic Number -1. >> +      } >> +    } >> + >>       Index = 3-EventIndex; > Magic Number 3. >>         while (1) { >