On 11/25/16 13:31, Laszlo Ersek wrote: > On 11/25/16 05:00, Michael S. Tsirkin wrote: >> On Thu, Nov 24, 2016 at 09:37:41AM +0100, Laszlo Ersek wrote: >>> On 11/24/16 05:29, Michael S. Tsirkin wrote: >>>> On Wed, Nov 23, 2016 at 07:38:35PM -0500, Kevin O'Connor wrote: >>>>> As a general comment - it does seem unfortunate that we keep building >>>>> adhoc interfaces to communicate information from firmware to QEMU. We >>>>> have a generic mechanism (fw_cfg) for passing adhoc information from >>>>> QEMU to the firmware, but the inverse seems to always involve magic >>>>> pci registers, magic io space registers, specific init ordering, etc. >>>> >>>> FWIW I posted a proposal >>>> fw-cfg: support writeable blobs >>>> a while ago to try to address that >>>> >>> >>> Yes, here's the discussion (Feb 2016): >>> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg354852.html >>> >>> and it was even part of a pull req (Mar 2016): >>> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg359348.html >>> >>> but it wasn't merged, apparently. >>> >>> If QEMU (re)gains this feature, I can try basing the broadcast SMI >>> negotiation on it. >>> >>> Thanks >>> Laszlo >> >> I dropped since it wasn't used yet. Go ahead and include it >> in your patchset if you like it. >> > > I can do that, but I'm no longer sure Paolo still approves of the > broadcast SMI idea. > > The way I see it, I can work on getting broadcast SMI functional (with > whichever negotiation method we deem suitable), and make the basic > feature set (which ignores VCPU hotplug entirely) reliable, for now. > Then, later, we can look into VCPU hotplug. VCPU hotplug is already > broken in OVMF and whatever we do for broadcast SMI cannot worsen the > user-observable VCPU hotplug status. > > (Note that VCPU hotplug will require a whole bunch of non-platform code > in edk2, such as UefiCpuPkg/Library/MpInitLib, UefiCpuPkg/CpuMpPei, and > UefiCpuPkg/CpuDxe, UefiCpuPkg/PiSmmCpuDxeSmm.) > > Or, we can delay (or even drop) broadcast SMI until we divine a design > that, with a lot of code everywhere, makes (a) the basic SMM feature set > reliable *and* (b) VCPU hotplug functional, at the exact same time. I'm > not saying this is impossible, but you'll need a better guy for that > than I am. I always work in incremental, small steps, especially where > the subject matter is hard for me to grasp. If you see me walking down > the wrong path and yank me back, that's appreciated, but the broadcast > SMI idea doesn't look wrong, considering feedback from Jordan as to what > real hardware does (and the edk2 UefiCpuPkg.dec default settings). > > So, I'm asking for guidance on: > > (1) what interface would be preferred for negotiating SMI: > (1a) APM_STS > (1b) writeable fw_cfg > (1c) another IO port > > (2) whether to prevent, and if so, how exactly, VCPU hotplug, when the > broadcast SMI is negotiated. By "how", I mean "what code to modify in QEMU". > > For (1), my preference is (1a), simply because it's ready. I do see > perspective in (1b) writeable fw_cfg, so if that's preferred, I can > include Michael's patch and rework my patches to utilize it. (As far as > I see, the textual documentation for fw_cfg is not extended by Michael's > patch, so I guess I'd have to do that too.)
If I understand correctly, one argument against the current state of writeable fw_cfg, captured in <https://www.mail-archive.com/qemu-devel@nongnu.org/msg354983.html>, is that callbacks on write are not supported. Apparently, QEMU code that uses the data written by the guest is supposed to just read that data, not to expect a notification about it. I'm unsure how this can work for actual negotiation, where the guest usually does a read/write/read cycle, and expects some kind of change between steps #2 and #3. I don't see how that can be implemented in QEMU without write callbacks (i.e. how QEMU can confirm or reject the negotiation attempt). Thanks Laszlo > (1c) is inferior to (1b) in > my opinion and shouldn't be chosen. > > Re (2), I'm clueless. Not sure we should care about it. Even if it is a > security problem, that problem exists within the guest, and triggering > it (i.e., hot-plugging a VCPU) requires a host admin action. The host > admin can cause a bunch of other security problems for the guest, via > different misconfigurations. So if we care about (2), it should be > something minimal, to catch "inadvertent" VCPU hotplug attempts. And > then please tell me what code to mess with. > > Thanks > Laszlo >
