From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web11.19477.1599735831840370144 for ; Thu, 10 Sep 2020 04:03:52 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=QZD5JDX3; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1599735831; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wHs8qpvtcT/iZw09wDRXwJIFW/x7yEMloTJ0o5XKTXE=; b=QZD5JDX3vTTqF6z7bKLNgfaQK2Y6UkC5jAsBNC+ofK+Sx6vt9Gnegp3buWhOJuWLL7n+KL JJicirCSsxnC2tPLOosOIuveHpNU2c5cBp9jUPzpT9fEgW/WubIg9OoYL3moQR/lk4tKNp bjq5/fGOdQPgw0Ost2yqTqW2U37EWAg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-321-_mG0t0qeMj-JL0BASRYaiQ-1; Thu, 10 Sep 2020 07:03:44 -0400 X-MC-Unique: _mG0t0qeMj-JL0BASRYaiQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BC4131882FA3; Thu, 10 Sep 2020 11:03:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-83.ams2.redhat.com [10.36.114.83]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1EB1A27BD0; Thu, 10 Sep 2020 11:03:40 +0000 (UTC) Subject: Re: [PATCH] UefiCpuPkg/RegisterCpuFeaturesLib: Support MpServices2 only case. To: Chasel Chiu , devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Rahul Kumar , Nate DeSimone References: <20200910090233.22784-1-chasel.chiu@intel.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 10 Sep 2020 13:03:40 +0200 MIME-Version: 1.0 In-Reply-To: <20200910090233.22784-1-chasel.chiu@intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0.002 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Hi Chasel, On 09/10/20 11:02, Chasel Chiu wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2883 > > MpServices Ppi can be replaced by MpServices2 Ppi and MpServices2 > Ppi is mandatory for RegisterCpuFeaturesLib functionality, > basing on this we can drop MpServices Ppi usage from the library > and the constraint that both Ppis must be installed. > > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > Cc: Nate DeSimone > Signed-off-by: Chasel Chiu > --- > UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c | 61 +++++++++++++++++++++++-------------------------------------- > UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 6 +++--- > 2 files changed, 26 insertions(+), 41 deletions(-) > > diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c > index 64768f7a74..4e558e9fee 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c > @@ -1,7 +1,7 @@ > /** @file > CPU Register Table Library functions. > > - Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.
> + Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -11,7 +11,6 @@ > #include > #include > #include > -#include > #include > > #include "RegisterCpuFeatures.h" > @@ -75,10 +74,10 @@ GetMpService ( > MP_SERVICES MpService; > > // > - // Get MP Services Protocol > + // Get MP Services2 Ppi > // > Status = PeiServicesLocatePpi ( > - &gEfiPeiMpServicesPpiGuid, > + &gEdkiiPeiMpServices2PpiGuid, > 0, > NULL, > (VOID **)&MpService.Ppi > @@ -100,17 +99,17 @@ GetProcessorIndex ( > ) > { > EFI_STATUS Status; > - EFI_PEI_MP_SERVICES_PPI *CpuMpPpi; > + EDKII_PEI_MP_SERVICES2_PPI *CpuMp2Ppi; > UINTN ProcessorIndex; > > - CpuMpPpi = CpuFeaturesData->MpService.Ppi; > + CpuMp2Ppi = CpuFeaturesData->MpService.Ppi; > > // > // For two reasons which use NULL for WhoAmI: > // 1. This function will be called by APs and AP should not use PeiServices Table > // 2. Check WhoAmI implementation, this parameter will not be used. > // > - Status = CpuMpPpi->WhoAmI(NULL, CpuMpPpi, &ProcessorIndex); > + Status = CpuMp2Ppi->WhoAmI (CpuMp2Ppi, &ProcessorIndex); > ASSERT_EFI_ERROR (Status); > return ProcessorIndex; > } > @@ -131,16 +130,15 @@ GetProcessorInformation ( > OUT EFI_PROCESSOR_INFORMATION *ProcessorInfoBuffer > ) > { > - EFI_PEI_MP_SERVICES_PPI *CpuMpPpi; > + EDKII_PEI_MP_SERVICES2_PPI *CpuMp2Ppi; > EFI_STATUS Status; > CPU_FEATURES_DATA *CpuFeaturesData; > > CpuFeaturesData = GetCpuFeaturesData (); > - CpuMpPpi = CpuFeaturesData->MpService.Ppi; > + CpuMp2Ppi = CpuFeaturesData->MpService.Ppi; > > - Status = CpuMpPpi->GetProcessorInfo ( > - GetPeiServicesTablePointer(), > - CpuMpPpi, > + Status = CpuMp2Ppi->GetProcessorInfo ( > + CpuMp2Ppi, > ProcessorNumber, > ProcessorInfoBuffer > ); > @@ -162,18 +160,17 @@ StartupAllAPsWorker ( > ) > { > EFI_STATUS Status; > - EFI_PEI_MP_SERVICES_PPI *CpuMpPpi; > + EDKII_PEI_MP_SERVICES2_PPI *CpuMp2Ppi; > CPU_FEATURES_DATA *CpuFeaturesData; > > CpuFeaturesData = GetCpuFeaturesData (); > - CpuMpPpi = CpuFeaturesData->MpService.Ppi; > + CpuMp2Ppi = CpuFeaturesData->MpService.Ppi; > > // > // Wakeup all APs for data collection. > // > - Status = CpuMpPpi->StartupAllAPs ( > - GetPeiServicesTablePointer (), > - CpuMpPpi, > + Status = CpuMp2Ppi->StartupAllAPs ( > + CpuMp2Ppi, > Procedure, > FALSE, > 0, > @@ -203,17 +200,7 @@ StartupAllCPUsWorker ( > // > // Get MP Services2 Ppi > // > - Status = PeiServicesLocatePpi ( > - &gEdkiiPeiMpServices2PpiGuid, > - 0, > - NULL, > - (VOID **)&CpuMp2Ppi > - ); > - ASSERT_EFI_ERROR (Status); > - > - // > - // Wakeup all APs for data collection. > - // > + CpuMp2Ppi = CpuFeaturesData->MpService.Ppi; > Status = CpuMp2Ppi->StartupAllCPUs ( > CpuMp2Ppi, > Procedure, > @@ -234,18 +221,17 @@ SwitchNewBsp ( > ) > { > EFI_STATUS Status; > - EFI_PEI_MP_SERVICES_PPI *CpuMpPpi; > + EDKII_PEI_MP_SERVICES2_PPI *CpuMp2Ppi; > CPU_FEATURES_DATA *CpuFeaturesData; > > CpuFeaturesData = GetCpuFeaturesData (); > - CpuMpPpi = CpuFeaturesData->MpService.Ppi; > + CpuMp2Ppi = CpuFeaturesData->MpService.Ppi; > > // > // Wakeup all APs for data collection. > // > - Status = CpuMpPpi->SwitchBSP ( > - GetPeiServicesTablePointer (), > - CpuMpPpi, > + Status = CpuMp2Ppi->SwitchBSP ( > + CpuMp2Ppi, > ProcessorNumber, > TRUE > ); > @@ -269,18 +255,17 @@ GetNumberOfProcessor ( > ) > { > EFI_STATUS Status; > - EFI_PEI_MP_SERVICES_PPI *CpuMpPpi; > + EDKII_PEI_MP_SERVICES2_PPI *CpuMp2Ppi; > CPU_FEATURES_DATA *CpuFeaturesData; > > CpuFeaturesData = GetCpuFeaturesData (); > - CpuMpPpi = CpuFeaturesData->MpService.Ppi; > + CpuMp2Ppi = CpuFeaturesData->MpService.Ppi; > > // > // Get the number of CPUs > // > - Status = CpuMpPpi->GetNumberOfProcessors ( > - GetPeiServicesTablePointer (), > - CpuMpPpi, > + Status = CpuMp2Ppi->GetNumberOfProcessors ( > + CpuMp2Ppi, > NumberOfCpus, > NumberOfEnabledProcessors > ); > diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h > index 53cb340b4c..e8a4aa644d 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h > @@ -10,7 +10,7 @@ > #define _REGISTER_CPU_FEATURES_H_ > #include > #include > -#include > +#include > #include > > #include > @@ -64,8 +64,8 @@ typedef struct { > } PROGRAM_CPU_REGISTER_FLAGS; > > typedef union { > - EFI_MP_SERVICES_PROTOCOL *Protocol; > - EFI_PEI_MP_SERVICES_PPI *Ppi; > + EFI_MP_SERVICES_PROTOCOL *Protocol; > + EDKII_PEI_MP_SERVICES2_PPI *Ppi; > } MP_SERVICES; > > typedef struct { > I defer to Eric and Ray on this patch. Thanks Laszlo