public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/4] KVM: Reorder IOCTLs in main kvm.h
  2009-10-15 17:05 [PATCH v2 0/4] Extensible VCPU state IOCTL Jan Kiszka
@ 2009-10-15 17:05 ` Jan Kiszka
  2009-10-15 17:05 ` [PATCH v2 3/4] KVM: x86: Add support for KVM_GET/SET_VCPU_STATE Jan Kiszka
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Jan Kiszka @ 2009-10-15 17:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

Obviously, people tend to extend this header at the bottom - more or
less blindly. Ensure that deprecated stuff gets its own corner again by
moving things to the top. Also add some comments and reindent IOCTLs to
make them more readable and reduce the risk of number collisions.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 include/linux/kvm.h |  228 ++++++++++++++++++++++++++-------------------------
 1 files changed, 114 insertions(+), 114 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index f8f8900..7d8c382 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -14,12 +14,76 @@
 
 #define KVM_API_VERSION 12
 
-/* for KVM_TRACE_ENABLE, deprecated */
+/* *** Deprecated interfaces *** */
+
+#define KVM_TRC_SHIFT           16
+
+#define KVM_TRC_ENTRYEXIT       (1 << KVM_TRC_SHIFT)
+#define KVM_TRC_HANDLER         (1 << (KVM_TRC_SHIFT + 1))
+
+#define KVM_TRC_VMENTRY         (KVM_TRC_ENTRYEXIT + 0x01)
+#define KVM_TRC_VMEXIT          (KVM_TRC_ENTRYEXIT + 0x02)
+#define KVM_TRC_PAGE_FAULT      (KVM_TRC_HANDLER + 0x01)
+
+#define KVM_TRC_HEAD_SIZE       12
+#define KVM_TRC_CYCLE_SIZE      8
+#define KVM_TRC_EXTRA_MAX       7
+
+#define KVM_TRC_INJ_VIRQ         (KVM_TRC_HANDLER + 0x02)
+#define KVM_TRC_REDELIVER_EVT    (KVM_TRC_HANDLER + 0x03)
+#define KVM_TRC_PEND_INTR        (KVM_TRC_HANDLER + 0x04)
+#define KVM_TRC_IO_READ          (KVM_TRC_HANDLER + 0x05)
+#define KVM_TRC_IO_WRITE         (KVM_TRC_HANDLER + 0x06)
+#define KVM_TRC_CR_READ          (KVM_TRC_HANDLER + 0x07)
+#define KVM_TRC_CR_WRITE         (KVM_TRC_HANDLER + 0x08)
+#define KVM_TRC_DR_READ          (KVM_TRC_HANDLER + 0x09)
+#define KVM_TRC_DR_WRITE         (KVM_TRC_HANDLER + 0x0A)
+#define KVM_TRC_MSR_READ         (KVM_TRC_HANDLER + 0x0B)
+#define KVM_TRC_MSR_WRITE        (KVM_TRC_HANDLER + 0x0C)
+#define KVM_TRC_CPUID            (KVM_TRC_HANDLER + 0x0D)
+#define KVM_TRC_INTR             (KVM_TRC_HANDLER + 0x0E)
+#define KVM_TRC_NMI              (KVM_TRC_HANDLER + 0x0F)
+#define KVM_TRC_VMMCALL          (KVM_TRC_HANDLER + 0x10)
+#define KVM_TRC_HLT              (KVM_TRC_HANDLER + 0x11)
+#define KVM_TRC_CLTS             (KVM_TRC_HANDLER + 0x12)
+#define KVM_TRC_LMSW             (KVM_TRC_HANDLER + 0x13)
+#define KVM_TRC_APIC_ACCESS      (KVM_TRC_HANDLER + 0x14)
+#define KVM_TRC_TDP_FAULT        (KVM_TRC_HANDLER + 0x15)
+#define KVM_TRC_GTLB_WRITE       (KVM_TRC_HANDLER + 0x16)
+#define KVM_TRC_STLB_WRITE       (KVM_TRC_HANDLER + 0x17)
+#define KVM_TRC_STLB_INVAL       (KVM_TRC_HANDLER + 0x18)
+#define KVM_TRC_PPC_INSTR        (KVM_TRC_HANDLER + 0x19)
+
 struct kvm_user_trace_setup {
-	__u32 buf_size; /* sub_buffer size of each per-cpu */
-	__u32 buf_nr; /* the number of sub_buffers of each per-cpu */
+	__u32 buf_size;
+	__u32 buf_nr;
 };
 
+#define __KVM_DEPRECATED_MAIN_W_0x06 \
+	_IOW(KVMIO, 0x06, struct kvm_user_trace_setup)
+#define __KVM_DEPRECATED_MAIN_0x07 _IO(KVMIO, 0x07)
+#define __KVM_DEPRECATED_MAIN_0x08 _IO(KVMIO, 0x08)
+
+#define __KVM_DEPRECATED_VM_R_0x70 _IOR(KVMIO, 0x70, struct kvm_assigned_irq)
+
+struct kvm_breakpoint {
+	__u32 enabled;
+	__u32 padding;
+	__u64 address;
+};
+
+struct kvm_debug_guest {
+	__u32 enabled;
+	__u32 pad;
+	struct kvm_breakpoint breakpoints[4];
+	__u32 singlestep;
+};
+
+#define __KVM_DEPRECATED_VCPU_W_0x87 _IOW(KVMIO, 0x87, struct kvm_debug_guest)
+
+/* *** End of deprecated interfaces *** */
+
+
 /* for KVM_CREATE_MEMORY_REGION */
 struct kvm_memory_region {
 	__u32 slot;
@@ -329,24 +393,6 @@ struct kvm_ioeventfd {
 	__u8  pad[36];
 };
 
-#define KVM_TRC_SHIFT           16
-/*
- * kvm trace categories
- */
-#define KVM_TRC_ENTRYEXIT       (1 << KVM_TRC_SHIFT)
-#define KVM_TRC_HANDLER         (1 << (KVM_TRC_SHIFT + 1)) /* only 12 bits */
-
-/*
- * kvm trace action
- */
-#define KVM_TRC_VMENTRY         (KVM_TRC_ENTRYEXIT + 0x01)
-#define KVM_TRC_VMEXIT          (KVM_TRC_ENTRYEXIT + 0x02)
-#define KVM_TRC_PAGE_FAULT      (KVM_TRC_HANDLER + 0x01)
-
-#define KVM_TRC_HEAD_SIZE       12
-#define KVM_TRC_CYCLE_SIZE      8
-#define KVM_TRC_EXTRA_MAX       7
-
 #define KVMIO 0xAE
 
 /*
@@ -367,12 +413,10 @@ struct kvm_ioeventfd {
  */
 #define KVM_GET_VCPU_MMAP_SIZE    _IO(KVMIO,   0x04) /* in bytes */
 #define KVM_GET_SUPPORTED_CPUID   _IOWR(KVMIO, 0x05, struct kvm_cpuid2)
-/*
- * ioctls for kvm trace
- */
-#define KVM_TRACE_ENABLE          _IOW(KVMIO, 0x06, struct kvm_user_trace_setup)
-#define KVM_TRACE_PAUSE           _IO(KVMIO,  0x07)
-#define KVM_TRACE_DISABLE         _IO(KVMIO,  0x08)
+#define KVM_TRACE_ENABLE          __KVM_DEPRECATED_MAIN_W_0x06
+#define KVM_TRACE_PAUSE           __KVM_DEPRECATED_MAIN_0x07
+#define KVM_TRACE_DISABLE         __KVM_DEPRECATED_MAIN_0x08
+
 /*
  * Extension capability list.
  */
@@ -500,52 +544,54 @@ struct kvm_irqfd {
 /*
  * ioctls for VM fds
  */
-#define KVM_SET_MEMORY_REGION     _IOW(KVMIO, 0x40, struct kvm_memory_region)
+#define KVM_SET_MEMORY_REGION     _IOW(KVMIO,  0x40, struct kvm_memory_region)
 /*
  * KVM_CREATE_VCPU receives as a parameter the vcpu slot, and returns
  * a vcpu fd.
  */
-#define KVM_CREATE_VCPU           _IO(KVMIO,  0x41)
-#define KVM_GET_DIRTY_LOG         _IOW(KVMIO, 0x42, struct kvm_dirty_log)
-#define KVM_SET_MEMORY_ALIAS      _IOW(KVMIO, 0x43, struct kvm_memory_alias)
-#define KVM_SET_NR_MMU_PAGES      _IO(KVMIO, 0x44)
-#define KVM_GET_NR_MMU_PAGES      _IO(KVMIO, 0x45)
-#define KVM_SET_USER_MEMORY_REGION _IOW(KVMIO, 0x46,\
+#define KVM_CREATE_VCPU           _IO(KVMIO,   0x41)
+#define KVM_GET_DIRTY_LOG         _IOW(KVMIO,  0x42, struct kvm_dirty_log)
+#define KVM_SET_MEMORY_ALIAS      _IOW(KVMIO,  0x43, struct kvm_memory_alias)
+#define KVM_SET_NR_MMU_PAGES      _IO(KVMIO,   0x44)
+#define KVM_GET_NR_MMU_PAGES      _IO(KVMIO,   0x45)
+#define KVM_SET_USER_MEMORY_REGION _IOW(KVMIO, 0x46, \
 					struct kvm_userspace_memory_region)
-#define KVM_SET_TSS_ADDR          _IO(KVMIO, 0x47)
-#define KVM_SET_IDENTITY_MAP_ADDR _IOW(KVMIO, 0x48, __u64)
+#define KVM_SET_TSS_ADDR          _IO(KVMIO,   0x47)
+#define KVM_SET_IDENTITY_MAP_ADDR _IOW(KVMIO,  0x48, __u64)
 /* Device model IOC */
-#define KVM_CREATE_IRQCHIP	  _IO(KVMIO,  0x60)
-#define KVM_IRQ_LINE		  _IOW(KVMIO, 0x61, struct kvm_irq_level)
-#define KVM_GET_IRQCHIP		  _IOWR(KVMIO, 0x62, struct kvm_irqchip)
-#define KVM_SET_IRQCHIP		  _IOR(KVMIO,  0x63, struct kvm_irqchip)
-#define KVM_CREATE_PIT		  _IO(KVMIO,  0x64)
-#define KVM_GET_PIT		  _IOWR(KVMIO, 0x65, struct kvm_pit_state)
-#define KVM_SET_PIT		  _IOR(KVMIO,  0x66, struct kvm_pit_state)
-#define KVM_IRQ_LINE_STATUS	  _IOWR(KVMIO, 0x67, struct kvm_irq_level)
+#define KVM_CREATE_IRQCHIP        _IO(KVMIO,   0x60)
+#define KVM_IRQ_LINE              _IOW(KVMIO,  0x61, struct kvm_irq_level)
+#define KVM_GET_IRQCHIP           _IOWR(KVMIO, 0x62, struct kvm_irqchip)
+#define KVM_SET_IRQCHIP           _IOR(KVMIO,  0x63, struct kvm_irqchip)
+#define KVM_CREATE_PIT            _IO(KVMIO,   0x64)
+#define KVM_GET_PIT               _IOWR(KVMIO, 0x65, struct kvm_pit_state)
+#define KVM_SET_PIT               _IOR(KVMIO,  0x66, struct kvm_pit_state)
+#define KVM_IRQ_LINE_STATUS       _IOWR(KVMIO, 0x67, struct kvm_irq_level)
 #define KVM_REGISTER_COALESCED_MMIO \
 			_IOW(KVMIO,  0x67, struct kvm_coalesced_mmio_zone)
 #define KVM_UNREGISTER_COALESCED_MMIO \
 			_IOW(KVMIO,  0x68, struct kvm_coalesced_mmio_zone)
-#define KVM_ASSIGN_PCI_DEVICE _IOR(KVMIO, 0x69, \
-				   struct kvm_assigned_pci_dev)
-#define KVM_SET_GSI_ROUTING       _IOW(KVMIO, 0x6a, struct kvm_irq_routing)
+#define KVM_ASSIGN_PCI_DEVICE     _IOR(KVMIO,  0x69, \
+				       struct kvm_assigned_pci_dev)
+#define KVM_SET_GSI_ROUTING       _IOW(KVMIO,  0x6a, struct kvm_irq_routing)
 /* deprecated, replaced by KVM_ASSIGN_DEV_IRQ */
