Date: Tue, 5 May 2015 20:40:34 +0200
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, bsd@redhat.com,
        guangrong.xiao@linux.intel.com, Yang Zhang <yang.z.zhang@intel.com>,
        wanpeng.li@linux.intel.com
Subject: Re: [RFC PATCH 00/13] KVM: x86: SMM support
Message-ID: <20150505184034.GA17718@potion.brq.redhat.com>
References: <1430393772-27208-1-git-send-email-pbonzini@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1430393772-27208-1-git-send-email-pbonzini@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2911
Lines: 66

2015-04-30 13:35+0200, Paolo Bonzini:
> This patch series introduces system management mode support.
> There is still some work to do, namely: test without unrestricted
> guest support, test on AMD, disable the capability if !unrestricted
> guest and !emulate invalid guest state(*), test with a QEMU that
> understand KVM_MEM_X86_SMRAM, actually post QEMU patches that let
> you use this.
> 
> 	(*) newer chipsets moved away from legacy SMRAM at 0xa0000,
> 	    thus support for real mode CS base above 1M is necessary
> 
> Because legacy SMRAM is a mess, I have tried these patches with Q35's
> high SMRAM (at 0xfeda0000).  This means that right now this isn't
> the easiest thing to test; you need QEMU patches that add support
> for high SMRAM, and SeaBIOS patches to use high SMRAM.  Until QEMU
> support for KVM_MEM_X86_SMRAM is in place, also, I'm keeping SMRAM
> open in SeaBIOS.
> 
> That said, even this clumsy and incomplete userspace configuration is
> enough to test all patches except 11 and 12.
> 
> The series is structured as follows.
> 
> Patch 1 is an unrelated bugfix (I think).  Patches 2 to 6 extend some
> infrastructure functions.  Patches 1 to 4 could be committed right now.
> 
> Patches 7 to 9 implement basic support for SMM in the KVM API
> and teach KVM about doing the world switch on SMI and RSM.
> 
> Patch 10 touches all places in KVM that read/write guest memory to
> go through an x86-specific function.  The x86-specific function takes
> a VCPU rather than a struct kvm.  This is used in patches 11 and 12
> to limits access to specially marked SMRAM slots unless the VCPU is
> in system management mode.
> 
> Finally, patch 13 exposes the new capability for userspace to probe.

I lost all concentration, so I'll write down design problems I noticed
while reviewing till now in case it helps:
(haven't mentioned the first three yet.)

- Whole SMRAM is writeable.  Spec says that parts of state should be
  read-only.  (This seems hard to fix without trapping all writes.)

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

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

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

- 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 ...
  Letting the flag in KVM_RUN also toggle SMM would be easiest.
  Otherwise, wouldn't GET/SET_ONE_REG be a better match for it?

- [10/13] :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/