[PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[Maxim Levitsky]

Recently KVM's SVM code switched to re-injecting software interrupt events,
if something prevented their delivery.

Task switch due to task gate in the IDT, however is an exception
to this rule, because in this case, INTn instruction causes
a task switch intercept and its emulation completes the INTn
emulation as well.

Add a missing case to task_switch_interception for that.

This fixes 32 bit kvm unit test taskswitch2.

Fixes: 7e5b5ef8dca322 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/svm/svm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 41bd7c5aa06a1e..638dd94f6e11f6 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2399,6 +2399,7 @@ static int task_switch_interception(struct kvm_vcpu *vcpu)
 			kvm_clear_exception_queue(vcpu);
 			break;
 		case SVM_EXITINTINFO_TYPE_INTR:
+		case SVM_EXITINTINFO_TYPE_SOFT:
 			kvm_clear_interrupt_queue(vcpu);
 			break;
 		default:
-- 
2.34.3

Re: [PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[Maciej S. Szmigiero]

On 14.07.2022 14:44, Maxim Levitsky wrote:
> Recently KVM's SVM code switched to re-injecting software interrupt events,
> if something prevented their delivery.
> 
> Task switch due to task gate in the IDT, however is an exception
> to this rule, because in this case, INTn instruction causes
> a task switch intercept and its emulation completes the INTn
> emulation as well.
> 
> Add a missing case to task_switch_interception for that.
> 
> This fixes 32 bit kvm unit test taskswitch2.
> 
> Fixes: 7e5b5ef8dca322 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---

That's a good catch, your patch looks totally sensible to me.
People running Win 3.x or OS/2 on top of KVM will surely be grateful for it :)

Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>

Thanks,
Maciej

Re: [PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[Maxim Levitsky]

On Thu, 2022-07-14 at 15:50 +0200, Maciej S. Szmigiero wrote:
> On 14.07.2022 14:44, Maxim Levitsky wrote:
> > Recently KVM's SVM code switched to re-injecting software interrupt events,
> > if something prevented their delivery.
> > 
> > Task switch due to task gate in the IDT, however is an exception
> > to this rule, because in this case, INTn instruction causes
> > a task switch intercept and its emulation completes the INTn
> > emulation as well.
> > 
> > Add a missing case to task_switch_interception for that.
> > 
> > This fixes 32 bit kvm unit test taskswitch2.
> > 
> > Fixes: 7e5b5ef8dca322 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> 
> That's a good catch, your patch looks totally sensible to me.
> People running Win 3.x or OS/2 on top of KVM will surely be grateful for it :)

Yes and also people who run 32 bit kvm unit tests :)

BTW, I do have a win98 VM which I run once in a while under KVM.
On Intel it works very well, on AMD, only works without NPT and without MMU
pre-fetching, due to fact that the OS doesn't correctly invalidate TLB entries.

I do need to test KVM with OS/2 on one of the weekends.... ;-)

Thanks for the review,
	Best regards,
		Maxim Levitsky

> 
> Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> 
> Thanks,
> Maciej
> 

Re: [PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[Paolo Bonzini]

Queued, thanks.

Paolo

Re: [PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[Maciej S. Szmigiero]

On 14.07.2022 15:57, Maxim Levitsky wrote:
> On Thu, 2022-07-14 at 15:50 +0200, Maciej S. Szmigiero wrote:
>> On 14.07.2022 14:44, Maxim Levitsky wrote:
>>> Recently KVM's SVM code switched to re-injecting software interrupt events,
>>> if something prevented their delivery.
>>>
>>> Task switch due to task gate in the IDT, however is an exception
>>> to this rule, because in this case, INTn instruction causes
>>> a task switch intercept and its emulation completes the INTn
>>> emulation as well.
>>>
>>> Add a missing case to task_switch_interception for that.
>>>
>>> This fixes 32 bit kvm unit test taskswitch2.
>>>
>>> Fixes: 7e5b5ef8dca322 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
>>>
>>> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>>> ---
>>
>> That's a good catch, your patch looks totally sensible to me.
>> People running Win 3.x or OS/2 on top of KVM will surely be grateful for it :)
> 
> Yes and also people who run 32 bit kvm unit tests :)

It looks like more people need to do this regularly :)

> BTW, I do have a win98 VM which I run once in a while under KVM.
> On Intel it works very well, on AMD, only works without NPT and without MMU
> pre-fetching, due to fact that the OS doesn't correctly invalidate TLB entries.

Interesting, maybe it is related to some operation in 90s CPUs implicitly
invalidating (or just replacing) enough TLB entries to actually make it work
(usually) - just a guess.

> I do need to test KVM with OS/2 on one of the weekends.... ;-)
> 
> Thanks for the review,
> 	Best regards,
> 		Maxim Levitsky
> 

Thanks,
Maciej

Re: [PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[Sean Christopherson]

On Fri, Jul 15, 2022, Maciej S. Szmigiero wrote:
> On 14.07.2022 15:57, Maxim Levitsky wrote:
> > On Thu, 2022-07-14 at 15:50 +0200, Maciej S. Szmigiero wrote:
> > > On 14.07.2022 14:44, Maxim Levitsky wrote:
> > > > Recently KVM's SVM code switched to re-injecting software interrupt events,
> > > > if something prevented their delivery.
> > > > 
> > > > Task switch due to task gate in the IDT, however is an exception
> > > > to this rule, because in this case, INTn instruction causes
> > > > a task switch intercept and its emulation completes the INTn
> > > > emulation as well.
> > > > 
> > > > Add a missing case to task_switch_interception for that.
> > > > 
> > > > This fixes 32 bit kvm unit test taskswitch2.
> > > > 
> > > > Fixes: 7e5b5ef8dca322 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
> > > > 
> > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > > > ---
> > > 
> > > That's a good catch, your patch looks totally sensible to me.
> > > People running Win 3.x or OS/2 on top of KVM will surely be grateful for it :)
> > 
> > Yes and also people who run 32 bit kvm unit tests :)
> 
> It looks like more people need to do this regularly :)

