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

[Laszlo Ersek]

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.) (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