KVM: mark segments accessed on HW task switch
PATCH 1/1 KVM: mark segments accessed on HW task switch
On HW task switch newly loaded segments should me marked as accessed.
arch/x86/kvm/x86.c +9 -13
@@ -4708,18 +4708,6 @@ static u16 get_segment_selector(struct kvm_vcpu *vcpu, int seg)return kvm_seg.selector; } -static int load_segment_descriptor_to_kvm_desct (struct kvm_vcpu *vcpu, -u16 selector, -struct kvm_segment *kvm_seg) -{ -struct desc_struct seg_desc; - -if (load_guest_segment_descriptor (vcpu, selector, &seg_desc)) -return 1 ; -seg_desct_to_kvm_desct (&seg_desc, selector, kvm_seg); -return 0 ; -} -static int kvm_load_realmode_segment (struct kvm_vcpu *vcpu,u16 selector,int seg) {struct kvm_segment segvar = {
@@ -4760,11 +4748,14 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, int type_bits, int seg) {struct kvm_segment kvm_seg; +struct desc_struct seg_desc;if (is_vm86_segment (vcpu, seg) || !(kvm_read_cr0_bits (vcpu,X86_CR0_PE )))return kvm_load_realmode_segment (vcpu, selector, seg); -if (load_segment_descriptor_to_kvm_desct (vcpu, selector, &kvm_seg)) + +if (load_guest_segment_descriptor (vcpu, selector, &seg_desc))return 1 ; +seg_desct_to_kvm_desct (&seg_desc, selector, &kvm_seg);kvm_check_segment_descriptor (vcpu, seg, selector); kvm_seg.type |= type_bits;
@@ -4775,6 +4766,11 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, kvm_seg.unusable =1 ;kvm_set_segment (vcpu, &kvm_seg, seg); +if (selector && !kvm_seg.unusable && kvm_seg.s) { +/* mark segment as accessed */ + seg_desc.type |=1 ; +save_guest_segment_descriptor (vcpu, selector, &seg_desc); + }return 0 ; }
--
Gleb.
Quoted 4 lines · Gleb Natapov
On HW task switch newly loaded segments should me marked as accessed.
arch/x86/kvm/x86.c +0
-0@@ -4775,6 +4766,11 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, kvm_seg.unusable =1 ;
arch/x86/kvm/x86.c int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, · line 4776
Quoted 10 lines
arch/x86/kvm/x86.c +5
-0kvm_set_segment (vcpu,&kvm_seg, seg); +if (selector&& !kvm_seg.unusable&& kvm_seg.s) { +/* mark segment as accessed */ + seg_desc.type |=1 ; +save_guest_segment_descriptor (vcpu, selector,&seg_desc); + }return 0 ; }
What about an error return from s_g_s_d?
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 6 lines · Avi Kivity
Quoted 5 lines · Gleb Natapov
On HW task switch newly loaded segments should me marked as accessed.
arch/x86/kvm/x86.c +0
-0@@ -4775,6 +4766,11 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, kvm_seg.unusable =1 ;
Quoted 11 lines
arch/x86/kvm/x86.c int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, · line 4775
Quoted 8 lines
arch/x86/kvm/x86.c +5
-0kvm_set_segment (vcpu,&kvm_seg, seg); +if (selector&& !kvm_seg.unusable&& kvm_seg.s) { +/* mark segment as accessed */ + seg_desc.type |=1 ; +save_guest_segment_descriptor (vcpu, selector,&seg_desc); + }return 0 ; }What about an error return from
s_g_s_d?
What can or should we do about it?
--
Gleb.
Quoted 21 lines · Gleb Natapov
Quoted 7 lines · Avi Kivity
Quoted 5 lines · Gleb Natapov
On HW task switch newly loaded segments should me marked as accessed.
arch/x86/kvm/x86.c +0
-0@@ -4775,6 +4766,11 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, kvm_seg.unusable =1 ;Quoted 12 lines
arch/x86/kvm/x86.c int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, · line 4776
Quoted 9 lines
arch/x86/kvm/x86.c +5
-0kvm_set_segment (vcpu,&kvm_seg, seg); +if (selector&& !kvm_seg.unusable&& kvm_seg.s) { +/* mark segment as accessed */ + seg_desc.type |=1 ; +save_guest_segment_descriptor (vcpu, selector,&seg_desc); + }return 0 ; }What about an error return from
s_g_s_d?
Quoted 3 lines
What can or should we do about it?
If -EFAULT, propagate to userspace.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 18 lines · Avi Kivity
Quoted 17 lines · Gleb Natapov
Quoted 16 lines · Avi Kivity
arch/x86/kvm/x86.c int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, · line 4775
Quoted 13 lines · Gleb Natapov
On HW task switch newly loaded segments should me marked as accessed.
arch/x86/kvm/x86.c +5
-0@@ -4775,6 +4766,11 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, kvm_seg.unusable =1 ;kvm_set_segment (vcpu,&kvm_seg, seg); +if (selector&& !kvm_seg.unusable&& kvm_seg.s) { +/* mark segment as accessed */ + seg_desc.type |=1 ; +save_guest_segment_descriptor (vcpu, selector,&seg_desc); + }return 0 ; }What about an error return from
s_g_s_d?
Quoted 5 lines
Quoted 2 lines
What can or should we do about it?
If
-EFAULT, propagate to userspace.
We don't handle it anywhere in task switch emulation. Separate patch?
--
Gleb.
Quoted 25 lines · Gleb Natapov
Quoted 23 lines · Avi Kivity
Quoted 21 lines · Gleb Natapov
Quoted 19 lines · Avi Kivity
Quoted 4 lines · Gleb Natapov
On HW task switch newly loaded segments should me marked as accessed.
arch/x86/kvm/x86.c +0
-0@@ -4775,6 +4766,11 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, kvm_seg.unusable =1 ;arch/x86/kvm/x86.c int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, · line 4776
Quoted 10 lines
arch/x86/kvm/x86.c +5
-0kvm_set_segment (vcpu,&kvm_seg, seg); +if (selector&& !kvm_seg.unusable&& kvm_seg.s) { +/* mark segment as accessed */ + seg_desc.type |=1 ; +save_guest_segment_descriptor (vcpu, selector,&seg_desc); + }return 0 ; }What about an error return from
s_g_s_d?
Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
What can or should we do about it?
If
-EFAULT, propagate to userspace.We don't handle it anywhere in task switch emulation. Separate patch?
Things like 'return kvm_write_guest_virt()' do handle it.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 20 lines · Avi Kivity
Quoted 19 lines · Gleb Natapov
Quoted 18 lines · Avi Kivity
Quoted 17 lines · Gleb Natapov
Quoted 16 lines · Avi Kivity
arch/x86/kvm/x86.c int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, · line 4775
Quoted 13 lines · Gleb Natapov
On HW task switch newly loaded segments should me marked as accessed.
arch/x86/kvm/x86.c +5
-0@@ -4775,6 +4766,11 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, kvm_seg.unusable =1 ;kvm_set_segment (vcpu,&kvm_seg, seg); +if (selector&& !kvm_seg.unusable&& kvm_seg.s) { +/* mark segment as accessed */ + seg_desc.type |=1 ; +save_guest_segment_descriptor (vcpu, selector,&seg_desc); + }return 0 ; }What about an error return from
s_g_s_d?
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
What can or should we do about it?
If
-EFAULT, propagate to userspace.We don't handle it anywhere in task switch emulation. Separate patch?
Things like 'return
kvm_write_guest_virt()' do handle it.
That what save_guest_segment_descriptor() calls, but error is not propagated to userspace anywhere in the task switch code. Lets apply this patch and I'll send follow up with fixes for error handling in task switch code.
--
Gleb.
Quoted 9 lines · Gleb Natapov
Quoted 4 lines
Things like 'return
kvm_write_guest_virt()' do handle it.That what
save_guest_segment_descriptor()calls, but error is not propagated to userspace anywhere in the task switch code. Lets apply this patch and I'll send follow up with fixes for error handling in task switch code.
Okay.
Signature · 1 line
error compiling committee.c: too many arguments to function
Marcelo Tosatti
AppliedQuoted 4 lines · Gleb Natapov
On HW task switch newly loaded segments should me marked as accessed.
Reported-by Lorenzo Martignoni <[email protected]>Signed-off-by Gleb Natapov <[email protected]>
Applied, thanks.