I do run KUT on 32-bit KVM, but until I hadn't done so on AMD for a long time and
so didn't realize the taskswitch2 failure was a regression.  My goal/hope is to
we'll get to a state where we're able to run the full gamut of tests before things
hit kvm/queue, but the number of permutations of configs and module params means
that's easier said than done.

Honestly, it'd be a waste of people's time to expect anyone else beyond us few
(and CI if we can get there) to test 32-bit KVM.  We do want to keep it healthy
for a variety of reasons, but I'm quite convinced that outside of us developers,
there's literally no one running 32-bit KVM.

Re: [PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[Jim Mattson]

On Thu, Jul 14, 2022 at 4:24 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Jul 15, 2022, Maciej S. Szmigiero wrote:
> > On 14.07.2022 15:57, Maxim Levitsky wrote:
> > > On Thu, 2022-07-14 at 15:50 +0200, Maciej S. Szmigiero wrote:
> > > > On 14.07.2022 14:44, Maxim Levitsky wrote:
> > > > > Recently KVM's SVM code switched to re-injecting software interrupt events,
> > > > > if something prevented their delivery.
> > > > >
> > > > > Task switch due to task gate in the IDT, however is an exception
> > > > > to this rule, because in this case, INTn instruction causes
> > > > > a task switch intercept and its emulation completes the INTn
> > > > > emulation as well.
> > > > >
> > > > > Add a missing case to task_switch_interception for that.
> > > > >
> > > > > This fixes 32 bit kvm unit test taskswitch2.
> > > > >
> > > > > Fixes: 7e5b5ef8dca322 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
> > > > >
> > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > > > > ---
> > > >
> > > > That's a good catch, your patch looks totally sensible to me.
> > > > People running Win 3.x or OS/2 on top of KVM will surely be grateful for it :)
> > >
> > > Yes and also people who run 32 bit kvm unit tests :)
> >
> > It looks like more people need to do this regularly :)
>
> I do run KUT on 32-bit KVM, but until I hadn't done so on AMD for a long time and
> so didn't realize the taskswitch2 failure was a regression.  My goal/hope is to
> we'll get to a state where we're able to run the full gamut of tests before things
> hit kvm/queue, but the number of permutations of configs and module params means
> that's easier said than done.
>
> Honestly, it'd be a waste of people's time to expect anyone else beyond us few
> (and CI if we can get there) to test 32-bit KVM.  We do want to keep it healthy
> for a variety of reasons, but I'm quite convinced that outside of us developers,
> there's literally no one running 32-bit KVM.

It shouldn't be necessary to run 32-bit KVM to run 32-bit guests! Or
am I not understanding the issue that was fixed here?

Re: [PATCH] KVM: SVM: fix task switch emulation on INTn instruction.

[Sean Christopherson]

On Thu, Jul 14, 2022, Jim Mattson wrote:
> On Thu, Jul 14, 2022 at 4:24 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Fri, Jul 15, 2022, Maciej S. Szmigiero wrote:
> > > On 14.07.2022 15:57, Maxim Levitsky wrote:
> > > > On Thu, 2022-07-14 at 15:50 +0200, Maciej S. Szmigiero wrote:
> > > > > On 14.07.2022 14:44, Maxim Levitsky wrote:
> > > > > > Recently KVM's SVM code switched to re-injecting software interrupt events,
> > > > > > if something prevented their delivery.
> > > > > >
> > > > > > Task switch due to task gate in the IDT, however is an exception
> > > > > > to this rule, because in this case, INTn instruction causes
> > > > > > a task switch intercept and its emulation completes the INTn
> > > > > > emulation as well.
> > > > > >
> > > > > > Add a missing case to task_switch_interception for that.
> > > > > >
> > > > > > This fixes 32 bit kvm unit test taskswitch2.
> > > > > >
> > > > > > Fixes: 7e5b5ef8dca322 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
> > > > > >
> > > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > > > > > ---
> > > > >
> > > > > That's a good catch, your patch looks totally sensible to me.
> > > > > People running Win 3.x or OS/2 on top of KVM will surely be grateful for it :)
> > > >
> > > > Yes and also people who run 32 bit kvm unit tests :)
> > >
> > > It looks like more people need to do this regularly :)
> >
> > I do run KUT on 32-bit KVM, but until I hadn't done so on AMD for a long time and
> > so didn't realize the taskswitch2 failure was a regression.  My goal/hope is to
> > we'll get to a state where we're able to run the full gamut of tests before things
> > hit kvm/queue, but the number of permutations of configs and module params means
> > that's easier said than done.
> >
> > Honestly, it'd be a waste of people's time to expect anyone else beyond us few
> > (and CI if we can get there) to test 32-bit KVM.  We do want to keep it healthy
> > for a variety of reasons, but I'm quite convinced that outside of us developers,
> > there's literally no one running 32-bit KVM.
> 
> It shouldn't be necessary to run 32-bit KVM to run 32-bit guests! Or
> am I not understanding the issue that was fixed here?

Ah, no, I'm the one off in the weeds.  I only ever run 32-bit KUT on 32-bit VMs
because I've been too lazy to "cross"-compile.  Time to remedy that...