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

[Igor Mammedov]

On Fri, 25 Nov 2016 13:31:17 +0100
Laszlo Ersek <address@hidden> 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.) (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.
I'd ignore (2) for now as VCPU hotplug is broken now anyway.

The only difference between broadcast and unicast SMIs, I see here is
that hotplugged VCPU will attempt to enter SMM handler at default
SMBASE when it gets SIPI while with unicast SMIs there won't be
pending SMI on hotplugged CPUs.
So in broadcast case SMBASE should be valid and do SMBASE relocation
on the first entry.


> 
> Thanks
> Laszlo