From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124])
 by mx.groups.io with SMTP id smtpd.web10.38042.1676971362982205271
 for <devel@edk2.groups.io>;
 Tue, 21 Feb 2023 01:22:43 -0800
Authentication-Results: mx.groups.io;
 dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Z2I2gmF4;
 spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
	s=mimecast20190719; t=1676971362;
	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:
	 in-reply-to:in-reply-to:references:references;
	bh=ItPyYNMkDbSbiVZEUZBkcsKQqDSrZKPLuw47QDlwkbY=;
	b=Z2I2gmF4p/INl8nxGe3lsbw6vQ4YE5baXB3eQzjE/v2/KE71aNewBLSbcT3fZcKdcuNr3v
	gUs8K+WKoVQvJBXqePvNUg1HM6zK0Oh2IH61M7w9Y7Kl1v/Fre/bl5DJmmfqrlIgSIRbzk
	YDTXqYimWvWdZECpVYeBy/aUnHvUO6c=
Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com
 [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS
 (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 us-mta-664-7wBNNL0SMgOYasYCfsBoMg-1; Tue, 21 Feb 2023 04:22:38 -0500
X-MC-Unique: 7wBNNL0SMgOYasYCfsBoMg-1
Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5])
	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4BAD7885622;
	Tue, 21 Feb 2023 09:22:38 +0000 (UTC)
Received: from sirius.home.kraxel.org (unknown [10.39.192.67])
	by smtp.corp.redhat.com (Postfix) with ESMTPS id 29C28440DC;
	Tue, 21 Feb 2023 09:22:37 +0000 (UTC)
Received: by sirius.home.kraxel.org (Postfix, from userid 1000)
	id 8163A1800925; Tue, 21 Feb 2023 10:22:35 +0100 (CET)
Date: Tue, 21 Feb 2023 10:22:35 +0100
From: "Gerd Hoffmann" <kraxel@redhat.com>
To: Yuanhao Xie <yuanhao.xie@intel.com>
Cc: devel@edk2.groups.io, Guo Dong <guo.dong@intel.com>,
	Ray Ni <ray.ni@intel.com>, Sean Rhodes <sean@starlabs.systems>,
	James Lu <james.lu@intel.com>, Gua Guo <gua.guo@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Laszlo Ersek <lersek@redhat.com>
Subject: Re: [Patch V2 1/5] UefiCpuPkg: Duplicate RelocateApLoop for the processors with SEV-ES.
Message-ID: <20230221092235.rzwbaseaibjh76wr@sirius.home.kraxel.org>
References: <20230220052022.9290-1-yuanhao.xie@intel.com>
 <20230220052022.9290-2-yuanhao.xie@intel.com>
MIME-Version: 1.0
In-Reply-To: <20230220052022.9290-2-yuanhao.xie@intel.com>
X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

  Hi,

>    if (CpuMpData->UseSevEsAPMethod) {
> -    StackStart = CpuMpData->SevEsAPResetStackStart;
> +    StackStart                  = CpuMpData->SevEsAPResetStackStart;
> +    AsmRelocateApLoopFuncAmdSev = (ASM_RELOCATE_AP_LOOP)(UINTN)mReservedApLoopFunc;
> +    AsmRelocateApLoopFuncAmdSev (
> +      MwaitSupport,
> +      CpuMpData->ApTargetCState,
> +      CpuMpData->PmCodeSegment,
> +      StackStart - ProcessorNumber * AP_SAFE_STACK_SIZE,
> +      (UINTN)&mNumberToFinish,
> +      CpuMpData->Pm16CodeSegment,
> +      CpuMpData->SevEsAPBuffer,
> +      CpuMpData->WakeupBuffer
> +      );

Good.  Thanks for updating it.

> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> index 7c2469f9c5..6b48913306 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> @@ -346,3 +346,172 @@ PM16Mode:
>      iret
>  
>  SwitchToRealProcEnd:
> +;-------------------------------------------------------------------------------------
> +;  AsmRelocateApLoopAmdSev (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer);
> +;-------------------------------------------------------------------------------------
> +
> +AsmRelocateApLoopStartAmdSev:
> +BITS 64

Hmm, so here you are adding a renamed copy of the AP loop.
Then, in patch #5, you are rewriting the code at the old location.

I'd suggest to move the code instead of copying it, i.e. have one patch
moving (and renaming) the AmdSev AP loop.  Have another patch adding the
new code for the generic AP loop.  That should result in patches which
are easier to read, especially the new generic AP loop code is not mixed
with AmdSev code removal.

take care,
  Gerd