From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 73BADAC0028 for ; Wed, 28 Feb 2024 03:48:43 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Vcj/tu/rB2r2srohg4QHKJ84xwMdanX0+YYV66bdEGA=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1709092121; v=1; b=b7QbKN7SgAIeVdzxu89912auPDw+GGzBg+YY4J84rxMfJnjbfprEMrks5ST55cyn9GEVw8Wc iMFjH0QqJ3RcSlw1j4FfJjeTsHDNnjntobZameZessGMUc1TJ8fzbVFDF/CUNW4pP/qRrqa9Nhb ILM64OHGyQ7WE4W03CBGh6AQ= X-Received: by 127.0.0.2 with SMTP id j7HnYY7687511xXktgY0zVsa; Tue, 27 Feb 2024 19:48:41 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web10.5734.1709092121205121829 for ; Tue, 27 Feb 2024 19:48:41 -0800 X-Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-491-IV24VZuDMOKOh0MYfzh5mQ-1; Tue, 27 Feb 2024 22:48:36 -0500 X-MC-Unique: IV24VZuDMOKOh0MYfzh5mQ-1 X-Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5DC073C0009F; Wed, 28 Feb 2024 03:48:36 +0000 (UTC) X-Received: from [10.39.192.46] (unknown [10.39.192.46]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8A4571C060AF; Wed, 28 Feb 2024 03:48:34 +0000 (UTC) Message-ID: <9a842e89-b73e-bc0f-6054-06db01c7bf73@redhat.com> Date: Wed, 28 Feb 2024 04:48:33 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: add struct MP_HAND_OFF_CONFIG To: devel@edk2.groups.io, kraxel@redhat.com Cc: Oliver Steffen , Ray Ni , Rahul Kumar References: <20240227114122.1140614-1-kraxel@redhat.com> From: "Laszlo Ersek" In-Reply-To: <20240227114122.1140614-1-kraxel@redhat.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: UhT88G5cQDcdT8WB6GZSfvrux7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=b7QbKN7S; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 2/27/24 12:41, Gerd Hoffmann wrote: > Move the WaitLoopExecutionMode and StartupSignalValue fields to a > separate HOB with the new struct. >=20 > Signed-off-by: Gerd Hoffmann > --- > UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 13 ++++++-- > UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 ++++++++++++++++++++---- > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 34 +++++++++++++-------- > 4 files changed, 66 insertions(+), 22 deletions(-) >=20 > diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h b/UefiCpuPkg/Librar= y/MpInitLib/MpHandOff.h > index 77854d6a81f8..ae93b7e3d7c9 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h > +++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h > @@ -15,7 +15,13 @@ > 0x11e2bd88, 0xed38, 0x4abd, {0xa3, 0x99, 0x21, 0xf2, 0x5f, 0xd0, 0x7= a, 0x60 } \ > } > =20 > +#define MP_HANDOFF_CONFIG_GUID \ > + { \ > + 0xdabbd793, 0x7b46, 0x4144, {0x8a, 0xd4, 0x10, 0x1c, 0x7c, 0x08, 0xe= b, 0xfa } \ > + } > + > extern EFI_GUID mMpHandOffGuid; > +extern EFI_GUID mMpHandOffConfigGuid; > =20 > // > // The information required to transfer from the PEI phase to the > @@ -43,8 +49,11 @@ typedef struct { > // > UINT32 ProcessorIndex; > UINT32 CpuCount; > - UINT32 WaitLoopExecutionMode; > - UINT32 StartupSignalValue; > PROCESSOR_HAND_OFF Info[]; > } MP_HAND_OFF; > + > +typedef struct { > + UINT32 WaitLoopExecutionMode; > + UINT32 StartupSignalValue; > +} MP_HAND_OFF_CONFIG; > #endif > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/Mp= InitLib/MpLib.h > index 3a7b9896cff4..d26035559f22 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h > @@ -482,7 +482,8 @@ GetWakeupBuffer ( > **/ > VOID > SwitchApContext ( > - IN CONST MP_HAND_OFF *FirstMpHandOff > + IN CONST MP_HAND_OFF_CONFIG *MpHandOffConfig, > + IN CONST MP_HAND_OFF *FirstMpHandOff > ); > =20 > /** > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/Mp= InitLib/MpLib.c > index 8c186211fb38..a22bcfc0bc30 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -15,6 +15,7 @@ > =20 > EFI_GUID mCpuInitMpLibHobGuid =3D CPU_INIT_MP_LIB_HOB_GUID; > EFI_GUID mMpHandOffGuid =3D MP_HANDOFF_GUID; > +EFI_GUID mMpHandOffConfigGuid =3D MP_HANDOFF_CONFIG_GUID; > =20 > /** > Save the volatile registers required to be restored following INIT IPI= . > @@ -1935,11 +1936,13 @@ GetBspNumber ( > This procedure allows the AP to switch to another section of > memory and continue its loop there. > =20 > - @param[in] FirstMpHandOff Pointer to first MP hand-off HOB body. > + @param[in] MpHandOffConfig Pointer to MP hand-off config HOB body. > + @param[in] FirstMpHandOff Pointer to first MP hand-off HOB body. > **/ > VOID > SwitchApContext ( > - IN CONST MP_HAND_OFF *FirstMpHandOff > + IN CONST MP_HAND_OFF_CONFIG *MpHandOffConfig, > + IN CONST MP_HAND_OFF *FirstMpHandOff > ) > { > UINTN Index; > @@ -1955,7 +1958,7 @@ SwitchApContext ( > for (Index =3D 0; Index < MpHandOff->CpuCount; Index++) { > if (MpHandOff->ProcessorIndex + Index !=3D BspNumber) { > *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = =3D (UINTN)SwitchContextPerAp; > - *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress = =3D MpHandOff->StartupSignalValue; > + *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress = =3D MpHandOffConfig->StartupSignalValue; > } > } > } > @@ -1975,6 +1978,26 @@ SwitchApContext ( > } > } > =20 > +/** > + Get pointer to MP_HAND_OFF_CONFIG GUIDed HOB body. > + > + @return The pointer to MP_HAND_OFF_CONFIG structure. > +**/ > +MP_HAND_OFF_CONFIG * > +GetMpHandOffConfigHob ( > + VOID > + ) > +{ > + EFI_HOB_GUID_TYPE *GuidHob; > + > + GuidHob =3D GetFirstGuidHob (&mMpHandOffConfigGuid); > + if (GuidHob =3D=3D NULL) { > + return NULL; > + } > + > + return (MP_HAND_OFF_CONFIG *)GET_GUID_HOB_DATA (GuidHob); > +} > + > /** > Get pointer to next MP_HAND_OFF GUIDed HOB body. > =20 > @@ -2022,6 +2045,7 @@ MpInitLibInitialize ( > VOID > ) > { > + MP_HAND_OFF_CONFIG *MpHandOffConfig; > MP_HAND_OFF *FirstMpHandOff; > MP_HAND_OFF *MpHandOff; > CPU_INFO_IN_HOB *CpuInfoInHob; > @@ -2239,13 +2263,15 @@ MpInitLibInitialize ( > } > } > =20 > + MpHandOffConfig =3D GetMpHandOffConfigHob (); > + ASSERT (MpHandOffConfig !=3D NULL); So, in connection to the other subthread Re: [edk2-devel] CodeQL Analysis in edk2 msgid: <80abb140-9a9c-43b8-ba0b-d8ea631d9051@linux.microsoft.com> https://edk2.groups.io/g/devel/message/116054 I suggest replacing this with: if (MpHandOffConfig =3D=3D NULL) { DEBUG (( DEBUG_ERROR, "%a: at least one MpHandOff HOB, but no MpHandOffConfig HOB\n", __func__ )); ASSERT (MpHandOffConfig !=3D NULL); CpuDeadLoop (); } (We should use a generic "panic" here; but for now, CpuDeadLoop() should do. There are two prior examples for that in MpInitLib already.) With that update: Reviewed-by: Laszlo Ersek Thanks Laszlo > DEBUG (( > DEBUG_INFO, > "FirstMpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *): %04= d\n", > - FirstMpHandOff->WaitLoopExecutionMode, > + MpHandOffConfig->WaitLoopExecutionMode, > sizeof (VOID *) > )); > - if (FirstMpHandOff->WaitLoopExecutionMode =3D=3D sizeof (VOID *)) { > + if (MpHandOffConfig->WaitLoopExecutionMode =3D=3D sizeof (VOID *)) { > ASSERT (CpuMpData->ApLoopMode !=3D ApInHltLoop); > =20 > CpuMpData->FinishedCount =3D 0; > @@ -2261,7 +2287,7 @@ MpInitLibInitialize ( > // enables the APs to switch to a different memory section and con= tinue their > // looping process there. > // > - SwitchApContext (FirstMpHandOff); > + SwitchApContext (MpHandOffConfig, FirstMpHandOff); > // > // Wait for all APs finished initialization > // > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library= /MpInitLib/PeiMpLib.c > index ec1aa666923d..4d3acb491f36 100644 > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > @@ -126,14 +126,15 @@ SaveCpuMpData ( > IN CPU_MP_DATA *CpuMpData > ) > { > - UINT32 MaxCpusPerHob; > - UINT32 CpusInHob; > - UINT64 Data64; > - UINT32 Index; > - UINT32 HobBase; > - CPU_INFO_IN_HOB *CpuInfoInHob; > - MP_HAND_OFF *MpHandOff; > - UINTN MpHandOffSize; > + UINT32 MaxCpusPerHob; > + UINT32 CpusInHob; > + UINT64 Data64; > + UINT32 Index; > + UINT32 HobBase; > + CPU_INFO_IN_HOB *CpuInfoInHob; > + MP_HAND_OFF *MpHandOff; > + MP_HAND_OFF_CONFIG MpHandOffConfig; > + UINTN MpHandOffSize; > =20 > MaxCpusPerHob =3D (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof (MP_HA= ND_OFF)) / sizeof (PROCESSOR_HAND_OFF); > =20 > @@ -155,11 +156,6 @@ SaveCpuMpData ( > =20 > MpHandOff->ProcessorIndex =3D HobBase; > MpHandOff->CpuCount =3D CpusInHob; > - > - if (CpuMpData->ApLoopMode !=3D ApInHltLoop) { > - MpHandOff->StartupSignalValue =3D MP_HAND_OFF_SIGNAL; > - MpHandOff->WaitLoopExecutionMode =3D sizeof (VOID *); > - } > } > =20 > MpHandOff->Info[Index-HobBase].ApicId =3D CpuInfoInHob[Index].ApicId= ; > @@ -170,6 +166,18 @@ SaveCpuMpData ( > } > } > =20 > + ZeroMem (&MpHandOffConfig, sizeof (MpHandOffConfig)); > + if (CpuMpData->ApLoopMode !=3D ApInHltLoop) { > + MpHandOffConfig.StartupSignalValue =3D MP_HAND_OFF_SIGNAL; > + MpHandOffConfig.WaitLoopExecutionMode =3D sizeof (VOID *); > + } > + > + BuildGuidDataHob ( > + &mMpHandOffConfigGuid, > + (VOID *)&MpHandOffConfig, > + sizeof (MpHandOffConfig) > + ); > + > // > // Build location of CPU MP DATA buffer in HOB > // -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116077): https://edk2.groups.io/g/devel/message/116077 Mute This Topic: https://groups.io/mt/104600810/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-