From: "Igor Mammedov" <imammedo@redhat.com>
To: "Laszlo Ersek" <lersek@redhat.com>
Cc: devel@edk2.groups.io, "Yao, Jiewen" <jiewen.yao@intel.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
"Chen, Yingwen" <yingwen.chen@intel.com>,
"phillip.goerl@oracle.com" <phillip.goerl@oracle.com>,
"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
"Nakajima, Jun" <jun.nakajima@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"boris.ostrovsky@oracle.com" <boris.ostrovsky@oracle.com>,
"rfc@edk2.groups.io" <rfc@edk2.groups.io>,
"joao.m.martins@oracle.com" <joao.m.martins@oracle.com>,
Brijesh Singh <brijesh.singh@amd.com>
Subject: Re: [edk2-devel] [Qemu-devel] [PATCH 1/2] q35: implement 128K SMRAM at default SMBASE address
Date: Fri, 4 Oct 2019 13:31:23 +0200 [thread overview]
Message-ID: <20191004133052.20373155@redhat.com> (raw)
In-Reply-To: <8f179aae-8ff4-2b2b-4024-1fd1156a3225@redhat.com>
On Tue, 1 Oct 2019 20:03:20 +0200
"Laszlo Ersek" <lersek@redhat.com> wrote:
> On 09/30/19 16:22, Yao, Jiewen wrote:
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Igor
> >> Mammedov
> >> Sent: Monday, September 30, 2019 8:37 PM
> >> To: Laszlo Ersek <lersek@redhat.com>
>
> >>> To me it looks like we need to figure out how QEMU can make the OS call
> >>> into SMM (in the GPE cpu hotplug handler), passing in parameters and
> >>> such. This would be step (03).
> >>>
> >>> Do you agree?
> >>>
> >>> If so, I'll ask Jiewen about such OS->SMM calls separately, because I
> >>> seem to remember that there used to be an "SMM communcation table" of
> >>> sorts, for flexible OS->SMM calls. However, it appears to be deprecated
> >>> lately.
> >> we can try to resurrect and put over it some kind of protocol
> >> to describe which CPUs to where hotplugged.
> >>
> >> or we could put a parameter into SMI status register (IO port 0xb3)
> >> and the trigger SMI from GPE handler to tell SMI handler that cpu
> >> hotplug happened and then use QEMU's cpu hotplug interface
> >> to enumerate hotplugged CPUs for SMI handler.
> >>
> >> The later is probably simpler as we won't need to reinvent the wheel
> >> (just reuse the interface that's already in use by GPE handler).
>
> Based on "docs/specs/acpi_cpu_hotplug.txt", this seems to boil down to a
> bunch of IO port accesses at base 0x0cd8.
>
> Is that correct?
yep, you can use it to iterate over hotplugged CPUs.
hw side (QEMU) uses cpu_hotplug_ops as IO write/read handlers
and firmware side (ACPI) scannig for hotplugged CPUs is implemented
in CPU_SCAN_METHOD.
What we can do on QEMU side is to write agreed upon value to command port (0xB2)
from CPU_SCAN_METHOD after taking ctrl_lock but before starting scan loop.
That way firmware will first bring up (from fw pov) all hotplugged CPUs
and then return control to OS to do the same from OS pov.
>
> > [Jiewen] The PI specification Volume 4 - SMM defines EFI_MM_COMMUNICATION_PROTOCOL.Communicate() - It can be used to communicate between OS and SMM handler. But it requires the runtime protocol call. I am not sure how OS loader passes this information to OS kernel.
> >
> > As such, I think using ACPI SCI/GPE -> software SMI handler is an easier way to achieve this. I also recommend this way.
> > For parameter passing, we can use 1) Port B2 (1 byte), 2) Port B3 (1 byte), 3) chipset scratch register (4 bytes or 8 bytes, based upon scratch register size), 4) ACPI NVS OPREGION, if the data structure is complicated.
>
> I'm confused about the details. In two categories:
> (1) what values to use,
> (2) how those values are passed.
>
> Assume a CPU is hotpluged, QEMU injects an SCI, and the ACPI GPE handler
> in the OS -- which also originates from QEMU -- writes a particular byte
> to the Data port (0xB3), and then to the Control port (0xB2),
> broadcasting an SMI.
>
> (1) What values to use.
>
> Note that values ICH9_APM_ACPI_ENABLE (2) and ICH9_APM_ACPI_DISABLE (3)
> are already reserved in QEMU for IO port 0xB2, for different purposes.
> So we can't use those in the GPE handler.
SeaBIOS writes 0x00 into command port, but it seems that's taken by
EFI_SMM_COMMUNICATION_PROTOCOL. So we can use the next unused value
(lets say 0x4). We probably don't have to use status port or
EFI_SMM_COMMUNICATION_PROTOCOL, since the value of written into 0xB2
is sufficient to distinguish hotplug event.
> Furthermote, OVMF's EFI_SMM_CONTROL2_PROTOCOL.Trigger() implementation
> (in "OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c") writes 0 to both ports,
> as long as the caller does not specify them.
>
> IoWrite8 (ICH9_APM_STS, DataPort == NULL ? 0 : *DataPort);
> IoWrite8 (ICH9_APM_CNT, CommandPort == NULL ? 0 : *CommandPort);
>
> And, there is only one Trigger() call site in edk2: namely in
> "MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c", in the
> SmmCommunicationCommunicate() function.
>
> (That function implements EFI_SMM_COMMUNICATION_PROTOCOL.Communicate().)
> This call site passes NULL for both DataPort and CommandPort.
>
> Yet further, EFI_SMM_COMMUNICATION_PROTOCOL.Communicate() is used for
> example by the UEFI variable driver, for talking between the
> unprivileged (runtime DXE) and privileged (SMM) half.
>
> As a result, all of the software SMIs currently in use in OVMF, related
> to actual firmware services, write 0 to both ports.
>
> I guess we can choose new values, as long as we avoid 2 and 3 for the
> control port (0xB2), because those are reserved in QEMU -- see
> ich9_apm_ctrl_changed() in "hw/isa/lpc_ich9.c".
>
>
> (2) How the parameters are passed.
>
>
> (2a) For the new CPU, the SMI remains pending, until it gets an
> INIT-SIPI-SIPI from one of the previously plugged CPUs (most likely, the
> BSP). At that point, the new CPU will execute the "initial SMI handler
> for hotplugged CPUs", at the default SMBASE.
>
> That's a routine we'll have to write in assembly, from zero. In this
> routine, we can read back IO ports 0xB2 and 0xB3. And QEMU will be happy
> to provide the values last written (see apm_ioport_readb() in
> "hw/isa/apm.c"). So we can receive the values in this routine. Alright.
Potentially we can can avoid writing custom SMI handler,
what do you think about following workflow:
on system boot after initial CPUs relocation, firmware set NOP SMI handler
at default SMBASE.
Then as reaction to GPE triggered SMI (on cpu hotplug), after SMI rendezvous,
a host cpu reads IO port 0xB2 and does hotplugged CPUs enumeration.
a) assuming we allow hotplug only in case of negotiated SMI broadcast
host CPU shoots down all in-flight INIT/SIPI/SIPI for hotpugged CPUs
to avoid race within relocation handler.
After that host CPU in loop
b) it prepares/initializes necessary CPU structures for a hotplugged
CPU if necessary and replaces NOP SMI handler with the relocation
SMI handler that is used during system boot.
c) a host CPU sends NOP INIT/SIPI/SIPI to the hotplugged CPU
d) the woken up hotplugged CPU, jumps to default SMBASE and
executes hotplug relocation handler.
e) after the hotplugged CPU is relocated and if there are more
hotplugged CPUs, a host CPU repeats b-d steps for the next
hotplugged CPU.
f) after all CPUs are relocated, restore NOP SMI handler at default
SMBASE.
> (2b) On all other CPUs, the SMM foundation already accepts the SMI.
>
> There point where it makes sense to start looking is SmmEntryPoint()
> [MdeModulePkg/Core/PiSmmCore/PiSmmCore.c].
>
> (2b1) This function first checks whether the SMI is synchronous. The
> logic for determining that is based on
> "gSmmCorePrivate->CommunicationBuffer" being non-NULL. This field is set
> to non-NULL in SmmCommunicationCommunicate() -- see above, in (1).
>
> In other words, the SMI is deemed synchronous if it was initiated with
> EFI_SMM_COMMUNICATION_PROTOCOL.Communicate(). In that case, the
> HandlerType GUID is extracted from the communication buffer, and passed
> to SmiManage(). In turn, SmiManage() locates the SMI handler registered
> with the same handler GUID, and delegates the SMI handling to that
> specific handler.
>
> This is how the UEFI variable driver is split in two halves:
>
> - in "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c", we have
> a call to gMmst->MmiHandlerRegister(), with HandlerType =
> "gEfiSmmVariableProtocolGuid"
>
> - in
> "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c", we
> format communication buffers with the header GUID set to the same
> "gEfiSmmVariableProtocolGuid".
>
> Of course, this is what does *not* apply to our use case, as the SMI is
> raised by the OS (via an ACPI method), and *not* by a firmware agent
> that calls EFI_SMM_COMMUNICATION_PROTOCOL.Communicate().
>
> Therefore, we need to look further in SmmEntryPoint()
> [MdeModulePkg/Core/PiSmmCore/PiSmmCore.c].
>
> (2b2) What's left there is only the following:
>
> //
> // Process Asynchronous SMI sources
> //
> SmiManage (NULL, NULL, NULL, NULL);
>
>
> So...
>
> - Are we supposed to write a new DXE_SMM_DRIVER for OvmfPkg, and call
> gMmst->MmiHandlerRegister() in it, with HandlerType=NULL? (I.e.,
> register a "root SMI handler"?)
>
> - And then in the handler, should we read IO ports 0xB2 / 0xB3?
>
> - Also, is that handler where we'd somehow sync up with the hot-plugged
> VCPU, and finally call EFI_SMM_CPU_SERVICE_PROTOCOL.SmmAddProcessor()?
>
> - Does it matter what (pre-existent) CPU executes the handler? (IOW,
> does it matter what the value of gMmst->CurrentlyExecutingCpu is?)
>
> Thanks,
> Laszlo
>
>
>
next prev parent reply other threads:[~2019-10-04 11:31 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-17 13:07 [PATCH 0/2] q35: mch: allow to lock down 128K RAM at default SMBASE address Igor Mammedov
2019-09-17 13:07 ` [PATCH 1/2] q35: implement 128K SMRAM " Igor Mammedov
2019-09-19 17:02 ` [Qemu-devel] " Laszlo Ersek
2019-09-20 8:28 ` [edk2-devel] " Igor Mammedov
2019-09-20 9:28 ` Laszlo Ersek
2019-09-23 18:35 ` Laszlo Ersek
2019-09-24 11:19 ` Igor Mammedov
2019-09-30 11:51 ` Laszlo Ersek
2019-09-30 12:36 ` Igor Mammedov
2019-09-30 14:22 ` Yao, Jiewen
2019-10-01 18:03 ` Laszlo Ersek
2019-10-04 11:31 ` Igor Mammedov [this message]
2019-10-07 9:44 ` Laszlo Ersek
2019-09-24 11:47 ` Paolo Bonzini
2019-09-17 13:07 ` [PATCH 2/2] tests: q35: MCH: add default SMBASE SMRAM lock test Igor Mammedov
2019-09-17 15:23 ` [edk2-devel] [PATCH 0/2] q35: mch: allow to lock down 128K RAM at default SMBASE address no-reply
2019-09-17 15:24 ` no-reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191004133052.20373155@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox