On 18/11/2016 11:36, Laszlo Ersek wrote:
> This is v3 of the series, with updates based on the v2 discussion:
> <http://lists.nongnu.org/archive/html/qemu-devel/2016-11/msg02687.html>.
>
> I've added feature negotiation via the APM_STS ("scratchpad") register.
> A new spec file called "docs/specs/q35-apm-sts.txt" is included.
>
> Tested with new OVMF patches (about to send out those as well).
> Regression tested with SeaBIOS (beyond simple functional tests with
> maximum SeaBIOS logging enabled, I used gdb to step through the new
> ich9_apm_status_changed() callback to see if it was behaving compatibly
> with SeaBIOS).
>
> The series was developed and tested on top of v2.7.0, because v2.8.0-rc0
> crashes very quickly for me when running OVMF:
>
> kvm_io_ioeventfd_add: error adding ioeventfd: File exists
>
> It is my understanding that there are patches on the list for this:
>
> [Qemu-devel] [PATCH v2 for-2.8 0/3] virtio fixes
>
> Anyway, the series rebases to v2.8.0-rc0 without as much as context
> differences.
Hi Laszlo,
sorry for the slightly delayed reply.
First of all, I'm wondering if we would be better off adding a new port
0xB1 that is QEMU-specific, instead of reusing 0xB3.
Second, I now remembered the reason why I was against broadcast SMI.
The reason is that it breaks hot-plug.
On hot-plug, the firmware (if it wants to use SMI for anything secure)
must relocate the SMBASE of the newly-hotplugged CPU before the OS has
any chance to put its fangs on it. This is because the OS can send
direct SMIs and is in control of the area at 0x30000. So OVMF is
already broken with respect to hotplug, but I am not yet sure if these
patches would break it further.
The full solution is to follow a protocol similar to what real hardware
does. On hot-plug, before the new CPU starts running, the DSDT should
generate an SMI (with a well-known value written to 0xB2). The handler
then:
1) waits for SMM rendezvous
2) unparks the hotplugged VCPU. Until the VCPU is unparked, it doesn't
react to either INITs or of course SIPIs (this would need to be
implemented separately for both TCG and KVM! but only in QEMU, not in
the kernel).
3) relocates SMBASE
4) records the presence of the new VCPU's APIC id for subsequent SMIs.
The other important things are:
* new CPU-hotplug controller (docs/specs/acpi_cpu_hotplug.txt)
interfaces. I think this would only need a new bit in the write
register at 0x4 ("CPU device control fields").
The 0x0-0x3 write register, currently reserved for reading, might
become read/write for easier communication with the SMI handler. The
SMI handler would write 1 to the new bit in order to unpark the VCPU.
* how to enable this. I think it would need a new SMM feature, just
like those that you are adding here, in order to make it negotiable. If
it is not negotiated, but broadcast SMI is negotiated, you'd need to do
something such as not allowing CPU-hotplug. (This is the only part that
I think is required for 2.9).
In order to trigger the SMI, the
ifctx = aml_if(aml_equal(ins_evt, one));
{
aml_append(ifctx,
aml_call2(CPU_NOTIFY_METHOD, cpu_data, dev_chk));
aml_append(ifctx, aml_store(one, ins_evt));
aml_append(ifctx, aml_store(one, has_event));
}
would be replaced by something like this pseudo-AML:
Store(And(smm_features, 2), Local1)
...
Store(next_cpu_cmd, cpu_cmd)
If (Equal(ins_evt, One)) {
If (Greater(Local1, Zero)) {
Store(CPU_HP_APM_CMD, apm_cmd)
}
CPU_NOTIFY_METHOD(cpu_data, dev_chk)
Store(One, ins_evt)
Store(One, has_event)
}
Of course all this is for OVMF only. SeaBIOS doesn't need to do
anything of this, because it actually likes to have its SMIs only on the
current CPU (and it had better be the BSP, since SMBASE is not relocated
elsewhere!).
Igor, any thoughts?
I understand that this is quite huge in both OVMF and QEMU, but we've
only been delaying it and we knew about it. :(
Paolo
> Cc: "Kevin O'Connor" <ke...@koconnor.net>
> Cc: "Michael S. Tsirkin" <m...@redhat.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Paolo Bonzini <pbonz...@redhat.com>
>
> Thanks
> Laszlo
>
> Laszlo Ersek (3):
> hw/isa/apm: introduce callback for APM_STS_IOPORT writes
> hw/isa/lpc_ich9: add SMI feature negotiation via APM_STS
> hw/isa/lpc_ich9: ICH9_APM_STS_F_BROADCAST_SMI: inject SMI on all VCPUs
>
> docs/specs/q35-apm-sts.txt | 80
> ++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/ich9.h | 9 ++++++
> include/hw/isa/apm.h | 9 +++---
> hw/acpi/piix4.c | 2 +-
> hw/isa/apm.c | 15 ++++++---
> hw/isa/lpc_ich9.c | 64 +++++++++++++++++++++++++++++++++++--
> hw/isa/vt82c686.c | 2 +-
> 7 files changed, 168 insertions(+), 13 deletions(-)
> create mode 100644 docs/specs/q35-apm-sts.txt
>
