Re: [Qemu-devel] [PATCH 00/31] target-i386: SMM improvements and partial support under KVM

[Michael S. Tsirkin]

On Mon, May 11, 2015 at 03:48:46PM +0200, Paolo Bonzini wrote:
> These patches implement almost everything that is needed for SMM
> support in OVMF and KVM.  The only missing bit is support for
> SMRAM regions in KVM, but it need not block review of these ones,
> and possibly inclusion of the first 26.

Overall this looks good to me.
Sent some comments, and IIUC there will v2 down the road?


> There are many small parts in this patches, but I am posting them
> together because each small part alone adds very little.
> 
> Patch 1 comes from mst's pull request.
> 
> Patches 2-6 are target-i386 patches.  They add support for memory
> attributes in target-i386, enabling the "secure" attribute whenever
> the CPU is in system management mode.  They also fix two SMM bugs
> found while working on KVM support.
> 
> Patches 7-9 add support for secure access to parallel flash.  If
> enabled, parallel flash behaves as ROM unless the "secure" memory
> transaction attribute is set.
> 
> Patches 10-12 are general infrastructure patches that didn't fit
> elsewhere.  Note that patch 10 introduces new command-line syntax.
> 
> Patches 13-16 rewrite the SMRAM handling in TCG mode, so that the
> SMRAM setup is done just once using the memory API, and then
> enabled/disabled by the CPU without intervention from the chipset.
> The resulting chipset code is simpler and...
> 
> ... patches 17-23 then rely on this to implement support for
> more q35 SMI features, in particular high SMRAM, TSEG and SMI_LOCK.
> This part was done almost entirely by Gerd.
> 
> Patches 24-26 are for q35 feature parity with PIIX4.  They are from Laszlo
> and they are included just because they conflict with the next few.
> 
> Patches 27 and 28 implement KVM support for SMM.  Note that this support
> is not yet upstream (will be in Linux 4.2); these patches will be
> rebased after the updated KVM headers are taken from kvm.git.
> 
> Patches 29-31 add a "-machine smm=on|off|auto" option (QOM property)
> that can be used to hide SMM or make it available on any accelerator.
> The compat gunk makes it available by default on TCG but not on KVM.
> 
> That's it.  Go ahead and review.
> 
> Paolo
> 
> 
> Gerd Hoffmann (6):
>   q35: fix ESMRAMC default
>   q35: add config space wmask for SMRAM and ESMRAMC
>   q35: implement SMRAM.D_LCK
>   q35: add test for SMRAM.D_LCK
>   q35: implement TSEG
>   ich9: implement SMI_LOCK
> 
> Jason Wang (1):
>   pc: add 2.4 machine types
> 
> Laszlo Ersek (3):
>   hw/acpi: acpi_pm1_cnt_init(): take "disable_s3" and "disable_s4"
>   hw/acpi: move "etc/system-states" fw_cfg file from PIIX4 to core
>   hw/acpi: piix4_pm_init(): take fw_cfg object no more
> 
> Paolo Bonzini (21):
>   target-i386: introduce cpu_get_mem_attrs
>   target-i386: Use correct memory attributes for memory accesses
>   target-i386: Use correct memory attributes for ioport accesses
>   target-i386: mask NMIs on entry to SMM
>   target-i386: set G=1 in SMM big real mode selectors
>   pflash_cfi01: change big-endian property to BIT type
>   pflash_cfi01: change to new-style MMIO accessors
>   pflash_cfi01: add secure property
>   vl: allow full-blown QemuOpts syntax for -global
>   qom: add object_property_add_const_link
>   vl: run "late" notifiers immediately
>   target-i386: create a separate AddressSpace for each CPU
>   hw/i386: add a separate region that tracks the SMRAME bit
>   target-i386: use memory API to implement SMRAM
>   hw/i386: remove smram_update
>   q35: implement high SMRAM
>   target-i386: add support for SMBASE MSR and SMIs
>   vga: disable chain4_alias if KVM supports SMRAM
>   pc_piix: rename kvm_enabled to smm_enabled
>   ich9: add smm_enabled field and arguments
>   pc: add SMM property
> 
>  bsd-user/main.c             |   4 -
>  hw/acpi/core.c              |  15 +-
>  hw/acpi/ich9.c              |  12 +-
>  hw/acpi/piix4.c             |  21 +--
>  hw/block/pflash_cfi01.c     | 204 +++++++++++----------------
>  hw/display/vga.c            |   8 +-
>  hw/display/vga_int.h        |   1 +
>  hw/i386/pc.c                |  72 +++++++---
>  hw/i386/pc_piix.c           |  53 +++++--
>  hw/i386/pc_q35.c            |  33 ++++-
>  hw/isa/lpc_ich9.c           |  23 ++-
>  hw/isa/vt82c686.c           |   2 +-
>  hw/mips/mips_malta.c        |   2 +-
>  hw/pci-host/pam.c           |  20 ---
>  hw/pci-host/piix.c          |  39 +++---
>  hw/pci-host/q35.c           | 137 ++++++++++++++++--
>  include/exec/memattrs.h     |   4 +-
>  include/hw/acpi/acpi.h      |   3 +-
>  include/hw/acpi/ich9.h      |   4 +-
>  include/hw/i386/ich9.h      |   8 +-
>  include/hw/i386/pc.h        |   7 +-
>  include/hw/pci-host/pam.h   |   4 -
>  include/hw/pci-host/q35.h   |  36 +++--
>  include/qom/object.h        |  18 +++
>  include/sysemu/kvm.h        |   1 +
>  kvm-all.c                   |   5 +
>  kvm-stub.c                  |   5 +
>  linux-headers/asm-x86/kvm.h |  11 +-
>  linux-headers/linux/kvm.h   |   5 +-
>  linux-user/main.c           |   4 -
>  qdev-monitor.c              |  18 ++-
>  qemu-options.hx             |   7 +-
>  qom/object.c                |  16 +++
>  target-i386/Makefile.objs   |   2 -
>  target-i386/cpu-qom.h       |   3 +
>  target-i386/cpu.c           |  43 ++++++
>  target-i386/cpu.h           |  41 ++++--
>  target-i386/helper.c        | 135 +++++++++++++++---
>  target-i386/helper.h        |  12 +-
>  target-i386/ioport-user.c   |  60 --------
>  target-i386/kvm.c           |  75 ++++++++--
>  target-i386/machine.c       |   3 +
>  target-i386/misc_helper.c   |  59 ++++++--
>  target-i386/seg_helper.c    |  12 +-
>  target-i386/smm_helper.c    | 331 
> +++++++++++++++++++++++---------------------
>  target-i386/svm_helper.c    | 230 +++++++++++++++---------------
>  target-i386/translate.c     |  12 +-
>  tests/Makefile              |   2 +
>  tests/smram-test.c          |  80 +++++++++++
>  vl.c                        |   6 +
>  50 files changed, 1220 insertions(+), 688 deletions(-)
>  delete mode 100644 target-i386/ioport-user.c
>  create mode 100644 tests/smram-test.c
> 
> -- 
> 1.8.3.1