On Wed, Nov 16, 2016 at 07:47:42AM -0500, Paolo Bonzini wrote:
>
> > If the consensus is that the patch is a QEMU bugfix (as opposed to a
> > feature) and that it is eligible for the currently supported upstream
> > stable branches, that's the best, no doubt.
>
> The currently supported upstream stable branches is just 2.7. :)
>
> I'm okay with bending the rules and including it in 2.8, but it's
> worrisome that you also needed to go back from relaxed to traditional
> delivery, meaning that old QEMU + new OVMF will take ages to boot.
>
> If this is the case, I still think this needs some kind of discovery
> mechanism, unless OVMF can just say "things were too broken, stop
> supporting SMM on QEMUs older than 2.8".
>
> For example:
>
> - OVMF should keep on using 0x00 (no broadcast) if the relaxed AP
> setting is used for the PCD; this would be backwards compatibility mode.
>
> - we could have another magic 0xB2 value, which is implemented directly
> in QEMU and sets 0xB3 to a magic value. Then OVMF can invoke it
> after SMBASE relocation and SMM IPL (so as not to crash on old QEMUs)
> to detect the new feature. It can fail to start if using traditional
> AP and the new feature is not there.
If we keep collecting these magic values, should architect it
and do a host/guest bitmap like virtio does?
> By the way, in case OVMF needs to use SmmSwDispatch in the future, I
> would make QEMU use broadcast behavior for all values in the 0x10-0xff
> range, or something like that.
>
> Paolo
It bothers me with all these ideas is that it's PV.
Unavoidable?
> > For reference, the OVMF documentation recommends QEMU 2.5+ for SMM. The
> > SMM enablement in libvirt enforces QEMU 2.4+. (Libvirt is actually
> > correct; when I was writing the OVMF docs, I must have misunderstood the
> > requirements and needlessly required 2.5+; 2.4+ should have been fine.)
> >
> > Which means the fix should be backported as far as stable-2.4.
> >
> > Should we proceed with that? CC'ing Mike Roth and the stable list.
> >
> > Thanks!
> > Laszlo
> >
> > >
> > >
> > >>>
> > >>> Paolo
> > >>>
> > >>>> ---
> > >>>> hw/isa/lpc_ich9.c | 12 +++++++++++-
> > >>>> 1 file changed, 11 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> > >>>> index 10d1ee8b9310..f2fe644fdaa4 100644
> > >>>> --- a/hw/isa/lpc_ich9.c
> > >>>> +++ b/hw/isa/lpc_ich9.c
> > >>>> @@ -372,6 +372,8 @@ void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool
> > >>>> smm_enabled)
> > >>>>
> > >>>> /* APM */
> > >>>>
> > >>>> +#define QEMU_ICH9_APM_STS_BROADCAST_SMI 'Q'
> > >>>> +
> > >>>> static void ich9_apm_ctrl_changed(uint32_t val, void *arg)
> > >>>> {
> > >>>> ICH9LPCState *lpc = arg;
> > >>>> @@ -386,7 +388,15 @@ static void ich9_apm_ctrl_changed(uint32_t val,
> > >>>> void *arg)
> > >>>>
> > >>>> /* SMI_EN = PMBASE + 30. SMI control and enable register */
> > >>>> if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN) {
> > >>>> - cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
> > >>>> + if (lpc->apm.apms == QEMU_ICH9_APM_STS_BROADCAST_SMI) {
> > >>>> + CPUState *cs;
> > >>>> +
> > >>>> + CPU_FOREACH(cs) {
> > >>>> + cpu_interrupt(cs, CPU_INTERRUPT_SMI);
> > >>>> + }
> > >>>> + } else {
> > >>>> + cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
> > >>>> + }
> > >>>> }
> > >>>> }
> > >>>>
> > >>>>
> >
> >
