* [PATCH v3 1/4] KVM: Reorder IOCTLs in main kvm.h
2009-11-02 16:20 [PATCH v3 0/4] Extensible VCPU state IOCTL Jan Kiszka
@ 2009-11-02 16:20 ` Jan Kiszka
2009-11-10 10:17 ` Avi Kivity
2009-11-02 16:20 ` [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states Jan Kiszka
` (2 subsequent siblings)
3 siblings, 1 reply; 22+ messages in thread
From: Jan Kiszka @ 2009-11-02 16:20 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 | 235 +++++++++++++++++++++++++--------------------------
1 files changed, 117 insertions(+), 118 deletions(-)
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 6ed1a12..ca62b8e 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.
*/
@@ -522,56 +566,57 @@ struct kvm_clock_data {
/*
* 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_XEN_HVM_CONFIG _IOW(KVMIO, 0x7a, struct kvm_xen_hvm_config)
-#define KVM_SET_CLOCK _IOW(KVMIO, 0x7b, struct kvm_clock_data)
-#define KVM_GET_CLOCK _IOR(KVMIO, 0x7c, struct kvm_clock_data)
-
+#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)
+#define KVM_XEN_HVM_CONFIG _IOW(KVMIO, 0x7a, struct kvm_xen_hvm_config)
+#define KVM_SET_CLOCK _IOW(KVMIO, 0x7b, struct kvm_clock_data)
+#define KVM_GET_CLOCK _IOR(KVMIO, 0x7c, struct kvm_clock_data)
+/* 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
@@ -584,7 +629,7 @@ struct kvm_clock_data {
#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)
@@ -596,7 +641,7 @@ struct kvm_clock_data {
#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 */
@@ -608,67 +653,21 @@ struct kvm_clock_data {
/* 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 {
@@ -722,4 +721,4 @@ struct kvm_assigned_msix_entry {
__u16 padding[3];
};
-#endif
+#endif /* __LINUX_KVM_H */
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states
2009-11-02 16:20 [PATCH v3 0/4] Extensible VCPU state IOCTL Jan Kiszka
2009-11-02 16:20 ` [PATCH v3 1/4] KVM: Reorder IOCTLs in main kvm.h Jan Kiszka
@ 2009-11-02 16:20 ` Jan Kiszka
2009-11-04 11:23 ` Avi Kivity
2009-11-05 8:25 ` [PATCH v4 " Jan Kiszka
2009-11-02 16:20 ` [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL Jan Kiszka
2009-11-02 16:20 ` [PATCH v3 3/4] KVM: x86: Add support for KVM_GET/SET_VCPU_STATE Jan Kiszka
3 siblings, 2 replies; 22+ messages in thread
From: Jan Kiszka @ 2009-11-02 16:20 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm
This new substate exports all yet user-invisible states related to
exceptions, interrupts, and NMIs. Together with appropriate user space
changes, this fixes sporadic problems of vmsave/restore, live migration
and system reset.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Documentation/kvm/api.txt | 28 ++++++++++++++
arch/x86/include/asm/kvm.h | 23 +++++++++++
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 | 79 +++++++++++++++++++++++++++++++++++++++
6 files changed, 184 insertions(+), 0 deletions(-)
diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 6421aee..66a0814 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -929,3 +929,31 @@ Deprecates: KVM_GET/SET_CPUID2
Architectures: x86
Payload: struct kvm_lapic
Deprecates: KVM_GET/SET_LAPIC
+
+6.8 KVM_X86_VCPU_STATE_EVENTS
+
+Architectures: x86
+Payload: struct kvm_x86_event_state
+Deprecates: interrupt_bitmap of struct kvm_sregs (pass it cleared)
+
+struct kvm_x86_event_state {
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 pad[2];
+ __u32 error_code;
+ } exception;
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 soft;
+ __u8 pad;
+ } interrupt;
+ struct {
+ __u8 injected;
+ __u8 pending;
+ __u8 masked;
+ __u8 pad;
+ } nmi;
+ __u32 sipi_vector;
+};
diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
index 42286ef..66c0843 100644
--- a/arch/x86/include/asm/kvm.h
+++ b/arch/x86/include/asm/kvm.h
@@ -257,5 +257,28 @@ 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_EVENTS 1003
+
+struct kvm_x86_event_state {
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 pad[2];
+ __u32 error_code;
+ } exception;
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 soft;
+ __u8 pad;
+ } interrupt;
+ struct {
+ __u8 injected;
+ __u8 pending;
+ __u8 masked;
+ __u8 pad;
+ } nmi;
+ __u32 sipi_vector;
+};
#endif /* _ASM_X86_KVM_H */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 26a74b7..06e0856 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -523,6 +523,8 @@ struct kvm_x86_ops {
bool has_error_code, u32 error_code);
int (*interrupt_allowed)(struct kvm_vcpu *vcpu);
int (*nmi_allowed)(struct kvm_vcpu *vcpu);
+ bool (*get_nmi_mask)(struct kvm_vcpu *vcpu);
+ void (*set_nmi_mask)(struct kvm_vcpu *vcpu, bool masked);
void (*enable_nmi_window)(struct kvm_vcpu *vcpu);
void (*enable_irq_window)(struct kvm_vcpu *vcpu);
void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 34b700f..3de0b37 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2499,6 +2499,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);
@@ -2946,6 +2966,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 b923f2a..63e4a50 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2633,6 +2633,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) &&
@@ -3973,6 +4001,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 3484787..8dc968d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4851,12 +4851,56 @@ out_free_lapic:
kfree(lapic);
break;
}
+ case KVM_X86_VCPU_STATE_EVENTS: {
+ struct kvm_x86_event_state events;
+
+ vcpu_load(vcpu);
+
+ events.exception.injected = vcpu->arch.exception.pending;
+ events.exception.nr = vcpu->arch.exception.nr;
+ events.exception.error_code = vcpu->arch.exception.error_code;
+
+ events.interrupt.injected = vcpu->arch.interrupt.pending;
+ events.interrupt.nr = vcpu->arch.interrupt.nr;
+ events.interrupt.soft = vcpu->arch.interrupt.soft;
+
+ events.nmi.injected = vcpu->arch.nmi_injected;
+ events.nmi.pending = vcpu->arch.nmi_pending;
+ events.nmi.masked = kvm_x86_ops->get_nmi_mask(vcpu);
+
+ events.sipi_vector = vcpu->arch.sipi_vector;
+
+ vcpu_put(vcpu);
+
+ r = -EFAULT;
+ if (copy_to_user(argp, &events,
+ sizeof(struct kvm_x86_event_state)))
+ break;
+ r = 0;
+ break;
+ }
default:
r = -EINVAL;
}
return r;
}
+static bool exception_has_error_code(int nr)
+{
+ switch (nr) {
+ case 8:
+ case 10:
+ case 11:
+ case 12:
+ case 13:
+ case 14:
+ case 17:
+ return true;
+ default:
+ return false;
+ }
+}
+
int kvm_arch_vcpu_set_substate(struct kvm_vcpu *vcpu, uint8_t __user *arg_base,
struct kvm_vcpu_substate *substate)
{
@@ -4901,6 +4945,40 @@ out_free_lapic:
kfree(lapic);
break;
}
+ case KVM_X86_VCPU_STATE_EVENTS: {
+ struct kvm_x86_event_state events;
+
+ r = -EFAULT;
+ if (copy_from_user(&events, argp,
+ sizeof(struct kvm_x86_event_state)))
+ break;
+
+ vcpu_load(vcpu);
+
+ vcpu->arch.exception.pending = events.exception.injected;
+ vcpu->arch.exception.nr = events.exception.nr;
+ vcpu->arch.exception.has_error_code =
+ exception_has_error_code(events.exception.nr);
+ vcpu->arch.exception.error_code = events.exception.error_code;
+
+ vcpu->arch.interrupt.pending = events.interrupt.injected;
+ vcpu->arch.interrupt.nr = events.interrupt.nr;
+ vcpu->arch.interrupt.soft = events.interrupt.soft;
+ if (vcpu->arch.interrupt.pending &&
+ irqchip_in_kernel(vcpu->kvm))
+ kvm_pic_clear_isr_ack(vcpu->kvm);
+
+ vcpu->arch.nmi_injected = events.nmi.injected;
+ vcpu->arch.nmi_pending = events.nmi.pending;
+ kvm_x86_ops->set_nmi_mask(vcpu, events.nmi.masked);
+
+ vcpu->arch.sipi_vector = events.sipi_vector;
+
+ vcpu_put(vcpu);
+
+ r = 0;
+ break;
+ }
default:
r = -EINVAL;
}
@@ -4913,6 +4991,7 @@ bool kvm_arch_check_substate(u32 type)
case KVM_X86_VCPU_STATE_MSRS:
case KVM_X86_VCPU_STATE_CPUID:
case KVM_X86_VCPU_STATE_LAPIC:
+ case KVM_X86_VCPU_STATE_EVENTS:
return true;
default:
return false;
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states
2009-11-02 16:20 ` [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states Jan Kiszka
@ 2009-11-04 11:23 ` Avi Kivity
2009-11-04 11:34 ` Jan Kiszka
2009-11-05 8:25 ` [PATCH v4 " Jan Kiszka
1 sibling, 1 reply; 22+ messages in thread
From: Avi Kivity @ 2009-11-04 11:23 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm
On 11/02/2009 06:20 PM, Jan Kiszka wrote:
> This new substate exports all yet user-invisible states related to
> exceptions, interrupts, and NMIs. Together with appropriate user space
> changes, this fixes sporadic problems of vmsave/restore, live migration
> and system reset.
>
> + events.exception.injected = vcpu->arch.exception.pending;
> + events.exception.nr = vcpu->arch.exception.nr;
> + events.exception.error_code = vcpu->arch.exception.error_code;
>
we're missing has_error_code here.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states
2009-11-04 11:23 ` Avi Kivity
@ 2009-11-04 11:34 ` Jan Kiszka
2009-11-04 12:51 ` Avi Kivity
0 siblings, 1 reply; 22+ messages in thread
From: Jan Kiszka @ 2009-11-04 11:34 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
Avi Kivity wrote:
> On 11/02/2009 06:20 PM, Jan Kiszka wrote:
>> This new substate exports all yet user-invisible states related to
>> exceptions, interrupts, and NMIs. Together with appropriate user space
>> changes, this fixes sporadic problems of vmsave/restore, live migration
>> and system reset.
>>
>
>> + events.exception.injected = vcpu->arch.exception.pending;
>> + events.exception.nr = vcpu->arch.exception.nr;
>> + events.exception.error_code = vcpu->arch.exception.error_code;
>>
>
> we're missing has_error_code here.
>
Isn't it statically defined by the arch? exception_has_error_code()
regenerates it (just like user land does).
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states
2009-11-04 11:34 ` Jan Kiszka
@ 2009-11-04 12:51 ` Avi Kivity
2009-11-04 14:44 ` Jan Kiszka
0 siblings, 1 reply; 22+ messages in thread
From: Avi Kivity @ 2009-11-04 12:51 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 11/04/2009 01:34 PM, Jan Kiszka wrote:
>
>> we're missing has_error_code here.
>>
>>
> Isn't it statically defined by the arch? exception_has_error_code()
> regenerates it (just like user land does).
>
In real mode exceptions lose their error code, so we have to look at
cr0, and now we depend on the order of loading of state...
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states
2009-11-04 12:51 ` Avi Kivity
@ 2009-11-04 14:44 ` Jan Kiszka
0 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2009-11-04 14:44 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
Avi Kivity wrote:
> On 11/04/2009 01:34 PM, Jan Kiszka wrote:
>>> we're missing has_error_code here.
>>>
>>>
>> Isn't it statically defined by the arch? exception_has_error_code()
>> regenerates it (just like user land does).
>>
>
> In real mode exceptions lose their error code, so we have to look at
> cr0, and now we depend on the order of loading of state...
I see. Will add it in v4.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 4/4] KVM: x86: Add VCPU substate for event states
2009-11-02 16:20 ` [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states Jan Kiszka
2009-11-04 11:23 ` Avi Kivity
@ 2009-11-05 8:25 ` Jan Kiszka
2009-11-05 11:00 ` Gleb Natapov
2009-11-05 11:51 ` [PATCH v5 " Jan Kiszka
1 sibling, 2 replies; 22+ messages in thread
From: Jan Kiszka @ 2009-11-05 8:25 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm
This new substate exports all yet user-invisible states related to
exceptions, interrupts, and NMIs. Together with appropriate user space
changes, this fixes sporadic problems of vmsave/restore, live migration
and system reset.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Changes from v3:
- added exception.has_error_code field
Documentation/kvm/api.txt | 28 +++++++++++++++++
arch/x86/include/asm/kvm.h | 24 ++++++++++++++
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 | 65 +++++++++++++++++++++++++++++++++++++++
6 files changed, 171 insertions(+), 0 deletions(-)
diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 6421aee..66a0814 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -929,3 +929,31 @@ Deprecates: KVM_GET/SET_CPUID2
Architectures: x86
Payload: struct kvm_lapic
Deprecates: KVM_GET/SET_LAPIC
+
+6.8 KVM_X86_VCPU_STATE_EVENTS
+
+Architectures: x86
+Payload: struct kvm_x86_event_state
+Deprecates: interrupt_bitmap of struct kvm_sregs (pass it cleared)
+
+struct kvm_x86_event_state {
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 pad[2];
+ __u32 error_code;
+ } exception;
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 soft;
+ __u8 pad;
+ } interrupt;
+ struct {
+ __u8 injected;
+ __u8 pending;
+ __u8 masked;
+ __u8 pad;
+ } nmi;
+ __u32 sipi_vector;
+};
diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
index 42286ef..ea2711f 100644
--- a/arch/x86/include/asm/kvm.h
+++ b/arch/x86/include/asm/kvm.h
@@ -257,5 +257,29 @@ 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_EVENTS 1003
+
+struct kvm_x86_event_state {
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 has_error_code;
+ __u8 pad;
+ __u32 error_code;
+ } exception;
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 soft;
+ __u8 pad;
+ } interrupt;
+ struct {
+ __u8 injected;
+ __u8 pending;
+ __u8 masked;
+ __u8 pad;
+ } nmi;
+ __u32 sipi_vector;
+};
#endif /* _ASM_X86_KVM_H */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 26a74b7..06e0856 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -523,6 +523,8 @@ struct kvm_x86_ops {
bool has_error_code, u32 error_code);
int (*interrupt_allowed)(struct kvm_vcpu *vcpu);
int (*nmi_allowed)(struct kvm_vcpu *vcpu);
+ bool (*get_nmi_mask)(struct kvm_vcpu *vcpu);
+ void (*set_nmi_mask)(struct kvm_vcpu *vcpu, bool masked);
void (*enable_nmi_window)(struct kvm_vcpu *vcpu);
void (*enable_irq_window)(struct kvm_vcpu *vcpu);
void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 34b700f..3de0b37 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2499,6 +2499,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);
@@ -2946,6 +2966,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 b923f2a..63e4a50 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2633,6 +2633,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) &&
@@ -3973,6 +4001,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 3484787..ade6c2d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4851,6 +4851,36 @@ out_free_lapic:
kfree(lapic);
break;
}
+ case KVM_X86_VCPU_STATE_EVENTS: {
+ struct kvm_x86_event_state events;
+
+ vcpu_load(vcpu);
+
+ events.exception.injected = vcpu->arch.exception.pending;
+ events.exception.nr = vcpu->arch.exception.nr;
+ events.exception.has_error_code =
+ vcpu->arch.exception.has_error_code;
+ events.exception.error_code = vcpu->arch.exception.error_code;
+
+ events.interrupt.injected = vcpu->arch.interrupt.pending;
+ events.interrupt.nr = vcpu->arch.interrupt.nr;
+ events.interrupt.soft = vcpu->arch.interrupt.soft;
+
+ events.nmi.injected = vcpu->arch.nmi_injected;
+ events.nmi.pending = vcpu->arch.nmi_pending;
+ events.nmi.masked = kvm_x86_ops->get_nmi_mask(vcpu);
+
+ events.sipi_vector = vcpu->arch.sipi_vector;
+
+ vcpu_put(vcpu);
+
+ r = -EFAULT;
+ if (copy_to_user(argp, &events,
+ sizeof(struct kvm_x86_event_state)))
+ break;
+ r = 0;
+ break;
+ }
default:
r = -EINVAL;
}
@@ -4901,6 +4931,40 @@ out_free_lapic:
kfree(lapic);
break;
}
+ case KVM_X86_VCPU_STATE_EVENTS: {
+ struct kvm_x86_event_state events;
+
+ r = -EFAULT;
+ if (copy_from_user(&events, argp,
+ sizeof(struct kvm_x86_event_state)))
+ break;
+
+ vcpu_load(vcpu);
+
+ vcpu->arch.exception.pending = events.exception.injected;
+ vcpu->arch.exception.nr = events.exception.nr;
+ vcpu->arch.exception.has_error_code =
+ events.exception.has_error_code;
+ vcpu->arch.exception.error_code = events.exception.error_code;
+
+ vcpu->arch.interrupt.pending = events.interrupt.injected;
+ vcpu->arch.interrupt.nr = events.interrupt.nr;
+ vcpu->arch.interrupt.soft = events.interrupt.soft;
+ if (vcpu->arch.interrupt.pending &&
+ irqchip_in_kernel(vcpu->kvm))
+ kvm_pic_clear_isr_ack(vcpu->kvm);
+
+ vcpu->arch.nmi_injected = events.nmi.injected;
+ vcpu->arch.nmi_pending = events.nmi.pending;
+ kvm_x86_ops->set_nmi_mask(vcpu, events.nmi.masked);
+
+ vcpu->arch.sipi_vector = events.sipi_vector;
+
+ vcpu_put(vcpu);
+
+ r = 0;
+ break;
+ }
default:
r = -EINVAL;
}
@@ -4913,6 +4977,7 @@ bool kvm_arch_check_substate(u32 type)
case KVM_X86_VCPU_STATE_MSRS:
case KVM_X86_VCPU_STATE_CPUID:
case KVM_X86_VCPU_STATE_LAPIC:
+ case KVM_X86_VCPU_STATE_EVENTS:
return true;
default:
return false;
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 4/4] KVM: x86: Add VCPU substate for event states
2009-11-05 8:25 ` [PATCH v4 " Jan Kiszka
@ 2009-11-05 11:00 ` Gleb Natapov
2009-11-05 11:51 ` [PATCH v5 " Jan Kiszka
1 sibling, 0 replies; 22+ messages in thread
From: Gleb Natapov @ 2009-11-05 11:00 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm
On Thu, Nov 05, 2009 at 09:25:39AM +0100, Jan Kiszka wrote:
> This new substate exports all yet user-invisible states related to
> exceptions, interrupts, and NMIs. Together with appropriate user space
> changes, this fixes sporadic problems of vmsave/restore, live migration
> and system reset.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Changes from v3:
> - added exception.has_error_code field
>
> Documentation/kvm/api.txt | 28 +++++++++++++++++
> arch/x86/include/asm/kvm.h | 24 ++++++++++++++
> 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 | 65 +++++++++++++++++++++++++++++++++++++++
> 6 files changed, 171 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
> index 6421aee..66a0814 100644
> --- a/Documentation/kvm/api.txt
> +++ b/Documentation/kvm/api.txt
> @@ -929,3 +929,31 @@ Deprecates: KVM_GET/SET_CPUID2
> Architectures: x86
> Payload: struct kvm_lapic
> Deprecates: KVM_GET/SET_LAPIC
> +
> +6.8 KVM_X86_VCPU_STATE_EVENTS
> +
> +Architectures: x86
> +Payload: struct kvm_x86_event_state
> +Deprecates: interrupt_bitmap of struct kvm_sregs (pass it cleared)
> +
> +struct kvm_x86_event_state {
> + struct {
> + __u8 injected;
> + __u8 nr;
> + __u8 pad[2];
Forgot to update documentation :(
> + __u32 error_code;
> + } exception;
> + struct {
> + __u8 injected;
> + __u8 nr;
> + __u8 soft;
> + __u8 pad;
> + } interrupt;
> + struct {
> + __u8 injected;
> + __u8 pending;
> + __u8 masked;
> + __u8 pad;
> + } nmi;
> + __u32 sipi_vector;
> +};
> diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> index 42286ef..ea2711f 100644
> --- a/arch/x86/include/asm/kvm.h
> +++ b/arch/x86/include/asm/kvm.h
> @@ -257,5 +257,29 @@ 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_EVENTS 1003
> +
> +struct kvm_x86_event_state {
> + struct {
> + __u8 injected;
> + __u8 nr;
> + __u8 has_error_code;
> + __u8 pad;
> + __u32 error_code;
> + } exception;
> + struct {
> + __u8 injected;
> + __u8 nr;
> + __u8 soft;
> + __u8 pad;
> + } interrupt;
> + struct {
> + __u8 injected;
> + __u8 pending;
> + __u8 masked;
> + __u8 pad;
> + } nmi;
> + __u32 sipi_vector;
> +};
>
> #endif /* _ASM_X86_KVM_H */
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 26a74b7..06e0856 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -523,6 +523,8 @@ struct kvm_x86_ops {
> bool has_error_code, u32 error_code);
> int (*interrupt_allowed)(struct kvm_vcpu *vcpu);
> int (*nmi_allowed)(struct kvm_vcpu *vcpu);
> + bool (*get_nmi_mask)(struct kvm_vcpu *vcpu);
> + void (*set_nmi_mask)(struct kvm_vcpu *vcpu, bool masked);
> void (*enable_nmi_window)(struct kvm_vcpu *vcpu);
> void (*enable_irq_window)(struct kvm_vcpu *vcpu);
> void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 34b700f..3de0b37 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -2499,6 +2499,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);
> @@ -2946,6 +2966,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 b923f2a..63e4a50 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2633,6 +2633,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) &&
> @@ -3973,6 +4001,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 3484787..ade6c2d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4851,6 +4851,36 @@ out_free_lapic:
> kfree(lapic);
> break;
> }
> + case KVM_X86_VCPU_STATE_EVENTS: {
> + struct kvm_x86_event_state events;
> +
> + vcpu_load(vcpu);
> +
> + events.exception.injected = vcpu->arch.exception.pending;
> + events.exception.nr = vcpu->arch.exception.nr;
> + events.exception.has_error_code =
> + vcpu->arch.exception.has_error_code;
> + events.exception.error_code = vcpu->arch.exception.error_code;
> +
> + events.interrupt.injected = vcpu->arch.interrupt.pending;
> + events.interrupt.nr = vcpu->arch.interrupt.nr;
> + events.interrupt.soft = vcpu->arch.interrupt.soft;
> +
> + events.nmi.injected = vcpu->arch.nmi_injected;
> + events.nmi.pending = vcpu->arch.nmi_pending;
> + events.nmi.masked = kvm_x86_ops->get_nmi_mask(vcpu);
> +
> + events.sipi_vector = vcpu->arch.sipi_vector;
> +
> + vcpu_put(vcpu);
> +
> + r = -EFAULT;
> + if (copy_to_user(argp, &events,
> + sizeof(struct kvm_x86_event_state)))
> + break;
> + r = 0;
> + break;
> + }
> default:
> r = -EINVAL;
> }
> @@ -4901,6 +4931,40 @@ out_free_lapic:
> kfree(lapic);
> break;
> }
> + case KVM_X86_VCPU_STATE_EVENTS: {
> + struct kvm_x86_event_state events;
> +
> + r = -EFAULT;
> + if (copy_from_user(&events, argp,
> + sizeof(struct kvm_x86_event_state)))
> + break;
> +
> + vcpu_load(vcpu);
> +
> + vcpu->arch.exception.pending = events.exception.injected;
> + vcpu->arch.exception.nr = events.exception.nr;
> + vcpu->arch.exception.has_error_code =
> + events.exception.has_error_code;
> + vcpu->arch.exception.error_code = events.exception.error_code;
> +
> + vcpu->arch.interrupt.pending = events.interrupt.injected;
> + vcpu->arch.interrupt.nr = events.interrupt.nr;
> + vcpu->arch.interrupt.soft = events.interrupt.soft;
> + if (vcpu->arch.interrupt.pending &&
> + irqchip_in_kernel(vcpu->kvm))
> + kvm_pic_clear_isr_ack(vcpu->kvm);
> +
> + vcpu->arch.nmi_injected = events.nmi.injected;
> + vcpu->arch.nmi_pending = events.nmi.pending;
> + kvm_x86_ops->set_nmi_mask(vcpu, events.nmi.masked);
> +
> + vcpu->arch.sipi_vector = events.sipi_vector;
> +
> + vcpu_put(vcpu);
> +
> + r = 0;
> + break;
> + }
> default:
> r = -EINVAL;
> }
> @@ -4913,6 +4977,7 @@ bool kvm_arch_check_substate(u32 type)
> case KVM_X86_VCPU_STATE_MSRS:
> case KVM_X86_VCPU_STATE_CPUID:
> case KVM_X86_VCPU_STATE_LAPIC:
> + case KVM_X86_VCPU_STATE_EVENTS:
> return true;
> default:
> return false;
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gleb.
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 4/4] KVM: x86: Add VCPU substate for event states
2009-11-05 8:25 ` [PATCH v4 " Jan Kiszka
2009-11-05 11:00 ` Gleb Natapov
@ 2009-11-05 11:51 ` Jan Kiszka
1 sibling, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2009-11-05 11:51 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm
This new substate exports all yet user-invisible states related to
exceptions, interrupts, and NMIs. Together with appropriate user space
changes, this fixes sporadic problems of vmsave/restore, live migration
and system reset.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Changes from v4:
- updated doc as well (thanks, Gleb)
Documentation/kvm/api.txt | 29 +++++++++++++++++
arch/x86/include/asm/kvm.h | 24 ++++++++++++++
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 | 65 +++++++++++++++++++++++++++++++++++++++
6 files changed, 172 insertions(+), 0 deletions(-)
diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 6421aee..374775e 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -929,3 +929,32 @@ Deprecates: KVM_GET/SET_CPUID2
Architectures: x86
Payload: struct kvm_lapic
Deprecates: KVM_GET/SET_LAPIC
+
+6.8 KVM_X86_VCPU_STATE_EVENTS
+
+Architectures: x86
+Payload: struct kvm_x86_event_state
+Deprecates: interrupt_bitmap of struct kvm_sregs (pass it cleared)
+
+struct kvm_x86_event_state {
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 has_error_code;
+ __u8 pad;
+ __u32 error_code;
+ } exception;
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 soft;
+ __u8 pad;
+ } interrupt;
+ struct {
+ __u8 injected;
+ __u8 pending;
+ __u8 masked;
+ __u8 pad;
+ } nmi;
+ __u32 sipi_vector;
+};
diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
index 42286ef..ea2711f 100644
--- a/arch/x86/include/asm/kvm.h
+++ b/arch/x86/include/asm/kvm.h
@@ -257,5 +257,29 @@ 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_EVENTS 1003
+
+struct kvm_x86_event_state {
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 has_error_code;
+ __u8 pad;
+ __u32 error_code;
+ } exception;
+ struct {
+ __u8 injected;
+ __u8 nr;
+ __u8 soft;
+ __u8 pad;
+ } interrupt;
+ struct {
+ __u8 injected;
+ __u8 pending;
+ __u8 masked;
+ __u8 pad;
+ } nmi;
+ __u32 sipi_vector;
+};
#endif /* _ASM_X86_KVM_H */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 26a74b7..06e0856 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -523,6 +523,8 @@ struct kvm_x86_ops {
bool has_error_code, u32 error_code);
int (*interrupt_allowed)(struct kvm_vcpu *vcpu);
int (*nmi_allowed)(struct kvm_vcpu *vcpu);
+ bool (*get_nmi_mask)(struct kvm_vcpu *vcpu);
+ void (*set_nmi_mask)(struct kvm_vcpu *vcpu, bool masked);
void (*enable_nmi_window)(struct kvm_vcpu *vcpu);
void (*enable_irq_window)(struct kvm_vcpu *vcpu);
void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 34b700f..3de0b37 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2499,6 +2499,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);
@@ -2946,6 +2966,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 b923f2a..63e4a50 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2633,6 +2633,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) &&
@@ -3973,6 +4001,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 3484787..ade6c2d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4851,6 +4851,36 @@ out_free_lapic:
kfree(lapic);
break;
}
+ case KVM_X86_VCPU_STATE_EVENTS: {
+ struct kvm_x86_event_state events;
+
+ vcpu_load(vcpu);
+
+ events.exception.injected = vcpu->arch.exception.pending;
+ events.exception.nr = vcpu->arch.exception.nr;
+ events.exception.has_error_code =
+ vcpu->arch.exception.has_error_code;
+ events.exception.error_code = vcpu->arch.exception.error_code;
+
+ events.interrupt.injected = vcpu->arch.interrupt.pending;
+ events.interrupt.nr = vcpu->arch.interrupt.nr;
+ events.interrupt.soft = vcpu->arch.interrupt.soft;
+
+ events.nmi.injected = vcpu->arch.nmi_injected;
+ events.nmi.pending = vcpu->arch.nmi_pending;
+ events.nmi.masked = kvm_x86_ops->get_nmi_mask(vcpu);
+
+ events.sipi_vector = vcpu->arch.sipi_vector;
+
+ vcpu_put(vcpu);
+
+ r = -EFAULT;
+ if (copy_to_user(argp, &events,
+ sizeof(struct kvm_x86_event_state)))
+ break;
+ r = 0;
+ break;
+ }
default:
r = -EINVAL;
}
@@ -4901,6 +4931,40 @@ out_free_lapic:
kfree(lapic);
break;
}
+ case KVM_X86_VCPU_STATE_EVENTS: {
+ struct kvm_x86_event_state events;
+
+ r = -EFAULT;
+ if (copy_from_user(&events, argp,
+ sizeof(struct kvm_x86_event_state)))
+ break;
+
+ vcpu_load(vcpu);
+
+ vcpu->arch.exception.pending = events.exception.injected;
+ vcpu->arch.exception.nr = events.exception.nr;
+ vcpu->arch.exception.has_error_code =
+ events.exception.has_error_code;
+ vcpu->arch.exception.error_code = events.exception.error_code;
+
+ vcpu->arch.interrupt.pending = events.interrupt.injected;
+ vcpu->arch.interrupt.nr = events.interrupt.nr;
+ vcpu->arch.interrupt.soft = events.interrupt.soft;
+ if (vcpu->arch.interrupt.pending &&
+ irqchip_in_kernel(vcpu->kvm))
+ kvm_pic_clear_isr_ack(vcpu->kvm);
+
+ vcpu->arch.nmi_injected = events.nmi.injected;
+ vcpu->arch.nmi_pending = events.nmi.pending;
+ kvm_x86_ops->set_nmi_mask(vcpu, events.nmi.masked);
+
+ vcpu->arch.sipi_vector = events.sipi_vector;
+
+ vcpu_put(vcpu);
+
+ r = 0;
+ break;
+ }
default:
r = -EINVAL;
}
@@ -4913,6 +4977,7 @@ bool kvm_arch_check_substate(u32 type)
case KVM_X86_VCPU_STATE_MSRS:
case KVM_X86_VCPU_STATE_CPUID:
case KVM_X86_VCPU_STATE_LAPIC:
+ case KVM_X86_VCPU_STATE_EVENTS:
return true;
default:
return false;
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-02 16:20 [PATCH v3 0/4] Extensible VCPU state IOCTL Jan Kiszka
2009-11-02 16:20 ` [PATCH v3 1/4] KVM: Reorder IOCTLs in main kvm.h Jan Kiszka
2009-11-02 16:20 ` [PATCH v3 4/4] KVM: x86: Add VCPU substate for event states Jan Kiszka
@ 2009-11-02 16:20 ` Jan Kiszka
2009-11-04 11:18 ` Avi Kivity
2009-11-10 10:14 ` Avi Kivity
2009-11-02 16:20 ` [PATCH v3 3/4] KVM: x86: Add support for KVM_GET/SET_VCPU_STATE Jan Kiszka
3 siblings, 2 replies; 22+ messages in thread
From: Jan Kiszka @ 2009-11-02 16:20 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, introduces KVM_CHECK_VCPU_STATES to query supported
substates, 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 | 94 +++++++++++
arch/ia64/kvm/kvm-ia64.c | 17 ++
arch/powerpc/kvm/powerpc.c | 17 ++
arch/s390/kvm/kvm-s390.c | 17 ++
arch/x86/kvm/x86.c | 17 ++
include/linux/kvm.h | 32 ++++
include/linux/kvm_host.h | 6 +
virt/kvm/kvm_main.c | 367 +++++++++++++++++++++++++++++++++-----------
8 files changed, 475 insertions(+), 92 deletions(-)
diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 36594ba..3c98d0a 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -653,6 +653,70 @@ struct kvm_clock_data {
__u32 pad[9];
};
+4.29 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)®s - (size_t)&request;
+
+4.30 KVM_CHECK_VCPU_STATES
+
+Capability: KVM_CAP_VCPU_STATE
+Architectures: all
+Type: system ioctl
+Parameters: struct kvm_vcpu_state_list (in/out)
+Returns: 0 on success, -1 on error
+
+Checks if the given list of vcpu substates is supported by the kernel.
+
+On entry, user space passes the list of substate types it wants to query in
+the following structure:
+
+struct kvm_vcpu_state_list {
+ __u32 ntypes;
+ __u32 types[0]; /* KVM_VCPU_STATE_* or KVM_$(ARCH)_VCPU_STATE_* */
+};
+
+The kernel will set all types it does not support to KVM_VCPU_STATE_INVALID on
+return.
+
5. The kvm_run structure
Application code obtains a pointer to the kvm_run structure by
@@ -817,3 +881,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..7f04603 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1991,3 +1991,20 @@ 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;
+}
+
+bool kvm_arch_check_substate(u32 type)
+{
+ return false;
+}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 5902bbc..67f4775 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -436,3 +436,20 @@ 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;
+}
+
+bool kvm_arch_check_substate(u32 type)
+{
+ return false;
+}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 5445058..058002b 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -450,6 +450,23 @@ 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;
+}
+
+bool kvm_arch_check_substate(u32 type)
+{
+ return false;
+}
+
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 1cc51ca..816e681 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4842,6 +4842,23 @@ 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;
+}
+
+bool kvm_arch_check_substate(u32 type)
+{
+ return false;
+}
+
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 ca62b8e..473eaec 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -393,6 +393,32 @@ struct kvm_ioeventfd {
__u8 pad[36];
};
+/* for KVM_GET_VCPU_STATE and KVM_SET_VCPU_STATE */
+#define KVM_VCPU_STATE_INVALID 0
+#define KVM_VCPU_STATE_REGS 1
+#define KVM_VCPU_STATE_SREGS 2
+#define KVM_VCPU_STATE_FPU 3
+#define KVM_VCPU_STATE_MP 4
+
+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];
+};
+
+struct kvm_vcpu_state_list {
+ __u32 ntypes;
+ __u32 types[0]; /* KVM_VCPU_STATE_* or KVM_$(ARCH)_VCPU_STATE_* */
+};
+
#define KVMIO 0xAE
/*
@@ -416,6 +442,8 @@ struct kvm_ioeventfd {
#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
+#define KVM_CHECK_VCPU_STATES _IOWR(KVMIO, 0x09, \
+ struct kvm_vcpu_state_list)
/*
* Extension capability list.
@@ -484,6 +512,7 @@ struct kvm_ioeventfd {
#define KVM_CAP_XEN_HVM 38
#endif
#define KVM_CAP_ADJUST_CLOCK 39
+#define KVM_CAP_VCPU_STATE 40
#ifdef KVM_CAP_IRQ_ROUTING
@@ -667,6 +696,9 @@ struct kvm_clock_data {
/* 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..8a39bd6 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -332,6 +332,12 @@ 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);
+bool kvm_arch_check_substate(u32 type);
+
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 bd44fb4..4623c18 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1247,124 +1247,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_SET_SREGS: {
+ 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_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;
}
@@ -1373,10 +1473,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;
}
@@ -1390,53 +1490,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;
}
@@ -1650,6 +1783,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
@@ -1664,6 +1798,19 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
return kvm_dev_ioctl_check_extension(arg);
}
+static bool kvm_check_substate(u32 type)
+{
+ switch (type) {
+ case KVM_VCPU_STATE_REGS:
+ case KVM_VCPU_STATE_SREGS:
+ case KVM_VCPU_STATE_FPU:
+ case KVM_VCPU_STATE_MP:
+ return true;
+ default:
+ return kvm_arch_check_substate(type);
+ }
+}
+
static long kvm_dev_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
@@ -1702,6 +1849,42 @@ static long kvm_dev_ioctl(struct file *filp,
case KVM_TRACE_DISABLE:
r = -EOPNOTSUPP;
break;
+ case KVM_CHECK_VCPU_STATES: {
+ struct kvm_vcpu_state_list __user *user_head = (void *)arg;
+ struct kvm_vcpu_state_list head;
+ __u32 *types;
+ size_t size;
+ int i;
+
+ r = -EFAULT;
+ if (copy_from_user(&head, user_head,
+ sizeof(struct kvm_vcpu_state_list)))
+ break;
+
+ r = -E2BIG;
+ if (head.ntypes > KVM_MAX_VCPU_SUBSTATES)
+ break;
+
+ size = head.ntypes * sizeof(__u32);
+ types = kmalloc(size, GFP_KERNEL);
+ r = -ENOMEM;
+ if (!types)
+ break;
+
+ r = -EFAULT;
+ if (copy_from_user(types, user_head->types, size))
+ goto vcpu_state_out;
+
+ for (i = 0; i < head.ntypes; i++)
+ if (!kvm_check_substate(types[i]))
+ types[i] = KVM_VCPU_STATE_INVALID;
+ r = 0;
+vcpu_state_out:
+ if (copy_to_user(&user_head->types, types, size))
+ r = -EFAULT;
+ kfree(types);
+ break;
+ }
default:
return kvm_arch_dev_ioctl(filp, ioctl, arg);
}
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-02 16:20 ` [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL Jan Kiszka
@ 2009-11-04 11:18 ` Avi Kivity
2009-11-04 11:35 ` Jan Kiszka
2009-11-10 10:14 ` Avi Kivity
1 sibling, 1 reply; 22+ messages in thread
From: Avi Kivity @ 2009-11-04 11:18 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm
On 11/02/2009 06:20 PM, Jan Kiszka wrote:
> 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, introduces KVM_CHECK_VCPU_STATES to query supported
> substates, 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.
>
> +
> +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 */
> +};
>
64-bit offset is a bit too much, and can't be made to work on 32-bit
hosts. Also, why signed? it's begging for trouble.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-04 11:18 ` Avi Kivity
@ 2009-11-04 11:35 ` Jan Kiszka
2009-11-04 12:52 ` Avi Kivity
0 siblings, 1 reply; 22+ messages in thread
From: Jan Kiszka @ 2009-11-04 11:35 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
Avi Kivity wrote:
> On 11/02/2009 06:20 PM, Jan Kiszka wrote:
>> 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, introduces KVM_CHECK_VCPU_STATES to query supported
>> substates, 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.
>>
>> +
>> +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 */
>> +};
>>
>
> 64-bit offset is a bit too much, and can't be made to work on 32-bit
> hosts. Also, why signed? it's begging for trouble.
Please elaborate what troubles you see precisely. The offset is supposed
to make the whole user's address space reachable relative to the
header's base address. Therefore 64 bit, therefore signed.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-04 11:35 ` Jan Kiszka
@ 2009-11-04 12:52 ` Avi Kivity
0 siblings, 0 replies; 22+ messages in thread
From: Avi Kivity @ 2009-11-04 12:52 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 11/04/2009 01:35 PM, Jan Kiszka wrote:
> Avi Kivity wrote:
>
>> On 11/02/2009 06:20 PM, Jan Kiszka wrote:
>>
>>> 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, introduces KVM_CHECK_VCPU_STATES to query supported
>>> substates, 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.
>>>
>>> +
>>> +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 */
>>> +};
>>>
>>>
>> 64-bit offset is a bit too much, and can't be made to work on 32-bit
>> hosts. Also, why signed? it's begging for trouble.
>>
> Please elaborate what troubles you see precisely. The offset is supposed
> to make the whole user's address space reachable relative to the
> header's base address. Therefore 64 bit, therefore signed.
>
Misread the code; patch is fine.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-02 16:20 ` [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL Jan Kiszka
2009-11-04 11:18 ` Avi Kivity
@ 2009-11-10 10:14 ` Avi Kivity
2009-11-10 12:03 ` Jan Kiszka
1 sibling, 1 reply; 22+ messages in thread
From: Avi Kivity @ 2009-11-10 10:14 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm
On 11/02/2009 06:20 PM, Jan Kiszka wrote:
> 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.
>
I'm having some second thoughts about this.
What does the new API buy us? Instead of declaring two new ioctls for
new/fixed substates, we only have to declare one. We still have the
capability check. We still have to declare a structure.
It's true that the internals are currently a mess. We can fix that with
a table-driven approach:
static struct kvm_state_ioctl state_ioctls[] = {
{
.get_ioctl = KVM_GET_FPU,
.set_ioctl = KVM_SET_FPU,
.get = kvm_get_fpu,
.set = kvm_set_fpu,
.size = sizeof(struct kvm_fpu), /* 0 for variable-size state */
},
};
(btw, the new stuff would also benefit from this).
So, what's the real justification for the new ABI?
Jan, my apologies for raising this at such a very late stage in the
review, after all the nits have been satisfactorily addressed. But I
want to make sure we don't bloat the interface without very good reasons.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-10 10:14 ` Avi Kivity
@ 2009-11-10 12:03 ` Jan Kiszka
2009-11-10 12:57 ` Avi Kivity
0 siblings, 1 reply; 22+ messages in thread
From: Jan Kiszka @ 2009-11-10 12:03 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
Avi Kivity wrote:
> On 11/02/2009 06:20 PM, Jan Kiszka wrote:
>> 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.
>>
>
> I'm having some second thoughts about this.
>
> What does the new API buy us? Instead of declaring two new ioctls for
> new/fixed substates, we only have to declare one. We still have the
> capability check. We still have to declare a structure.
Right, we still need CAPs to protect us against undefined types. So
KVM_CHECK_VCPU_STATES is actually pointless - well, someone asked for it...
>
> It's true that the internals are currently a mess. We can fix that with
> a table-driven approach:
>
> static struct kvm_state_ioctl state_ioctls[] = {
> {
> .get_ioctl = KVM_GET_FPU,
> .set_ioctl = KVM_SET_FPU,
> .get = kvm_get_fpu,
> .set = kvm_set_fpu,
> .size = sizeof(struct kvm_fpu), /* 0 for variable-size state */
Even a variable-sized state has a fixed-size header. The handlers would
have to deal with this, or we would need to define which field in the
header holds the extension size, and what is its multiplier.
> },
> };
>
> (btw, the new stuff would also benefit from this).
Right.
>
> So, what's the real justification for the new ABI?
The remaining differences are:
- single kernel call possible
- slightly higher regularity (the IOCTL space is rather chaotic)
>
> Jan, my apologies for raising this at such a very late stage in the
> review, after all the nits have been satisfactorily addressed. But I
> want to make sure we don't bloat the interface without very good reasons.
I think we came from the idea: "Let's have one new IOCTL that will fit
it all - now and then." That's obviously not cheaply achievable. So the
valid question is what our extension concept of the future should be,
the existing multi-IOCTL approach or the substates? I only have a slight
bias towards the latter but the strong wish to achieve to a final decision.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-10 12:03 ` Jan Kiszka
@ 2009-11-10 12:57 ` Avi Kivity
2009-11-10 13:22 ` Jan Kiszka
0 siblings, 1 reply; 22+ messages in thread
From: Avi Kivity @ 2009-11-10 12:57 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 11/10/2009 02:03 PM, Jan Kiszka wrote:
>
>> I'm having some second thoughts about this.
>>
>> What does the new API buy us? Instead of declaring two new ioctls for
>> new/fixed substates, we only have to declare one. We still have the
>> capability check. We still have to declare a structure.
>>
> Right, we still need CAPs to protect us against undefined types. So
> KVM_CHECK_VCPU_STATES is actually pointless - well, someone asked for it...
>
It's not pointless - you can do a compile-time check for
KVM_VCPU_STATE_... and a runtime check using KVM_CHECK_VCPU_STATES. But
it does duplicate the existing KVM_CAP_ functionality.
>> It's true that the internals are currently a mess. We can fix that with
>> a table-driven approach:
>>
>> static struct kvm_state_ioctl state_ioctls[] = {
>> {
>> .get_ioctl = KVM_GET_FPU,
>> .set_ioctl = KVM_SET_FPU,
>> .get = kvm_get_fpu,
>> .set = kvm_set_fpu,
>> .size = sizeof(struct kvm_fpu), /* 0 for variable-size state */
>>
> Even a variable-sized state has a fixed-size header. The handlers would
> have to deal with this, or we would need to define which field in the
> header holds the extension size, and what is its multiplier.
>
Since we have very few variable-size states, and their number is
unlikely to increase, ad-hoc handling should be sufficient.
>> So, what's the real justification for the new ABI?
>>
> The remaining differences are:
> - single kernel call possible
>
Is there a real advantage in this? It's not a high performance call,
typically only called during save/restore, reset, and for vmware's
wonderful ioport interface.
> - slightly higher regularity (the IOCTL space is rather chaotic)
>
But still, actually handling the state is not regular either on the
userspace or kernel side.
>> Jan, my apologies for raising this at such a very late stage in the
>> review, after all the nits have been satisfactorily addressed. But I
>> want to make sure we don't bloat the interface without very good reasons.
>>
> I think we came from the idea: "Let's have one new IOCTL that will fit
> it all - now and then." That's obviously not cheaply achievable. So the
> valid question is what our extension concept of the future should be,
> the existing multi-IOCTL approach or the substates? I only have a slight
> bias towards the latter but the strong wish to achieve to a final decision.
>
It would have been better to start from substates in the first place,
since there is less duplication: instead of 2 x NR_STATES ioctls, we
define 2 ioctls + NR_STATES defines. It's more regular and less chance
for errors (like misspelling _IOR/_IOW).
But given that we already do have the old interface, perhaps it's best
to stick with it and concentrate on improving the internals.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-10 12:57 ` Avi Kivity
@ 2009-11-10 13:22 ` Jan Kiszka
2009-11-10 13:31 ` Avi Kivity
0 siblings, 1 reply; 22+ messages in thread
From: Jan Kiszka @ 2009-11-10 13:22 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
Avi Kivity wrote:
> On 11/10/2009 02:03 PM, Jan Kiszka wrote:
>>> I'm having some second thoughts about this.
>>>
>>> What does the new API buy us? Instead of declaring two new ioctls for
>>> new/fixed substates, we only have to declare one. We still have the
>>> capability check. We still have to declare a structure.
>>>
>> Right, we still need CAPs to protect us against undefined types. So
>> KVM_CHECK_VCPU_STATES is actually pointless - well, someone asked for it...
>>
>
> It's not pointless - you can do a compile-time check for
> KVM_VCPU_STATE_... and a runtime check using KVM_CHECK_VCPU_STATES. But
> it does duplicate the existing KVM_CAP_ functionality.
It's redundant, therefore I considered it pointless.
>
>>> It's true that the internals are currently a mess. We can fix that with
>>> a table-driven approach:
>>>
>>> static struct kvm_state_ioctl state_ioctls[] = {
>>> {
>>> .get_ioctl = KVM_GET_FPU,
>>> .set_ioctl = KVM_SET_FPU,
>>> .get = kvm_get_fpu,
>>> .set = kvm_set_fpu,
>>> .size = sizeof(struct kvm_fpu), /* 0 for variable-size state */
>>>
>> Even a variable-sized state has a fixed-size header. The handlers would
>> have to deal with this, or we would need to define which field in the
>> header holds the extension size, and what is its multiplier.
>>
>
> Since we have very few variable-size states, and their number is
> unlikely to increase, ad-hoc handling should be sufficient.
Regarding CPU states, there is actually only the MSR interface.
>
>>> So, what's the real justification for the new ABI?
>>>
>> The remaining differences are:
>> - single kernel call possible
>>
>
> Is there a real advantage in this? It's not a high performance call,
> typically only called during save/restore, reset, and for vmware's
> wonderful ioport interface.
And debugging. But, true, this is all fairly uncritical.
>
>> - slightly higher regularity (the IOCTL space is rather chaotic)
>>
>
> But still, actually handling the state is not regular either on the
> userspace or kernel side.
>
>>> Jan, my apologies for raising this at such a very late stage in the
>>> review, after all the nits have been satisfactorily addressed. But I
>>> want to make sure we don't bloat the interface without very good reasons.
>>>
>> I think we came from the idea: "Let's have one new IOCTL that will fit
>> it all - now and then." That's obviously not cheaply achievable. So the
>> valid question is what our extension concept of the future should be,
>> the existing multi-IOCTL approach or the substates? I only have a slight
>> bias towards the latter but the strong wish to achieve to a final decision.
>>
>
> It would have been better to start from substates in the first place,
> since there is less duplication: instead of 2 x NR_STATES ioctls, we
> define 2 ioctls + NR_STATES defines. It's more regular and less chance
> for errors (like misspelling _IOR/_IOW).
>
> But given that we already do have the old interface, perhaps it's best
> to stick with it and concentrate on improving the internals.
So the new roadmap shall be like this:
o Add KVM_X86_GET/SET_EVENT_STATES (instead of
KVM_X86_VCPU_STATE_EVENTS)
o Refactor in-kernel VCPU state IOCTLs to use table-driven dispatching
and shared argument passing
o Maybe refactor user space as well towards a table-driven state sync
(need to think about this a bit more)
Any other comments or does everyone agree?
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-10 13:22 ` Jan Kiszka
@ 2009-11-10 13:31 ` Avi Kivity
2009-11-10 13:41 ` Jan Kiszka
0 siblings, 1 reply; 22+ messages in thread
From: Avi Kivity @ 2009-11-10 13:31 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 11/10/2009 03:22 PM, Jan Kiszka wrote:
>
>> Since we have very few variable-size states, and their number is
>> unlikely to increase, ad-hoc handling should be sufficient.
>>
> Regarding CPU states, there is actually only the MSR interface.
>
>
cpuid internal states too.
> So the new roadmap shall be like this:
>
> o Add KVM_X86_GET/SET_EVENT_STATES (instead of
> KVM_X86_VCPU_STATE_EVENTS)
>
> o Refactor in-kernel VCPU state IOCTLs to use table-driven dispatching
> and shared argument passing
>
> o Maybe refactor user space as well towards a table-driven state sync
> (need to think about this a bit more)
>
> Any other comments or does everyone agree?
>
Looks good to me. These are all independent, of course.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL
2009-11-10 13:31 ` Avi Kivity
@ 2009-11-10 13:41 ` Jan Kiszka
0 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2009-11-10 13:41 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
Avi Kivity wrote:
> On 11/10/2009 03:22 PM, Jan Kiszka wrote:
>>> Since we have very few variable-size states, and their number is
>>> unlikely to increase, ad-hoc handling should be sufficient.
>>>
>> Regarding CPU states, there is actually only the MSR interface.
>>
>>
>
> cpuid internal states too.
Oh, yes.
>
>> So the new roadmap shall be like this:
>>
>> o Add KVM_X86_GET/SET_EVENT_STATES (instead of
>> KVM_X86_VCPU_STATE_EVENTS)
>>
>> o Refactor in-kernel VCPU state IOCTLs to use table-driven dispatching
>> and shared argument passing
>>
>> o Maybe refactor user space as well towards a table-driven state sync
>> (need to think about this a bit more)
>>
>> Any other comments or does everyone agree?
>>
>
> Looks good to me. These are all independent, of course.
Yep. The first one will come first and ASAP, should be easy to derive
from existing series. The others have to wait a bit now.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 3/4] KVM: x86: Add support for KVM_GET/SET_VCPU_STATE
2009-11-02 16:20 [PATCH v3 0/4] Extensible VCPU state IOCTL Jan Kiszka
` (2 preceding siblings ...)
2009-11-02 16:20 ` [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL Jan Kiszka
@ 2009-11-02 16:20 ` Jan Kiszka
3 siblings, 0 replies; 22+ messages in thread
From: Jan Kiszka @ 2009-11-02 16:20 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 | 230 ++++++++++++++++++++++++++++----------------
3 files changed, 170 insertions(+), 86 deletions(-)
diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 3c98d0a..6421aee 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -911,3 +911,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 ef9b4b7..42286ef 100644
--- a/arch/x86/include/asm/kvm.h
+++ b/arch/x86/include/asm/kvm.h
@@ -151,7 +151,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];
};
@@ -252,4 +252,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 816e681..3484787 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1276,11 +1276,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;
@@ -1301,15 +1301,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);
@@ -1888,61 +1895,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: {
- r = -EINVAL;
- if (!vcpu->arch.apic)
- goto out;
- 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: {
- r = -EINVAL;
- if (!vcpu->arch.apic)
- goto out;
- 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;
}
@@ -1952,60 +1934,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;
};
@@ -2014,10 +1976,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;
@@ -2027,7 +1989,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;
}
@@ -2036,15 +1998,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;
}
@@ -4845,18 +4805,118 @@ 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;
+
+ r = -EINVAL;
+ if (!vcpu->arch.apic)
+ break;
+ 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;
+
+ r = -EINVAL;
+ if (!vcpu->arch.apic)
+ break;
+ 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;
}
bool kvm_arch_check_substate(u32 type)
{
- return false;
+ switch (type) {
+ case KVM_X86_VCPU_STATE_MSRS:
+ case KVM_X86_VCPU_STATE_CPUID:
+ case KVM_X86_VCPU_STATE_LAPIC:
+ return true;
+ default:
+ return false;
+ }
}
void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
^ permalink raw reply related [flat|nested] 22+ messages in thread