[PATCH-for-4.11] xpti: fix bug in double fault handling

[PATCH-for-4.11] xpti: fix bug in double fault handling

[Juergen Gross]

When entering the hypervisor via the double fault handler resetting
xen_cr3 was missing. This led to switching to pv_cr3 when returning
from the next following interrupt, e.g. after re-enabling interrupts
in machine_restart().

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/firmware/xen-dir/shim.config | 2 +-
 xen/arch/x86/x86_64/entry.S        | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/xen-dir/shim.config b/tools/firmware/xen-dir/shim.config
index 4d5630f87a..0b9e001436 100644
--- a/tools/firmware/xen-dir/shim.config
+++ b/tools/firmware/xen-dir/shim.config
@@ -1,6 +1,6 @@
 #
 # Automatically generated file; DO NOT EDIT.
-# Xen/x86 4.11-unstable Configuration
+# Xen/x86 4.11-rc Configuration
 #
 CONFIG_X86_64=y
 CONFIG_X86=y
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 45d9842d09..d12fbcb55d 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -772,6 +772,7 @@ ENTRY(double_fault)
         jns   .Ldblf_cr3_load
         neg   %rbx
 .Ldblf_cr3_load:
+        movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
         mov   %rbx, %cr3
 .Ldblf_cr3_okay:
 
-- 
2.13.6


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH-for-4.11] xpti: fix bug in double fault handling

[Jan Beulich]

>>> On 23.04.18 at 13:37, <jgross@suse.com> wrote:
> When entering the hypervisor via the double fault handler resetting
> xen_cr3 was missing. This led to switching to pv_cr3 when returning
> from the next following interrupt, e.g. after re-enabling interrupts
> in machine_restart().

Pointing at bad behavior to justify a change is not very helpful, I think.
Andrew's argument of exception handling wanting to continue to work
even after a #DF is a better one imo.

> --- a/tools/firmware/xen-dir/shim.config
> +++ b/tools/firmware/xen-dir/shim.config
> @@ -1,6 +1,6 @@
>  #
>  # Automatically generated file; DO NOT EDIT.
> -# Xen/x86 4.11-unstable Configuration
> +# Xen/x86 4.11-rc Configuration
>  #
>  CONFIG_X86_64=y
>  CONFIG_X86=y

Stray change?

> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -772,6 +772,7 @@ ENTRY(double_fault)
>          jns   .Ldblf_cr3_load
>          neg   %rbx
>  .Ldblf_cr3_load:
> +        movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
>          mov   %rbx, %cr3
>  .Ldblf_cr3_okay:

Just like for the other code paths this write should be after the CR3 load,
or else NMI or #MC occurring between the two would fail to update CR3.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH-for-4.11] xpti: fix bug in double fault handling

[Juergen Gross]

On 23/04/18 14:11, Jan Beulich wrote:
>>>> On 23.04.18 at 13:37, <jgross@suse.com> wrote:
>> When entering the hypervisor via the double fault handler resetting
>> xen_cr3 was missing. This led to switching to pv_cr3 when returning
>> from the next following interrupt, e.g. after re-enabling interrupts
>> in machine_restart().
> 
> Pointing at bad behavior to justify a change is not very helpful, I think.
> Andrew's argument of exception handling wanting to continue to work
> even after a #DF is a better one imo.

Okay.

> 
>> --- a/tools/firmware/xen-dir/shim.config
>> +++ b/tools/firmware/xen-dir/shim.config
>> @@ -1,6 +1,6 @@
>>  #
>>  # Automatically generated file; DO NOT EDIT.
>> -# Xen/x86 4.11-unstable Configuration
>> +# Xen/x86 4.11-rc Configuration
>>  #
>>  CONFIG_X86_64=y
>>  CONFIG_X86=y
> 
> Stray change?

Oh, that one again. Sorry. Maybe we should really take my related
patch to avoid issues like this one.

> 
>> --- a/xen/arch/x86/x86_64/entry.S
>> +++ b/xen/arch/x86/x86_64/entry.S
>> @@ -772,6 +772,7 @@ ENTRY(double_fault)
>>          jns   .Ldblf_cr3_load
>>          neg   %rbx
>>  .Ldblf_cr3_load:
>> +        movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
>>          mov   %rbx, %cr3
>>  .Ldblf_cr3_okay:
> 
> Just like for the other code paths this write should be after the CR3 load,
> or else NMI or #MC occurring between the two would fail to update CR3.

Aah, right.

Will send V2 soon.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH-for-4.11] xpti: fix bug in double fault handling

[Jan Beulich]

>>> On 23.04.18 at 14:22, <jgross@suse.com> wrote:
> On 23/04/18 14:11, Jan Beulich wrote:
>>>>> On 23.04.18 at 13:37, <jgross@suse.com> wrote:
>>> --- a/tools/firmware/xen-dir/shim.config
>>> +++ b/tools/firmware/xen-dir/shim.config
>>> @@ -1,6 +1,6 @@
>>>  #
>>>  # Automatically generated file; DO NOT EDIT.
>>> -# Xen/x86 4.11-unstable Configuration
>>> +# Xen/x86 4.11-rc Configuration
>>>  #
>>>  CONFIG_X86_64=y
>>>  CONFIG_X86=y
>> 
>> Stray change?
> 
> Oh, that one again. Sorry. Maybe we should really take my related
> patch to avoid issues like this one.

I agree, but I'm not a maintainer of the file.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel