Re: [edk2-devel] [PATCH wave 1 00/10] support QEMU's "SMRAM at default SMBASE" feature

["Yao, Jiewen" <jiewen.yao@intel.com>]

Sounds good.

Maybe you can write that info in the commit message. A simple sentence such as :
No CSM support is needed because legacy BIOS boot don't use SMM.

So other people won't have same question in the future.

With the commit message change, the series reviewed-by: jiewen.yao@intel.com

Thank you
Yao Jiewen


> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Thursday, September 26, 2019 10:52 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Boris Ostrovsky
> <boris.ostrovsky@oracle.com>; Brijesh Singh <brijesh.singh@amd.com>; Igor
> Mammedov <imammedo@redhat.com>; Joao M Martins
> <joao.m.martins@oracle.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Nakajima, Jun <jun.nakajima@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Paolo Bonzini <pbonzini@redhat.com>; Phillip
> Goerl <phillip.goerl@oracle.com>; Chen, Yingwen <yingwen.chen@intel.com>
> Subject: Re: [edk2-devel] [PATCH wave 1 00/10] support QEMU's "SMRAM at
> default SMBASE" feature
> 
> On 09/26/19 10:46, Yao, Jiewen wrote:
> > Hi Laszlo
> > May I know if you want to support legacy BIOS?
> 
> No, thanks.
> 
> The end-goal is to implement VCPU hotplug at OS runtime, when the OS is
> booted with OVMF, Secure Boot is enabled, and the firmware is protected
> with SMM.
> 
> In other words, the goal is to support VCPU hotplug when there is a
> privilege barrier between the firmware and the OS. (The whole complexity
> of the feature arises because a hotplugged VCPU is a risk for that barrier.)
> 
> With legacy BIOS, there is no such barrier, and VCPU hotplug already
> works with SeaBIOS.
> 
> Furthermore, if you build OVMF *without* "-D SMM_REQUIRE", then there is
> no such barrier again, and VCPU hotplug already works well, in that case
> too. (I had tested it extensively, in RHBZ#1454803.)
> 
> In effect, I consider "-D SMM_REQUIRE" and "-D CSM_ENABLE" mutually
> exclusive, for OVMF. And the present work is only relevant for "-D
> SMM_REQUIRE".
> 
> We could perhaps add an "!error" directive to the OVMF DSC files,
> conditional on both flags being enabled (SMM_REQUIRE and CSM_ENABLE). I
> might propose such a patch in the future, but until it becomes a
> practical problem, I don't want to spend cycles on it.
> 
> Thanks
> Laszlo
> 
> > The legacy BIOS memory map is reported in E820, which is different with UEFI
> memory map.
> >
> > In OvmfPkg\Csm\LegacyBiosDxe\LegacyBootSupport.c, LegacyBiosBuildE820()
> will hardcode below 640K address to be OS usage without consulting UEFI
> memory map.
> >
> >   //
> >   // First entry is 0 to (640k - EBDA)
> >   //
> >   ACCESS_PAGE0_CODE (
> >     E820Table[0].BaseAddr  = 0;
> >     E820Table[0].Length    = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
> >     E820Table[0].Type      = EfiAcpiAddressRangeMemory;
> >   );
> >
> > If we want to support this feature in legacy BIOS, we also need reserve the
> black hole here.
> >
> > Thank you
> > Yao Jiewen
> >
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> >> Sent: Tuesday, September 24, 2019 7:35 PM
> >> To: edk2-devel-groups-io <devel@edk2.groups.io>
> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Boris Ostrovsky
> >> <boris.ostrovsky@oracle.com>; Brijesh Singh <brijesh.singh@amd.com>; Igor
> >> Mammedov <imammedo@redhat.com>; Yao, Jiewen
> <jiewen.yao@intel.com>;
> >> Joao M Martins <joao.m.martins@oracle.com>; Justen, Jordan L
> >> <jordan.l.justen@intel.com>; Nakajima, Jun <jun.nakajima@intel.com>;
> Kinney,
> >> Michael D <michael.d.kinney@intel.com>; Paolo Bonzini
> >> <pbonzini@redhat.com>; Phillip Goerl <phillip.goerl@oracle.com>; Chen,
> >> Yingwen <yingwen.chen@intel.com>
> >> Subject: [edk2-devel] [PATCH wave 1 00/10] support QEMU's "SMRAM at
> >> default SMBASE" feature
> >>
> >> Ref:    https://bugzilla.tianocore.org/show_bug.cgi?id=1512
> >> Repo:   https://github.com/lersek/edk2.git
> >> Branch: smram_at_default_smbase_bz_1512_wave_1
> >>
> >> This is the firmware-side patch series for the QEMU feature that Igor is
> >> proposing in
> >>
> >>   [Qemu-devel] [PATCH 0/2]
> >>   q35: mch: allow to lock down 128K RAM at default SMBASE address
> >>
> >> posted at
> >>
> >>   http://mid.mail-archive.com/20190917130708.10281-1-
> >> imammedo@redhat.com
> >>   https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg03538.html
> >>   https://edk2.groups.io/g/devel/message/47364
> >>
> >> This OVMF patch series is supposed to be the first "wave" of patch sets,
> >> working towards VCPU hotplug with SMM enabled.
> >>
> >> We want the hot-plugged VCPU to execute its very first instruction from
> >> SMRAM, running in SMM, for protection from OS and PCI DMA interference.
> >> In the (long!) discussion at
> >>
> >>   http://mid.mail-archive.com/20190905174503.2acaa46a@redhat.com
> >>   https://edk2.groups.io/g/devel/message/46910
> >>   https://bugzilla.tianocore.org/show_bug.cgi?id=1512#c9
> >>
> >> we considered diverging from the specs and changing the default SMBASE
> >> to an address different from 0x3_0000, so that it would point into
> >> SMRAM. However, Igor had the idea to keep the default SMBASE intact, and
> >> replace the underlying system RAM (128 KB of it) with lockable SMRAM.
> >> This conforms to the language of the Intel SDM, namely, "[t]he actual
> >> physical location of the SMRAM can be in system memory or in a separate
> >> RAM memory". When firmware locks this QEMU-specific SMRAM, the latter
> >> completely disappears from the address space of code that runs outside
> >> of SMM (including PCI devices doing DMA). We call the remaining MMIO
> >> area a "black hole".
> >>
> >> The present patch set detects the QEMU feature, locks the SMRAM at the
> >> right times, and informs firmware and OS components to stay away form
> >> the SMRAM at the default SMBASE.
> >>
> >> I've tested the set extensively:
> >>
> >>   http://mid.mail-archive.com/c18f1ada-3eca-d5f1-da4f-
> >> e74181c5a862@redhat.com
> >>   https://edk2.groups.io/g/devel/message/47864
> >>
> >> Going forward, if I understand correctly, the plan is to populate the
> >> new SMRAM with the hotplug SMI handler. (This would likely be put in
> >> place by OvmfPkg/PlatformPei, from NASM source code. The
> >> SmmRelocateBases() function in UefiCpuPkg/PiSmmCpuDxeSmm backs up
> the
> >> original contents before, and restores them after, the initial SMBASE
> >> relocation of the cold-plugged VCPUs. Thus the hotplug SMI handler would
> >> survive the initial relocation of the cold-plugged VCPUs.)
> >>
> >> Further, when a VCPU is hot-plugged, we seem to intend to raise an SMI
> >> for it (perhaps internally to QEMU?), which will remain pending until
> >> the VCPU is launched with INIT-SIPI-SIPI. Once the INIT-SIPI-SIPI is
> >> sent, the VCPU will immediately enter SMM, and run the SMI handler in
> >> the locked SMRAM at the default SMBASE. At RSM, it may continue at the
> >> vector requested in the INIT-SIPI-SIPI.
> >>
> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> >> Cc: Brijesh Singh <brijesh.singh@amd.com>
> >> Cc: Igor Mammedov <imammedo@redhat.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Joao M Martins <joao.m.martins@oracle.com>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >> Cc: Jun Nakajima <jun.nakajima@intel.com>
> >> Cc: Michael Kinney <michael.d.kinney@intel.com>
> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >> Cc: Phillip Goerl <phillip.goerl@oracle.com>
> >> Cc: Yingwen Chen <yingwen.chen@intel.com>
> >>
> >> Thanks
> >> Laszlo
> >>
> >> Laszlo Ersek (10):
> >>   OvmfPkg: introduce PcdQ35SmramAtDefaultSmbase
> >>   OvmfPkg/IndustryStandard: increase vertical whitespace in Q35 macro
> >>     defs
> >>   OvmfPkg/IndustryStandard: add MCH_DEFAULT_SMBASE* register macros
> >>   OvmfPkg/PlatformPei: factor out Q35BoardVerification()
> >>   OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (skeleton)
> >>   OvmfPkg/PlatformPei: assert there's no permanent PEI RAM at default
> >>     SMBASE
> >>   OvmfPkg/PlatformPei: reserve the SMRAM at the default SMBASE, if it
> >>     exists
> >>   OvmfPkg/SEV: don't manage the lifecycle of the SMRAM at the default
> >>     SMBASE
> >>   OvmfPkg/SmmAccess: close and lock SMRAM at default SMBASE
> >>   OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (for real)
> >>
> >>  OvmfPkg/Include/IndustryStandard/Q35MchIch9.h           | 106
> +++++++++++----
> >> -----
> >>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c   |  21 +++-
> >>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf |   4 +
> >>  OvmfPkg/OvmfPkg.dec                                     |   6 ++
> >>  OvmfPkg/OvmfPkgIa32.dsc                                 |   1 +
> >>  OvmfPkg/OvmfPkgIa32X64.dsc                              |   1 +
> >>  OvmfPkg/OvmfPkgX64.dsc                                  |   1 +
> >>  OvmfPkg/PlatformPei/AmdSev.c                            |  24 ++++-
> >>  OvmfPkg/PlatformPei/MemDetect.c                         |  87 +++++++++++++---
> >>  OvmfPkg/PlatformPei/Platform.c                          |  24 +++++
> >>  OvmfPkg/PlatformPei/Platform.h                          |   7 ++
> >>  OvmfPkg/PlatformPei/PlatformPei.inf                     |   1 +
> >>  OvmfPkg/SmmAccess/SmmAccess2Dxe.c                       |   7 ++
> >>  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf                     |   1 +
> >>  OvmfPkg/SmmAccess/SmmAccessPei.c                        |   6 ++
> >>  OvmfPkg/SmmAccess/SmmAccessPei.inf                      |   1 +
> >>  OvmfPkg/SmmAccess/SmramInternal.c                       |  25 +++++
> >>  OvmfPkg/SmmAccess/SmramInternal.h                       |   8 ++
> >>  18 files changed, 260 insertions(+), 71 deletions(-)
> >>
> >> --
> >> 2.19.1.3.g30247aa5d201
> >>
> >>
> >> -=-=-=-=-=-=
> >> Groups.io Links: You receive all messages sent to this group.
> >>
> >> View/Reply Online (#47924): https://edk2.groups.io/g/devel/message/47924
> >> Mute This Topic: https://groups.io/mt/34274934/1772286
> >> Group Owner: devel+owner@edk2.groups.io
> >> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [jiewen.yao@intel.com]
> >> -=-=-=-=-=-=
> >