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.129.124]) by mx.groups.io with SMTP id smtpd.web11.53907.1673527056879212249 for ; Thu, 12 Jan 2023 04:37:37 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Y/Ks5URq; spf=pass (domain: redhat.com, ip: 170.10.129.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673527056; 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=ZUz7cbYevigCHL9Il5r4jwwznta1TBRxesGFEvDic7o=; b=Y/Ks5URqBfrRb6vVJRGycam7dnFuUOUhOUCnBzqZBdx/lOTLI6IbKfGvH0PYZ35KkuWjkc p0Nc1JaUp25zTHVCUezWPJVjzWmioY/N+xkL+WccErSuDfKzc9QrKYCmoAG0OQP06UlY4q PpCaw1pfAsd6ih6luue/Oqb3+K1bVDc= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-231-9LJTrhA1OUmKRuB035MNow-1; Thu, 12 Jan 2023 07:37:33 -0500 X-MC-Unique: 9LJTrhA1OUmKRuB035MNow-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 8FCD53C0257E; Thu, 12 Jan 2023 12:37:32 +0000 (UTC) Received: from [10.39.192.93] (unknown [10.39.192.93]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 591871759E; Thu, 12 Jan 2023 12:37:31 +0000 (UTC) Message-ID: <5caa15ee-4c89-c2bc-6c0f-77707cfbcf61@redhat.com> Date: Thu, 12 Jan 2023 13:37:30 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v2] UefiCpuPkg: Support SMM Relocated SmBase handling To: devel@edk2.groups.io, jiaxin.wu@intel.com Cc: Eric Dong , Ray Ni , Zeng Star , Gerd Hoffmann References: <20230111083507.8792-1-jiaxin.wu@intel.com> From: "Laszlo Ersek" In-Reply-To: <20230111083507.8792-1-jiaxin.wu@intel.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 1/11/23 09:35, Wu, Jiaxin wrote: > Mainly changes as below: > 1. Add Smm Base HOB, which is used to store the information of > Smm Relocated SmBase array for each Processors; > 2. Combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate) into one > (gcSmiHandlerTemplate), the new SMI handler needs to run to 2 paths: one > to SmmCpuFeaturesInitializeProcessor(), the other to SMM Core Entry Point. > 3. Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for SMM init > before normal SMI sources happen. > 4. Call SmmCpuFeaturesInitializeProcessor() in parallel. > > v2: > - Refine the coding style > - Rename hob to gSmmBaseHobGuid > - Update SmmInitHandler() to handle the SMM relocation > - Correct the S3 for SMM relocation > > v1: > - Thread: https://edk2.groups.io/g/devel/message/97748 > > Change-Id: Iec7bf25166bfeefb44a202285465a35b5debbce4 (1) Please don't include this in upstream patches. > Cc: Eric Dong > Cc: Ray Ni > Cc: Zeng Star > Signed-off-by: Jiaxin Wu (2) This patch is for UefiCpuPkg, but Gerd has not been CC'd, as far as I can tell. CC'ing him now. (Please refer to commit 0aca5901e344, "Maintainers.txt: designate Gerd Hoffmann as UefiCpuPkg reviewer", 2023-01-06). > --- > UefiCpuPkg/Include/Guid/SmmBaseHob.h | 36 +++++ > .../Library/SmmCpuFeaturesLib/CpuFeaturesLib.h | 2 + > .../SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c | 24 +++- > .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 + > .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf | 1 + > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c | 1 - > .../StandaloneMmCpuFeaturesLib.inf | 4 + > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 39 +++++- > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 25 +++- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 149 ++++++++++++++++----- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 21 ++- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + > UefiCpuPkg/UefiCpuPkg.dec | 3 + > 13 files changed, 261 insertions(+), 49 deletions(-) > create mode 100644 UefiCpuPkg/Include/Guid/SmmBaseHob.h (3) The patch extends the interface between PiSmmCpuDxeSmm and multipe SmmCpuFeaturesLib instances. There is no concise and complete design description, either in the commit message, or in a TianoCore bugzilla ticket (no reference in your commit message), or in the new header file "SmmBaseHob.h". (4) The commit modifies multiple modules at once. In a producer-consumer scenario (which is usually characteristic of HOBs), we tend to extend the producer first (if there are multiple producers, then each in separation), and then the consumers. Usually the consumers are expected to keep compatibility with the lack of the HOB, if possible. The compatibility aspects are not explained, and the modifications are squashed together in a single patch. Unacceptable. (5) OvmfPkg includes its own SmmCpuFeaturesLib instance, but there's not a peep about OvmfPkg in the patch (code or commit message), not even an explanation why OvmfPkg is supposed to be unaffected. Unacceptable. Nacked-by: Laszlo Ersek