From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43168) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YlCZx-00069x-7k for qemu-devel@nongnu.org; Thu, 23 Apr 2015 04:33:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YlCZr-0004ia-SI for qemu-devel@nongnu.org; Thu, 23 Apr 2015 04:33:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51498) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YlCZr-0004iI-HT for qemu-devel@nongnu.org; Thu, 23 Apr 2015 04:33:43 -0400 Message-ID: <5538AE61.8090107@redhat.com> Date: Thu, 23 Apr 2015 10:33:37 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1429521560-2743-1-git-send-email-kraxel@redhat.com> <1429521560-2743-5-git-send-email-kraxel@redhat.com> <55365C33.2090101@redhat.com> <1429628650.21164.24.camel@nilsson.home.kraxel.org> <55381571.4000201@redhat.com> <553817EC.3080608@redhat.com> <1429772550.26641.8.camel@nilsson.home.kraxel.org> <5538A22D.7020302@redhat.com> In-Reply-To: <5538A22D.7020302@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/6] [wip] tseg, part1, not (yet) tested List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, "Jordan Justen (Intel address)" , mst@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 jljusten, are you still online? (I guess you are, from your recent email) lersek: yeah. it's getting a bit late though. :) jljusten, okay, just briefly then I can send an email too if you prefer that the question is dynamic detection of SMM/SMRAM support it is quite messy and has a large number of ties into things several modules / module types are affected but the worst question is how we'd select a LockBox library instance dynamically dependent on the presence of the SMM/SMRAM capability in QEMU one idea I had is quite ugly but could work ah, yikes (1) introduce a new PPI and a new protocol with some GUID we generate this PPI and protocol would abstract the LockBox library interface the PPI for PEIMs the protocol for DXE modules (2) we'd build two such drivers for each of the phases one pair would back the PPI / protocol with the SMM-based (unprivileged) lockbox library instance another pair would back the PPI / protocol with our current (insecure) lockbox library instance (3) we'd build both pairs of drivers into OVMF and their entry points would check for SMM/SMRAM only one pair would remain active and install the PPI / protocol (4) we'd introduce another LockBox library instance that would depend on the PPI / protocol on the depex level (PPI depexes can also be stated for PEIMs) wow. any chance this could be a build time option? :) then all modules using this library instance would be delayed until after the corect driver exactly! so you can see where this is going a huge mess I actually *want* to make this a build time option but I was worried you'd ask me to do it dynamically basing it on -D SMM_ENABLE, statically would make things hugely easier not just for the LockBox lib instance but a number of other things as well so if you agree to make SMM/SMRAM support a static build time config option that would be awesome 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. Of course, that could change over time... right so let's agree on the -D SMM_ENABLE for now thank you very much another question quickly while we're at it how would you prefer checks for this in the C code? feature PCD? controlled by the build flag? I think that's usually the best method for stuff like this it's better than conditional inclusion (ie. #ifdef etc) because all code gets verified by the compiler I'll also add some sanity checks of course Yeah, I think so. so that a -D SMM_ENABLE build rejects running early, if qemu doesn't actually provide the feature jljusten, thank you 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... 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. 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) anyway we can perhaps refine this in one of the versions of the patchset-to-be the important thing is that I can now start out with a static config flag, knowing that I won't have to rewrite *that*. :) thanks! I think SMM is not initialized until DXE, but yeah, I somehow doubt video would work in that case. jljusten, indeed, SMM *drivers* only come into the picture in DXE but until then the SMRAM area (TSEG on Q35/MCH) needs to be removed from the system memory HOB we install in PlatformPei so that no memory allocations are served from there 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) we need to set that aside while in PEI even the permanent PEI ram cannot be installed the way it is now (last 64 MB under top of RAM below 4GB) so this affects a number of things I'll also have to audit all of the special memory ranges that OVMF uses (see the "comprehensive memory map" section in the whitepaper) 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 so for example we can't let SEC just reuse the pre-decompressed PEIFV on S3 resume if SMM/SMRAM is selected because a malicious OS may have overwritten the PEIFV and then if it suspends, then the user resumes the firmware will run the OS-injected code as part of PEI, with the SMRAM open etc it's going to be hugely intrusive Right. I think most real platforms run code from flash in that case. exactly 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 You ought to be able to run from the SMRAM region though. considering the above the first idea I've come up with is to just decompress the firmware volumes again on S3 resume overwriting whatever a potentially malicious runtime guest OS may have poked into there but of course given the way we compress things, for the sake of good compression ratio that means we'd decompress not just PEIFV but DXEFV too and for that we'll need to set aside much more RAM even from a non-malicious OS because decompressing DXEFV too during S3 resume must not overwrite OS data so this would be an argument for reverting to a more traditional layout where PEI is uncompressed and runs from flash and DXEFV is separately decompressed in any case, let me just run with this "decompress them both on S3" initial idea we can discuss things better when there are patches for illustration it doesn't have to be perfect in v1 We could trust code in SMRAM right? see the original S3 series :) that's one idea as well I did think of it we could place PEI in SMRAM there are at least two problems with it though (1) it would require making SEC too smart configuring the SMRAM control registers and stuff but that's the smaller issue the big issue is the S3 PEIM itself (2) that one has builtin knowledge about SMRAM and it explicitly uses the SMM ACCESS2 PPI to open and close smram obviously if it is running from SMRAM it doesn't help if it closes down SMRAM underneath itself. :) anyway what we know now should be enough for prototyping without committing to directions that would be certainly wrong in the future I'll send out a transcript of this discussion in the qemu-devel thread I'll CC you too, and then from the message ID you'll be able to find the entire thread on gmane