-#define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
-			    struct kvm_assigned_irq)
-#define KVM_ASSIGN_DEV_IRQ        _IOW(KVMIO, 0x70, struct kvm_assigned_irq)
-#define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
-#define KVM_DEASSIGN_PCI_DEVICE _IOW(KVMIO, 0x72, \
-				     struct kvm_assigned_pci_dev)
-#define KVM_ASSIGN_SET_MSIX_NR \
-			_IOW(KVMIO, 0x73, struct kvm_assigned_msix_nr)
-#define KVM_ASSIGN_SET_MSIX_ENTRY \
-			_IOW(KVMIO, 0x74, struct kvm_assigned_msix_entry)
-#define KVM_DEASSIGN_DEV_IRQ       _IOW(KVMIO, 0x75, struct kvm_assigned_irq)
-#define KVM_IRQFD                  _IOW(KVMIO, 0x76, struct kvm_irqfd)
-#define KVM_CREATE_PIT2		   _IOW(KVMIO, 0x77, struct kvm_pit_config)
-#define KVM_SET_BOOT_CPU_ID        _IO(KVMIO, 0x78)
-#define KVM_IOEVENTFD             _IOW(KVMIO, 0x79, struct kvm_ioeventfd)
+#define KVM_ASSIGN_IRQ            __KVM_DEPRECATED_VM_R_0x70
+#define KVM_ASSIGN_DEV_IRQ        _IOW(KVMIO,  0x70, struct kvm_assigned_irq)
+#define KVM_REINJECT_CONTROL      _IO(KVMIO,   0x71)
+#define KVM_DEASSIGN_PCI_DEVICE   _IOW(KVMIO,  0x72, \
+				       struct kvm_assigned_pci_dev)
+#define KVM_ASSIGN_SET_MSIX_NR    _IOW(KVMIO,  0x73, \
+				       struct kvm_assigned_msix_nr)
+#define KVM_ASSIGN_SET_MSIX_ENTRY _IOW(KVMIO,  0x74, \
+				       struct kvm_assigned_msix_entry)
+#define KVM_DEASSIGN_DEV_IRQ      _IOW(KVMIO,  0x75, struct kvm_assigned_irq)
+#define KVM_IRQFD                 _IOW(KVMIO,  0x76, struct kvm_irqfd)
+#define KVM_CREATE_PIT2		  _IOW(KVMIO,  0x77, struct kvm_pit_config)
+#define KVM_SET_BOOT_CPU_ID       _IO(KVMIO,   0x78)
+#define KVM_IOEVENTFD             _IOW(KVMIO,  0x79, struct kvm_ioeventfd)
+/* Available with KVM_CAP_PIT_STATE2 */
+#define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct kvm_pit_state2)
+#define KVM_SET_PIT2              _IOW(KVMIO,  0xa0, struct kvm_pit_state2)
 
 /*
  * ioctls for vcpu fds
@@ -558,7 +604,7 @@ struct kvm_irqfd {
 #define KVM_TRANSLATE             _IOWR(KVMIO, 0x85, struct kvm_translation)
 #define KVM_INTERRUPT             _IOW(KVMIO,  0x86, struct kvm_interrupt)
 /* KVM_DEBUG_GUEST is no longer supported, use KVM_SET_GUEST_DEBUG instead */
-#define KVM_DEBUG_GUEST           __KVM_DEPRECATED_DEBUG_GUEST
+#define KVM_DEBUG_GUEST           __KVM_DEPRECATED_VCPU_W_0x87
 #define KVM_GET_MSRS              _IOWR(KVMIO, 0x88, struct kvm_msrs)
 #define KVM_SET_MSRS              _IOW(KVMIO,  0x89, struct kvm_msrs)
 #define KVM_SET_CPUID             _IOW(KVMIO,  0x8a, struct kvm_cpuid)
