Re: [PATCH v2 00/13] SMM implementation for KVM

["Radim Krčmář" <rkrcmar@redhat.com>]

2015-05-27 19:05+0200, Paolo Bonzini:
> This brings together the remaining parts of SMM.  For now I've left the
> "weird" interaction between SMM and NMI blocking, and I'm using the same
> format for the state save area (which is also the one used by QEMU) as
> the RFC.
> 
> It builds on the previous cleanup patches, which (with the exception
> of "KVM: x86: pass kvm_mmu_page to gfn_to_rmap") are now in kvm/queue.
> The first six patches are more or less the same as the previous version,
> while the address spaces part hopefully touches all affected functions
> now.
> 
> Patches 1-6 implement the SMM API and world switch; patches 7-12
> implements the multiple address spaces; patch 13 ties the loose
> ends and advertises the capability.
> 
> Tested with SeaBIOS and OVMF, where SMM provides the trusted base
> for secure boot.

Good work!  ABI looks ok and unknown bugs shouldn't be major.
With ~HF_SMM_INSIDE_NMI_MASK [04/13], early return [06/13] and
INIT [14/13] fixed,
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>


---
A list of minor stuff I noticed (you'll probably agree with [12/13]):
[06/13] get_smstate can't be a valid C function, CAPS would be clearer
[06/13] get_smstate uses kvm_read_guest_virt_system for every field,
        buffering the whole block should be much faster
[06/13] mangled whitespace in trace_kvm_enter_smm
[08/13] '(u16)' or '0xffff &' seem better than '65535 &'
[10/13] try_async_pf() could pass memslot to kvm_arch_setup_async_pf()
[11/13] '#define kvm_for_each_address_space' would fit a lot of places
[12/13] add role.smm to Documentation/virtual/kvm/mmu.txt
[14/13] kvm_lapic_latched_init could use a linebreak