Re: [RFC PATCH 00/13] KVM: x86: SMM support

[Paolo Bonzini <pbonzini@redhat.com>]

On 05/05/2015 20:40, Radim Krčmář wrote:
> - Whole SMRAM is writeable.  Spec says that parts of state should be
>   read-only.  (This seems hard to fix without trapping all writes.)

Read-only here just means that you shouldn't touch it.  It says "Some
register images are read-only, and must not be modified (modifying these
registers will result in unpredictable behavior)".

But actually the behavior is very predictable, and can be very fun.  You
can do stuff such as interrupting a VM86 task with an SMI, and prepare
an SMM handler that returns to VM86 with CPL=0 (by setting SS.DPL=0 in
the SS access rights field).  That's very illegal compared to big real
mode. :)

Or you can fake a processor reset straight after RSM, which includes
setting the right segment base, limit and access rights (again you need
to set SS.DPL=0 to affect the CPL).

Worst case, you get a failed VM entry (e.g. if you set up an invalid
combination of segment limit and segment G flag).  If you care, disable
unrestricted_guest. :)

> - I/O restarting is not enabled.  (APM 2:10.2.4 SMM-Revision Identifier
>   says that AMD64 always sets this bit.)

Yes, unfortunately if I do enable it SeaBIOS breaks.  So it's left for
later.

I/O restarting is meant for stuff like emulating the i8042 on top of a
USB keyboard.  We luckily don't care (do not get strange ideas about
reducing the QEMU attack surface).

> - NMI handling has some quirks.  (Software can enable NMI and another
>   SMI should mask them again.)

Fixed now.  The handling is pretty insane, and requires another field
smi.rsm_unmasks_nmi in KVM_GET_VCPU_EVENTS.

> - SMIs received while in SMM aren't handled.  (One can be pending.)

Yup, fixed.

> - SMM and userspace.
>   We can get if smm is enabled at two separate places (flag from KVM_RUN
>   and in KVM_GET_VCPU_EVENTS) and toggle it via KVM_SET_VCPU_EVENTS.
> 
>   It's not an event, so I wouldn't include it in EVENTS API ...

Well, neither is nmi.masked or interrupt.shadow.  In the end, smi.smm is
just "smi.masked" (except that it also doubles as "is RSM allowed/is
SMRAM accessible").

>   Letting the flag in KVM_RUN also toggle SMM would be easiest.

I'm worried about breaking userspace with that.  I would probably have
to enable the SMM capability manually.

By comparison, the current implementation is entirely transparent as
long as the guest only generates SMIs through the APIC: all QEMU changes
are needed to support SMRAM and generation of SMIs through port 0xB2,
but the feature otherwise has zero impact on userspace.

But the main point in favor of "smi.smm" IMO is that it doubles as
"smi.masked".

>   Otherwise, wouldn't GET/SET_ONE_REG be a better match for it?

Perhaps, but then smi.pending would still be a better match for
KVM_GET_VCPU_EVENTS than for ONE_REG.  (And again, so would
"smi.masked"---it just happens that "masked SMIs == CPU in SMM").

Paolo