@@ -570,7 +616,7 @@ struct kvm_irqfd {
 #define KVM_SET_CPUID2            _IOW(KVMIO,  0x90, struct kvm_cpuid2)
 #define KVM_GET_CPUID2            _IOWR(KVMIO, 0x91, struct kvm_cpuid2)
 /* Available with KVM_CAP_VAPIC */
-#define KVM_TPR_ACCESS_REPORTING  _IOWR(KVMIO,  0x92, struct kvm_tpr_access_ctl)
+#define KVM_TPR_ACCESS_REPORTING  _IOWR(KVMIO, 0x92, struct kvm_tpr_access_ctl)
 /* Available with KVM_CAP_VAPIC */
 #define KVM_SET_VAPIC_ADDR        _IOW(KVMIO,  0x93, struct kvm_vapic_addr)
 /* valid for virtual machine (for floating interrupt)_and_ vcpu */
@@ -582,67 +628,21 @@ struct kvm_irqfd {
 /* initial ipl psw for s390 */
 #define KVM_S390_SET_INITIAL_PSW  _IOW(KVMIO,  0x96, struct kvm_s390_psw)
 /* initial reset for s390 */
-#define KVM_S390_INITIAL_RESET    _IO(KVMIO,  0x97)
+#define KVM_S390_INITIAL_RESET    _IO(KVMIO,   0x97)
 #define KVM_GET_MP_STATE          _IOR(KVMIO,  0x98, struct kvm_mp_state)
 #define KVM_SET_MP_STATE          _IOW(KVMIO,  0x99, struct kvm_mp_state)
 /* Available with KVM_CAP_NMI */
-#define KVM_NMI                   _IO(KVMIO,  0x9a)
+#define KVM_NMI                   _IO(KVMIO,   0x9a)
 /* Available with KVM_CAP_SET_GUEST_DEBUG */
 #define KVM_SET_GUEST_DEBUG       _IOW(KVMIO,  0x9b, struct kvm_guest_debug)
 /* MCE for x86 */
 #define KVM_X86_SETUP_MCE         _IOW(KVMIO,  0x9c, __u64)
 #define KVM_X86_GET_MCE_CAP_SUPPORTED _IOR(KVMIO,  0x9d, __u64)
 #define KVM_X86_SET_MCE           _IOW(KVMIO,  0x9e, struct kvm_x86_mce)
-
-/*
- * Deprecated interfaces
- */
-struct kvm_breakpoint {
-	__u32 enabled;
-	__u32 padding;
-	__u64 address;
-};
-
-struct kvm_debug_guest {
-	__u32 enabled;
-	__u32 pad;
-	struct kvm_breakpoint breakpoints[4];
-	__u32 singlestep;
-};
-
-#define __KVM_DEPRECATED_DEBUG_GUEST _IOW(KVMIO,  0x87, struct kvm_debug_guest)
-
+/* IA64 stack access */
 #define KVM_IA64_VCPU_GET_STACK   _IOR(KVMIO,  0x9a, void *)
 #define KVM_IA64_VCPU_SET_STACK   _IOW(KVMIO,  0x9b, void *)
 
-#define KVM_GET_PIT2   _IOR(KVMIO,   0x9f, struct kvm_pit_state2)
-#define KVM_SET_PIT2   _IOW(KVMIO,   0xa0, struct kvm_pit_state2)
-
-#define KVM_TRC_INJ_VIRQ         (KVM_TRC_HANDLER + 0x02)
-#define KVM_TRC_REDELIVER_EVT    (KVM_TRC_HANDLER + 0x03)
-#define KVM_TRC_PEND_INTR        (KVM_TRC_HANDLER + 0x04)
-#define KVM_TRC_IO_READ          (KVM_TRC_HANDLER + 0x05)
-#define KVM_TRC_IO_WRITE         (KVM_TRC_HANDLER + 0x06)
-#define KVM_TRC_CR_READ          (KVM_TRC_HANDLER + 0x07)
-#define KVM_TRC_CR_WRITE         (KVM_TRC_HANDLER + 0x08)
-#define KVM_TRC_DR_READ          (KVM_TRC_HANDLER + 0x09)
-#define KVM_TRC_DR_WRITE         (KVM_TRC_HANDLER + 0x0A)
-#define KVM_TRC_MSR_READ         (KVM_TRC_HANDLER + 0x0B)
-#define KVM_TRC_MSR_WRITE        (KVM_TRC_HANDLER + 0x0C)
-#define KVM_TRC_CPUID            (KVM_TRC_HANDLER + 0x0D)
-#define KVM_TRC_INTR             (KVM_TRC_HANDLER + 0x0E)
-#define KVM_TRC_NMI              (KVM_TRC_HANDLER + 0x0F)
-#define KVM_TRC_VMMCALL          (KVM_TRC_HANDLER + 0x10)
-#define KVM_TRC_HLT              (KVM_TRC_HANDLER + 0x11)
-#define KVM_TRC_CLTS             (KVM_TRC_HANDLER + 0x12)
-#define KVM_TRC_LMSW             (KVM_TRC_HANDLER + 0x13)
-#define KVM_TRC_APIC_ACCESS      (KVM_TRC_HANDLER + 0x14)
-#define KVM_TRC_TDP_FAULT        (KVM_TRC_HANDLER + 0x15)
-#define KVM_TRC_GTLB_WRITE       (KVM_TRC_HANDLER + 0x16)
-#define KVM_TRC_STLB_WRITE       (KVM_TRC_HANDLER + 0x17)
-#define KVM_TRC_STLB_INVAL       (KVM_TRC_HANDLER + 0x18)
-#define KVM_TRC_PPC_INSTR        (KVM_TRC_HANDLER + 0x19)
-
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 
 struct kvm_assigned_pci_dev {
@@ -696,4 +696,4 @@ struct kvm_assigned_msix_entry {
 	__u16 padding[3];
 };
 
-#endif
+#endif /* __LINUX_KVM_H */


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 0/4] Extensible VCPU state IOCTL
@ 2009-10-15 17:05 Jan Kiszka
  2009-10-15 17:05 ` [PATCH v2 1/4] KVM: Reorder IOCTLs in main kvm.h Jan Kiszka
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Jan Kiszka @ 2009-10-15 17:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

This version addresses the review comments:
 - rename KVM[_X86]_VCPU_* -> KVM[_X86]_VCPU_STATE_*
 - more padding for kvm_nmi_state
 - use bool in get/set_nmi_mask
 - add basic documentation.

Find this series also at git://git.kiszka.org/linux-kvm.git queues/vcpu-state

Jan Kiszka (4):
      KVM: Reorder IOCTLs in main kvm.h
      KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
      KVM: x86: Add support for KVM_GET/SET_VCPU_STATE
      KVM: x86: Add VCPU substate for NMI states

 Documentation/kvm/api.txt       |  103 +++++++++++++
 arch/ia64/kvm/kvm-ia64.c        |   12 ++
 arch/powerpc/kvm/powerpc.c      |   12 ++
 arch/s390/kvm/kvm-s390.c        |   12 ++
 arch/x86/include/asm/kvm.h      |   15 ++-
 arch/x86/include/asm/kvm_host.h |    2 +
 arch/x86/kvm/svm.c              |   22 +++
 arch/x86/kvm/vmx.c              |   30 ++++
 arch/x86/kvm/x86.c              |  243 ++++++++++++++++++++---------
 include/linux/kvm.h             |  246 +++++++++++++++++--------------
 include/linux/kvm_host.h        |    5 +
 virt/kvm/kvm_main.c             |  318 +++++++++++++++++++++++++++-----------
 12 files changed, 740 insertions(+), 280 deletions(-)



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-15 17:05 [PATCH v2 0/4] Extensible VCPU state IOCTL Jan Kiszka
  2009-10-15 17:05 ` [PATCH v2 1/4] KVM: Reorder IOCTLs in main kvm.h Jan Kiszka
  2009-10-15 17:05 ` [PATCH v2 3/4] KVM: x86: Add support for KVM_GET/SET_VCPU_STATE Jan Kiszka
@ 2009-10-15 17:05 ` Jan Kiszka
  2009-10-19 20:32   ` Marcelo Tosatti
  2009-10-15 17:05 ` [PATCH v2 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL Jan Kiszka
  3 siblings, 1 reply; 15+ messages in thread
From: Jan Kiszka @ 2009-10-15 17:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

This plugs an NMI-related hole in the VCPU synchronization between
kernel and user space. So far, neither pending NMIs nor the inhibit NMI
mask was properly read/set which was able to cause problems on
vmsave/restore, live migration and system reset. Fix it by making use
of the new VCPU substate interface.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 Documentation/kvm/api.txt       |   12 ++++++++++++
 arch/x86/include/asm/kvm.h      |    7 +++++++
 arch/x86/include/asm/kvm_host.h |    2 ++
 arch/x86/kvm/svm.c              |   22 ++++++++++++++++++++++
 arch/x86/kvm/vmx.c              |   30 ++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c              |   26 ++++++++++++++++++++++++++
 6 files changed, 99 insertions(+), 0 deletions(-)

diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index bee5bbd..e483edb 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -848,3 +848,15 @@ Deprecates: KVM_GET/SET_CPUID2
 Architectures: x86
 Payload: struct kvm_lapic
 Deprecates: KVM_GET/SET_LAPIC
+
+6.8 KVM_X86_VCPU_STATE_NMI
+
+Architectures: x86
+Payload: struct kvm_nmi_state
+Deprecates: -
+
+struct kvm_nmi_state {
+       __u8 pending;
+       __u8 masked;
+       __u8 pad1[6];
+};
diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
index 326615a..6ad4448 100644
--- a/arch/x86/include/asm/kvm.h
+++ b/arch/x86/include/asm/kvm.h
@@ -256,5 +256,12 @@ struct kvm_reinject_control {
 #define KVM_X86_VCPU_STATE_MSRS		1000
 #define KVM_X86_VCPU_STATE_CPUID	1001
 #define KVM_X86_VCPU_STATE_LAPIC	1002
+#define KVM_X86_VCPU_STATE_NMI		1003
+
+struct kvm_nmi_state {
+       __u8 pending;
+       __u8 masked;
+       __u8 pad1[6];
+};
 
 #endif /* _ASM_X86_KVM_H */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 179a919..b6b2db4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -513,6 +513,8 @@ struct kvm_x86_ops {
 				unsigned char *hypercall_addr);
 	void (*set_irq)(struct kvm_vcpu *vcpu);
 	void (*set_nmi)(struct kvm_vcpu *vcpu);
+	bool (*get_nmi_mask)(struct kvm_vcpu *vcpu);
+	void (*set_nmi_mask)(struct kvm_vcpu *vcpu, bool masked);
 	void (*queue_exception)(struct kvm_vcpu *vcpu, unsigned nr,
 				bool has_error_code, u32 error_code);
 	int (*interrupt_allowed)(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 170b2d9..a16ee6e 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2498,6 +2498,26 @@ static int svm_nmi_allowed(struct kvm_vcpu *vcpu)
 		!(svm->vcpu.arch.hflags & HF_NMI_MASK);
 }
 
+static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	return !!(svm->vcpu.arch.hflags & HF_NMI_MASK);
+}
+
+static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (masked) {
+		svm->vcpu.arch.hflags |= HF_NMI_MASK;
+		svm->vmcb->control.intercept |= (1UL << INTERCEPT_IRET);
+	} else {
+		svm->vcpu.arch.hflags &= ~HF_NMI_MASK;
+		svm->vmcb->control.intercept &= ~(1UL << INTERCEPT_IRET);
+	}
+}
+
 static int svm_interrupt_allowed(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -2945,6 +2965,8 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.queue_exception = svm_queue_exception,
 	.interrupt_allowed = svm_interrupt_allowed,
 	.nmi_allowed = svm_nmi_allowed,
+	.get_nmi_mask = svm_get_nmi_mask,
+	.set_nmi_mask = svm_set_nmi_mask,
 	.enable_nmi_window = enable_nmi_window,
 	.enable_irq_window = enable_irq_window,
 	.update_cr8_intercept = update_cr8_intercept,
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 364263a..6e032e4 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2655,6 +2655,34 @@ static int vmx_nmi_allowed(struct kvm_vcpu *vcpu)
 				GUEST_INTR_STATE_NMI));
 }
 
+static bool vmx_get_nmi_mask(struct kvm_vcpu *vcpu)
+{
+	if (!cpu_has_virtual_nmis())
+		return to_vmx(vcpu)->soft_vnmi_blocked;
+	else
+		return !!(vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) &
+			  GUEST_INTR_STATE_NMI);
+}
+
+static void vmx_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
+{
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
+
+	if (!cpu_has_virtual_nmis()) {
+		if (vmx->soft_vnmi_blocked != masked) {
+			vmx->soft_vnmi_blocked = masked;
+			vmx->vnmi_blocked_time = 0;
+		}
+	} else {
+		if (masked)
+			vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
+				      GUEST_INTR_STATE_NMI);
+		else
+			vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO,
+					GUEST_INTR_STATE_NMI);
+	}
+}
+
 static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu)
 {
 	return (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
@@ -4006,6 +4034,8 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.queue_exception = vmx_queue_exception,
 	.interrupt_allowed = vmx_interrupt_allowed,
 	.nmi_allowed = vmx_nmi_allowed,
+	.get_nmi_mask = vmx_get_nmi_mask,
+	.set_nmi_mask = vmx_set_nmi_mask,
 	.enable_nmi_window = enable_nmi_window,
 	.enable_irq_window = enable_irq_window,
 	.update_cr8_intercept = update_cr8_intercept,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 46fad88..e7ce505 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4686,6 +4686,19 @@ out_free_lapic:
 		kfree(lapic);
 		break;
 	}
+	case KVM_X86_VCPU_STATE_NMI: {
+		struct kvm_nmi_state nmi;
+
+		vcpu_load(vcpu);
+		nmi.pending = vcpu->arch.nmi_pending;
+		nmi.masked = kvm_x86_ops->get_nmi_mask(vcpu);
+		vcpu_put(vcpu);
+		r = -EFAULT;
+		if (copy_to_user(argp, &nmi, sizeof(struct kvm_nmi_state)))
+			break;
+		r = 0;
+		break;
+	}
 	default:
 		r = -EINVAL;
 	}
@@ -4733,6 +4746,19 @@ out_free_lapic:
 		kfree(lapic);
 		break;
 	}
