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.web12.22695.1599162953528103801 for ; Thu, 03 Sep 2020 12:55:53 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=XQoxiJoG; 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=1599162952; 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=bw46uCPxumw079eDMTqynbLHzmgqE0ztQeTtRqnS7z4=; b=XQoxiJoG7QHcbhDZ+3aXQ9tQ2h+22u3HdF7PH6P9blQw5F1RwoqQQfiiX1D9fiZ8erTZdm wDR/F9ULqChtL/cGN93ltBl0ol6W3GQ8F5Rb53cfBIZilR5iqzaIXDrntoTtHzKyZz6xlY sgJKK5BKEd/ojM0G15jCVYfWWLzeAME= 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-493-OVIKaAowO4-ndTdBmHia2Q-1; Thu, 03 Sep 2020 15:55:51 -0400 X-MC-Unique: OVIKaAowO4-ndTdBmHia2Q-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 F055D1084D66; Thu, 3 Sep 2020 19:55:49 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-168.ams2.redhat.com [10.36.112.168]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2BAAB19C78; Thu, 3 Sep 2020 19:55:48 +0000 (UTC) Subject: Re: [PATCH v2] UefiCpuPkg/MpInitLib: Add check for CR3/GDT/IDT. From: "Laszlo Ersek" To: Eric Dong , devel@edk2.groups.io Cc: Ray Ni References: <20200903151147.1196-1-eric.dong@intel.com> Message-ID: <9c9d8289-4f8e-75d8-2816-750195a54842@redhat.com> Date: Thu, 3 Sep 2020 21:55:47 +0200 MIME-Version: 1.0 In-Reply-To: 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 On 09/03/20 21:00, Laszlo Ersek wrote: > (10) More importantly, ValidCR3GdtIdtCheck() should not be called in the > Worker functions for StartupAllAPs, StartupThisAP, SwitchBSP, and > EnableDisableAP, in "UefiCpuPkg/Library/MpInitLib/MpLib.c". > > Instead, the calls should be made in the DXE instance of the library > ("UefiCpuPkg/Library/MpInitLib/DxeMpLib.c"), at the very top of the > functions: > > - MpInitLibStartupAllAPs > - MpInitLibStartupThisAP > - MpInitLibSwitchBSP > - MpInitLibEnableDisableAP > > Here's why: > > (a) The symptom does not affect the PEI phase -- by the time the UEFI > application is executed, the PEI phase has ended; there's no need to > modify the PEI instance of the library. > > (b) The currently proposed failure exits are too late. For example, in > the SwitchBSPWorker() function, by the time we exit, we have called > DisableApicTimerInterrupt(), SaveAndDisableInterrupts(), and > DisableLvtInterrupts() -- and the error path does not restore the > original environment. > > (c) According to the PI spec (v1.7), the StartupAllAPs(), > StartupThisAP(), SwitchBSP(), EnableDisableAP() member functions of > EFI_MP_SERVICES_PROTOCOL may only be called on the (current) BSP. > Because of this, it is OK to call ValidCR3GdtIdtCheck() as the very > first action in the above-listed DxeMpLib functions. > > (Assuming the protocol members are called from an AP, and consequently > we check CR3 / GDTR / IDTR on the AP (and not on the BSP), that's the > *caller's* fault, per spec!) This means we can move the ValidCr3GdtIdtCheck() function to "DxeMpLib.c", and it is not necessary to modify "MpLib.h". Thanks Laszlo