KVM: x86: Add SYNC REGS functionality.
Subsystems (4)
v4 cover KVM: x86: Add SYNC REGS functionality.
This patch set adds KVM_CAP_SYNC_REGS functionality for x86.
Changes from v3 <https://www.spinics.net/lists/kvm/msg162834.html>SYNC_REGS_SIZE_BYTES patch (1/2)
- No changes.KVM_CAP_SYNC_REGS patch (2/2)
- KVM_SYNC_X86_NUM_FIELDS removed.
- Avoid overwriting error code in kvm_arch_vcpu_ioctl_run.
- Shortened __kvm_arch_vcpu_ioctl_{{set,get}_{sregs,regs} to __{set,get}_{sregs,regs}.
- Renamed sync_regs/store_regs to match s390.
Changes from v2 <https://patchwork.kernel.org/patch/10161941/>
- v3 splits v2 changes into two separate patches, moving SYNC_REGS_SIZE_BYTES #define change to its own patch.
- Refactors kvm_arch_vcpu_ioctl_set_{regs,sregs} to avoid redundant calls to both vcpu_load and vcpu_put in their usage by sync regs code.
Please see the individual patches for details.
Ken Hofsass (2):
KVM: x86: add SYNC_REGS_SIZE_BYTES #define.
KVM: x86: KVM_CAP_SYNC_REGS
Documentation/virtual/kvm/api.txt | 42 ++++++++++++++- arch/x86/include/uapi/asm/kvm.h | 19 ++++++- arch/x86/kvm/x86.c | 108 +++++++++++++++++++++++++++++++++----- include/uapi/linux/kvm.h | 6 ++- 4 files changed, 158 insertions(+), 17 deletions(-)
Signature · 1 line
2.16.0.rc1.238.g530d649a79-goog
PATCH v4 1/2 KVM: x86: add SYNC_REGS_SIZE_BYTES #define.
Replace hardcoded padding size value for struct kvm_sync_regs with #define SYNC_REGS_SIZE_BYTES.
Also update the value specified in api.txt from outdated hardcoded value to SYNC_REGS_SIZE_BYTES.
Signed-off-by Ken Hofsass <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
--- Documentation/virtual/kvm/api.txt | 2 +- include/uapi/linux/kvm.h | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-)
Documentation/virtual/kvm/api.txt +1 -1
@@ -3809,7 +3809,7 @@ in userspace. __u64 kvm_dirty_regs; union { struct kvm_sync_regs regs; - char padding[1024]; + char padding[SYNC_REGS_SIZE_BYTES]; } s; If KVM_CAP_SYNC_REGS is defined, these fields allow userspace to access
include/uapi/linux/kvm.h +5 -1
@@ -396,6 +396,10 @@ struct kvm_run {char padding[256 ]; }; +/* 2048 is the size of the char array used to bound/pad the size +* of the union that holds sync regs. +*/ +#define SYNC_REGS_SIZE_BYTES 2048/* * shared registers between kvm and userspace. * kvm_valid_regs specifies the register classes set by the host
@@ -407,7 +411,7 @@ struct kvm_run {__u64 kvm_dirty_regs;union {struct kvm_sync_regs regs; -char padding[2048 ]; +char padding[SYNC_REGS_SIZE_BYTES ]; } s; };
Signature · 1 line
2.16.0.rc1.238.g530d649a79-goog
PATCH v4 2/2 KVM: x86: KVM_CAP_SYNC_REGS
In reply to Ken HofsassThis commit implements an enhanced x86 version of S390 KVM_CAP_SYNC_REGS functionality. KVM_CAP_SYNC_REGS "allow[s] userspace to access certain guest registers without having to call SET/GET_*REGS”. This reduces ioctl overhead which is particularly important when userspace is making synchronous guest state modifications (e.g. when emulating and/or intercepting instructions).
Originally implemented upstream for the S390, the x86 differences follow:
- userspace can select the register sets to be synchronized with kvm_run using bit-flags in the kvm_valid_registers and kvm_dirty_registers fields.
- vcpu_events is available in addition to the regs and sregs register sets.
Signed-off-by Ken Hofsass <[email protected]>
--- Documentation/virtual/kvm/api.txt | 40 ++++++++++++++ arch/x86/include/uapi/asm/kvm.h | 19 ++++++- arch/x86/kvm/x86.c | 108 +++++++++++++++++++++++++++++++++----- 3 files changed, 152 insertions(+), 15 deletions(-)
Documentation/virtual/kvm/api.txt +40 -0
@@ -4014,6 +4014,46 @@ Once this is done the KVM_REG_MIPS_VEC_* and KVM_REG_MIPS_MSA_* registers can be accessed, and the Config5.MSAEn bit is accessible via the KVM API and also from the guest. +6.74 KVM_CAP_SYNC_REGS +Architectures: s390, x86 +Target: s390: always enabled, x86: vcpu +Parameters: none +Returns: x86: KVM_CHECK_EXTENSION returns a bit-array indicating which register +sets are supported (bitfields defined in arch/x86/include/uapi/asm/kvm.h). + +As described above in the kvm_sync_regs struct info in section 5 (kvm_run): +KVM_CAP_SYNC_REGS "allow[s] userspace to access certain guest registers +without having to call SET/GET_*REGS". This reduces overhead by eliminating +repeated ioctl calls for setting and/or getting register values. This is +particularly important when userspace is making synchronous guest state +modifications, e.g. when emulating and/or intercepting instructions in +userspace. + +For s390 specifics, please refer to the source code. + +For x86: +- the register sets to be copied out to kvm_run are selectable + by userspace (rather that all sets being copied out for every exit). +- vcpu_events are available in addition to regs and sregs. + +For x86, the 'kvm_valid_regs' field of struct kvm_run is overloaded to +function as an input bit-array field set by userspace to indicate the +specific register sets to be copied out on the next exit. + +To indicate when userspace has modified values that should be copied into +the vCPU, the all architecture bitarray field, 'kvm_dirty_regs' must be set. +This is done using the same bitflags as for the 'kvm_valid_regs' field. +If the dirty bit is not set, then the register set values will not be copied +into the vCPU even if they've been modified. + +Unused bitfields in the bitarrays must be set to zero. + +struct kvm_sync_regs { + struct kvm_regs regs; + struct kvm_sregs sregs; + struct kvm_vcpu_events events; +}; + 7. Capabilities that can be enabled on VMs ------------------------------------------
arch/x86/include/uapi/asm/kvm.h +18 -1
@@ -354,8 +354,25 @@ struct kvm_xcrs {__u64 padding[16 ]; }; -/* definition of registers in kvm_run */ +#define KVM_SYNC_X86_REGS (1UL << 0) +#define KVM_SYNC_X86_SREGS (1UL << 1) +#define KVM_SYNC_X86_EVENTS (1UL << 2) + +#define KVM_SYNC_X86_VALID_FIELDS \ + (KVM_SYNC_X86_REGS| \ + KVM_SYNC_X86_SREGS| \ + KVM_SYNC_X86_EVENTS) + +/* kvm_sync_regs struct included by kvm_run struct */ struct kvm_sync_regs { +/* Members of this structure are potentially malicious. +* Care must be taken by code reading, esp. interpreting, +* data fields from them inside KVM to prevent TOCTOU and +* double-fetch types of vulnerabilities. +*/ +struct kvm_regs regs; +struct kvm_sregs sregs; +struct kvm_vcpu_events events; };#define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)
arch/x86/kvm/x86.c +94 -14
204 lines not on this page · raw message
Signature · 1 line
2.16.0.rc1.238.g530d649a79-goog
Re: [PATCH v4 2/2] KVM: x86: KVM_CAP_SYNC_REGS
arch/x86/include/uapi/asm/kvm.h struct kvm_xcrs { · line 372
Quoted 8 lines
arch/x86/include/uapi/asm/kvm.h +6
-0+/* kvm_sync_regs struct included by kvm_run struct */ struct kvm_sync_regs { +/* Members of this structure are potentially malicious. +* Care must be taken by code reading, esp. interpreting, +* data fields from them inside KVM to prevent TOCTOU and +* double-fetch types of vulnerabilities. +*/
Indeed, did you check if the set functions are safe?
Quoted 44 lines
arch/x86/include/uapi/asm/kvm.h +3
-0+struct kvm_regs regs; +struct kvm_sregs sregs; +struct kvm_vcpu_events events; };#define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)arch/x86/kvm/x86.c +12
-0@@ -100,6 +100,9 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu);static void process_nmi (struct kvm_vcpu *vcpu);static void enter_smm (struct kvm_vcpu *vcpu);static void __kvm_set_rflags (struct kvm_vcpu *vcpu,unsigned long rflags); +static void store_regs (struct kvm_vcpu *vcpu); +static int sync_regs (struct kvm_vcpu *vcpu); +static int check_valid_regs (struct kvm_vcpu *vcpu);struct kvm_x86_ops *kvm_x86_ops__read_mostly ;EXPORT_SYMBOL_GPL (kvm_x86_ops);@@ -2743,6 +2746,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)case KVM_CAP_IMMEDIATE_EXIT : r =1 ;break ; +case KVM_CAP_SYNC_REGS : + r =KVM_SYNC_X86_VALID_FIELDS ; +break ;case KVM_CAP_ADJUST_CLOCK : r =KVM_CLOCK_TSC_STABLE ;break ;@@ -7325,6 +7331,12 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)return 0 ; } +static int check_valid_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_valid_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; +return 0 ; +}int kvm_arch_vcpu_ioctl_run (struct kvm_vcpu *vcpu,struct kvm_run *kvm_run) {
arch/x86/kvm/x86.c int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) · line 7366
Quoted 4 lines
arch/x86/kvm/x86.c +1
-0@@ -7351,6 +7363,17 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) goto out; } +if (vcpu->run->kvm_valid_regs) {
You can check directly, without this if. (and not sure if the extra function is needed here).
Quoted 14 lines
arch/x86/kvm/x86.c +10
-0+ r =check_valid_regs (vcpu); +if (r !=0 ) + goto out; + } +if (vcpu->run->kvm_dirty_regs) { + r =sync_regs (vcpu); +if (r !=0 ) + goto out; + } +/* re-sync apic's tpr */ if (!lapic_in_kernel (vcpu)) {if (kvm_set_cr8 (vcpu, kvm_run->cr8) !=0 ) {
arch/x86/kvm/x86.c int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) · line 7398
Quoted 1 line
arch/x86/kvm/x86.c +0
-0@@ -7375,6 +7398,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
Looks good to me
Reviewed-by David Hildenbrand <[email protected]>
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v4 2/2] KVM: x86: KVM_CAP_SYNC_REGS
2018-02-01 14:05+0100, David Hildenbrand:
Quoted 8 lines
arch/x86/kvm/x86.c int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) · line 7366
Quoted 5 lines
arch/x86/kvm/x86.c +1
-0@@ -7351,6 +7363,17 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) goto out; } +if (vcpu->run->kvm_valid_regs) {You can check directly, without this if. (and not sure if the extra function is needed here).
Right, it's unlikely that we'll be doing more checks on kvm_valid_regs, so I've moved it here and replaced the block with
if (vcpu->run->kvm_valid_regs & ~KVM_SYNC_X86_VALID_FIELDS) {
r = -EINVAL;
goto out;
}You can see the result in kvm/queue.
Btw. the userspace doesn't know what the next userspace exit is going to be, so it the purpose of kvm_valid_regs to avoid needless writes of registers on old userspaces?
Thanks.
Quoted 4 lines
Quoted 4 lines
arch/x86/kvm/x86.c +4
-0+ r =check_valid_regs (vcpu); +if (r !=0 ) + goto out; + }
Re: [PATCH v4 2/2] KVM: x86: KVM_CAP_SYNC_REGS
Radim,
Quoted 9 lines · Radim Krčmář
Right, it's unlikely that we'll be doing more checks on
kvm_valid_regs, so I've moved it here and replaced the block withif (vcpu->run->kvm_valid_regs & ~KVM_SYNC_X86_VALID_FIELDS) { r = -EINVAL; goto out; }You can see the result in kvm/queue.
Thanks for cleaning that up & queuing the commits!
Quoted 3 lines
Btw. the userspace doesn't know what the next userspace exit is going to be, so it the purpose of
kvm_valid_regsto avoid needless writes of registers on old userspaces?
If you mean userspace agents that don't have code to use SYNC_REGS, then yes. Or new userspace agents that have not enabled features that use S_R or features that only need a subset of available register sets. Originally I had FPU and debug_regs as well and expected that those would not be as broadly used and wanted to enable turning them on/off. Also, I wanted to keep the flexibility because I have related patches extending and building on SYNC_REGS that I will be sending for review in the near future.
thanks again,
Ken
Re: [PATCH v4 2/2] KVM: x86: KVM_CAP_SYNC_REGS
In reply to Ken Hofsass2018-02-01 8:03 GMT+08:00 Ken Hofsass <[email protected]>:
Quoted 16 lines
This commit implements an enhanced x86 version of S390
KVM_CAP_SYNC_REGSfunctionality.KVM_CAP_SYNC_REGS"allow[s] userspace to access certain guest registers without having to call SET/GET_*REGS”. This reduces ioctl overhead which is particularly important when userspace is making synchronous guest state modifications (e.g. when emulating and/or intercepting instructions).Originally implemented upstream for the S390, the x86 differences follow:
- userspace can select the register sets to be synchronized withkvm_runusing bit-flags in thekvm_valid_registersandkvm_dirty_registersfields.
-vcpu_eventsis available in addition to the regs and sregs register sets.
Quoted 1 line
Signed-off-by Ken Hofsass <[email protected]>
This commit in kvm/queue results in:
general protection fault: 0000 [#1] PREEMPT SMP PTI CPU: 7 PID: 2348 Comm: qemu-system-x86 Not tainted 4.15.0+ #4 Hardware name: LENOVO ThinkCentre M8500t-N000/SHARKBAY, BIOS FBKTC1AUS 02/16/2016 RIP: 0010:preempt_notifier_unregister+0x13/0x40 RSP: 0018:ffffbe6541c07da8 EFLAGS: 00010286 RAX: dead000000000100 RBX: ffff99ed46ca8000 RCX: 0000000037271e7b RDX: dead000000000200 RSI: 0000000000000000 RDI: ffff99ed46ca8008 RBP: ffffbe6541c07da8 R08: 00000000fe0913b4 R09: 442c1f7a00000000 R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000 R13: ffff99ed46ca8000 R14: ffff99ed46ca8058 R15: ffff99ed47635400 FS: 00007f3c2b474700(0000) GS:ffff99ed8e200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f3c1c001000 CR3: 00000003c73f4006 CR4: 00000000001626e0 Call Trace: vcpu_put+0x28/0x50 [kvm] kvm_arch_vcpu_ioctl_set_sregs+0x2d/0x40 [kvm] kvm_vcpu_ioctl+0x54a/0x720 [kvm] ? __fget+0xfc/0x210 ? __fget+0xfc/0x210 do_vfs_ioctl+0xa4/0x6a0 ? __fget+0x11d/0x210 SyS_ioctl+0x79/0x90 entry_SYSCALL_64_fastpath+0x25/0x9c RIP: 0033:0x7f3c36fd0f47 RSP: 002b:00007f3c2b473668 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000555a11f78c10 RCX: 00007f3c36fd0f47 RDX: 00007f3c2b473710 RSI: 000000004138ae84 RDI: 000000000000000f RBP: 0000000000000003 R08: 0000555a107825b0 R09: 00007f3c1c001000 R10: 00007f3c1c003010 R11: 0000000000000246 R12: 00007f3c2b473870 R13: 00007f3c1c001000 R14: 00007f3c2b4749c0 R15: 0000555a11f78c10 RIP: preempt_notifier_unregister+0x13/0x40 RSP: ffffbe6541c07da8 ---[ end trace 318a26af2b7ef2bc ]---
Regards,
Wanpeng Li
Quoted 313 lines
--- Documentation/virtual/kvm/api.txt | 40 ++++++++++++++ arch/x86/include/uapi/asm/kvm.h | 19 ++++++- arch/x86/kvm/x86.c | 108 +++++++++++++++++++++++++++++++++----- 3 files changed, 152 insertions(+), 15 deletions(-)Documentation/virtual/kvm/api.txt +40
-0@@ -4014,6 +4014,46 @@ Once this is done the KVM_REG_MIPS_VEC_* and KVM_REG_MIPS_MSA_* registers can be accessed, and the Config5.MSAEn bit is accessible via the KVM API and also from the guest. +6.74 KVM_CAP_SYNC_REGS +Architectures: s390, x86 +Target: s390: always enabled, x86: vcpu +Parameters: none +Returns: x86: KVM_CHECK_EXTENSION returns a bit-array indicating which register +sets are supported (bitfields defined in arch/x86/include/uapi/asm/kvm.h). + +As described above in the kvm_sync_regs struct info in section 5 (kvm_run): +KVM_CAP_SYNC_REGS "allow[s] userspace to access certain guest registers +without having to call SET/GET_*REGS". This reduces overhead by eliminating +repeated ioctl calls for setting and/or getting register values. This is +particularly important when userspace is making synchronous guest state +modifications, e.g. when emulating and/or intercepting instructions in +userspace. + +For s390 specifics, please refer to the source code. + +For x86: +- the register sets to be copied out to kvm_run are selectable + by userspace (rather that all sets being copied out for every exit). +- vcpu_events are available in addition to regs and sregs. + +For x86, the 'kvm_valid_regs' field of struct kvm_run is overloaded to +function as an input bit-array field set by userspace to indicate the +specific register sets to be copied out on the next exit. + +To indicate when userspace has modified values that should be copied into +the vCPU, the all architecture bitarray field, 'kvm_dirty_regs' must be set. +This is done using the same bitflags as for the 'kvm_valid_regs' field. +If the dirty bit is not set, then the register set values will not be copied +into the vCPU even if they've been modified. + +Unused bitfields in the bitarrays must be set to zero. + +struct kvm_sync_regs { + struct kvm_regs regs; + struct kvm_sregs sregs; + struct kvm_vcpu_events events; +}; + 7. Capabilities that can be enabled on VMs ------------------------------------------arch/x86/include/uapi/asm/kvm.h +18
-1@@ -354,8 +354,25 @@ struct kvm_xcrs {__u64 padding[16 ]; }; -/* definition of registers in kvm_run */ +#define KVM_SYNC_X86_REGS (1UL << 0) +#define KVM_SYNC_X86_SREGS (1UL << 1) +#define KVM_SYNC_X86_EVENTS (1UL << 2) + +#define KVM_SYNC_X86_VALID_FIELDS \ + (KVM_SYNC_X86_REGS| \ + KVM_SYNC_X86_SREGS| \ + KVM_SYNC_X86_EVENTS) + +/* kvm_sync_regs struct included by kvm_run struct */ struct kvm_sync_regs { +/* Members of this structure are potentially malicious. +* Care must be taken by code reading, esp. interpreting, +* data fields from them inside KVM to prevent TOCTOU and +* double-fetch types of vulnerabilities. +*/ +struct kvm_regs regs; +struct kvm_sregs sregs; +struct kvm_vcpu_events events; };#define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)arch/x86/kvm/x86.c +94
-15@@ -100,6 +100,9 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu);static void process_nmi (struct kvm_vcpu *vcpu);static void enter_smm (struct kvm_vcpu *vcpu);static void __kvm_set_rflags (struct kvm_vcpu *vcpu,unsigned long rflags); +static void store_regs (struct kvm_vcpu *vcpu); +static int sync_regs (struct kvm_vcpu *vcpu); +static int check_valid_regs (struct kvm_vcpu *vcpu);struct kvm_x86_ops *kvm_x86_ops__read_mostly ;EXPORT_SYMBOL_GPL (kvm_x86_ops);@@ -2743,6 +2746,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)case KVM_CAP_IMMEDIATE_EXIT : r =1 ;break ; +case KVM_CAP_SYNC_REGS : + r =KVM_SYNC_X86_VALID_FIELDS ; +break ;case KVM_CAP_ADJUST_CLOCK : r =KVM_CLOCK_TSC_STABLE ;break ;@@ -7325,6 +7331,12 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)return 0 ; } +static int check_valid_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_valid_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; +return 0 ; +}int kvm_arch_vcpu_ioctl_run (struct kvm_vcpu *vcpu,struct kvm_run *kvm_run) {@@ -7351,6 +7363,17 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) goto out; } +if (vcpu->run->kvm_valid_regs) { + r =check_valid_regs (vcpu); +if (r !=0 ) + goto out; + } +if (vcpu->run->kvm_dirty_regs) { + r =sync_regs (vcpu); +if (r !=0 ) + goto out; + } +/* re-sync apic's tpr */ if (!lapic_in_kernel (vcpu)) {if (kvm_set_cr8 (vcpu, kvm_run->cr8) !=0 ) {@@ -7375,6 +7398,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) out:kvm_put_guest_fpu (vcpu); +if (vcpu->run->kvm_valid_regs) +store_regs (vcpu);post_kvm_run_save (vcpu);kvm_sigset_deactivate (vcpu);@@ -7382,10 +7407,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)return r; } -int kvm_arch_vcpu_ioctl_get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +static void __get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) { -vcpu_load (vcpu); -if (vcpu->arch.emulate_regs_need_sync_to_vcpu) {/* * We are here if userspace calls get_regs() in the middle of @@ -7418,15 +7441,18 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) regs->rip =kvm_rip_read (vcpu); regs->rflags =kvm_get_rflags (vcpu); +} +int kvm_arch_vcpu_ioctl_get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +{ +vcpu_load (vcpu); +__get_regs (vcpu, regs);vcpu_put (vcpu);return 0 ; } -int kvm_arch_vcpu_ioctl_set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +static void __set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) { -vcpu_load (vcpu); - vcpu->arch.emulate_regs_need_sync_from_vcpu = true; vcpu->arch.emulate_regs_need_sync_to_vcpu = false;@@ -7455,7 +7481,12 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) vcpu->arch.exception.pending = false;kvm_make_request (KVM_REQ_EVENT , vcpu); +} +int kvm_arch_vcpu_ioctl_set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +{ +vcpu_load (vcpu); +__set_regs (vcpu, regs);vcpu_put (vcpu);return 0 ; }@@ -7470,13 +7501,10 @@ void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu, int *db, int *l) }EXPORT_SYMBOL_GPL (kvm_get_cs_db_l_bits); -int kvm_arch_vcpu_ioctl_get_sregs (struct kvm_vcpu *vcpu, -struct kvm_sregs *sregs) +static void __get_sregs (struct kvm_vcpu *vcpu,struct kvm_sregs *sregs) {struct desc_ptr dt; -vcpu_load (vcpu); -kvm_get_segment (vcpu, &sregs->cs,VCPU_SREG_CS );kvm_get_segment (vcpu, &sregs->ds,VCPU_SREG_DS );kvm_get_segment (vcpu, &sregs->es,VCPU_SREG_ES );@@ -7507,7 +7535,13 @@ int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,if (vcpu->arch.interrupt.pending && !vcpu->arch.interrupt.soft)set_bit (vcpu->arch.interrupt.nr, (unsigned long *)sregs->interrupt_bitmap); +} +int kvm_arch_vcpu_ioctl_get_sregs (struct kvm_vcpu *vcpu, +struct kvm_sregs *sregs) +{ +vcpu_load (vcpu); +__get_sregs (vcpu, sregs);vcpu_put (vcpu);return 0 ; }@@ -7579,8 +7613,7 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index, }EXPORT_SYMBOL_GPL (kvm_task_switch); -int kvm_arch_vcpu_ioctl_set_sregs (struct kvm_vcpu *vcpu, -struct kvm_sregs *sregs) +static int __set_sregs (struct kvm_vcpu *vcpu,struct kvm_sregs *sregs) {struct msr_data apic_base_msr;int mmu_reset_needed =0 ;@@ -7588,8 +7621,6 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,struct desc_ptr dt;int ret = -EINVAL ; -vcpu_load (vcpu); -if (!guest_cpuid_has (vcpu,X86_FEATURE_XSAVE ) && (sregs->cr4 &X86_CR4_OSXSAVE )) goto out;@@ -7665,6 +7696,16 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, ret =0 ; out: +return ret; +} + +int kvm_arch_vcpu_ioctl_set_sregs (struct kvm_vcpu *vcpu, +struct kvm_sregs *sregs) +{ +int ret; + +vcpu_load (vcpu); + ret =__set_sregs (vcpu, sregs);vcpu_put (vcpu);return ret; }@@ -7791,6 +7832,45 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)return 0 ; } +static void store_regs (struct kvm_vcpu *vcpu) +{ +BUILD_BUG_ON (sizeof (struct kvm_sync_regs ) >SYNC_REGS_SIZE_BYTES ); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_REGS ) +__get_regs (vcpu, &vcpu->run->s.regs.regs); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_SREGS ) +__get_sregs (vcpu, &vcpu->run->s.regs.sregs); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_EVENTS ) +kvm_vcpu_ioctl_x86_get_vcpu_events ( + vcpu, &vcpu->run->s.regs.events); +} + +static int sync_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_dirty_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; + +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_REGS ) { +__set_regs (vcpu, &vcpu->run->s.regs.regs); + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_REGS ; + } +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_SREGS ) { +if (__set_sregs (vcpu, &vcpu->run->s.regs.sregs)) +return -EINVAL ; + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_SREGS ; + } +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_EVENTS ) { +if (kvm_vcpu_ioctl_x86_set_vcpu_events ( + vcpu, &vcpu->run->s.regs.events)) +return -EINVAL ; + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_EVENTS ; + } + +return 0 ; +} +static void fx_init (struct kvm_vcpu *vcpu) {fpstate_init (&vcpu->arch.guest_fpu.state); --2.16.0.rc1.238.g530d649a79-goog
Re: [PATCH v4 2/2] KVM: x86: KVM_CAP_SYNC_REGS
Hi Wanpeng, that's disturbing! Could you give me more context? Did this occur while running part of the KVM unit tests? Does this happen every time? Do you know if this is this the first invocation of kvm_arch_vcpu_ioctl_set_sregs? When I ran against kvm-unit-tests w/qemu, I didn't hit this. Nor when I ran our other internal tests.
thanks,
Ken Hofsass
Quoted 371 lines · Wanpeng Li
2018-02-01 8:03 GMT+08:00 Ken Hofsass <[email protected]>:
Quoted 16 lines
This commit implements an enhanced x86 version of S390
KVM_CAP_SYNC_REGSfunctionality.KVM_CAP_SYNC_REGS"allow[s] userspace to access certain guest registers without having to call SET/GET_*REGS”. This reduces ioctl overhead which is particularly important when userspace is making synchronous guest state modifications (e.g. when emulating and/or intercepting instructions).Originally implemented upstream for the S390, the x86 differences follow:
- userspace can select the register sets to be synchronized withkvm_runusing bit-flags in thekvm_valid_registersandkvm_dirty_registersfields.
-vcpu_eventsis available in addition to the regs and sregs register sets.Quoted 1 line
Signed-off-by Ken Hofsass <[email protected]>This commit in kvm/queue results in:
general protection fault: 0000 [#1] PREEMPT SMP PTI CPU: 7 PID: 2348 Comm: qemu-system-x86 Not tainted 4.15.0+ #4 Hardware name: LENOVO ThinkCentre M8500t-N000/SHARKBAY, BIOS FBKTC1AUS 02/16/2016 RIP: 0010:preempt_notifier_unregister+0x13/0x40 RSP: 0018:ffffbe6541c07da8 EFLAGS: 00010286 RAX: dead000000000100 RBX: ffff99ed46ca8000 RCX: 0000000037271e7b RDX: dead000000000200 RSI: 0000000000000000 RDI: ffff99ed46ca8008 RBP: ffffbe6541c07da8 R08: 00000000fe0913b4 R09: 442c1f7a00000000 R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000 R13: ffff99ed46ca8000 R14: ffff99ed46ca8058 R15: ffff99ed47635400 FS: 00007f3c2b474700(0000) GS:ffff99ed8e200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f3c1c001000 CR3: 00000003c73f4006 CR4: 00000000001626e0 Call Trace: vcpu_put+0x28/0x50 [kvm] kvm_arch_vcpu_ioctl_set_sregs+0x2d/0x40 [kvm] kvm_vcpu_ioctl+0x54a/0x720 [kvm] ? __fget+0xfc/0x210 ? __fget+0xfc/0x210 do_vfs_ioctl+0xa4/0x6a0 ? __fget+0x11d/0x210 SyS_ioctl+0x79/0x90 entry_SYSCALL_64_fastpath+0x25/0x9c RIP: 0033:0x7f3c36fd0f47 RSP: 002b:00007f3c2b473668 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000555a11f78c10 RCX: 00007f3c36fd0f47 RDX: 00007f3c2b473710 RSI: 000000004138ae84 RDI: 000000000000000f RBP: 0000000000000003 R08: 0000555a107825b0 R09: 00007f3c1c001000 R10: 00007f3c1c003010 R11: 0000000000000246 R12: 00007f3c2b473870 R13: 00007f3c1c001000 R14: 00007f3c2b4749c0 R15: 0000555a11f78c10 RIP: preempt_notifier_unregister+0x13/0x40 RSP: ffffbe6541c07da8 ---[ end trace 318a26af2b7ef2bc ]---Regards,
Wanpeng LiQuoted 313 lines
--- Documentation/virtual/kvm/api.txt | 40 ++++++++++++++ arch/x86/include/uapi/asm/kvm.h | 19 ++++++- arch/x86/kvm/x86.c | 108 +++++++++++++++++++++++++++++++++----- 3 files changed, 152 insertions(+), 15 deletions(-)Documentation/virtual/kvm/api.txt +40
-0@@ -4014,6 +4014,46 @@ Once this is done the KVM_REG_MIPS_VEC_* and KVM_REG_MIPS_MSA_* registers can be accessed, and the Config5.MSAEn bit is accessible via the KVM API and also from the guest. +6.74 KVM_CAP_SYNC_REGS +Architectures: s390, x86 +Target: s390: always enabled, x86: vcpu +Parameters: none +Returns: x86: KVM_CHECK_EXTENSION returns a bit-array indicating which register +sets are supported (bitfields defined in arch/x86/include/uapi/asm/kvm.h). + +As described above in the kvm_sync_regs struct info in section 5 (kvm_run): +KVM_CAP_SYNC_REGS "allow[s] userspace to access certain guest registers +without having to call SET/GET_*REGS". This reduces overhead by eliminating +repeated ioctl calls for setting and/or getting register values. This is +particularly important when userspace is making synchronous guest state +modifications, e.g. when emulating and/or intercepting instructions in +userspace. + +For s390 specifics, please refer to the source code. + +For x86: +- the register sets to be copied out to kvm_run are selectable + by userspace (rather that all sets being copied out for every exit). +- vcpu_events are available in addition to regs and sregs. + +For x86, the 'kvm_valid_regs' field of struct kvm_run is overloaded to +function as an input bit-array field set by userspace to indicate the +specific register sets to be copied out on the next exit. + +To indicate when userspace has modified values that should be copied into +the vCPU, the all architecture bitarray field, 'kvm_dirty_regs' must be set. +This is done using the same bitflags as for the 'kvm_valid_regs' field. +If the dirty bit is not set, then the register set values will not be copied +into the vCPU even if they've been modified. + +Unused bitfields in the bitarrays must be set to zero. + +struct kvm_sync_regs { + struct kvm_regs regs; + struct kvm_sregs sregs; + struct kvm_vcpu_events events; +}; + 7. Capabilities that can be enabled on VMs ------------------------------------------arch/x86/include/uapi/asm/kvm.h +18
-1@@ -354,8 +354,25 @@ struct kvm_xcrs {__u64 padding[16 ]; }; -/* definition of registers in kvm_run */ +#define KVM_SYNC_X86_REGS (1UL << 0) +#define KVM_SYNC_X86_SREGS (1UL << 1) +#define KVM_SYNC_X86_EVENTS (1UL << 2) + +#define KVM_SYNC_X86_VALID_FIELDS \ + (KVM_SYNC_X86_REGS| \ + KVM_SYNC_X86_SREGS| \ + KVM_SYNC_X86_EVENTS) + +/* kvm_sync_regs struct included by kvm_run struct */ struct kvm_sync_regs { +/* Members of this structure are potentially malicious. +* Care must be taken by code reading, esp. interpreting, +* data fields from them inside KVM to prevent TOCTOU and +* double-fetch types of vulnerabilities. +*/ +struct kvm_regs regs; +struct kvm_sregs sregs; +struct kvm_vcpu_events events; };#define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)arch/x86/kvm/x86.c +94
-15@@ -100,6 +100,9 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu);static void process_nmi (struct kvm_vcpu *vcpu);static void enter_smm (struct kvm_vcpu *vcpu);static void __kvm_set_rflags (struct kvm_vcpu *vcpu,unsigned long rflags); +static void store_regs (struct kvm_vcpu *vcpu); +static int sync_regs (struct kvm_vcpu *vcpu); +static int check_valid_regs (struct kvm_vcpu *vcpu);struct kvm_x86_ops *kvm_x86_ops__read_mostly ;EXPORT_SYMBOL_GPL (kvm_x86_ops);@@ -2743,6 +2746,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)case KVM_CAP_IMMEDIATE_EXIT : r =1 ;break ; +case KVM_CAP_SYNC_REGS : + r =KVM_SYNC_X86_VALID_FIELDS ; +break ;case KVM_CAP_ADJUST_CLOCK : r =KVM_CLOCK_TSC_STABLE ;break ;@@ -7325,6 +7331,12 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)return 0 ; } +static int check_valid_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_valid_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; +return 0 ; +}int kvm_arch_vcpu_ioctl_run (struct kvm_vcpu *vcpu,struct kvm_run *kvm_run) {@@ -7351,6 +7363,17 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) goto out; } +if (vcpu->run->kvm_valid_regs) { + r =check_valid_regs (vcpu); +if (r !=0 ) + goto out; + } +if (vcpu->run->kvm_dirty_regs) { + r =sync_regs (vcpu); +if (r !=0 ) + goto out; + } +/* re-sync apic's tpr */ if (!lapic_in_kernel (vcpu)) {if (kvm_set_cr8 (vcpu, kvm_run->cr8) !=0 ) {@@ -7375,6 +7398,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) out:kvm_put_guest_fpu (vcpu); +if (vcpu->run->kvm_valid_regs) +store_regs (vcpu);post_kvm_run_save (vcpu);kvm_sigset_deactivate (vcpu);@@ -7382,10 +7407,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)return r; } -int kvm_arch_vcpu_ioctl_get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +static void __get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) { -vcpu_load (vcpu); -if (vcpu->arch.emulate_regs_need_sync_to_vcpu) {/* * We are here if userspace calls get_regs() in the middle of @@ -7418,15 +7441,18 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) regs->rip =kvm_rip_read (vcpu); regs->rflags =kvm_get_rflags (vcpu); +} +int kvm_arch_vcpu_ioctl_get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +{ +vcpu_load (vcpu); +__get_regs (vcpu, regs);vcpu_put (vcpu);return 0 ; } -int kvm_arch_vcpu_ioctl_set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +static void __set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) { -vcpu_load (vcpu); - vcpu->arch.emulate_regs_need_sync_from_vcpu = true; vcpu->arch.emulate_regs_need_sync_to_vcpu = false;@@ -7455,7 +7481,12 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) vcpu->arch.exception.pending = false;kvm_make_request (KVM_REQ_EVENT , vcpu); +} +int kvm_arch_vcpu_ioctl_set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +{ +vcpu_load (vcpu); +__set_regs (vcpu, regs);vcpu_put (vcpu);return 0 ; }@@ -7470,13 +7501,10 @@ void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu, int *db, int *l) }EXPORT_SYMBOL_GPL (kvm_get_cs_db_l_bits); -int kvm_arch_vcpu_ioctl_get_sregs (struct kvm_vcpu *vcpu, -struct kvm_sregs *sregs) +static void __get_sregs (struct kvm_vcpu *vcpu,struct kvm_sregs *sregs) {struct desc_ptr dt; -vcpu_load (vcpu); -kvm_get_segment (vcpu, &sregs->cs,VCPU_SREG_CS );kvm_get_segment (vcpu, &sregs->ds,VCPU_SREG_DS );kvm_get_segment (vcpu, &sregs->es,VCPU_SREG_ES );@@ -7507,7 +7535,13 @@ int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,if (vcpu->arch.interrupt.pending && !vcpu->arch.interrupt.soft)set_bit (vcpu->arch.interrupt.nr, (unsigned long *)sregs->interrupt_bitmap); +} +int kvm_arch_vcpu_ioctl_get_sregs (struct kvm_vcpu *vcpu, +struct kvm_sregs *sregs) +{ +vcpu_load (vcpu); +__get_sregs (vcpu, sregs);vcpu_put (vcpu);return 0 ; }@@ -7579,8 +7613,7 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index, }EXPORT_SYMBOL_GPL (kvm_task_switch); -int kvm_arch_vcpu_ioctl_set_sregs (struct kvm_vcpu *vcpu, -struct kvm_sregs *sregs) +static int __set_sregs (struct kvm_vcpu *vcpu,struct kvm_sregs *sregs) {struct msr_data apic_base_msr;int mmu_reset_needed =0 ;@@ -7588,8 +7621,6 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,struct desc_ptr dt;int ret = -EINVAL ; -vcpu_load (vcpu); -if (!guest_cpuid_has (vcpu,X86_FEATURE_XSAVE ) && (sregs->cr4 &X86_CR4_OSXSAVE )) goto out;@@ -7665,6 +7696,16 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, ret =0 ; out: +return ret; +} + +int kvm_arch_vcpu_ioctl_set_sregs (struct kvm_vcpu *vcpu, +struct kvm_sregs *sregs) +{ +int ret; + +vcpu_load (vcpu); + ret =__set_sregs (vcpu, sregs);vcpu_put (vcpu);return ret; }@@ -7791,6 +7832,45 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)return 0 ; } +static void store_regs (struct kvm_vcpu *vcpu) +{ +BUILD_BUG_ON (sizeof (struct kvm_sync_regs ) >SYNC_REGS_SIZE_BYTES ); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_REGS ) +__get_regs (vcpu, &vcpu->run->s.regs.regs); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_SREGS ) +__get_sregs (vcpu, &vcpu->run->s.regs.sregs); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_EVENTS ) +kvm_vcpu_ioctl_x86_get_vcpu_events ( + vcpu, &vcpu->run->s.regs.events); +} + +static int sync_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_dirty_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; + +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_REGS ) { +__set_regs (vcpu, &vcpu->run->s.regs.regs); + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_REGS ; + } +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_SREGS ) { +if (__set_sregs (vcpu, &vcpu->run->s.regs.sregs)) +return -EINVAL ; + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_SREGS ; + } +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_EVENTS ) { +if (kvm_vcpu_ioctl_x86_set_vcpu_events ( + vcpu, &vcpu->run->s.regs.events)) +return -EINVAL ; + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_EVENTS ; + } + +return 0 ; +} +static void fx_init (struct kvm_vcpu *vcpu) {fpstate_init (&vcpu->arch.guest_fpu.state); --2.16.0.rc1.238.g530d649a79-goog
Re: [PATCH v4 2/2] KVM: x86: KVM_CAP_SYNC_REGS
2018-02-06 8:32 GMT+08:00 Ken Hofsass <[email protected]>:
Quoted 5 lines
Hi Wanpeng, that's disturbing! Could you give me more context? Did this occur while running part of the KVM unit tests? Does this happen every time? Do you know if this is this the first invocation of
kvm_arch_vcpu_ioctl_set_sregs? When I ran against kvm-unit-tests w/qemu, I didn't hit this. Nor when I ran our other internal tests.
I just run a kvm guest (ubuntu 16.04 4.15-rc3) against kvm/queue w/ CONFIG_PREEMPT enabled.
Regards,
Wanpeng Li
Quoted 376 lines
thanks,
Ken HofsassQuoted 371 lines · Wanpeng Li
2018-02-01 8:03 GMT+08:00 Ken Hofsass <[email protected]>:
Quoted 16 lines
This commit implements an enhanced x86 version of S390
KVM_CAP_SYNC_REGSfunctionality.KVM_CAP_SYNC_REGS"allow[s] userspace to access certain guest registers without having to call SET/GET_*REGS”. This reduces ioctl overhead which is particularly important when userspace is making synchronous guest state modifications (e.g. when emulating and/or intercepting instructions).Originally implemented upstream for the S390, the x86 differences follow:
- userspace can select the register sets to be synchronized withkvm_runusing bit-flags in thekvm_valid_registersandkvm_dirty_registersfields.
-vcpu_eventsis available in addition to the regs and sregs register sets.Quoted 1 line
Signed-off-by Ken Hofsass <[email protected]>This commit in kvm/queue results in:
general protection fault: 0000 [#1] PREEMPT SMP PTI CPU: 7 PID: 2348 Comm: qemu-system-x86 Not tainted 4.15.0+ #4 Hardware name: LENOVO ThinkCentre M8500t-N000/SHARKBAY, BIOS FBKTC1AUS 02/16/2016 RIP: 0010:preempt_notifier_unregister+0x13/0x40 RSP: 0018:ffffbe6541c07da8 EFLAGS: 00010286 RAX: dead000000000100 RBX: ffff99ed46ca8000 RCX: 0000000037271e7b RDX: dead000000000200 RSI: 0000000000000000 RDI: ffff99ed46ca8008 RBP: ffffbe6541c07da8 R08: 00000000fe0913b4 R09: 442c1f7a00000000 R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000 R13: ffff99ed46ca8000 R14: ffff99ed46ca8058 R15: ffff99ed47635400 FS: 00007f3c2b474700(0000) GS:ffff99ed8e200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f3c1c001000 CR3: 00000003c73f4006 CR4: 00000000001626e0 Call Trace: vcpu_put+0x28/0x50 [kvm] kvm_arch_vcpu_ioctl_set_sregs+0x2d/0x40 [kvm] kvm_vcpu_ioctl+0x54a/0x720 [kvm] ? __fget+0xfc/0x210 ? __fget+0xfc/0x210 do_vfs_ioctl+0xa4/0x6a0 ? __fget+0x11d/0x210 SyS_ioctl+0x79/0x90 entry_SYSCALL_64_fastpath+0x25/0x9c RIP: 0033:0x7f3c36fd0f47 RSP: 002b:00007f3c2b473668 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000555a11f78c10 RCX: 00007f3c36fd0f47 RDX: 00007f3c2b473710 RSI: 000000004138ae84 RDI: 000000000000000f RBP: 0000000000000003 R08: 0000555a107825b0 R09: 00007f3c1c001000 R10: 00007f3c1c003010 R11: 0000000000000246 R12: 00007f3c2b473870 R13: 00007f3c1c001000 R14: 00007f3c2b4749c0 R15: 0000555a11f78c10 RIP: preempt_notifier_unregister+0x13/0x40 RSP: ffffbe6541c07da8 ---[ end trace 318a26af2b7ef2bc ]---Regards,
Wanpeng LiQuoted 313 lines
--- Documentation/virtual/kvm/api.txt | 40 ++++++++++++++ arch/x86/include/uapi/asm/kvm.h | 19 ++++++- arch/x86/kvm/x86.c | 108 +++++++++++++++++++++++++++++++++----- 3 files changed, 152 insertions(+), 15 deletions(-)Documentation/virtual/kvm/api.txt +40
-0@@ -4014,6 +4014,46 @@ Once this is done the KVM_REG_MIPS_VEC_* and KVM_REG_MIPS_MSA_* registers can be accessed, and the Config5.MSAEn bit is accessible via the KVM API and also from the guest. +6.74 KVM_CAP_SYNC_REGS +Architectures: s390, x86 +Target: s390: always enabled, x86: vcpu +Parameters: none +Returns: x86: KVM_CHECK_EXTENSION returns a bit-array indicating which register +sets are supported (bitfields defined in arch/x86/include/uapi/asm/kvm.h). + +As described above in the kvm_sync_regs struct info in section 5 (kvm_run): +KVM_CAP_SYNC_REGS "allow[s] userspace to access certain guest registers +without having to call SET/GET_*REGS". This reduces overhead by eliminating +repeated ioctl calls for setting and/or getting register values. This is +particularly important when userspace is making synchronous guest state +modifications, e.g. when emulating and/or intercepting instructions in +userspace. + +For s390 specifics, please refer to the source code. + +For x86: +- the register sets to be copied out to kvm_run are selectable + by userspace (rather that all sets being copied out for every exit). +- vcpu_events are available in addition to regs and sregs. + +For x86, the 'kvm_valid_regs' field of struct kvm_run is overloaded to +function as an input bit-array field set by userspace to indicate the +specific register sets to be copied out on the next exit. + +To indicate when userspace has modified values that should be copied into +the vCPU, the all architecture bitarray field, 'kvm_dirty_regs' must be set. +This is done using the same bitflags as for the 'kvm_valid_regs' field. +If the dirty bit is not set, then the register set values will not be copied +into the vCPU even if they've been modified. + +Unused bitfields in the bitarrays must be set to zero. + +struct kvm_sync_regs { + struct kvm_regs regs; + struct kvm_sregs sregs; + struct kvm_vcpu_events events; +}; + 7. Capabilities that can be enabled on VMs ------------------------------------------arch/x86/include/uapi/asm/kvm.h +18
-1@@ -354,8 +354,25 @@ struct kvm_xcrs {__u64 padding[16 ]; }; -/* definition of registers in kvm_run */ +#define KVM_SYNC_X86_REGS (1UL << 0) +#define KVM_SYNC_X86_SREGS (1UL << 1) +#define KVM_SYNC_X86_EVENTS (1UL << 2) + +#define KVM_SYNC_X86_VALID_FIELDS \ + (KVM_SYNC_X86_REGS| \ + KVM_SYNC_X86_SREGS| \ + KVM_SYNC_X86_EVENTS) + +/* kvm_sync_regs struct included by kvm_run struct */ struct kvm_sync_regs { +/* Members of this structure are potentially malicious. +* Care must be taken by code reading, esp. interpreting, +* data fields from them inside KVM to prevent TOCTOU and +* double-fetch types of vulnerabilities. +*/ +struct kvm_regs regs; +struct kvm_sregs sregs; +struct kvm_vcpu_events events; };#define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)arch/x86/kvm/x86.c +94
-15@@ -100,6 +100,9 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu);static void process_nmi (struct kvm_vcpu *vcpu);static void enter_smm (struct kvm_vcpu *vcpu);static void __kvm_set_rflags (struct kvm_vcpu *vcpu,unsigned long rflags); +static void store_regs (struct kvm_vcpu *vcpu); +static int sync_regs (struct kvm_vcpu *vcpu); +static int check_valid_regs (struct kvm_vcpu *vcpu);struct kvm_x86_ops *kvm_x86_ops__read_mostly ;EXPORT_SYMBOL_GPL (kvm_x86_ops);@@ -2743,6 +2746,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)case KVM_CAP_IMMEDIATE_EXIT : r =1 ;break ; +case KVM_CAP_SYNC_REGS : + r =KVM_SYNC_X86_VALID_FIELDS ; +break ;case KVM_CAP_ADJUST_CLOCK : r =KVM_CLOCK_TSC_STABLE ;break ;@@ -7325,6 +7331,12 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)return 0 ; } +static int check_valid_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_valid_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; +return 0 ; +}int kvm_arch_vcpu_ioctl_run (struct kvm_vcpu *vcpu,struct kvm_run *kvm_run) {@@ -7351,6 +7363,17 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) goto out; } +if (vcpu->run->kvm_valid_regs) { + r =check_valid_regs (vcpu); +if (r !=0 ) + goto out; + } +if (vcpu->run->kvm_dirty_regs) { + r =sync_regs (vcpu); +if (r !=0 ) + goto out; + } +/* re-sync apic's tpr */ if (!lapic_in_kernel (vcpu)) {if (kvm_set_cr8 (vcpu, kvm_run->cr8) !=0 ) {@@ -7375,6 +7398,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) out:kvm_put_guest_fpu (vcpu); +if (vcpu->run->kvm_valid_regs) +store_regs (vcpu);post_kvm_run_save (vcpu);kvm_sigset_deactivate (vcpu);@@ -7382,10 +7407,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)return r; } -int kvm_arch_vcpu_ioctl_get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +static void __get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) { -vcpu_load (vcpu); -if (vcpu->arch.emulate_regs_need_sync_to_vcpu) {/* * We are here if userspace calls get_regs() in the middle of @@ -7418,15 +7441,18 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) regs->rip =kvm_rip_read (vcpu); regs->rflags =kvm_get_rflags (vcpu); +} +int kvm_arch_vcpu_ioctl_get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +{ +vcpu_load (vcpu); +__get_regs (vcpu, regs);vcpu_put (vcpu);return 0 ; } -int kvm_arch_vcpu_ioctl_set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +static void __set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) { -vcpu_load (vcpu); - vcpu->arch.emulate_regs_need_sync_from_vcpu = true; vcpu->arch.emulate_regs_need_sync_to_vcpu = false;@@ -7455,7 +7481,12 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) vcpu->arch.exception.pending = false;kvm_make_request (KVM_REQ_EVENT , vcpu); +} +int kvm_arch_vcpu_ioctl_set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +{ +vcpu_load (vcpu); +__set_regs (vcpu, regs);vcpu_put (vcpu);return 0 ; }@@ -7470,13 +7501,10 @@ void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu, int *db, int *l) }EXPORT_SYMBOL_GPL (kvm_get_cs_db_l_bits); -int kvm_arch_vcpu_ioctl_get_sregs (struct kvm_vcpu *vcpu, -struct kvm_sregs *sregs) +static void __get_sregs (struct kvm_vcpu *vcpu,struct kvm_sregs *sregs) {struct desc_ptr dt; -vcpu_load (vcpu); -kvm_get_segment (vcpu, &sregs->cs,VCPU_SREG_CS );kvm_get_segment (vcpu, &sregs->ds,VCPU_SREG_DS );kvm_get_segment (vcpu, &sregs->es,VCPU_SREG_ES );@@ -7507,7 +7535,13 @@ int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,if (vcpu->arch.interrupt.pending && !vcpu->arch.interrupt.soft)set_bit (vcpu->arch.interrupt.nr, (unsigned long *)sregs->interrupt_bitmap); +} +int kvm_arch_vcpu_ioctl_get_sregs (struct kvm_vcpu *vcpu, +struct kvm_sregs *sregs) +{ +vcpu_load (vcpu); +__get_sregs (vcpu, sregs);vcpu_put (vcpu);return 0 ; }@@ -7579,8 +7613,7 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index, }EXPORT_SYMBOL_GPL (kvm_task_switch); -int kvm_arch_vcpu_ioctl_set_sregs (struct kvm_vcpu *vcpu, -struct kvm_sregs *sregs) +static int __set_sregs (struct kvm_vcpu *vcpu,struct kvm_sregs *sregs) {struct msr_data apic_base_msr;int mmu_reset_needed =0 ;@@ -7588,8 +7621,6 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,struct desc_ptr dt;int ret = -EINVAL ; -vcpu_load (vcpu); -if (!guest_cpuid_has (vcpu,X86_FEATURE_XSAVE ) && (sregs->cr4 &X86_CR4_OSXSAVE )) goto out;@@ -7665,6 +7696,16 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, ret =0 ; out: +return ret; +} + +int kvm_arch_vcpu_ioctl_set_sregs (struct kvm_vcpu *vcpu, +struct kvm_sregs *sregs) +{ +int ret; + +vcpu_load (vcpu); + ret =__set_sregs (vcpu, sregs);vcpu_put (vcpu);return ret; }@@ -7791,6 +7832,45 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)return 0 ; } +static void store_regs (struct kvm_vcpu *vcpu) +{ +BUILD_BUG_ON (sizeof (struct kvm_sync_regs ) >SYNC_REGS_SIZE_BYTES ); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_REGS ) +__get_regs (vcpu, &vcpu->run->s.regs.regs); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_SREGS ) +__get_sregs (vcpu, &vcpu->run->s.regs.sregs); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_EVENTS ) +kvm_vcpu_ioctl_x86_get_vcpu_events ( + vcpu, &vcpu->run->s.regs.events); +} + +static int sync_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_dirty_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; + +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_REGS ) { +__set_regs (vcpu, &vcpu->run->s.regs.regs); + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_REGS ; + } +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_SREGS ) { +if (__set_sregs (vcpu, &vcpu->run->s.regs.sregs)) +return -EINVAL ; + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_SREGS ; + } +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_EVENTS ) { +if (kvm_vcpu_ioctl_x86_set_vcpu_events ( + vcpu, &vcpu->run->s.regs.events)) +return -EINVAL ; + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_EVENTS ; + } + +return 0 ; +} +static void fx_init (struct kvm_vcpu *vcpu) {fpstate_init (&vcpu->arch.guest_fpu.state); --2.16.0.rc1.238.g530d649a79-goog
Re: [PATCH v4 2/2] KVM: x86: KVM_CAP_SYNC_REGS
Wanpeng, thanks for noticing that! I was able to replicate the problem and identified the cause.
Radim, in the merged kvm/queue version of arch/x86/kvm/x86.c, vcpu_put() was mistakenly deleted from kvm_arch_vcpu_ioctl_set_mpstate() and inserted into __set_sregs().
So, kvm_arch_vcpu_ioctl_set_sregs() executes two vcpu_put() calls and kvm_arch_vcpu_ioctl_set_mpstate() executes none. #GP soon follows! ;-)
I can send a patch but since the issue on kvm/queue, I'm sure what the protocol is. Anyhow, here's a diff of the corrected code.
--- arch/x86/kvm/x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kvm/x86.c +1 -0
@@ -7678,6 +7678,7 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, ret =0 ; out: +vcpu_put (vcpu);return ret; }
@@ -7811,7 +7812,6 @@ static int __set_sregs(struct kvm_vcpu *vcpu,
struct kvm_sregs *sregs)
ret = 0;
out:
- vcpu_put(vcpu);
return ret;
}
Quoted 389 lines · Wanpeng Li
2018-02-06 8:32 GMT+08:00 Ken Hofsass <[email protected]>:
Quoted 5 lines
Hi Wanpeng, that's disturbing! Could you give me more context? Did this occur while running part of the KVM unit tests? Does this happen every time? Do you know if this is this the first invocation of
kvm_arch_vcpu_ioctl_set_sregs? When I ran against kvm-unit-tests w/qemu, I didn't hit this. Nor when I ran our other internal tests.I just run a kvm guest (ubuntu 16.04 4.15-rc3) against kvm/queue w/
CONFIG_PREEMPTenabled.Regards,
Wanpeng LiQuoted 376 lines
thanks,
Ken HofsassQuoted 371 lines · Wanpeng Li
2018-02-01 8:03 GMT+08:00 Ken Hofsass <[email protected]>:
Quoted 16 lines
This commit implements an enhanced x86 version of S390
KVM_CAP_SYNC_REGSfunctionality.KVM_CAP_SYNC_REGS"allow[s] userspace to access certain guest registers without having to call SET/GET_*REGS”. This reduces ioctl overhead which is particularly important when userspace is making synchronous guest state modifications (e.g. when emulating and/or intercepting instructions).Originally implemented upstream for the S390, the x86 differences follow:
- userspace can select the register sets to be synchronized withkvm_runusing bit-flags in thekvm_valid_registersandkvm_dirty_registersfields.
-vcpu_eventsis available in addition to the regs and sregs register sets.Quoted 1 line
Signed-off-by Ken Hofsass <[email protected]>This commit in kvm/queue results in:
general protection fault: 0000 [#1] PREEMPT SMP PTI CPU: 7 PID: 2348 Comm: qemu-system-x86 Not tainted 4.15.0+ #4 Hardware name: LENOVO ThinkCentre M8500t-N000/SHARKBAY, BIOS FBKTC1AUS 02/16/2016 RIP: 0010:preempt_notifier_unregister+0x13/0x40 RSP: 0018:ffffbe6541c07da8 EFLAGS: 00010286 RAX: dead000000000100 RBX: ffff99ed46ca8000 RCX: 0000000037271e7b RDX: dead000000000200 RSI: 0000000000000000 RDI: ffff99ed46ca8008 RBP: ffffbe6541c07da8 R08: 00000000fe0913b4 R09: 442c1f7a00000000 R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000 R13: ffff99ed46ca8000 R14: ffff99ed46ca8058 R15: ffff99ed47635400 FS: 00007f3c2b474700(0000) GS:ffff99ed8e200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f3c1c001000 CR3: 00000003c73f4006 CR4: 00000000001626e0 Call Trace: vcpu_put+0x28/0x50 [kvm] kvm_arch_vcpu_ioctl_set_sregs+0x2d/0x40 [kvm] kvm_vcpu_ioctl+0x54a/0x720 [kvm] ? __fget+0xfc/0x210 ? __fget+0xfc/0x210 do_vfs_ioctl+0xa4/0x6a0 ? __fget+0x11d/0x210 SyS_ioctl+0x79/0x90 entry_SYSCALL_64_fastpath+0x25/0x9c RIP: 0033:0x7f3c36fd0f47 RSP: 002b:00007f3c2b473668 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000555a11f78c10 RCX: 00007f3c36fd0f47 RDX: 00007f3c2b473710 RSI: 000000004138ae84 RDI: 000000000000000f RBP: 0000000000000003 R08: 0000555a107825b0 R09: 00007f3c1c001000 R10: 00007f3c1c003010 R11: 0000000000000246 R12: 00007f3c2b473870 R13: 00007f3c1c001000 R14: 00007f3c2b4749c0 R15: 0000555a11f78c10 RIP: preempt_notifier_unregister+0x13/0x40 RSP: ffffbe6541c07da8 ---[ end trace 318a26af2b7ef2bc ]---Regards,
Wanpeng LiQuoted 313 lines
--- Documentation/virtual/kvm/api.txt | 40 ++++++++++++++ arch/x86/include/uapi/asm/kvm.h | 19 ++++++- arch/x86/kvm/x86.c | 108 +++++++++++++++++++++++++++++++++----- 3 files changed, 152 insertions(+), 15 deletions(-)Documentation/virtual/kvm/api.txt +40
-0@@ -4014,6 +4014,46 @@ Once this is done the KVM_REG_MIPS_VEC_* and KVM_REG_MIPS_MSA_* registers can be accessed, and the Config5.MSAEn bit is accessible via the KVM API and also from the guest. +6.74 KVM_CAP_SYNC_REGS +Architectures: s390, x86 +Target: s390: always enabled, x86: vcpu +Parameters: none +Returns: x86: KVM_CHECK_EXTENSION returns a bit-array indicating which register +sets are supported (bitfields defined in arch/x86/include/uapi/asm/kvm.h). + +As described above in the kvm_sync_regs struct info in section 5 (kvm_run): +KVM_CAP_SYNC_REGS "allow[s] userspace to access certain guest registers +without having to call SET/GET_*REGS". This reduces overhead by eliminating +repeated ioctl calls for setting and/or getting register values. This is +particularly important when userspace is making synchronous guest state +modifications, e.g. when emulating and/or intercepting instructions in +userspace. + +For s390 specifics, please refer to the source code. + +For x86: +- the register sets to be copied out to kvm_run are selectable + by userspace (rather that all sets being copied out for every exit). +- vcpu_events are available in addition to regs and sregs. + +For x86, the 'kvm_valid_regs' field of struct kvm_run is overloaded to +function as an input bit-array field set by userspace to indicate the +specific register sets to be copied out on the next exit. + +To indicate when userspace has modified values that should be copied into +the vCPU, the all architecture bitarray field, 'kvm_dirty_regs' must be set. +This is done using the same bitflags as for the 'kvm_valid_regs' field. +If the dirty bit is not set, then the register set values will not be copied +into the vCPU even if they've been modified. + +Unused bitfields in the bitarrays must be set to zero. + +struct kvm_sync_regs { + struct kvm_regs regs; + struct kvm_sregs sregs; + struct kvm_vcpu_events events; +}; + 7. Capabilities that can be enabled on VMs ------------------------------------------arch/x86/include/uapi/asm/kvm.h +18
-1@@ -354,8 +354,25 @@ struct kvm_xcrs {__u64 padding[16 ]; }; -/* definition of registers in kvm_run */ +#define KVM_SYNC_X86_REGS (1UL << 0) +#define KVM_SYNC_X86_SREGS (1UL << 1) +#define KVM_SYNC_X86_EVENTS (1UL << 2) + +#define KVM_SYNC_X86_VALID_FIELDS \ + (KVM_SYNC_X86_REGS| \ + KVM_SYNC_X86_SREGS| \ + KVM_SYNC_X86_EVENTS) + +/* kvm_sync_regs struct included by kvm_run struct */ struct kvm_sync_regs { +/* Members of this structure are potentially malicious. +* Care must be taken by code reading, esp. interpreting, +* data fields from them inside KVM to prevent TOCTOU and +* double-fetch types of vulnerabilities. +*/ +struct kvm_regs regs; +struct kvm_sregs sregs; +struct kvm_vcpu_events events; };#define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)arch/x86/kvm/x86.c +94
-15@@ -100,6 +100,9 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu);static void process_nmi (struct kvm_vcpu *vcpu);static void enter_smm (struct kvm_vcpu *vcpu);static void __kvm_set_rflags (struct kvm_vcpu *vcpu,unsigned long rflags); +static void store_regs (struct kvm_vcpu *vcpu); +static int sync_regs (struct kvm_vcpu *vcpu); +static int check_valid_regs (struct kvm_vcpu *vcpu);struct kvm_x86_ops *kvm_x86_ops__read_mostly ;EXPORT_SYMBOL_GPL (kvm_x86_ops);@@ -2743,6 +2746,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)case KVM_CAP_IMMEDIATE_EXIT : r =1 ;break ; +case KVM_CAP_SYNC_REGS : + r =KVM_SYNC_X86_VALID_FIELDS ; +break ;case KVM_CAP_ADJUST_CLOCK : r =KVM_CLOCK_TSC_STABLE ;break ;@@ -7325,6 +7331,12 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)return 0 ; } +static int check_valid_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_valid_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; +return 0 ; +}int kvm_arch_vcpu_ioctl_run (struct kvm_vcpu *vcpu,struct kvm_run *kvm_run) {@@ -7351,6 +7363,17 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) goto out; } +if (vcpu->run->kvm_valid_regs) { + r =check_valid_regs (vcpu); +if (r !=0 ) + goto out; + } +if (vcpu->run->kvm_dirty_regs) { + r =sync_regs (vcpu); +if (r !=0 ) + goto out; + } +/* re-sync apic's tpr */ if (!lapic_in_kernel (vcpu)) {if (kvm_set_cr8 (vcpu, kvm_run->cr8) !=0 ) {@@ -7375,6 +7398,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) out:kvm_put_guest_fpu (vcpu); +if (vcpu->run->kvm_valid_regs) +store_regs (vcpu);post_kvm_run_save (vcpu);kvm_sigset_deactivate (vcpu);@@ -7382,10 +7407,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)return r; } -int kvm_arch_vcpu_ioctl_get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +static void __get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) { -vcpu_load (vcpu); -if (vcpu->arch.emulate_regs_need_sync_to_vcpu) {/* * We are here if userspace calls get_regs() in the middle of @@ -7418,15 +7441,18 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) regs->rip =kvm_rip_read (vcpu); regs->rflags =kvm_get_rflags (vcpu); +} +int kvm_arch_vcpu_ioctl_get_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +{ +vcpu_load (vcpu); +__get_regs (vcpu, regs);vcpu_put (vcpu);return 0 ; } -int kvm_arch_vcpu_ioctl_set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +static void __set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) { -vcpu_load (vcpu); - vcpu->arch.emulate_regs_need_sync_from_vcpu = true; vcpu->arch.emulate_regs_need_sync_to_vcpu = false;@@ -7455,7 +7481,12 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) vcpu->arch.exception.pending = false;kvm_make_request (KVM_REQ_EVENT , vcpu); +} +int kvm_arch_vcpu_ioctl_set_regs (struct kvm_vcpu *vcpu,struct kvm_regs *regs) +{ +vcpu_load (vcpu); +__set_regs (vcpu, regs);vcpu_put (vcpu);return 0 ; }@@ -7470,13 +7501,10 @@ void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu, int *db, int *l) }EXPORT_SYMBOL_GPL (kvm_get_cs_db_l_bits); -int kvm_arch_vcpu_ioctl_get_sregs (struct kvm_vcpu *vcpu, -struct kvm_sregs *sregs) +static void __get_sregs (struct kvm_vcpu *vcpu,struct kvm_sregs *sregs) {struct desc_ptr dt; -vcpu_load (vcpu); -kvm_get_segment (vcpu, &sregs->cs,VCPU_SREG_CS );kvm_get_segment (vcpu, &sregs->ds,VCPU_SREG_DS );kvm_get_segment (vcpu, &sregs->es,VCPU_SREG_ES );@@ -7507,7 +7535,13 @@ int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,if (vcpu->arch.interrupt.pending && !vcpu->arch.interrupt.soft)set_bit (vcpu->arch.interrupt.nr, (unsigned long *)sregs->interrupt_bitmap); +} +int kvm_arch_vcpu_ioctl_get_sregs (struct kvm_vcpu *vcpu, +struct kvm_sregs *sregs) +{ +vcpu_load (vcpu); +__get_sregs (vcpu, sregs);vcpu_put (vcpu);return 0 ; }@@ -7579,8 +7613,7 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index, }EXPORT_SYMBOL_GPL (kvm_task_switch); -int kvm_arch_vcpu_ioctl_set_sregs (struct kvm_vcpu *vcpu, -struct kvm_sregs *sregs) +static int __set_sregs (struct kvm_vcpu *vcpu,struct kvm_sregs *sregs) {struct msr_data apic_base_msr;int mmu_reset_needed =0 ;@@ -7588,8 +7621,6 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,struct desc_ptr dt;int ret = -EINVAL ; -vcpu_load (vcpu); -if (!guest_cpuid_has (vcpu,X86_FEATURE_XSAVE ) && (sregs->cr4 &X86_CR4_OSXSAVE )) goto out;@@ -7665,6 +7696,16 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, ret =0 ; out: +return ret; +} + +int kvm_arch_vcpu_ioctl_set_sregs (struct kvm_vcpu *vcpu, +struct kvm_sregs *sregs) +{ +int ret; + +vcpu_load (vcpu); + ret =__set_sregs (vcpu, sregs);vcpu_put (vcpu);return ret; }@@ -7791,6 +7832,45 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)return 0 ; } +static void store_regs (struct kvm_vcpu *vcpu) +{ +BUILD_BUG_ON (sizeof (struct kvm_sync_regs ) >SYNC_REGS_SIZE_BYTES ); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_REGS ) +__get_regs (vcpu, &vcpu->run->s.regs.regs); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_SREGS ) +__get_sregs (vcpu, &vcpu->run->s.regs.sregs); + +if (vcpu->run->kvm_valid_regs &KVM_SYNC_X86_EVENTS ) +kvm_vcpu_ioctl_x86_get_vcpu_events ( + vcpu, &vcpu->run->s.regs.events); +} + +static int sync_regs (struct kvm_vcpu *vcpu) +{ +if (vcpu->run->kvm_dirty_regs & ~KVM_SYNC_X86_VALID_FIELDS ) +return -EINVAL ; + +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_REGS ) { +__set_regs (vcpu, &vcpu->run->s.regs.regs); + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_REGS ; + } +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_SREGS ) { +if (__set_sregs (vcpu, &vcpu->run->s.regs.sregs)) +return -EINVAL ; + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_SREGS ; + } +if (vcpu->run->kvm_dirty_regs &KVM_SYNC_X86_EVENTS ) { +if (kvm_vcpu_ioctl_x86_set_vcpu_events ( + vcpu, &vcpu->run->s.regs.events)) +return -EINVAL ; + vcpu->run->kvm_dirty_regs &= ~KVM_SYNC_X86_EVENTS ; + } + +return 0 ; +} +static void fx_init (struct kvm_vcpu *vcpu) {fpstate_init (&vcpu->arch.guest_fpu.state); --2.16.0.rc1.238.g530d649a79-goog