+	case KVM_X86_VCPU_STATE_NMI: {
+		struct kvm_nmi_state nmi;
+
+		r = -EFAULT;
+		if (copy_from_user(&nmi, argp, sizeof(struct kvm_nmi_state)))
+			break;
+		vcpu_load(vcpu);
+		vcpu->arch.nmi_pending = nmi.pending;
+		kvm_x86_ops->set_nmi_mask(vcpu, nmi.masked);
+		vcpu_put(vcpu);
+		r = 0;
+		break;
+	}
 	default:
 		r = -EINVAL;
 	}


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
  2009-10-15 17:05 [PATCH v2 0/4] Extensible VCPU state IOCTL Jan Kiszka
                   ` (2 preceding siblings ...)
  2009-10-15 17:05 ` [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states Jan Kiszka
@ 2009-10-15 17:05 ` Jan Kiszka
  3 siblings, 0 replies; 15+ messages in thread
From: Jan Kiszka @ 2009-10-15 17:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

Add a new IOCTL pair to retrieve or set the VCPU state in one chunk.
More precisely, the IOCTL is able to process a list of substates to be
read or written. This list is easily extensible without breaking the
existing ABI, thus we will no longer have to add new IOCTLs when we
discover a missing VCPU state field or want to support new hardware
features.

This patch establishes the generic infrastructure for KVM_GET/
SET_VCPU_STATE and adds support for the generic substates REGS, SREGS,
FPU, and MP. To avoid code duplication, the entry point for the
corresponding original IOCTLs are converted to make use of the new
infrastructure internally, too.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 Documentation/kvm/api.txt  |   73 ++++++++++
 arch/ia64/kvm/kvm-ia64.c   |   12 ++
 arch/powerpc/kvm/powerpc.c |   12 ++
 arch/s390/kvm/kvm-s390.c   |   12 ++
 arch/x86/kvm/x86.c         |   12 ++
 include/linux/kvm.h        |   24 +++
 include/linux/kvm_host.h   |    5 +
 virt/kvm/kvm_main.c        |  318 +++++++++++++++++++++++++++++++-------------
 8 files changed, 376 insertions(+), 92 deletions(-)

diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 5a4bc8c..7c0be8d 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -593,6 +593,49 @@ struct kvm_irqchip {
 	} chip;
 };
 
+4.27 KVM_GET/SET_VCPU_STATE
+
+Capability: KVM_CAP_VCPU_STATE
+Architectures: all (substate support may vary across architectures)
+Type: vcpu ioctl
+Parameters: struct kvm_vcpu_state (in/out)
+Returns: 0 on success, -1 on error
+
+Reads or sets one or more vcpu substates.
+
+The data structures exchanged between user space and kernel are organized
+in two layers. Layer one is the header structure kvm_vcpu_state:
+
+struct kvm_vcpu_state {
+	__u32 nsubstates; /* number of elements in substates */
+	__u32 nprocessed; /* return value: successfully processed substates */
+	struct kvm_vcpu_substate substates[0];
+};
+
+The kernel accepts up to KVM_MAX_VCPU_SUBSTATES elements in the substates
+array. An element is described by kvm_vcpu_substate:
+
+struct kvm_vcpu_substate {
+	__u32 type;	/* KVM_VCPU_STATE_* or KVM_$(ARCH)_VCPU_STATE_* */
+	__u32 pad;
+	__s64 offset;	/* payload offset to kvm_vcpu_state in bytes */
+};
+
+Layer two are the substate-specific payload structures. See section 6 for a
+list of supported substates and their payload format.
+
+Exemplary setup for a single-substate query via KVM_GET_VCPU_STATE:
+
+	struct {
+		struct kvm_vcpu_state header;
+		struct kvm_vcpu_substate substates[1];
+	} request;
+	struct kvm_regs regs;
+
+	request.header.nsubstates = 1;
+	request.header.substates[0].type = KVM_VCPU_STATE_REGS;
+	request.header.substates[0].offset = (size_t)&regs - (size_t)&request;
+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by
@@ -757,3 +800,33 @@ powerpc specific.
 		char padding[256];
 	};
 };
+
+6. Supported vcpu substates
+
+6.1 KVM_VCPU_STATE_REGS
+
+Architectures: all
+Payload: struct kvm_regs (see KVM_GET_REGS)
+Deprecates: KVM_GET/SET_REGS
+
+6.2 KVM_VCPU_STATE_SREGS
+
+Architectures: all
+Payload: struct kvm_sregs (see KVM_GET_SREGS)
+Deprecates: KVM_GET/SET_SREGS
+
+6.3 KVM_VCPU_STATE_FPU
+
+Architectures: all
+Payload: struct kvm_fpu (see KVM_GET_FPU)
+Deprecates: KVM_GET/SET_FPU
+
+6.4 KVM_VCPU_STATE_MP
+
+Architectures: x86, ia64
+Payload: struct kvm_mp_state
+Deprecates: KVM_GET/SET_MP_STATE
+
+struct kvm_mp_state {
+	__u32 mp_state;	/* KVM_MP_STATE_* */
+};
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 5fdeec5..c3450a6 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1991,3 +1991,15 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 	vcpu_put(vcpu);
 	return r;
 }
+
+int kvm_arch_vcpu_get_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate)
+{
+	return -EINVAL;
+}
+
+int kvm_arch_vcpu_set_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate)
+{
+	return -EINVAL;
+}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 5902bbc..3336ad5 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -436,3 +436,15 @@ int kvm_arch_init(void *opaque)
 void kvm_arch_exit(void)
 {
 }
+
+int kvm_arch_vcpu_get_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate)
+{
+	return -EINVAL;
+}
+
+int kvm_arch_vcpu_set_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate)
+{
+	return -EINVAL;
+}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 5445058..978ed6c 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -450,6 +450,18 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 	return -EINVAL; /* not implemented yet */
 }
 
+int kvm_arch_vcpu_get_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate)
+{
+	return -EINVAL;
+}
+
+int kvm_arch_vcpu_set_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate)
+{
+	return -EINVAL;
+}
+
 static void __vcpu_run(struct kvm_vcpu *vcpu)
 {
 	memcpy(&vcpu->arch.sie_block->gg14, &vcpu->arch.guest_gprs[14], 16);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9601bc6..685215b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4674,6 +4674,18 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_put_guest_fpu);
 
+int kvm_arch_vcpu_get_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate)
+{
+	return -EINVAL;
+}
+
+int kvm_arch_vcpu_set_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate)
+{
+	return -EINVAL;
+}
+
 void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
 {
 	if (vcpu->arch.time_page) {
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 7d8c382..421dbf8 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -393,6 +393,26 @@ struct kvm_ioeventfd {
 	__u8  pad[36];
 };
 
+/* for KVM_GET_VCPU_STATE and KVM_SET_VCPU_STATE */
+#define KVM_VCPU_STATE_REGS		0
+#define KVM_VCPU_STATE_SREGS		1
+#define KVM_VCPU_STATE_FPU		2
+#define KVM_VCPU_STATE_MP		3
+
+struct kvm_vcpu_substate {
+	__u32 type;	/* KVM_VCPU_STATE_* or KVM_$(ARCH)_VCPU_STATE_* */
+	__u32 pad;
+	__s64 offset;	/* payload offset to kvm_vcpu_state in bytes */
+};
+
+#define KVM_MAX_VCPU_SUBSTATES		64
+
+struct kvm_vcpu_state {
+	__u32 nsubstates; /* number of elements in substates */
+	__u32 nprocessed; /* return value: successfully processed substates */
+	struct kvm_vcpu_substate substates[0];
+};
+
 #define KVMIO 0xAE
 
 /*
@@ -480,6 +500,7 @@ struct kvm_ioeventfd {
 #endif
 #define KVM_CAP_IOEVENTFD 36
 #define KVM_CAP_SET_IDENTITY_MAP_ADDR 37
+#define KVM_CAP_VCPU_STATE 38
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -642,6 +663,9 @@ struct kvm_irqfd {
 /* IA64 stack access */
 #define KVM_IA64_VCPU_GET_STACK   _IOR(KVMIO,  0x9a, void *)
 #define KVM_IA64_VCPU_SET_STACK   _IOW(KVMIO,  0x9b, void *)
+/* Available with KVM_CAP_VCPU_STATE */
+#define KVM_GET_VCPU_STATE        _IOR(KVMIO,  0x9f, struct kvm_vcpu_state)
+#define KVM_SET_VCPU_STATE        _IOW(KVMIO,  0xa0, struct kvm_vcpu_state)
 
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index bd5a616..7419f32 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -332,6 +332,11 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 					struct kvm_guest_debug *dbg);
 int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run);
 
+int kvm_arch_vcpu_get_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate);
+int kvm_arch_vcpu_set_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
+			       struct kvm_vcpu_substate *substate);
+
 int kvm_arch_init(void *opaque);
 void kvm_arch_exit(void);
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c4289c0..d8dac51 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1246,124 +1246,224 @@ static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
 	return 0;
 }
 
-static long kvm_vcpu_ioctl(struct file *filp,
-			   unsigned int ioctl, unsigned long arg)
+static int kvm_vcpu_get_substate(struct kvm_vcpu *vcpu,
+				 uint8_t __user *arg_base,
+				 struct kvm_vcpu_substate *substate)
 {
-	struct kvm_vcpu *vcpu = filp->private_data;
-	void __user *argp = (void __user *)arg;
+	void __user *argp = (void __user *)arg_base + substate->offset;
 	int r;
-	struct kvm_fpu *fpu = NULL;
-	struct kvm_sregs *kvm_sregs = NULL;
 
-	if (vcpu->kvm->mm != current->mm)
-		return -EIO;
-	switch (ioctl) {
-	case KVM_RUN:
-		r = -EINVAL;
-		if (arg)
-			goto out;
-		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
-		break;
-	case KVM_GET_REGS: {
+	switch (substate->type) {
+	case KVM_VCPU_STATE_REGS: {
 		struct kvm_regs *kvm_regs;
 
-		r = -ENOMEM;
 		kvm_regs = kzalloc(sizeof(struct kvm_regs), GFP_KERNEL);
+		r = -ENOMEM;
 		if (!kvm_regs)
-			goto out;
+			break;
 		r = kvm_arch_vcpu_ioctl_get_regs(vcpu, kvm_regs);
 		if (r)
-			goto out_free1;
+			goto out_free_regs;
 		r = -EFAULT;
 		if (copy_to_user(argp, kvm_regs, sizeof(struct kvm_regs)))
-			goto out_free1;
+			goto out_free_regs;
 		r = 0;
-out_free1:
+out_free_regs:
 		kfree(kvm_regs);
 		break;
 	}
-	case KVM_SET_REGS: {
-		struct kvm_regs *kvm_regs;
+	case KVM_VCPU_STATE_SREGS: {
+		struct kvm_sregs *kvm_sregs;
 
+		kvm_sregs = kzalloc(sizeof(struct kvm_sregs), GFP_KERNEL);
 		r = -ENOMEM;
-		kvm_regs = kzalloc(sizeof(struct kvm_regs), GFP_KERNEL);
-		if (!kvm_regs)
-			goto out;
-		r = -EFAULT;
-		if (copy_from_user(kvm_regs, argp, sizeof(struct kvm_regs)))
-			goto out_free2;
-		r = kvm_arch_vcpu_ioctl_set_regs(vcpu, kvm_regs);
+		if (!kvm_sregs)
+			break;
+		r = kvm_arch_vcpu_ioctl_get_sregs(vcpu, kvm_sregs);
 		if (r)
-			goto out_free2;
+			goto out_free_sregs;
+		r = -EFAULT;
+		if (copy_to_user(argp, kvm_sregs, sizeof(struct kvm_sregs)))
+			goto out_free_sregs;
 		r = 0;
-out_free2:
-		kfree(kvm_regs);
+out_free_sregs:
+		kfree(kvm_sregs);
 		break;
 	}
-	case KVM_GET_SREGS: {
-		kvm_sregs = kzalloc(sizeof(struct kvm_sregs), GFP_KERNEL);
+	case KVM_VCPU_STATE_FPU: {
+		struct kvm_fpu *kvm_fpu;
+
+		kvm_fpu = kzalloc(sizeof(struct kvm_fpu), GFP_KERNEL);
 		r = -ENOMEM;
-		if (!kvm_sregs)
-			goto out;
-		r = kvm_arch_vcpu_ioctl_get_sregs(vcpu, kvm_sregs);
+		if (!kvm_fpu)
+			break;
+		r = kvm_arch_vcpu_ioctl_get_fpu(vcpu, kvm_fpu);
 		if (r)
-			goto out;
+			break;
 		r = -EFAULT;
-		if (copy_to_user(argp, kvm_sregs, sizeof(struct kvm_sregs)))
-			goto out;
+		if (copy_to_user(argp, kvm_fpu, sizeof(struct kvm_fpu)))
+			goto out_free_fpu;
+		r = 0;
+out_free_fpu:
+		kfree(kvm_fpu);
+		break;
+	}
+	case KVM_VCPU_STATE_MP: {
+		struct kvm_mp_state mp_state;
+
+		r = kvm_arch_vcpu_ioctl_get_mpstate(vcpu, &mp_state);
+		if (r)
+			break;
+		r = -EFAULT;
+		if (copy_to_user(argp, &mp_state, sizeof(struct kvm_mp_state)))
+			break;
+		r = 0;
+		break;
+	}
+	default:
+		r = kvm_arch_vcpu_get_substate(vcpu, arg_base, substate);
+	}
+	return r;
+}
+
+static int kvm_vcpu_set_substate(struct kvm_vcpu *vcpu,
+				 uint8_t __user *arg_base,
+				 struct kvm_vcpu_substate *substate)
+{
+	void __user *argp = (void __user *)arg_base + substate->offset;
+	int r;
+
+	switch (substate->type) {
+	case KVM_VCPU_STATE_REGS: {
+		struct kvm_regs *kvm_regs;
+
+		kvm_regs = kzalloc(sizeof(struct kvm_regs), GFP_KERNEL);
+		r = -ENOMEM;
+		if (!kvm_regs)
+			break;
+		r = -EFAULT;
+		if (copy_from_user(kvm_regs, argp, sizeof(struct kvm_regs)))
+			goto out_free_regs;
+		r = kvm_arch_vcpu_ioctl_set_regs(vcpu, kvm_regs);
+		if (r)
+			goto out_free_regs;
 		r = 0;
+out_free_regs:
+		kfree(kvm_regs);
 		break;
 	}
-	case KVM_SET_SREGS: {
+	case KVM_VCPU_STATE_SREGS: {
+		struct kvm_sregs *kvm_sregs;
+
 		kvm_sregs = kmalloc(sizeof(struct kvm_sregs), GFP_KERNEL);
 		r = -ENOMEM;
 		if (!kvm_sregs)
-			goto out;
+			break;
 		r = -EFAULT;
 		if (copy_from_user(kvm_sregs, argp, sizeof(struct kvm_sregs)))
-			goto out;
+			goto out_free_sregs;
 		r = kvm_arch_vcpu_ioctl_set_sregs(vcpu, kvm_sregs);
 		if (r)
-			goto out;
+			goto out_free_sregs;
 		r = 0;
+out_free_sregs:
+		kfree(kvm_sregs);
 		break;
 	}
-	case KVM_GET_MP_STATE: {
-		struct kvm_mp_state mp_state;
+	case KVM_VCPU_STATE_FPU: {
+		struct kvm_fpu *kvm_fpu;
 
-		r = kvm_arch_vcpu_ioctl_get_mpstate(vcpu, &mp_state);
-		if (r)
-			goto out;
+		kvm_fpu = kmalloc(sizeof(struct kvm_fpu), GFP_KERNEL);
+		r = -ENOMEM;
+		if (!kvm_fpu)
+			break;
 		r = -EFAULT;
-		if (copy_to_user(argp, &mp_state, sizeof mp_state))
-			goto out;
+		if (copy_from_user(kvm_fpu, argp, sizeof(struct kvm_fpu)))
+			goto out_free_fpu;
+		r = kvm_arch_vcpu_ioctl_set_fpu(vcpu, kvm_fpu);
+		if (r)
+			goto out_free_fpu;
 		r = 0;
+out_free_fpu:
+		kfree(kvm_fpu);
 		break;
 	}
-	case KVM_SET_MP_STATE: {
+	case KVM_VCPU_STATE_MP: {
 		struct kvm_mp_state mp_state;
 
 		r = -EFAULT;
-		if (copy_from_user(&mp_state, argp, sizeof mp_state))
-			goto out;
+		if (copy_from_user(&mp_state, argp,
+				   sizeof(struct kvm_mp_state)))
+			break;
 		r = kvm_arch_vcpu_ioctl_set_mpstate(vcpu, &mp_state);
-		if (r)
-			goto out;
-		r = 0;
 		break;
 	}
+	default:
+		r = kvm_arch_vcpu_set_substate(vcpu, arg_base, substate);
+	}
+	return r;
+}
+
+
+static long kvm_vcpu_ioctl(struct file *filp,
+			   unsigned int ioctl, unsigned long arg)
+{
+	struct kvm_vcpu *vcpu = filp->private_data;
+	void __user *argp = (void __user *)arg;
+	struct kvm_vcpu_substate substate;
+	int r;
+
+	if (vcpu->kvm->mm != current->mm)
+		return -EIO;
+	switch (ioctl) {
+	case KVM_RUN:
+		r = -EINVAL;
+		if (arg)
+			break;
+		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
+		break;
+	case KVM_GET_REGS:
+		substate.type = KVM_VCPU_STATE_REGS;
+		substate.offset = 0;
+		r = kvm_vcpu_get_substate(vcpu, argp, &substate);
+		break;
+	case KVM_SET_REGS:
+		substate.type = KVM_VCPU_STATE_REGS;
+		substate.offset = 0;
+		r = kvm_vcpu_set_substate(vcpu, argp, &substate);
+		break;
+	case KVM_GET_SREGS:
+		substate.type = KVM_VCPU_STATE_SREGS;
+		substate.offset = 0;
+		r = kvm_vcpu_get_substate(vcpu, argp, &substate);
+		break;
+	case KVM_SET_SREGS:
+		substate.type = KVM_VCPU_STATE_SREGS;
+		substate.offset = 0;
+		r = kvm_vcpu_set_substate(vcpu, argp, &substate);
+		break;
+	case KVM_GET_MP_STATE:
+		substate.type = KVM_VCPU_STATE_MP;
+		substate.offset = 0;
+		r = kvm_vcpu_get_substate(vcpu, argp, &substate);
+		break;
+	case KVM_SET_MP_STATE:
+		substate.type = KVM_VCPU_STATE_MP;
+		substate.offset = 0;
+		r = kvm_vcpu_set_substate(vcpu, argp, &substate);
+		break;
 	case KVM_TRANSLATE: {
 		struct kvm_translation tr;
 
 		r = -EFAULT;
 		if (copy_from_user(&tr, argp, sizeof tr))
-			goto out;
+			break;
 		r = kvm_arch_vcpu_ioctl_translate(vcpu, &tr);
 		if (r)
-			goto out;
+			break;
 		r = -EFAULT;
 		if (copy_to_user(argp, &tr, sizeof tr))
-			goto out;
+			break;
 		r = 0;
 		break;
 	}
@@ -1372,10 +1472,10 @@ out_free2:
 
 		r = -EFAULT;
 		if (copy_from_user(&dbg, argp, sizeof dbg))
-			goto out;
+			break;
 		r = kvm_arch_vcpu_ioctl_set_guest_debug(vcpu, &dbg);
 		if (r)
-			goto out;
+			break;
 		r = 0;
 		break;
 	}
@@ -1389,53 +1489,86 @@ out_free2:
 			r = -EFAULT;
 			if (copy_from_user(&kvm_sigmask, argp,
 					   sizeof kvm_sigmask))
-				goto out;
+				break;
 			r = -EINVAL;
 			if (kvm_sigmask.len != sizeof sigset)
-				goto out;
+				break;
 			r = -EFAULT;
 			if (copy_from_user(&sigset, sigmask_arg->sigset,
 					   sizeof sigset))
-				goto out;
+				break;
 			p = &sigset;
 		}
 		r = kvm_vcpu_ioctl_set_sigmask(vcpu, &sigset);
 		break;
 	}
-	case KVM_GET_FPU: {
-		fpu = kzalloc(sizeof(struct kvm_fpu), GFP_KERNEL);
-		r = -ENOMEM;
-		if (!fpu)
-			goto out;
-		r = kvm_arch_vcpu_ioctl_get_fpu(vcpu, fpu);
-		if (r)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(argp, fpu, sizeof(struct kvm_fpu)))
-			goto out;
-		r = 0;
+	case KVM_GET_FPU:
+		substate.type = KVM_VCPU_STATE_FPU;
+		substate.offset = 0;
+		r = kvm_vcpu_get_substate(vcpu, argp, &substate);
 		break;
-	}
-	case KVM_SET_FPU: {
-		fpu = kmalloc(sizeof(struct kvm_fpu), GFP_KERNEL);
-		r = -ENOMEM;
-		if (!fpu)
-			goto out;
+	case KVM_SET_FPU:
+		substate.type = KVM_VCPU_STATE_FPU;
+		substate.offset = 0;
+		r = kvm_vcpu_set_substate(vcpu, argp, &substate);
+		break;
+	case KVM_GET_VCPU_STATE:
+	case KVM_SET_VCPU_STATE: {
+		struct kvm_vcpu_state __user *user_head = argp;
+		struct kvm_vcpu_substate *substates = NULL;
+		uint8_t __user *arg_base = argp;
+		struct kvm_vcpu_state head;
+		size_t size;
+		int i;
+
 		r = -EFAULT;
-		if (copy_from_user(fpu, argp, sizeof(struct kvm_fpu)))
-			goto out;
-		r = kvm_arch_vcpu_ioctl_set_fpu(vcpu, fpu);
-		if (r)
-			goto out;
+		if (copy_from_user(&head, user_head,
+				   sizeof(struct kvm_vcpu_state)))
+			break;
+
+		head.nprocessed = 0;
+
+		size = head.nsubstates * sizeof(struct kvm_vcpu_substate);
+		if (head.nsubstates <= 1) {
+			substates = &substate;
+		} else {
+			r = -E2BIG;
+			if (head.nsubstates > KVM_MAX_VCPU_SUBSTATES)
+				goto vcpu_state_out;
+
+			substates = kmalloc(size, GFP_KERNEL);
+			r = -ENOMEM;
+			if (!substates)
+				goto vcpu_state_out;
+		}
+
+		r = -EFAULT;
+		if (copy_from_user(substates, user_head->substates, size))
+			goto vcpu_state_out;
+
+		for (i = 0; i < head.nsubstates; i++) {
+			if (ioctl == KVM_GET_VCPU_STATE)
+				r = kvm_vcpu_get_substate(vcpu, arg_base,
+							  &substates[i]);
+			else
+				r = kvm_vcpu_set_substate(vcpu, arg_base,
+							  &substates[i]);
+			if (r < 0)
+				goto vcpu_state_out;
+			head.nprocessed++;
+		}
 		r = 0;
+vcpu_state_out:
+		if (copy_to_user(&user_head->nprocessed, &head.nprocessed,
+				 sizeof(head.nprocessed)))
+			r = -EFAULT;
+		if (head.nsubstates > 1)
+			kfree(substates);
 		break;
 	}
 	default:
 		r = kvm_arch_vcpu_ioctl(filp, ioctl, arg);
 	}
-out:
-	kfree(fpu);
-	kfree(kvm_sregs);
 	return r;
 }
 
@@ -1601,6 +1734,7 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	case KVM_CAP_USER_MEMORY:
 	case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
 	case KVM_CAP_JOIN_MEMORY_REGIONS_WORKS:
+	case KVM_CAP_VCPU_STATE:
 #ifdef CONFIG_KVM_APIC_ARCHITECTURE
 	case KVM_CAP_SET_BOOT_CPU_ID:
 #endif


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/4] KVM: x86: Add support for KVM_GET/SET_VCPU_STATE
  2009-10-15 17:05 [PATCH v2 0/4] Extensible VCPU state IOCTL Jan Kiszka
  2009-10-15 17:05 ` [PATCH v2 1/4] KVM: Reorder IOCTLs in main kvm.h Jan Kiszka
@ 2009-10-15 17:05 ` Jan Kiszka
  2009-10-15 17:05 ` [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states Jan Kiszka
  2009-10-15 17:05 ` [PATCH v2 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL Jan Kiszka
  3 siblings, 0 replies; 15+ messages in thread
From: Jan Kiszka @ 2009-10-15 17:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

Add support for getting/setting MSRs, CPUID tree, and the LACPIC via the
new VCPU state interface. Also in this case we convert the existing
IOCTLs to use the new infrastructure internally.

The MSR interface has to be extended to pass back the number of
processed MSRs via the header structure instead of the return code as
the latter is not available with the new IOCTL. The semantic of the
original KVM_GET/SET_MSRS is not affected by this change.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 Documentation/kvm/api.txt  |   18 ++++
 arch/x86/include/asm/kvm.h |    8 +-
 arch/x86/kvm/x86.c         |  209 ++++++++++++++++++++++++++++----------------
 3 files changed, 156 insertions(+), 79 deletions(-)

diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 7c0be8d..bee5bbd 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -830,3 +830,21 @@ Deprecates: KVM_GET/SET_MP_STATE
 struct kvm_mp_state {
 	__u32 mp_state;	/* KVM_MP_STATE_* */
 };
+
+6.5 KVM_X86_VCPU_STATE_MSRS
+
+Architectures: x86
+Payload: struct kvm_msrs (see KVM_GET_MSRS)
+Deprecates: KVM_GET/SET_MSRS
+
+6.6 KVM_X86_VCPU_STATE_CPUID
+
+Architectures: x86
+Payload: struct kvm_cpuid2
+Deprecates: KVM_GET/SET_CPUID2
+
+6.7 KVM_X86_VCPU_STATE_LAPIC
+
+Architectures: x86
+Payload: struct kvm_lapic
+Deprecates: KVM_GET/SET_LAPIC
diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
index f02e87a..326615a 100644
--- a/arch/x86/include/asm/kvm.h
+++ b/arch/x86/include/asm/kvm.h
@@ -150,7 +150,7 @@ struct kvm_msr_entry {
 /* for KVM_GET_MSRS and KVM_SET_MSRS */
 struct kvm_msrs {
 	__u32 nmsrs; /* number of msrs in entries */
-	__u32 pad;
+	__u32 nprocessed; /* return value: successfully processed entries */
 
 	struct kvm_msr_entry entries[0];
 };
@@ -251,4 +251,10 @@ struct kvm_reinject_control {
 	__u8 pit_reinject;
 	__u8 reserved[31];
 };
+
+/* for KVM_GET/SET_VCPU_STATE */
+#define KVM_X86_VCPU_STATE_MSRS		1000
+#define KVM_X86_VCPU_STATE_CPUID	1001
+#define KVM_X86_VCPU_STATE_LAPIC	1002
+
 #endif /* _ASM_X86_KVM_H */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 685215b..46fad88 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1182,11 +1182,11 @@ static int __msr_io(struct kvm_vcpu *vcpu, struct kvm_msrs *msrs,
 static int msr_io(struct kvm_vcpu *vcpu, struct kvm_msrs __user *user_msrs,
 		  int (*do_msr)(struct kvm_vcpu *vcpu,
 				unsigned index, u64 *data),
-		  int writeback)
+		  int writeback, int write_nprocessed)
 {
 	struct kvm_msrs msrs;
 	struct kvm_msr_entry *entries;
-	int r, n;
+	int r;
 	unsigned size;
 
 	r = -EFAULT;
@@ -1207,15 +1207,22 @@ static int msr_io(struct kvm_vcpu *vcpu, struct kvm_msrs __user *user_msrs,
 	if (copy_from_user(entries, user_msrs->entries, size))
 		goto out_free;
 
-	r = n = __msr_io(vcpu, &msrs, entries, do_msr);
+	r = __msr_io(vcpu, &msrs, entries, do_msr);
 	if (r < 0)
 		goto out_free;
 
+	msrs.nprocessed = r;
+
 	r = -EFAULT;
+	if (write_nprocessed &&
+	    copy_to_user(&user_msrs->nprocessed, &msrs.nprocessed,
+			 sizeof(msrs.nprocessed)))
+		goto out_free;
+
 	if (writeback && copy_to_user(user_msrs->entries, entries, size))
 		goto out_free;
 
-	r = n;
+	r = msrs.nprocessed;
 
 out_free:
 	vfree(entries);
@@ -1792,55 +1799,36 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 {
 	struct kvm_vcpu *vcpu = filp->private_data;
 	void __user *argp = (void __user *)arg;
+	struct kvm_vcpu_substate substate;
 	int r;
-	struct kvm_lapic_state *lapic = NULL;
 
 	switch (ioctl) {
-	case KVM_GET_LAPIC: {
-		lapic = kzalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
-
-		r = -ENOMEM;
-		if (!lapic)
-			goto out;
-		r = kvm_vcpu_ioctl_get_lapic(vcpu, lapic);
-		if (r)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(argp, lapic, sizeof(struct kvm_lapic_state)))
-			goto out;
-		r = 0;
+	case KVM_GET_LAPIC:
+		substate.type = KVM_X86_VCPU_STATE_LAPIC;
+		substate.offset = 0;
+		r = kvm_arch_vcpu_get_substate(vcpu, argp, &substate);
 		break;
-	}
-	case KVM_SET_LAPIC: {
-		lapic = kmalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
-		r = -ENOMEM;
-		if (!lapic)
-			goto out;
-		r = -EFAULT;
-		if (copy_from_user(lapic, argp, sizeof(struct kvm_lapic_state)))
-			goto out;
-		r = kvm_vcpu_ioctl_set_lapic(vcpu, lapic);
-		if (r)
-			goto out;
-		r = 0;
+	case KVM_SET_LAPIC:
+		substate.type = KVM_X86_VCPU_STATE_LAPIC;
+		substate.offset = 0;
+		r = kvm_arch_vcpu_set_substate(vcpu, argp, &substate);
 		break;
-	}
 	case KVM_INTERRUPT: {
 		struct kvm_interrupt irq;
 
 		r = -EFAULT;
 		if (copy_from_user(&irq, argp, sizeof irq))
-			goto out;
+			break;
 		r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
 		if (r)
-			goto out;
+			break;
 		r = 0;
 		break;
 	}
 	case KVM_NMI: {
 		r = kvm_vcpu_ioctl_nmi(vcpu);
 		if (r)
-			goto out;
+			break;
 		r = 0;
 		break;
 	}
@@ -1850,60 +1838,40 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		r = -EFAULT;
 		if (copy_from_user(&cpuid, cpuid_arg, sizeof cpuid))
-			goto out;
+			break;
 		r = kvm_vcpu_ioctl_set_cpuid(vcpu, &cpuid, cpuid_arg->entries);
 		if (r)
-			goto out;
+			break;
 		break;
 	}
-	case KVM_SET_CPUID2: {
-		struct kvm_cpuid2 __user *cpuid_arg = argp;
-		struct kvm_cpuid2 cpuid;
-
-		r = -EFAULT;
-		if (copy_from_user(&cpuid, cpuid_arg, sizeof cpuid))
-			goto out;
-		r = kvm_vcpu_ioctl_set_cpuid2(vcpu, &cpuid,
-					      cpuid_arg->entries);
-		if (r)
-			goto out;
+	case KVM_GET_CPUID2:
+		substate.type = KVM_X86_VCPU_STATE_CPUID;
+		substate.offset = 0;
+		r = kvm_arch_vcpu_get_substate(vcpu, argp, &substate);
 		break;
-	}
-	case KVM_GET_CPUID2: {
-		struct kvm_cpuid2 __user *cpuid_arg = argp;
-		struct kvm_cpuid2 cpuid;
-
-		r = -EFAULT;
-		if (copy_from_user(&cpuid, cpuid_arg, sizeof cpuid))
-			goto out;
-		r = kvm_vcpu_ioctl_get_cpuid2(vcpu, &cpuid,
-					      cpuid_arg->entries);
-		if (r)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(cpuid_arg, &cpuid, sizeof cpuid))
-			goto out;
-		r = 0;
+	case KVM_SET_CPUID2:
+		substate.type = KVM_X86_VCPU_STATE_CPUID;
+		substate.offset = 0;
+		r = kvm_arch_vcpu_set_substate(vcpu, argp, &substate);
 		break;
-	}
 	case KVM_GET_MSRS:
-		r = msr_io(vcpu, argp, kvm_get_msr, 1);
+		r = msr_io(vcpu, argp, kvm_get_msr, 1, 0);
 		break;
 	case KVM_SET_MSRS:
-		r = msr_io(vcpu, argp, do_set_msr, 0);
+		r = msr_io(vcpu, argp, do_set_msr, 0, 0);
 		break;
 	case KVM_TPR_ACCESS_REPORTING: {
 		struct kvm_tpr_access_ctl tac;
 
 		r = -EFAULT;
 		if (copy_from_user(&tac, argp, sizeof tac))
-			goto out;
+			break;
 		r = vcpu_ioctl_tpr_access_reporting(vcpu, &tac);
 		if (r)
-			goto out;
+			break;
 		r = -EFAULT;
 		if (copy_to_user(argp, &tac, sizeof tac))
-			goto out;
+			break;
 		r = 0;
 		break;
 	};
@@ -1912,10 +1880,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		r = -EINVAL;
 		if (!irqchip_in_kernel(vcpu->kvm))
-			goto out;
+			break;
 		r = -EFAULT;
 		if (copy_from_user(&va, argp, sizeof va))
-			goto out;
+			break;
 		r = 0;
 		kvm_lapic_set_vapic_addr(vcpu, va.vapic_addr);
 		break;
@@ -1925,7 +1893,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		r = -EFAULT;
 		if (copy_from_user(&mcg_cap, argp, sizeof mcg_cap))
-			goto out;
+			break;
 		r = kvm_vcpu_ioctl_x86_setup_mce(vcpu, mcg_cap);
 		break;
 	}
@@ -1934,15 +1902,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		r = -EFAULT;
 		if (copy_from_user(&mce, argp, sizeof mce))
-			goto out;
+			break;
 		r = kvm_vcpu_ioctl_x86_set_mce(vcpu, &mce);
 		break;
 	}
 	default:
 		r = -EINVAL;
 	}
-out:
-	kfree(lapic);
 	return r;
 }
 
@@ -4677,13 +4643,100 @@ EXPORT_SYMBOL_GPL(kvm_put_guest_fpu);
 int kvm_arch_vcpu_get_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
 			       struct kvm_vcpu_substate *substate)
 {
-	return -EINVAL;
+	void __user *argp = (void __user *)arg_base + substate->offset;
+	int r;
+
+	switch (substate->type) {
+	case KVM_X86_VCPU_STATE_MSRS:
+		r = msr_io(vcpu, argp, kvm_get_msr, 1, 1);
+		break;
+	case KVM_X86_VCPU_STATE_CPUID: {
+		struct kvm_cpuid2 __user *cpuid_arg = argp;
+		struct kvm_cpuid2 cpuid;
+
+		r = -EFAULT;
+		if (copy_from_user(&cpuid, cpuid_arg,
+				   sizeof(struct kvm_cpuid2)))
+			break;
+		r = kvm_vcpu_ioctl_get_cpuid2(vcpu, &cpuid,
+					      cpuid_arg->entries);
+		if (r)
+			break;
+		r = -EFAULT;
+		if (copy_to_user(cpuid_arg, &cpuid, sizeof(struct kvm_cpuid2)))
+			break;
+		r = 0;
+		break;
+	}
+	case KVM_X86_VCPU_STATE_LAPIC: {
+		struct kvm_lapic_state *lapic;
+
+		lapic = kzalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
+		r = -ENOMEM;
+		if (!lapic)
+			break;
+		r = kvm_vcpu_ioctl_get_lapic(vcpu, lapic);
+		if (r)
+			goto out_free_lapic;
+		r = -EFAULT;
+		if (copy_to_user(argp, lapic, sizeof(struct kvm_lapic_state)))
+			goto out_free_lapic;
+		r = 0;
+out_free_lapic:
+		kfree(lapic);
+		break;
+	}
+	default:
+		r = -EINVAL;
+	}
+	return r;
 }
 
 int kvm_arch_vcpu_set_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
 			       struct kvm_vcpu_substate *substate)
 {
-	return -EINVAL;
+	void __user *argp = (void __user *)arg_base + substate->offset;
+	int r;
+
+	switch (substate->type) {
+	case KVM_X86_VCPU_STATE_MSRS:
+		r = msr_io(vcpu, argp, do_set_msr, 0, 1);
+		break;
+	case KVM_X86_VCPU_STATE_CPUID: {
+		struct kvm_cpuid2 __user *cpuid_arg = argp;
+		struct kvm_cpuid2 cpuid;
+
+		r = -EFAULT;
+		if (copy_from_user(&cpuid, cpuid_arg,
+				   sizeof(struct kvm_cpuid2)))
+			break;
+		r = kvm_vcpu_ioctl_set_cpuid2(vcpu, &cpuid,
+					      cpuid_arg->entries);
+		break;
+	}
+	case KVM_X86_VCPU_STATE_LAPIC: {
+		struct kvm_lapic_state *lapic;
+
+		lapic = kmalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
+		r = -ENOMEM;
+		if (!lapic)
+			break;
+		r = -EFAULT;
+		if (copy_from_user(lapic, argp,
+				   sizeof(struct kvm_lapic_state)))
+			goto out_free_lapic;
+		r = kvm_vcpu_ioctl_set_lapic(vcpu, lapic);
+		if (r)
+			goto out_free_lapic;
+		r = 0;
+out_free_lapic:
+		kfree(lapic);
+		break;
+	}
+	default:
+		r = -EINVAL;
+	}
+	return r;
 }
 
 void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-15 17:05 ` [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states Jan Kiszka
@ 2009-10-19 20:32   ` Marcelo Tosatti
  2009-10-19 20:39     ` Gleb Natapov
  2009-10-20  8:52     ` Jan Kiszka
  0 siblings, 2 replies; 15+ messages in thread
From: Marcelo Tosatti @ 2009-10-19 20:32 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, kvm

On Thu, Oct 15, 2009 at 07:05:36PM +0200, Jan Kiszka wrote:
> This plugs an NMI-related hole in the VCPU synchronization between
> kernel and user space. So far, neither pending NMIs nor the inhibit NMI
> mask was properly read/set which was able to cause problems on
> vmsave/restore, live migration and system reset. Fix it by making use
> of the new VCPU substate interface.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
>  Documentation/kvm/api.txt       |   12 ++++++++++++
>  arch/x86/include/asm/kvm.h      |    7 +++++++
>  arch/x86/include/asm/kvm_host.h |    2 ++
>  arch/x86/kvm/svm.c              |   22 ++++++++++++++++++++++
>  arch/x86/kvm/vmx.c              |   30 ++++++++++++++++++++++++++++++
>  arch/x86/kvm/x86.c              |   26 ++++++++++++++++++++++++++
>  6 files changed, 99 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
> index bee5bbd..e483edb 100644
> --- a/Documentation/kvm/api.txt
> +++ b/Documentation/kvm/api.txt
> @@ -848,3 +848,15 @@ Deprecates: KVM_GET/SET_CPUID2
>  Architectures: x86
>  Payload: struct kvm_lapic
>  Deprecates: KVM_GET/SET_LAPIC
> +
> +6.8 KVM_X86_VCPU_STATE_NMI
> +
> +Architectures: x86
> +Payload: struct kvm_nmi_state
> +Deprecates: -
> +
> +struct kvm_nmi_state {
> +       __u8 pending;
> +       __u8 masked;
> +       __u8 pad1[6];

Don't you also have to save "nmi_injected", in case of failure during
NMI delivery.

BTW, what happens to exceptions that fail to be delivered? Can't see
where they are saved/restored across migration.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-19 20:32   ` Marcelo Tosatti
@ 2009-10-19 20:39     ` Gleb Natapov
  2009-10-19 23:34       ` Avi Kivity
  2009-10-20  8:52     ` Jan Kiszka
  1 sibling, 1 reply; 15+ messages in thread
From: Gleb Natapov @ 2009-10-19 20:39 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Jan Kiszka, Avi Kivity, kvm

On Mon, Oct 19, 2009 at 06:32:54PM -0200, Marcelo Tosatti wrote:
> On Thu, Oct 15, 2009 at 07:05:36PM +0200, Jan Kiszka wrote:
> > This plugs an NMI-related hole in the VCPU synchronization between
> > kernel and user space. So far, neither pending NMIs nor the inhibit NMI
> > mask was properly read/set which was able to cause problems on
> > vmsave/restore, live migration and system reset. Fix it by making use
> > of the new VCPU substate interface.
> > 
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> > 
> >  Documentation/kvm/api.txt       |   12 ++++++++++++
> >  arch/x86/include/asm/kvm.h      |    7 +++++++
> >  arch/x86/include/asm/kvm_host.h |    2 ++
> >  arch/x86/kvm/svm.c              |   22 ++++++++++++++++++++++
> >  arch/x86/kvm/vmx.c              |   30 ++++++++++++++++++++++++++++++
> >  arch/x86/kvm/x86.c              |   26 ++++++++++++++++++++++++++
> >  6 files changed, 99 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
> > index bee5bbd..e483edb 100644
> > --- a/Documentation/kvm/api.txt
> > +++ b/Documentation/kvm/api.txt
> > @@ -848,3 +848,15 @@ Deprecates: KVM_GET/SET_CPUID2
> >  Architectures: x86
> >  Payload: struct kvm_lapic
> >  Deprecates: KVM_GET/SET_LAPIC
> > +
> > +6.8 KVM_X86_VCPU_STATE_NMI
> > +
> > +Architectures: x86
> > +Payload: struct kvm_nmi_state
> > +Deprecates: -
> > +
> > +struct kvm_nmi_state {
> > +       __u8 pending;
> > +       __u8 masked;
> > +       __u8 pad1[6];
> 
> Don't you also have to save "nmi_injected", in case of failure during
> NMI delivery.
> 
> BTW, what happens to exceptions that fail to be delivered? Can't see
> where they are saved/restored across migration.
> 
The instruction that caused an exception will be re-executed after
migration and exception will be regenerated. But I think we should
migrate exception anyway for completeness.

--
			Gleb.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-19 20:39     ` Gleb Natapov
@ 2009-10-19 23:34       ` Avi Kivity
  2009-10-20  8:56         ` Jan Kiszka
  0 siblings, 1 reply; 15+ messages in thread
From: Avi Kivity @ 2009-10-19 23:34 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Marcelo Tosatti, Jan Kiszka, kvm

On 10/20/2009 05:39 AM, Gleb Natapov wrote:
>
>> BTW, what happens to exceptions that fail to be delivered? Can't see
>> where they are saved/restored across migration.
>>
>>      
> The instruction that caused an exception will be re-executed after
> migration and exception will be regenerated.

Except for debug exceptions (traps).

> But I think we should
> migrate exception anyway for completeness.
>    

Yes.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-19 20:32   ` Marcelo Tosatti
  2009-10-19 20:39     ` Gleb Natapov
@ 2009-10-20  8:52     ` Jan Kiszka
  1 sibling, 0 replies; 15+ messages in thread
From: Jan Kiszka @ 2009-10-20  8:52 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm@vger.kernel.org

Marcelo Tosatti wrote:
> On Thu, Oct 15, 2009 at 07:05:36PM +0200, Jan Kiszka wrote:
>> This plugs an NMI-related hole in the VCPU synchronization between
>> kernel and user space. So far, neither pending NMIs nor the inhibit NMI
>> mask was properly read/set which was able to cause problems on
>> vmsave/restore, live migration and system reset. Fix it by making use
>> of the new VCPU substate interface.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>>  Documentation/kvm/api.txt       |   12 ++++++++++++
>>  arch/x86/include/asm/kvm.h      |    7 +++++++
>>  arch/x86/include/asm/kvm_host.h |    2 ++
>>  arch/x86/kvm/svm.c              |   22 ++++++++++++++++++++++
>>  arch/x86/kvm/vmx.c              |   30 ++++++++++++++++++++++++++++++
>>  arch/x86/kvm/x86.c              |   26 ++++++++++++++++++++++++++
>>  6 files changed, 99 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
>> index bee5bbd..e483edb 100644
>> --- a/Documentation/kvm/api.txt
>> +++ b/Documentation/kvm/api.txt
>> @@ -848,3 +848,15 @@ Deprecates: KVM_GET/SET_CPUID2
>>  Architectures: x86
>>  Payload: struct kvm_lapic
>>  Deprecates: KVM_GET/SET_LAPIC
>> +
>> +6.8 KVM_X86_VCPU_STATE_NMI
>> +
>> +Architectures: x86
>> +Payload: struct kvm_nmi_state
>> +Deprecates: -
>> +
>> +struct kvm_nmi_state {
>> +       __u8 pending;
>> +       __u8 masked;
>> +       __u8 pad1[6];
> 
> Don't you also have to save "nmi_injected", in case of failure during
> NMI delivery.
> 

Something made me think it's not required. Don't ask me what, it was
wrong anyway. Will roll out -v3 for this patch.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-19 23:34       ` Avi Kivity
@ 2009-10-20  8:56         ` Jan Kiszka
  2009-10-20  9:06           ` Avi Kivity
  0 siblings, 1 reply; 15+ messages in thread
From: Jan Kiszka @ 2009-10-20  8:56 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Gleb Natapov, Marcelo Tosatti, kvm@vger.kernel.org

Avi Kivity wrote:
> On 10/20/2009 05:39 AM, Gleb Natapov wrote:
>>> BTW, what happens to exceptions that fail to be delivered? Can't see
>>> where they are saved/restored across migration.
>>>
>>>      
>> The instruction that caused an exception will be re-executed after
>> migration and exception will be regenerated.
> 
> Except for debug exceptions (traps).
> 
>> But I think we should
>> migrate exception anyway for completeness.
>>    
> 
> Yes.

So save/restore kvm_vcpu_arch::exception? As another substate or as part
of a generalized NMI substate?

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-20  8:56         ` Jan Kiszka
@ 2009-10-20  9:06           ` Avi Kivity
  2009-10-20  9:08             ` Gleb Natapov
  0 siblings, 1 reply; 15+ messages in thread
From: Avi Kivity @ 2009-10-20  9:06 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Gleb Natapov, Marcelo Tosatti, kvm@vger.kernel.org

On 10/20/2009 05:56 PM, Jan Kiszka wrote:
> So save/restore kvm_vcpu_arch::exception? As another substate or as part
> of a generalized NMI substate?
>    

Yes.  It's not part of an nmi substate, but both can be part of an 
exception substate (but need to look at the docs vewy cawefuwy to make 
sure we don't screw up again).

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-20  9:06           ` Avi Kivity
@ 2009-10-20  9:08             ` Gleb Natapov
  2009-10-20  9:14               ` Avi Kivity
  0 siblings, 1 reply; 15+ messages in thread
From: Gleb Natapov @ 2009-10-20  9:08 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Jan Kiszka, Marcelo Tosatti, kvm@vger.kernel.org

On Tue, Oct 20, 2009 at 06:06:36PM +0900, Avi Kivity wrote:
> On 10/20/2009 05:56 PM, Jan Kiszka wrote:
> >So save/restore kvm_vcpu_arch::exception? As another substate or as part
> >of a generalized NMI substate?
> 
> Yes.  It's not part of an nmi substate, but both can be part of an
> exception substate (but need to look at the docs vewy cawefuwy to
> make sure we don't screw up again).
> 
What do you mean? How they can be both part of exception substate?

--
			Gleb.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-20  9:08             ` Gleb Natapov
@ 2009-10-20  9:14               ` Avi Kivity
  2009-10-20 11:13                 ` Marcelo Tosatti
  0 siblings, 1 reply; 15+ messages in thread
From: Avi Kivity @ 2009-10-20  9:14 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Jan Kiszka, Marcelo Tosatti, kvm@vger.kernel.org

On 10/20/2009 06:08 PM, Gleb Natapov wrote:
> On Tue, Oct 20, 2009 at 06:06:36PM +0900, Avi Kivity wrote:
>    
>> On 10/20/2009 05:56 PM, Jan Kiszka wrote:
>>      
>>> So save/restore kvm_vcpu_arch::exception? As another substate or as part
>>> of a generalized NMI substate?
>>>        
>> Yes.  It's not part of an nmi substate, but both can be part of an
>> exception substate (but need to look at the docs vewy cawefuwy to
>> make sure we don't screw up again).
>>
>>      
> What do you mean? How they can be both part of exception substate?
>
>    

Sorry, nomenclature failure.  We need NMI state, Interrupt state 
(already provided), and pending exception state (which can be a fault or 
a trap).  There's also some extra state associated with pending debug 
exceptions (maybe we can copy it into dr6).

We can either put all of these into one substate, or into separate 
substates.  I'm not sure which is best.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-20  9:14               ` Avi Kivity
@ 2009-10-20 11:13                 ` Marcelo Tosatti
  2009-10-20 12:44                   ` Gleb Natapov
  0 siblings, 1 reply; 15+ messages in thread
From: Marcelo Tosatti @ 2009-10-20 11:13 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Gleb Natapov, Jan Kiszka, kvm@vger.kernel.org

On Tue, Oct 20, 2009 at 06:14:04PM +0900, Avi Kivity wrote:
> On 10/20/2009 06:08 PM, Gleb Natapov wrote:
>> On Tue, Oct 20, 2009 at 06:06:36PM +0900, Avi Kivity wrote:
>>    
>>> On 10/20/2009 05:56 PM, Jan Kiszka wrote:
>>>      
>>>> So save/restore kvm_vcpu_arch::exception? As another substate or as part
>>>> of a generalized NMI substate?
>>>>        
>>> Yes.  It's not part of an nmi substate, but both can be part of an
>>> exception substate (but need to look at the docs vewy cawefuwy to
>>> make sure we don't screw up again).
>>>
>>>      
>> What do you mean? How they can be both part of exception substate?
>>
>>    
>
> Sorry, nomenclature failure.  We need NMI state, Interrupt state  
> (already provided), and pending exception state (which can be a fault or  
> a trap).  There's also some extra state associated with pending debug  
> exceptions (maybe we can copy it into dr6).

KVM_REQ_TRIPLE_FAULT can also be lost, but i don't think anybody cares?

>
> We can either put all of these into one substate, or into separate  
> substates.  I'm not sure which is best.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states
  2009-10-20 11:13                 ` Marcelo Tosatti
@ 2009-10-20 12:44                   ` Gleb Natapov
  0 siblings, 0 replies; 15+ messages in thread
From: Gleb Natapov @ 2009-10-20 12:44 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, Jan Kiszka, kvm@vger.kernel.org

On Tue, Oct 20, 2009 at 09:13:02AM -0200, Marcelo Tosatti wrote:
> On Tue, Oct 20, 2009 at 06:14:04PM +0900, Avi Kivity wrote:
> > On 10/20/2009 06:08 PM, Gleb Natapov wrote:
> >> On Tue, Oct 20, 2009 at 06:06:36PM +0900, Avi Kivity wrote:
> >>    
> >>> On 10/20/2009 05:56 PM, Jan Kiszka wrote:
> >>>      
> >>>> So save/restore kvm_vcpu_arch::exception? As another substate or as part
> >>>> of a generalized NMI substate?
> >>>>        
> >>> Yes.  It's not part of an nmi substate, but both can be part of an
> >>> exception substate (but need to look at the docs vewy cawefuwy to
> >>> make sure we don't screw up again).
> >>>
> >>>      
> >> What do you mean? How they can be both part of exception substate?
> >>
> >>    
> >
> > Sorry, nomenclature failure.  We need NMI state, Interrupt state  
> > (already provided), and pending exception state (which can be a fault or  
> > a trap).  There's also some extra state associated with pending debug  
> > exceptions (maybe we can copy it into dr6).
> 
> KVM_REQ_TRIPLE_FAULT can also be lost, but i don't think anybody cares?
> 
If pending exception will be migrated KVM_REQ_TRIPLE_FAULT will be restored
after guest will try to re-execute instruction that caused it. One more
reason to migrate pending exceptions. And why not migrate
KVM_REQ_TRIPLE_FAULT while we are at it.

> >
> > We can either put all of these into one substate, or into separate  
> > substates.  I'm not sure which is best.

--
			Gleb.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2009-10-20 12:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-15 17:05 [PATCH v2 0/4] Extensible VCPU state IOCTL Jan Kiszka
2009-10-15 17:05 ` [PATCH v2 1/4] KVM: Reorder IOCTLs in main kvm.h Jan Kiszka
2009-10-15 17:05 ` [PATCH v2 3/4] KVM: x86: Add support for KVM_GET/SET_VCPU_STATE Jan Kiszka
2009-10-15 17:05 ` [PATCH v2 4/4] KVM: x86: Add VCPU substate for NMI states Jan Kiszka
2009-10-19 20:32   ` Marcelo Tosatti
2009-10-19 20:39     ` Gleb Natapov
2009-10-19 23:34       ` Avi Kivity
2009-10-20  8:56         ` Jan Kiszka
2009-10-20  9:06           ` Avi Kivity
2009-10-20  9:08             ` Gleb Natapov
2009-10-20  9:14               ` Avi Kivity
2009-10-20 11:13                 ` Marcelo Tosatti
2009-10-20 12:44                   ` Gleb Natapov
2009-10-20  8:52     ` Jan Kiszka
2009-10-15 17:05 ` [PATCH v2 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox