From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0E48821E48F50 for ; Wed, 24 Jan 2018 07:43:02 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F3592E89B; Wed, 24 Jan 2018 15:48:29 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-13.rdu2.redhat.com [10.10.121.13]) by smtp.corp.redhat.com (Postfix) with ESMTP id E23055D724; Wed, 24 Jan 2018 15:48:28 +0000 (UTC) To: Dandan Bi , edk2-devel@lists.01.org Cc: Eric Dong , Liming Gao References: <1516780916-6364-1-git-send-email-dandan.bi@intel.com> <1516780916-6364-2-git-send-email-dandan.bi@intel.com> From: Laszlo Ersek Message-ID: <00ecccd6-6e43-9433-7db1-244adc00f0d7@redhat.com> Date: Wed, 24 Jan 2018 16:48:28 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1516780916-6364-2-git-send-email-dandan.bi@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 24 Jan 2018 15:48:30 +0000 (UTC) Subject: Re: [patch 1/3] UefiCpuPkg/S3Resume: Remove useless pref code X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jan 2018 15:43:02 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/24/18 09:01, Dandan Bi wrote: > Our new performance infrastructure can support to dump performance > date form ACPI table in OS. So we can remove the old pref code to > write performance data to OS. > > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Liming Gao > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Dandan Bi > --- > UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 131 --------------------- > .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 3 +- > 2 files changed, 1 insertion(+), 133 deletions(-) > > diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > index d7d2a4d..4d77689 100644 > --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > @@ -20,11 +20,10 @@ > > #include > > #include > #include > -#include > #include > #include > #include > #include > #include > @@ -284,136 +283,10 @@ GLOBAL_REMOVE_IF_UNREFERENCED IA32_GDT mGdtEntries[] = { > GLOBAL_REMOVE_IF_UNREFERENCED CONST IA32_DESCRIPTOR mGdt = { > sizeof (mGdtEntries) - 1, > (UINTN) mGdtEntries > }; > > -/** > - Performance measure function to get S3 detailed performance data. > - > - This function will getS3 detailed performance data and saved in pre-reserved ACPI memory. > -**/ > -VOID > -WriteToOsS3PerformanceData ( > - VOID > - ) > -{ > - EFI_STATUS Status; > - EFI_PHYSICAL_ADDRESS mAcpiLowMemoryBase; > - PERF_HEADER *PerfHeader; > - PERF_DATA *PerfData; > - UINT64 Ticker; > - UINTN Index; > - EFI_PEI_READ_ONLY_VARIABLE2_PPI *VariableServices; > - UINTN VarSize; > - UINTN LogEntryKey; > - CONST VOID *Handle; > - CONST CHAR8 *Token; > - CONST CHAR8 *Module; > - UINT64 StartTicker; > - UINT64 EndTicker; > - UINT64 StartValue; > - UINT64 EndValue; > - BOOLEAN CountUp; > - UINT64 Freq; > - > - // > - // Retrieve time stamp count as early as possible > - // > - Ticker = GetPerformanceCounter (); > - > - Freq = GetPerformanceCounterProperties (&StartValue, &EndValue); > - > - Freq = DivU64x32 (Freq, 1000); > - > - Status = PeiServicesLocatePpi ( > - &gEfiPeiReadOnlyVariable2PpiGuid, > - 0, > - NULL, > - (VOID **) &VariableServices > - ); > - if (EFI_ERROR (Status)) { > - return; > - } > - > - VarSize = sizeof (EFI_PHYSICAL_ADDRESS); > - Status = VariableServices->GetVariable ( > - VariableServices, > - L"PerfDataMemAddr", > - &gPerformanceProtocolGuid, > - NULL, > - &VarSize, > - &mAcpiLowMemoryBase > - ); > - if (EFI_ERROR (Status)) { > - DEBUG ((EFI_D_ERROR, "Fail to retrieve variable to log S3 performance data \n")); > - return; > - } > - > - PerfHeader = (PERF_HEADER *) (UINTN) mAcpiLowMemoryBase; > - > - if (PerfHeader->Signiture != PERFORMANCE_SIGNATURE) { > - DEBUG ((EFI_D_ERROR, "Performance data in ACPI memory get corrupted! \n")); > - return; > - } > - > - // > - // Record total S3 resume time. > - // > - if (EndValue >= StartValue) { > - PerfHeader->S3Resume = Ticker - StartValue; > - CountUp = TRUE; > - } else { > - PerfHeader->S3Resume = StartValue - Ticker; > - CountUp = FALSE; > - } > - > - // > - // Get S3 detailed performance data > - // > - Index = 0; > - LogEntryKey = 0; > - while ((LogEntryKey = GetPerformanceMeasurement ( > - LogEntryKey, > - &Handle, > - &Token, > - &Module, > - &StartTicker, > - &EndTicker)) != 0) { > - if (EndTicker != 0) { > - PerfData = &PerfHeader->S3Entry[Index]; > - > - // > - // Use File Handle to specify the different performance log for PEIM. > - // File Handle is the base address of PEIM FFS file. > - // > - if ((AsciiStrnCmp (Token, "PEIM", PEI_PERFORMANCE_STRING_SIZE) == 0) && (Handle != NULL)) { > - AsciiSPrint (PerfData->Token, PERF_TOKEN_LENGTH, "0x%11p", Handle); > - } else { > - AsciiStrnCpyS (PerfData->Token, PERF_TOKEN_SIZE, Token, PERF_TOKEN_LENGTH); > - } > - if (StartTicker == 1) { > - StartTicker = StartValue; > - } > - if (EndTicker == 1) { > - EndTicker = StartValue; > - } > - Ticker = CountUp? (EndTicker - StartTicker) : (StartTicker - EndTicker); > - PerfData->Duration = (UINT32) DivU64x32 (Ticker, (UINT32) Freq); > - > - // > - // Only Record > 1ms performance data so that more big performance can be recorded. > - // > - if ((Ticker > Freq) && (++Index >= PERF_PEI_ENTRY_MAX_NUM)) { > - // > - // Reach the maximum number of PEI performance log entries. > - // > - break; > - } > - } > - } > - PerfHeader->S3EntryNum = (UINT32) Index; > -} > > /** > The function will check if current waking vector is long mode. > > @param AcpiS3Context a pointer to a structure of ACPI_S3_CONTEXT > @@ -602,14 +475,10 @@ S3ResumeBootOs ( > // > // report status code on S3 resume > // > REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_PC_OS_WAKE); > > - PERF_CODE ( > - WriteToOsS3PerformanceData (); > - ); > - > AsmTransferControl = (ASM_TRANSFER_CONTROL)(UINTN)PeiS3ResumeState->AsmTransferControl; > if (Facs->XFirmwareWakingVector != 0) { > // > // Switch to native waking vector > // > diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf > index 15fa2d1..9522ede 100644 > --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf > +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf > @@ -3,11 +3,11 @@ > # > # This module works with StandAloneBootScriptExecutor to S3 resume to OS. > # This module will excute the boot script saved during last boot and after that, > # control is passed to OS waking up handler. > # > -# Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
> +# Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> # Copyright (c) 2017, AMD Incorporated. 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 > @@ -71,11 +71,10 @@ > PrintLib > > [Guids] > gEfiBootScriptExecutorVariableGuid ## SOMETIMES_CONSUMES ## UNDEFINED # LockBox > gEfiBootScriptExecutorContextGuid ## SOMETIMES_CONSUMES ## UNDEFINED # LockBox > - gPerformanceProtocolGuid ## SOMETIMES_CONSUMES ## Variable:L"PerfDataMemAddr" > ## SOMETIMES_CONSUMES ## HOB > ## SOMETIMES_CONSUMES ## UNDEFINED # LockBox > gEfiAcpiVariableGuid > gEfiAcpiS3ContextGuid ## SOMETIMES_CONSUMES ## UNDEFINED # LockBox > gEdkiiEndOfS3ResumeGuid ## SOMETIMES_CONSUMES ## UNDEFINED # Used to do smm communication > Looks good to me. Specifically for use in OVMF, this logic has always been inactive, since we don't yet have PEI-phase (r/o) variable access. Reviewed-by: Laszlo Ersek Thanks Laszlo