* [PATCH] KVM: mark segments accessed on HW task switch
@ 2010-01-25 10:01 Gleb Natapov
2010-01-25 11:08 ` Avi Kivity
2010-01-25 21:34 ` Marcelo Tosatti
0 siblings, 2 replies; 9+ messages in thread
From: Gleb Natapov @ 2010-01-25 10:01 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm
On HW task switch newly loaded segments should me marked as accessed.
Reported-by: Lorenzo Martignoni <martignlo@gmail.com>
Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 47c6e23..b5a2a88 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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.
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] KVM: mark segments accessed on HW task switch
2010-01-25 10:01 [PATCH] KVM: mark segments accessed on HW task switch Gleb Natapov
@ 2010-01-25 11:08 ` Avi Kivity
2010-01-25 11:11 ` Gleb Natapov
2010-01-25 21:34 ` Marcelo Tosatti
1 sibling, 1 reply; 9+ messages in thread
From: Avi Kivity @ 2010-01-25 11:08 UTC (permalink / raw)
To: Gleb Natapov; +Cc: mtosatti, kvm
On 01/25/2010 12:01 PM, Gleb Natapov wrote:
> On HW task switch newly loaded segments should me marked as accessed.
>
> @@ -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?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] KVM: mark segments accessed on HW task switch
2010-01-25 11:08 ` Avi Kivity
@ 2010-01-25 11:11 ` Gleb Natapov
2010-01-25 11:12 ` Avi Kivity
0 siblings, 1 reply; 9+ messages in thread
From: Gleb Natapov @ 2010-01-25 11:11 UTC (permalink / raw)
To: Avi Kivity; +Cc: mtosatti, kvm
On Mon, Jan 25, 2010 at 01:08:13PM +0200, Avi Kivity wrote:
> On 01/25/2010 12:01 PM, Gleb Natapov wrote:
> >On HW task switch newly loaded segments should me marked as accessed.
> >
> >@@ -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?
>
What can or should we do about it?
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] KVM: mark segments accessed on HW task switch
2010-01-25 11:11 ` Gleb Natapov
@ 2010-01-25 11:12 ` Avi Kivity
2010-01-25 12:24 ` Gleb Natapov
0 siblings, 1 reply; 9+ messages in thread
From: Avi Kivity @ 2010-01-25 11:12 UTC (permalink / raw)
To: Gleb Natapov; +Cc: mtosatti, kvm
On 01/25/2010 01:11 PM, Gleb Natapov wrote:
> On Mon, Jan 25, 2010 at 01:08:13PM +0200, Avi Kivity wrote:
>
>> On 01/25/2010 12:01 PM, Gleb Natapov wrote:
>>
>>> On HW task switch newly loaded segments should me marked as accessed.
>>>
>>> @@ -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?
>>
>>
> What can or should we do about it?
>
>
If -EFAULT, propagate to userspace.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] KVM: mark segments accessed on HW task switch
2010-01-25 11:12 ` Avi Kivity
@ 2010-01-25 12:24 ` Gleb Natapov
2010-01-25 12:53 ` Avi Kivity
0 siblings, 1 reply; 9+ messages in thread
From: Gleb Natapov @ 2010-01-25 12:24 UTC (permalink / raw)
To: Avi Kivity; +Cc: mtosatti, kvm
On Mon, Jan 25, 2010 at 01:12:36PM +0200, Avi Kivity wrote:
> On 01/25/2010 01:11 PM, Gleb Natapov wrote:
> >On Mon, Jan 25, 2010 at 01:08:13PM +0200, Avi Kivity wrote:
> >>On 01/25/2010 12:01 PM, Gleb Natapov wrote:
> >>>On HW task switch newly loaded segments should me marked as accessed.
> >>>
> >>>@@ -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?
> >>
> >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.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] KVM: mark segments accessed on HW task switch
2010-01-25 12:24 ` Gleb Natapov
@ 2010-01-25 12:53 ` Avi Kivity
2010-01-25 14:22 ` Gleb Natapov
0 siblings, 1 reply; 9+ messages in thread
From: Avi Kivity @ 2010-01-25 12:53 UTC (permalink / raw)
To: Gleb Natapov; +Cc: mtosatti, kvm
On 01/25/2010 02:24 PM, Gleb Natapov wrote:
> On Mon, Jan 25, 2010 at 01:12:36PM +0200, Avi Kivity wrote:
>
>> On 01/25/2010 01:11 PM, Gleb Natapov wrote:
>>
>>> On Mon, Jan 25, 2010 at 01:08:13PM +0200, Avi Kivity wrote:
>>>
>>>> On 01/25/2010 12:01 PM, Gleb Natapov wrote:
>>>>
>>>>> On HW task switch newly loaded segments should me marked as accessed.
>>>>>
>>>>> @@ -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?
>>>>
>>>>
>>> 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.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] KVM: mark segments accessed on HW task switch
2010-01-25 12:53 ` Avi Kivity
@ 2010-01-25 14:22 ` Gleb Natapov
2010-01-25 14:52 ` Avi Kivity
0 siblings, 1 reply; 9+ messages in thread
From: Gleb Natapov @ 2010-01-25 14:22 UTC (permalink / raw)
To: Avi Kivity; +Cc: mtosatti, kvm
On Mon, Jan 25, 2010 at 02:53:09PM +0200, Avi Kivity wrote:
> On 01/25/2010 02:24 PM, Gleb Natapov wrote:
> >On Mon, Jan 25, 2010 at 01:12:36PM +0200, Avi Kivity wrote:
> >>On 01/25/2010 01:11 PM, Gleb Natapov wrote:
> >>>On Mon, Jan 25, 2010 at 01:08:13PM +0200, Avi Kivity wrote:
> >>>>On 01/25/2010 12:01 PM, Gleb Natapov wrote:
> >>>>>On HW task switch newly loaded segments should me marked as accessed.
> >>>>>
> >>>>>@@ -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?
> >>>>
> >>>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.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] KVM: mark segments accessed on HW task switch
2010-01-25 14:22 ` Gleb Natapov
@ 2010-01-25 14:52 ` Avi Kivity
0 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2010-01-25 14:52 UTC (permalink / raw)
To: Gleb Natapov; +Cc: mtosatti, kvm
On 01/25/2010 04:22 PM, Gleb Natapov wrote:
>>
>> 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.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: mark segments accessed on HW task switch
2010-01-25 10:01 [PATCH] KVM: mark segments accessed on HW task switch Gleb Natapov
2010-01-25 11:08 ` Avi Kivity
@ 2010-01-25 21:34 ` Marcelo Tosatti
1 sibling, 0 replies; 9+ messages in thread
From: Marcelo Tosatti @ 2010-01-25 21:34 UTC (permalink / raw)
To: Gleb Natapov; +Cc: avi, kvm
On Mon, Jan 25, 2010 at 12:01:04PM +0200, Gleb Natapov wrote:
> On HW task switch newly loaded segments should me marked as accessed.
>
> Reported-by: Lorenzo Martignoni <martignlo@gmail.com>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-01-25 21:35 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-25 10:01 [PATCH] KVM: mark segments accessed on HW task switch Gleb Natapov
2010-01-25 11:08 ` Avi Kivity
2010-01-25 11:11 ` Gleb Natapov
2010-01-25 11:12 ` Avi Kivity
2010-01-25 12:24 ` Gleb Natapov
2010-01-25 12:53 ` Avi Kivity
2010-01-25 14:22 ` Gleb Natapov
2010-01-25 14:52 ` Avi Kivity
2010-01-25 21:34 ` Marcelo Tosatti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox