diff mbox

Fixed KVM problems with old DOS programs. Compatibility can be forced by module parameter.

Message ID 563A5D7D.4080201@wiesinger.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gerhard Wiesinger Nov. 4, 2015, 7:33 p.m. UTC
Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com>
---
  arch/x86/kvm/svm.c | 7 +++++++
  1 file changed, 7 insertions(+)

init_event)
  	if (npt_enabled) {
  		/* Setup VMCB for Nested Paging */
  		control->nested_ctl = 1;
+		if (!npt_task_switch_emulation) {
+			clr_intercept(svm, INTERCEPT_TASK_SWITCH);
+		}
  		clr_intercept(svm, INTERCEPT_INVLPG);
  		clr_exception_intercept(svm, PF_VECTOR);
  		clr_cr_intercept(svm, INTERCEPT_CR3_READ);

Comments

Paolo Bonzini Nov. 4, 2015, 9:27 p.m. UTC | #1
On 04/11/2015 20:33, Gerhard Wiesinger wrote:
> Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com>
> ---
>  arch/x86/kvm/svm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 2f9ed1f..e0b00fc 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -198,6 +198,10 @@ static bool npt_enabled;
>  static int npt = true;
>  module_param(npt, int, S_IRUGO);
>  +/* allow backward compatibility with e.g. old DOS application */
> +static int npt_task_switch_emulation = true;
> +module_param(npt_task_switch_emulation, int, S_IRUGO);
> +
>  /* allow nested virtualization in KVM/SVM */
>  static int nested = true;
>  module_param(nested, int, S_IRUGO);
> @@ -1177,6 +1181,9 @@ static void init_vmcb(struct vcpu_svm *svm, bool
> init_event)
>      if (npt_enabled) {
>          /* Setup VMCB for Nested Paging */
>          control->nested_ctl = 1;
> +        if (!npt_task_switch_emulation) {
> +            clr_intercept(svm, INTERCEPT_TASK_SWITCH);
> +        }
>          clr_intercept(svm, INTERCEPT_INVLPG);
>          clr_exception_intercept(svm, PF_VECTOR);
>          clr_cr_intercept(svm, INTERCEPT_CR3_READ);

What is the problem you are seeing?  KVM can emulate task switches; the
intercept is set here because of a processor erratum that can mess them
up even though, in theory, AMD supports task switching from guest mode.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gerhard Wiesinger Nov. 4, 2015, 9:33 p.m. UTC | #2
On 04.11.2015 22:27, Paolo Bonzini wrote:
>
> On 04/11/2015 20:33, Gerhard Wiesinger wrote:
>> Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com>
>> ---
>>   arch/x86/kvm/svm.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 2f9ed1f..e0b00fc 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -198,6 +198,10 @@ static bool npt_enabled;
>>   static int npt = true;
>>   module_param(npt, int, S_IRUGO);
>>   +/* allow backward compatibility with e.g. old DOS application */
>> +static int npt_task_switch_emulation = true;
>> +module_param(npt_task_switch_emulation, int, S_IRUGO);
>> +
>>   /* allow nested virtualization in KVM/SVM */
>>   static int nested = true;
>>   module_param(nested, int, S_IRUGO);
>> @@ -1177,6 +1181,9 @@ static void init_vmcb(struct vcpu_svm *svm, bool
>> init_event)
>>       if (npt_enabled) {
>>           /* Setup VMCB for Nested Paging */
>>           control->nested_ctl = 1;
>> +        if (!npt_task_switch_emulation) {
>> +            clr_intercept(svm, INTERCEPT_TASK_SWITCH);
>> +        }
>>           clr_intercept(svm, INTERCEPT_INVLPG);
>>           clr_exception_intercept(svm, PF_VECTOR);
>>           clr_cr_intercept(svm, INTERCEPT_CR3_READ);
> What is the problem you are seeing?  KVM can emulate task switches; the
> intercept is set here because of a processor erratum that can mess them
> up even though, in theory, AMD supports task switching from guest mode.

See old thread:
https://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg01506.html

Ciao,
Gerhard

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini Nov. 4, 2015, 10:04 p.m. UTC | #3
On 04/11/2015 22:33, Gerhard Wiesinger wrote:
>>>
>> What is the problem you are seeing?  KVM can emulate task switches; the
>> intercept is set here because of a processor erratum that can mess them
>> up even though, in theory, AMD supports task switching from guest mode.
> 
> See old thread:
> https://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg01506.html

Can you obtain the traces you were asked for at the time?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gerhard Wiesinger Nov. 5, 2015, 4:07 p.m. UTC | #4
On 04.11.2015 23:04, Paolo Bonzini wrote:
>
> On 04/11/2015 22:33, Gerhard Wiesinger wrote:
>>> What is the problem you are seeing?  KVM can emulate task switches; the
>>> intercept is set here because of a processor erratum that can mess them
>>> up even though, in theory, AMD supports task switching from guest mode.
>> See old thread:
>> https://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg01506.html
> Can you obtain the traces you were asked for at the time?

./trace-cmd record -b 20000 -e kvm
./trace-cmd report | grep -i task_switch
  qemu-system-x86-6024  [001] 792774.719297: kvm_exit: reason 
task_switch rip 0x4883 info 158 4000000000

But I can't interpret it. But I know my patch works well. Since it is 
just a module parameter it is fully backward compatible by default and 
because of the one liner no side effects are possible. So a intergration 
would be good.

Ciao,
Gerhard

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini Nov. 5, 2015, 4:15 p.m. UTC | #5
On 05/11/2015 17:07, Gerhard Wiesinger wrote:
>>
> 
> ./trace-cmd record -b 20000 -e kvm
> ./trace-cmd report | grep -i task_switch
>  qemu-system-x86-6024  [001] 792774.719297: kvm_exit: reason task_switch
> rip 0x4883 info 158 4000000000

0x158 is the segment selector of the incoming TSS, and the task switch
was caused by a far jump.

> But I can't interpret it.

Neither can I; you have to send the whole trace.

> But I know my patch works well. Since it is
> just a module parameter it is fully backward compatible by default and
> because of the one liner no side effects are possible. So a intergration
> would be good.

It's also papering over a bug, and likely the bug still triggers on
Intel systems.  So it's not acceptable.

Can you provide reproduction instructions please?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini Nov. 5, 2015, 4:19 p.m. UTC | #6
On 05/11/2015 17:15, Paolo Bonzini wrote:
> 
> 
> On 05/11/2015 17:07, Gerhard Wiesinger wrote:
>>>
>>
>> ./trace-cmd record -b 20000 -e kvm
>> ./trace-cmd report | grep -i task_switch
>>  qemu-system-x86-6024  [001] 792774.719297: kvm_exit: reason task_switch
>> rip 0x4883 info 158 4000000000
> 
> 0x158 is the segment selector of the incoming TSS, and the task switch
> was caused by a far jump.
> 
>> But I can't interpret it.
> 
> Neither can I; you have to send the whole trace.
> 
>> But I know my patch works well. Since it is
>> just a module parameter it is fully backward compatible by default and
>> because of the one liner no side effects are possible. So a intergration
>> would be good.
> 
> It's also papering over a bug, and likely the bug still triggers on
> Intel systems.  So it's not acceptable.
> 
> Can you provide reproduction instructions please?

At the very least, does it reproduce without KVM?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2f9ed1f..e0b00fc 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -198,6 +198,10 @@  static bool npt_enabled;
  static int npt = true;
  module_param(npt, int, S_IRUGO);
  +/* allow backward compatibility with e.g. old DOS application */
+static int npt_task_switch_emulation = true;
+module_param(npt_task_switch_emulation, int, S_IRUGO);
+
  /* allow nested virtualization in KVM/SVM */
  static int nested = true;
  module_param(nested, int, S_IRUGO);
@@ -1177,6 +1181,9 @@  static void init_vmcb(struct vcpu_svm *svm, bool