Re: [Qemu-devel] [PATCH v3 for-2.9 0/3] q35: add negotiable broadcast SMI

[Laszlo Ersek]

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/address@hidden/msg354852.html
>>>
>>> and it was even part of a pull req (Mar 2016):
>>>
>>> https://www.mail-archive.com/address@hidden/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/address@hidden/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
>