From: Laszlo Ersek <lersek@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org,
"Jordan Justen (Intel address)" <jordan.l.justen@intel.com>,
mst@redhat.com
Subject: Re: [Qemu-devel] [PATCH 5/6] [wip] tseg, part1, not (yet) tested
Date: Thu, 23 Apr 2015 10:33:37 +0200 [thread overview]
Message-ID: <5538AE61.8090107@redhat.com> (raw)
In-Reply-To: <5538A22D.7020302@redhat.com>
On 04/23/15 09:41, Laszlo Ersek wrote:
> On 04/23/15 09:02, Gerd Hoffmann wrote:
>> Hi,
>>
>>>> The third one is messy. It relies on SMI_EN, which is an ioport at
>>>> PMBA+0x30. It requires a configured PMBA.
>>
>> Isn't that initialized early anyway, because the pm timer lives there?
>
> TimerLib (which is based in OVMF's case on the PM timer) is not needed /
> used before PEI in the default case. It is used in SEC only when -D
> SOURCE_DEBUG_ENABLE is passed for the build (which is "never" in practice).
>
>> It's not a regular pci bar exactly to allow early init, so the full pci
>> bus scan + bar allocation done later will not mess up things.
>>
>> [ just a question, could very well be that even when initialized early
>> it isn't early enough because you need to know the memory layout
>> before uncompressing the firmware modules ]
>
> I'm trying to avoid that; the decompression happens to a low fixed range.
>
>>> There's another problem with basing this decision in OVMF on
>>> SMI_EN.APMC_EN: it is not an idempotent check. At some point the
>>> firmware itself has to set SMI_EN.APMC_EN.
>>
>> Yep, you'll need some variable saying whenever smm is there or not and
>> check that. Because of this. Or (in case we are going for the fw_cfg
>> file) because you don't want dig into fw_cfg each time you need to know
>> this.
>
> The annoying limitation with SEC is that it cannot use normal C static
> variables, nor PCDs. It is okay if SEC is a bit wasteful on fw_cfg (and
> even in SEC I might be able to cache the result in a local variable and
> pass it around). In PEI I can already use static variables (because OVMF
> is unorthodox and runs PEI modules from RAM, not flash), plus I can set
> PCDs (because in PEI the PCDs live in a dedicated HOB, and that HOB is
> allocated from RAM (independently of OVMF)).
>
> I'll ask Jordan too about the dynamic feature detection, because there's
> another big hurdle with it (selecting a LockBox library instance at
> runtime, dependent on the presence of SMM). Deciding about SMM support
> at build time would make things hugely easier (because that's how
> firmware for physical platforms is built as well).
Okay, looks like Jordan agrees with using a static build option.
This makes things much easier, and I think I won't need a new fw_cfg file.
I will certainly do a sanity check (based on the condition we discussed
eariler in this thread, using the PCI config and iospace registers).
It's going to be "somewhere" in the firmware (still thinking about the
best place), but this way I won't have to handle everything (fall back
to non-SMM behavior) if the sanity check fails; in that case I can just
stop.
Public IRC transcript follows.
Thanks!
Laszlo
* Loaded log from Wed Apr 22 21:45:19 2015
* Now talking on #edk2
* Topic for #edk2 is: EDK II / OVMF / UEFI development discussions |
http://www.tianocore.org/edk2
* Topic for #edk2 set by
jljusten!~jljusten@static-50-43-41-168.bvtn.or.frontiernet.net at Thu
Feb 5 03:36:47 2015
<lersek> jljusten, are you still online?
<lersek> (I guess you are, from your recent email)
<jljusten> lersek: yeah. it's getting a bit late though. :)
<lersek> jljusten, okay, just briefly then
<lersek> I can send an email too if you prefer that
<lersek> the question is dynamic detection of SMM/SMRAM
support
<lersek> it is quite messy
<lersek> and has a large number of ties into things
<lersek> several modules / module types are affected
<lersek> but the worst question is how we'd select a
LockBox library instance dynamically
<lersek> dependent on the presence of the SMM/SMRAM
capability in QEMU
<lersek> one idea I had
<lersek> is quite ugly
<lersek> but could work
<jljusten> ah, yikes
<lersek> (1) introduce a new PPI and a new protocol
<lersek> with some GUID we generate
<lersek> this PPI and protocol would abstract the LockBox
library interface
<lersek> the PPI for PEIMs
<lersek> the protocol for DXE modules
<lersek> (2) we'd build two such drivers
<lersek> for each of the phases
<lersek> one pair would back the PPI / protocol with the
SMM-based (unprivileged) lockbox library instance
<lersek> another pair would back the PPI / protocol with
our current (insecure) lockbox library instance
<lersek> (3) we'd build both pairs of drivers into OVMF
<lersek> and their entry points would check for SMM/SMRAM
<lersek> only one pair would remain active and install the
PPI / protocol
<lersek> (4) we'd introduce another LockBox library
instance
<lersek> that would depend on the PPI / protocol
<lersek> on the depex level
<lersek> (PPI depexes can also be stated for PEIMs)
<jljusten> wow. any chance this could be a build time
option? :)
<lersek> then all modules using this library instance
would be delayed until after the corect driver
<lersek> exactly!
<lersek> so you can see where this is going
<lersek> a huge mess
<lersek> I actually *want* to make this a build time option
<lersek> but I was worried you'd ask me to do it
dynamically
<lersek> basing it on -D SMM_ENABLE, statically
<lersek> would make things hugely easier
<lersek> not just for the LockBox lib instance
<lersek> but a number of other things as well
<lersek> so if you agree to make SMM/SMRAM support a
static build time config option
<lersek> that would be awesome
<jljusten> Whenever I think of SMM, I think, yeah that'd be
cool to have a sample platform in EDK II, but it never
sounds like something we'd want to enable by default.
<jljusten> Of course, that could change over time...
<lersek> right
<lersek> so let's agree on the -D SMM_ENABLE for now
<lersek> thank you very much
<lersek> another question quickly while we're at it
<lersek> how would you prefer checks for this in the C
code?
<lersek> feature PCD?
<lersek> controlled by the build flag?
<lersek> I think that's usually the best method for stuff
like this
<lersek> it's better than conditional inclusion (ie.
#ifdef etc)
<lersek> because all code gets verified by the compiler
<lersek> I'll also add some sanity checks of course
<jljusten> Yeah, I think so.
<lersek> so that a -D SMM_ENABLE build rejects running
early, if qemu doesn't actually provide the feature
<lersek> jljusten, thank you
<jljusten> Any chance to get video running before then?
Probably not critical. If they are using the SMM build, they
should know they need a newer QEMU...
<lersek> jljusten, hm, I think video is initialized quite
late; it depends on a UEFI driver and is connected in BDS --
presence of SMM/SMRAM communicates that the user wants
things to be secure, so it affects even SEC and PEI.
<lersek> If those phases expect SMM/SMRAM and it's not
there, the best would be just to stop right there (after
logging an error message)
<lersek> anyway we can perhaps refine this in one of the
versions of the patchset-to-be
<lersek> the important thing is that I can now start out
with a static config flag, knowing that I won't have to
rewrite *that*. :)
<lersek> thanks!
<jljusten> I think SMM is not initialized until DXE, but
yeah, I somehow doubt video would work in that case.
<lersek> jljusten, indeed, SMM *drivers* only come into
the picture in DXE
<lersek> but until then the SMRAM area (TSEG on Q35/MCH)
needs to be removed from the system memory HOB we install in
PlatformPei
<lersek> so that no memory allocations are served from
there
<lersek> the TSEG area is the last 1, 2, or 8 MB just
below the end of the low RAM (ie. top of RAM under 4GB)
<lersek> we need to set that aside while in PEI
<lersek> even the permanent PEI ram cannot be installed
the way it is now
<lersek> (last 64 MB under top of RAM below 4GB)
<lersek> so this affects a number of things
<lersek> I'll also have to audit all of the special memory
ranges that OVMF uses (see the "comprehensive memory map"
section in the whitepaper)
<lersek> because those ranges are sensitive (SMRAM is open
while SEC and PEI run) and we must not allow the OS to poke
data directly into some of them
<lersek> so for example
<lersek> we can't let SEC just reuse the pre-decompressed
PEIFV on S3 resume
<lersek> if SMM/SMRAM is selected
<lersek> because a malicious OS may have overwritten the
PEIFV
<lersek> and then if it suspends, then the user resumes
<lersek> the firmware will run the OS-injected code as
part of PEI, with the SMRAM open
<lersek> etc
<lersek> it's going to be hugely intrusive
<jljusten> Right. I think most real platforms run code from
flash in that case.
<lersek> exactly
<lersek> so that's one area where our smartness (ie.
running PEI from RAM, and only SEC from flash) is hurting us
wrt. SMM a little bit
<jljusten> You ought to be able to run from the SMRAM region
though.
<lersek> considering the above
<lersek> the first idea I've come up with
<lersek> is to just decompress the firmware volumes again
<lersek> on S3 resume
<lersek> overwriting whatever a potentially malicious
runtime guest OS may have poked into there
<lersek> but
<lersek> of course
<lersek> given the way we compress things, for the sake of
good compression ratio
<lersek> that means we'd decompress not just PEIFV but
DXEFV too
<lersek> and for that we'll need to set aside much more RAM
<lersek> even from a non-malicious OS
<lersek> because decompressing DXEFV too during S3 resume
must not overwrite OS data
<lersek> so this would be an argument for reverting to a
more traditional layout
<lersek> where PEI is uncompressed and runs from flash
<lersek> and DXEFV is separately decompressed
<lersek> in any case,
<lersek> let me just run with this "decompress them both
on S3" initial idea
<lersek> we can discuss things better when there are
patches for illustration
<lersek> it doesn't have to be perfect in v1
<jljusten> We could trust code in SMRAM right?
<lersek> see the original S3 series :)
<lersek> that's one idea as well
<lersek> I did think of it
<lersek> we could place PEI in SMRAM
<lersek> there are at least two problems with it though
<lersek> (1) it would require making SEC too smart
<lersek> configuring the SMRAM control registers and stuff
<lersek> but that's the smaller issue
<lersek> the big issue is the S3 PEIM itself
<lersek> (2) that one has builtin knowledge about SMRAM
<lersek> and it explicitly uses the SMM ACCESS2 PPI
<lersek> to open and close smram
<lersek> obviously
<lersek> if it is running from SMRAM
<lersek> it doesn't help if it closes down SMRAM
underneath itself. :)
<lersek> anyway what we know now should be enough for
prototyping
<lersek> without committing to directions that would be
certainly wrong in the future
<lersek> I'll send out a transcript of this discussion in
the qemu-devel thread
<lersek> I'll CC you too, and then from the message ID
you'll be able to find the entire thread on gmane
next prev parent reply other threads:[~2015-04-23 8:33 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-20 9:19 [Qemu-devel] [PATCH 1/6] [fixup] add ESMRAMC default Gerd Hoffmann
2015-04-20 9:19 ` [Qemu-devel] [PATCH 2/6] add SMRAM+ESMRAMC wmask Gerd Hoffmann
2015-04-20 12:05 ` Michael S. Tsirkin
2015-04-20 9:19 ` [Qemu-devel] [PATCH 3/6] q35: implement SMRAM.D_LCK Gerd Hoffmann
2015-04-20 12:06 ` Michael S. Tsirkin
2015-04-20 9:19 ` [Qemu-devel] [PATCH 4/6] q35: add test for SMRAM.D_LCK Gerd Hoffmann
2015-04-20 12:06 ` Michael S. Tsirkin
2015-04-20 9:19 ` [Qemu-devel] [PATCH 5/6] [wip] tseg, part1, not (yet) tested Gerd Hoffmann
2015-04-20 11:45 ` Paolo Bonzini
2015-04-21 14:18 ` Laszlo Ersek
2015-04-21 15:04 ` Gerd Hoffmann
2015-04-21 15:08 ` Paolo Bonzini
2015-04-21 15:16 ` Gerd Hoffmann
2015-04-21 18:46 ` Laszlo Ersek
2015-04-22 6:07 ` Gerd Hoffmann
2015-04-22 8:09 ` Gerd Hoffmann
2015-04-22 8:52 ` Laszlo Ersek
2015-04-22 9:33 ` Gerd Hoffmann
2015-04-22 21:41 ` Laszlo Ersek
2015-04-22 21:51 ` Laszlo Ersek
2015-04-23 7:02 ` Gerd Hoffmann
2015-04-23 7:41 ` Laszlo Ersek
2015-04-23 8:33 ` Laszlo Ersek [this message]
2015-04-23 8:34 ` Gerd Hoffmann
2015-04-23 8:42 ` Laszlo Ersek
2015-04-23 10:27 ` Paolo Bonzini
2015-04-20 9:19 ` [Qemu-devel] [PATCH 6/6] [wip] tseg, part2, " Gerd Hoffmann
2015-04-21 14:30 ` Laszlo Ersek
2015-04-21 14:38 ` Paolo Bonzini
2015-04-21 15:05 ` Laszlo Ersek
2015-04-21 15:14 ` Gerd Hoffmann
2015-04-21 15:21 ` Paolo Bonzini
2015-04-21 20:31 ` [Qemu-devel] implementing EFI_SMM_CONTROL2_PROTOCOL.Trigger() (was: [PATCH 6/6] [wip] tseg, part2, not (yet) tested) Laszlo Ersek
2015-04-21 20:58 ` [Qemu-devel] implementing EFI_SMM_CONTROL2_PROTOCOL.Trigger() Paolo Bonzini
2015-04-24 11:56 ` [Qemu-devel] [edk2] implementing EFI_SMM_CONTROL2_PROTOCOL.Trigger() (was: [PATCH 6/6] [wip] tseg, part2, not (yet) tested) Yao, Jiewen
2015-04-24 13:00 ` [Qemu-devel] [edk2] implementing EFI_SMM_CONTROL2_PROTOCOL.Trigger() Paolo Bonzini
2015-04-24 13:16 ` Yao, Jiewen
2015-04-24 14:50 ` [Qemu-devel] [edk2] implementing EFI_SMM_CONTROL2_PROTOCOL.Trigger() (was: [PATCH 6/6] [wip] tseg, part2, not (yet) tested) Yao, Jiewen
2015-04-24 16:46 ` [Qemu-devel] [edk2] implementing EFI_SMM_CONTROL2_PROTOCOL.Trigger() Laszlo Ersek
2015-04-21 15:12 ` [Qemu-devel] [PATCH 6/6] [wip] tseg, part2, not (yet) tested Gerd Hoffmann
2015-04-20 12:07 ` [Qemu-devel] [PATCH 1/6] [fixup] add ESMRAMC default Michael S. Tsirkin
2015-04-20 12:27 ` Paolo Bonzini
2015-04-20 13:23 ` Gerd Hoffmann
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-all 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=5538AE61.8090107@redhat.com \
--to=lersek@redhat.com \
--cc=jordan.l.justen@intel.com \
--cc=kraxel@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.