subject:"\[Xen\-devel\] \[PATCH\] x86\/XPTI\: fix S3 resume \(and CPU offlining in general\)"

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-28 Thread Juergen Gross

On 24/05/18 15:41, Jan Beulich wrote:
> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
> I've failed to remember the fact that multiple CPUs share a stub
> mapping page. Therefore it is wrong to unconditionally zap the mapping
> when bringing down a CPU; it may only be unmapped when no other online
> CPU uses that same page.
> 
> Reported-by: Simon Gaiser 
> Signed-off-by: Jan Beulich 

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Simon Gaiser

Jan Beulich:
 On 24.05.18 at 17:10,  wrote:
>> Jan Beulich:
>> On 24.05.18 at 16:14,  wrote:
 Jan Beulich:
 On 24.05.18 at 16:00,  wrote:
>> Jan Beulich:
>>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>>> I've failed to remember the fact that multiple CPUs share a stub
>>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>>> when bringing down a CPU; it may only be unmapped when no other online
>>> CPU uses that same page.
>>>
>>> Reported-by: Simon Gaiser 
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>>  
>>>  free_xen_pagetable(rpt);
>>>  
>>> -/* Also zap the stub mapping for this CPU. */
>>> +/*
>>> + * Also zap the stub mapping for this CPU, if no other online one 
>>> uses
>>> + * the same page.
>>> + */
>>> +if ( stub_linear )
>>> +{
>>> +unsigned int other;
>>> +
>>> +for_each_online_cpu(other)
>>> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
>>> PAGE_SHIFT) 
>> )
>>> +{
>>> +stub_linear = 0;
>>> +break;
>>> +}
>>> +}
>>>  if ( stub_linear )
>>>  {
>>>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>>
>> Tried this on-top of staging (fc5805daef) and I still get the same
>> double fault.
>
> Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. 
> What
> size a system are you testing on? Mine has got only 12 CPUs, i.e. all 
> stubs
> are in the same page (and I'd never unmap anything here at all).

 4 cores + HT, so 8 CPUs from Xen's PoV.
>>>
>>> May I ask you to do two things:
>>> 1) confirm that you can offline CPUs successfully using xen-hptool,
>>> 2) add a printk() to the code above making clear whether/when any
>>> of the mappings actually get zapped?
>>
>> There seem to be two failure modes now. It seems that both can be
>> triggered either by offlining a cpu or by suspend. Using cpu offlining
>> below since during suspend I often loose part of the serial output.
>>
>> Failure mode 1, the double fault as before:
>>
>> root@localhost:~# xen-hptool cpu-offline 3
>> Prepare to offline CPU 3
>> (XEN) Broke affinity for irq 9
>> (XEN) Broke affinity for irq 29
>> (XEN) dbg: stub_linear't1 = 18446606431818858880
>> (XEN) dbg: first stub_linear if
>> (XEN) dbg: stub_linear't2 = 18446606431818858880
>> (XEN) dbg: second stub_linear if
>> CPU 3 offlined successfully
>> root@localhost:~# (XEN) *** DOUBLE FAULT ***
>> (XEN) [ Xen-4.11-rc  x86_64  debug=y   Not tainted ]
>> (XEN) CPU:0
>> (XEN) RIP:e008:[] handle_exception+0x9c/0xff
>> (XEN) RFLAGS: 00010006   CONTEXT: hypervisor
>> (XEN) rax: c90040cdc0a8   rbx:    rcx: 0006
>> (XEN) rdx:    rsi:    rdi: 
>> (XEN) rbp: 36ffbf323f37   rsp: c90040cdc000   r8:  
>> (XEN) r9:     r10:    r11: 
>> (XEN) r12:    r13:    r14: c90040cd
>> (XEN) r15:    cr0: 8005003b   cr4: 00042660
>> (XEN) cr3: 000128109000   cr2: c90040cdbff8
>> (XEN) fsb: 7fc01c3c6dc0   gsb: 88021e70   gss: 
>> (XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
>> (XEN) Xen code around  (handle_exception+0x9c/0xff):
>> (XEN)  00 f3 90 0f ae e8 eb f9  07 00 00 00 f3 90 0f ae e8 eb f9 83 e9 
>> 01 
>> 75
>> (XEN) Current stack base c90040cd8000 differs from expected 
>> 8300cec88000
>> (XEN) Valid stack range: c90040cde000-c90040ce, 
>> sp=c90040cdc000, tss.rsp0=8300cec8ffa0
>> (XEN) No stack overflow detected. Skipping stack trace.
>> (XEN) 
>> (XEN) 
>> (XEN) Panic on CPU 0:
>> (XEN) DOUBLE FAULT -- system shutdown
>> (XEN) 
>> (XEN) 
>> (XEN) Reboot in five seconds...
> 
> Oh, so CPU 0 gets screwed by offlining CPU 3. How about this alternative
> (but so far untested) patch:
> 
> --- unstable.orig/xen/arch/x86/smpboot.c
> +++ unstable/xen/arch/x86/smpboot.c
> @@ -874,7 +874,7 @@ static void cleanup_cpu_root_pgt(unsigne
>  l2_pgentry_t *l2t = l3e_to_l2e(l3t[l3_table_offset(stub_linear)]);
>  l1_pgentry_t *l1t = l2e_to_l1e(l2t[l2_table_offset(stub_linear)]);
>  
> -l1t[l2_table_offset(stub_linear)] = l1e_empty();
> +l1t[l1_table_offset(stub_linear)] = l1e_empty();
>  }
>  }
>

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Jan Beulich

>>> On 24.05.18 at 17:10,  wrote:
> Jan Beulich:
> On 24.05.18 at 16:14,  wrote:
>>> Jan Beulich:
>>> On 24.05.18 at 16:00,  wrote:
> Jan Beulich:
>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>> I've failed to remember the fact that multiple CPUs share a stub
>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>> when bringing down a CPU; it may only be unmapped when no other online
>> CPU uses that same page.
>>
>> Reported-by: Simon Gaiser 
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>  
>>  free_xen_pagetable(rpt);
>>  
>> -/* Also zap the stub mapping for this CPU. */
>> +/*
>> + * Also zap the stub mapping for this CPU, if no other online one 
>> uses
>> + * the same page.
>> + */
>> +if ( stub_linear )
>> +{
>> +unsigned int other;
>> +
>> +for_each_online_cpu(other)
>> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
>> PAGE_SHIFT) 
> )
>> +{
>> +stub_linear = 0;
>> +break;
>> +}
>> +}
>>  if ( stub_linear )
>>  {
>>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>
> Tried this on-top of staging (fc5805daef) and I still get the same
> double fault.

 Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. 
 What
 size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
 are in the same page (and I'd never unmap anything here at all).
>>>
>>> 4 cores + HT, so 8 CPUs from Xen's PoV.
>> 
>> May I ask you to do two things:
>> 1) confirm that you can offline CPUs successfully using xen-hptool,
>> 2) add a printk() to the code above making clear whether/when any
>> of the mappings actually get zapped?
> 
> There seem to be two failure modes now. It seems that both can be
> triggered either by offlining a cpu or by suspend. Using cpu offlining
> below since during suspend I often loose part of the serial output.
> 
> Failure mode 1, the double fault as before:
> 
> root@localhost:~# xen-hptool cpu-offline 3
> Prepare to offline CPU 3
> (XEN) Broke affinity for irq 9
> (XEN) Broke affinity for irq 29
> (XEN) dbg: stub_linear't1 = 18446606431818858880
> (XEN) dbg: first stub_linear if
> (XEN) dbg: stub_linear't2 = 18446606431818858880
> (XEN) dbg: second stub_linear if
> CPU 3 offlined successfully
> root@localhost:~# (XEN) *** DOUBLE FAULT ***
> (XEN) [ Xen-4.11-rc  x86_64  debug=y   Not tainted ]
> (XEN) CPU:0
> (XEN) RIP:e008:[] handle_exception+0x9c/0xff
> (XEN) RFLAGS: 00010006   CONTEXT: hypervisor
> (XEN) rax: c90040cdc0a8   rbx:    rcx: 0006
> (XEN) rdx:    rsi:    rdi: 
> (XEN) rbp: 36ffbf323f37   rsp: c90040cdc000   r8:  
> (XEN) r9:     r10:    r11: 
> (XEN) r12:    r13:    r14: c90040cd
> (XEN) r15:    cr0: 8005003b   cr4: 00042660
> (XEN) cr3: 000128109000   cr2: c90040cdbff8
> (XEN) fsb: 7fc01c3c6dc0   gsb: 88021e70   gss: 
> (XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
> (XEN) Xen code around  (handle_exception+0x9c/0xff):
> (XEN)  00 f3 90 0f ae e8 eb f9  07 00 00 00 f3 90 0f ae e8 eb f9 83 e9 01 
> 75
> (XEN) Current stack base c90040cd8000 differs from expected 
> 8300cec88000
> (XEN) Valid stack range: c90040cde000-c90040ce, 
> sp=c90040cdc000, tss.rsp0=8300cec8ffa0
> (XEN) No stack overflow detected. Skipping stack trace.
> (XEN) 
> (XEN) 
> (XEN) Panic on CPU 0:
> (XEN) DOUBLE FAULT -- system shutdown
> (XEN) 
> (XEN) 
> (XEN) Reboot in five seconds...

Oh, so CPU 0 gets screwed by offlining CPU 3. How about this alternative
(but so far untested) patch:

--- unstable.orig/xen/arch/x86/smpboot.c
+++ unstable/xen/arch/x86/smpboot.c
@@ -874,7 +874,7 @@ static void cleanup_cpu_root_pgt(unsigne
 l2_pgentry_t *l2t = l3e_to_l2e(l3t[l3_table_offset(stub_linear)]);
 l1_pgentry_t *l1t = l2e_to_l1e(l2t[l2_table_offset(stub_linear)]);
 
-l1t[l2_table_offset(stub_linear)] = l1e_empty();
+l1t[l1_table_offset(stub_linear)] = l1e_empty();
 }
 }
 

Jan


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Jan Beulich

>>> On 24.05.18 at 17:10,  wrote:
> Jan Beulich:
> On 24.05.18 at 16:14,  wrote:
>>> Jan Beulich:
>>> On 24.05.18 at 16:00,  wrote:
> Jan Beulich:
>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>> I've failed to remember the fact that multiple CPUs share a stub
>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>> when bringing down a CPU; it may only be unmapped when no other online
>> CPU uses that same page.
>>
>> Reported-by: Simon Gaiser 
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>  
>>  free_xen_pagetable(rpt);
>>  
>> -/* Also zap the stub mapping for this CPU. */
>> +/*
>> + * Also zap the stub mapping for this CPU, if no other online one 
>> uses
>> + * the same page.
>> + */
>> +if ( stub_linear )
>> +{
>> +unsigned int other;
>> +
>> +for_each_online_cpu(other)
>> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
>> PAGE_SHIFT) 
> )
>> +{
>> +stub_linear = 0;
>> +break;
>> +}
>> +}
>>  if ( stub_linear )
>>  {
>>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>
> Tried this on-top of staging (fc5805daef) and I still get the same
> double fault.

 Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. 
 What
 size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
 are in the same page (and I'd never unmap anything here at all).
>>>
>>> 4 cores + HT, so 8 CPUs from Xen's PoV.
>> 
>> May I ask you to do two things:
>> 1) confirm that you can offline CPUs successfully using xen-hptool,
>> 2) add a printk() to the code above making clear whether/when any
>> of the mappings actually get zapped?
> 
> There seem to be two failure modes now. It seems that both can be
> triggered either by offlining a cpu or by suspend. Using cpu offlining
> below since during suspend I often loose part of the serial output.
> 
> Failure mode 1, the double fault as before:
> 
> root@localhost:~# xen-hptool cpu-offline 3
> Prepare to offline CPU 3
> (XEN) Broke affinity for irq 9
> (XEN) Broke affinity for irq 29
> (XEN) dbg: stub_linear't1 = 18446606431818858880
> (XEN) dbg: first stub_linear if
> (XEN) dbg: stub_linear't2 = 18446606431818858880
> (XEN) dbg: second stub_linear if
> CPU 3 offlined successfully
> root@localhost:~# (XEN) *** DOUBLE FAULT ***
> (XEN) [ Xen-4.11-rc  x86_64  debug=y   Not tainted ]
> (XEN) CPU:0
> (XEN) RIP:e008:[] handle_exception+0x9c/0xff
> (XEN) RFLAGS: 00010006   CONTEXT: hypervisor
> (XEN) rax: c90040cdc0a8   rbx:    rcx: 0006
> (XEN) rdx:    rsi:    rdi: 
> (XEN) rbp: 36ffbf323f37   rsp: c90040cdc000   r8:  
> (XEN) r9:     r10:    r11: 
> (XEN) r12:    r13:    r14: c90040cd
> (XEN) r15:    cr0: 8005003b   cr4: 00042660
> (XEN) cr3: 000128109000   cr2: c90040cdbff8
> (XEN) fsb: 7fc01c3c6dc0   gsb: 88021e70   gss: 
> (XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
> (XEN) Xen code around  (handle_exception+0x9c/0xff):
> (XEN)  00 f3 90 0f ae e8 eb f9  07 00 00 00 f3 90 0f ae e8 eb f9 83 e9 01 
> 75
> (XEN) Current stack base c90040cd8000 differs from expected 
> 8300cec88000
> (XEN) Valid stack range: c90040cde000-c90040ce, 
> sp=c90040cdc000, tss.rsp0=8300cec8ffa0
> (XEN) No stack overflow detected. Skipping stack trace.
> (XEN) 
> (XEN) 
> (XEN) Panic on CPU 0:
> (XEN) DOUBLE FAULT -- system shutdown
> (XEN) 
> (XEN) 
> (XEN) Reboot in five seconds...

And I see now why I thought the patch works - I should have removed
"xpti=no" from the command line. The diagnosis was wrong altogether:
While we share physical pages for stubs, we don#t share virtual space.
See alloc_stub_page().

But I'm pretty sure it has something to do with setting up stub space.
Looking around again ...

Jan


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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Simon Gaiser

Andrew Cooper:
> On 24/05/18 15:35, Simon Gaiser wrote:
>> Andrew Cooper:
>>> On 24/05/18 15:14, Simon Gaiser wrote:
 Jan Beulich:
 On 24.05.18 at 16:00,  wrote:
>> Jan Beulich:
>>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>>> I've failed to remember the fact that multiple CPUs share a stub
>>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>>> when bringing down a CPU; it may only be unmapped when no other online
>>> CPU uses that same page.
>>>
>>> Reported-by: Simon Gaiser 
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>>  
>>>  free_xen_pagetable(rpt);
>>>  
>>> -/* Also zap the stub mapping for this CPU. */
>>> +/*
>>> + * Also zap the stub mapping for this CPU, if no other online one 
>>> uses
>>> + * the same page.
>>> + */
>>> +if ( stub_linear )
>>> +{
>>> +unsigned int other;
>>> +
>>> +for_each_online_cpu(other)
>>> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
>>> PAGE_SHIFT) )
>>> +{
>>> +stub_linear = 0;
>>> +break;
>>> +}
>>> +}
>>>  if ( stub_linear )
>>>  {
>>>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>> Tried this on-top of staging (fc5805daef) and I still get the same
>> double fault.
> Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. 
> What
> size a system are you testing on? Mine has got only 12 CPUs, i.e. all 
> stubs
> are in the same page (and I'd never unmap anything here at all).
 4 cores + HT, so 8 CPUs from Xen's PoV.
>>> Can you try with the "x86/traps: Dump the instruction stream even for
>>> double faults" patch I've just posted, and show the full #DF panic log
>>> please?  (Its conceivable that there are multiple different issues here.)
>> With Jan's and your patch:
>>
>> (XEN) mce_intel.c:782: MCA Capability: firstbank 0, extended MCE MSR 0, 
>> BCAST, CMCI
>> (XEN) CPU0 CMCI LVT vector (0xf2) already installed
>> (XEN) Finishing wakeup from ACPI S3 state.
>> (XEN) Enabling non-boot CPUs  ...
>> (XEN) emul-priv-op.c:1166:d0v1 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee0
>> (XEN) emul-priv-op.c:1166:d0v1 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee00800
>> (XEN) emul-priv-op.c:1166:d0v2 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee0
>> (XEN) emul-priv-op.c:1166:d0v2 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee00800
>> (XEN) emul-priv-op.c:1166:d0v3 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee0
>> (XEN) emul-priv-op.c:1166:d0v3 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee00800
>> (XEN) emul-priv-op.c:1166:d0v4 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee0
>> (XEN) emul-priv-op.c:1166:d0v4 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee00800
> 
> /sigh - Naughty Linux.  The PVOps really ought to know that they don't
> have an APIC to play with, not that this related to the crash.
> 
>> (XEN) *** DOUBLE FAULT ***
>> (XEN) [ Xen-4.11-rc  x86_64  debug=y   Not tainted ]
>> (XEN) CPU:0
>> (XEN) RIP:e008:[] handle_exception+0x9c/0xff
>> (XEN) RFLAGS: 00010006   CONTEXT: hypervisor
>> (XEN) rax: c90040ce40d8   rbx:    rcx: 0003
>> (XEN) rdx:    rsi:    rdi: 
>> (XEN) rbp: 36ffbf31bf07   rsp: c90040ce4000   r8:  
>> (XEN) r9:     r10:    r11: 
>> (XEN) r12:    r13:    r14: c90040ce7fff
>> (XEN) r15:    cr0: 8005003b   cr4: 00042660
>> (XEN) cr3: 00022200a000   cr2: c90040ce3ff8
>> (XEN) fsb: 7fa9e7909740   gsb: 88021e74   gss: 
>> (XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
>> (XEN) Xen code around  (handle_exception+0x9c/0xff):
>> (XEN)  00 f3 90 0f ae e8 eb f9  07 00 00 00 f3 90 0f ae e8 eb f9 83 e9 
>> 01 75
>> (XEN) Current stack base c90040ce differs from expected 
>> 8300cec88000
>> (XEN) Valid stack range: c90040ce6000-c90040ce8000, 
>> sp=c90040ce4000, tss.rsp0=8300cec8ffa0
>> (XEN) No stack overflow detected. Skipping stack trace.
> 
> Ok - this is the same as George's crash, and yes - I did misdiagnose the
> stack we were on.  I presume this hardware doesn't have SMAP? (or

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Simon Gaiser

Jan Beulich:
 On 24.05.18 at 16:14,  wrote:
>> Jan Beulich:
>> On 24.05.18 at 16:00,  wrote:
 Jan Beulich:
> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
> I've failed to remember the fact that multiple CPUs share a stub
> mapping page. Therefore it is wrong to unconditionally zap the mapping
> when bringing down a CPU; it may only be unmapped when no other online
> CPU uses that same page.
>
> Reported-by: Simon Gaiser 
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>  
>  free_xen_pagetable(rpt);
>  
> -/* Also zap the stub mapping for this CPU. */
> +/*
> + * Also zap the stub mapping for this CPU, if no other online one 
> uses
> + * the same page.
> + */
> +if ( stub_linear )
> +{
> +unsigned int other;
> +
> +for_each_online_cpu(other)
> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
> PAGE_SHIFT) )
> +{
> +stub_linear = 0;
> +break;
> +}
> +}
>  if ( stub_linear )
>  {
>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);

 Tried this on-top of staging (fc5805daef) and I still get the same
 double fault.
>>>
>>> Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. What
>>> size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
>>> are in the same page (and I'd never unmap anything here at all).
>>
>> 4 cores + HT, so 8 CPUs from Xen's PoV.
> 
> May I ask you to do two things:
> 1) confirm that you can offline CPUs successfully using xen-hptool,
> 2) add a printk() to the code above making clear whether/when any
> of the mappings actually get zapped?

There seem to be two failure modes now. It seems that both can be
triggered either by offlining a cpu or by suspend. Using cpu offlining
below since during suspend I often loose part of the serial output.

Failure mode 1, the double fault as before:

root@localhost:~# xen-hptool cpu-offline 3
Prepare to offline CPU 3
(XEN) Broke affinity for irq 9
(XEN) Broke affinity for irq 29
(XEN) dbg: stub_linear't1 = 18446606431818858880
(XEN) dbg: first stub_linear if
(XEN) dbg: stub_linear't2 = 18446606431818858880
(XEN) dbg: second stub_linear if
CPU 3 offlined successfully
root@localhost:~# (XEN) *** DOUBLE FAULT ***
(XEN) [ Xen-4.11-rc  x86_64  debug=y   Not tainted ]
(XEN) CPU:0
(XEN) RIP:e008:[] handle_exception+0x9c/0xff
(XEN) RFLAGS: 00010006   CONTEXT: hypervisor
(XEN) rax: c90040cdc0a8   rbx:    rcx: 0006
(XEN) rdx:    rsi:    rdi: 
(XEN) rbp: 36ffbf323f37   rsp: c90040cdc000   r8:  
(XEN) r9:     r10:    r11: 
(XEN) r12:    r13:    r14: c90040cd
(XEN) r15:    cr0: 8005003b   cr4: 00042660
(XEN) cr3: 000128109000   cr2: c90040cdbff8
(XEN) fsb: 7fc01c3c6dc0   gsb: 88021e70   gss: 
(XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
(XEN) Xen code around  (handle_exception+0x9c/0xff):
(XEN)  00 f3 90 0f ae e8 eb f9  07 00 00 00 f3 90 0f ae e8 eb f9 83 e9 01 75
(XEN) Current stack base c90040cd8000 differs from expected 8300cec88000
(XEN) Valid stack range: c90040cde000-c90040ce, 
sp=c90040cdc000, tss.rsp0=8300cec8ffa0
(XEN) No stack overflow detected. Skipping stack trace.
(XEN) 
(XEN) 
(XEN) Panic on CPU 0:
(XEN) DOUBLE FAULT -- system shutdown
(XEN) 
(XEN) 
(XEN) Reboot in five seconds...

Failure mode 2, dom0 kernel panic (Debian unstable default kernel):

root@localhost:~# xen-hptool cpu-offline 3
Prepare to offline CPU 3
(XEN) Broke affinity for irq 9
(XEN) Broke affinity for irq 26
(XEN) Broke affinity for irq 28
(XEN) dbg: stub_linear't1 = 18446606431818858880
(XEN) dbg: first stub_linear if
(XEN) dbg: stub_linear't2 = 18446606431818858880
(XEN) dbg: second stub_linear if
CPU 3 offlined successfully
root@localhost:~# [   42.976030] BUG: unable to handle kernel NULL pointer 
dereference at 
[   42.976178] IP: __evtchn_fifo_handle_events+0x43/0x1a0
[   42.976256] PGD 0 P4D 0 
[   42.976305] Oops: 0002 [#1] SMP NOPTI
[   42.976367] Modules linked in: ctr ccm bnep ip6t_REJECT nf_reject_ipv6 
ip6table_filter ip6_tables ipt_REJECT nf_reject_ipv4 xt_tcpudp iptable_filter 
arc4 snd_hda_codec_hdmi dell_rbtn iwldvm nouveau intel_rapl intel_powerclamp

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread George Dunlap



> On May 24, 2018, at 3:53 PM, Andrew Cooper  wrote:
> 
> On 24/05/18 15:35, Simon Gaiser wrote:
>> Andrew Cooper:
>>> On 24/05/18 15:14, Simon Gaiser wrote:
 Jan Beulich:
 On 24.05.18 at 16:00,  wrote:
>> Jan Beulich:
>>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>>> I've failed to remember the fact that multiple CPUs share a stub
>>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>>> when bringing down a CPU; it may only be unmapped when no other online
>>> CPU uses that same page.
>>> 
>>> Reported-by: Simon Gaiser 
>>> Signed-off-by: Jan Beulich 
>>> 
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>> 
>>> free_xen_pagetable(rpt);
>>> 
>>> -/* Also zap the stub mapping for this CPU. */
>>> +/*
>>> + * Also zap the stub mapping for this CPU, if no other online one 
>>> uses
>>> + * the same page.
>>> + */
>>> +if ( stub_linear )
>>> +{
>>> +unsigned int other;
>>> +
>>> +for_each_online_cpu(other)
>>> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
>>> PAGE_SHIFT) )
>>> +{
>>> +stub_linear = 0;
>>> +break;
>>> +}
>>> +}
>>> if ( stub_linear )
>>> {
>>> l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>> Tried this on-top of staging (fc5805daef) and I still get the same
>> double fault.
> Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. 
> What
> size a system are you testing on? Mine has got only 12 CPUs, i.e. all 
> stubs
> are in the same page (and I'd never unmap anything here at all).
 4 cores + HT, so 8 CPUs from Xen's PoV.
>>> Can you try with the "x86/traps: Dump the instruction stream even for
>>> double faults" patch I've just posted, and show the full #DF panic log
>>> please?  (Its conceivable that there are multiple different issues here.)
>> With Jan's and your patch:
>> 
>> (XEN) mce_intel.c:782: MCA Capability: firstbank 0, extended MCE MSR 0, 
>> BCAST, CMCI
>> (XEN) CPU0 CMCI LVT vector (0xf2) already installed
>> (XEN) Finishing wakeup from ACPI S3 state.
>> (XEN) Enabling non-boot CPUs  ...
>> (XEN) emul-priv-op.c:1166:d0v1 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee0
>> (XEN) emul-priv-op.c:1166:d0v1 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee00800
>> (XEN) emul-priv-op.c:1166:d0v2 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee0
>> (XEN) emul-priv-op.c:1166:d0v2 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee00800
>> (XEN) emul-priv-op.c:1166:d0v3 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee0
>> (XEN) emul-priv-op.c:1166:d0v3 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee00800
>> (XEN) emul-priv-op.c:1166:d0v4 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee0
>> (XEN) emul-priv-op.c:1166:d0v4 Domain attempted WRMSR 001b from 
>> 0xfee00c00 to 0xfee00800
> 
> /sigh - Naughty Linux.  The PVOps really ought to know that they don't
> have an APIC to play with, not that this related to the crash.
> 
>> (XEN) *** DOUBLE FAULT ***
>> (XEN) [ Xen-4.11-rc  x86_64  debug=y   Not tainted ]
>> (XEN) CPU:0
>> (XEN) RIP:e008:[] handle_exception+0x9c/0xff
>> (XEN) RFLAGS: 00010006   CONTEXT: hypervisor
>> (XEN) rax: c90040ce40d8   rbx:    rcx: 0003
>> (XEN) rdx:    rsi:    rdi: 
>> (XEN) rbp: 36ffbf31bf07   rsp: c90040ce4000   r8:  
>> (XEN) r9:     r10:    r11: 
>> (XEN) r12:    r13:    r14: c90040ce7fff
>> (XEN) r15:    cr0: 8005003b   cr4: 00042660
>> (XEN) cr3: 00022200a000   cr2: c90040ce3ff8
>> (XEN) fsb: 7fa9e7909740   gsb: 88021e74   gss: 
>> (XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
>> (XEN) Xen code around  (handle_exception+0x9c/0xff):
>> (XEN)  00 f3 90 0f ae e8 eb f9  07 00 00 00 f3 90 0f ae e8 eb f9 83 e9 
>> 01 75
>> (XEN) Current stack base c90040ce differs from expected 
>> 8300cec88000
>> (XEN) Valid stack range: c90040ce6000-c90040ce8000, 
>> sp=c90040ce4000, tss.rsp0=8300cec8ffa0
>> (XEN) No stack overflow detected. Skipping stack trace.
> 
> Ok - this is the same as George's crash, and yes - I did misdiagnose the

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Andrew Cooper

On 24/05/18 15:35, Simon Gaiser wrote:
> Andrew Cooper:
>> On 24/05/18 15:14, Simon Gaiser wrote:
>>> Jan Beulich:
>>> On 24.05.18 at 16:00,  wrote:
> Jan Beulich:
>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>> I've failed to remember the fact that multiple CPUs share a stub
>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>> when bringing down a CPU; it may only be unmapped when no other online
>> CPU uses that same page.
>>
>> Reported-by: Simon Gaiser 
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>  
>>  free_xen_pagetable(rpt);
>>  
>> -/* Also zap the stub mapping for this CPU. */
>> +/*
>> + * Also zap the stub mapping for this CPU, if no other online one 
>> uses
>> + * the same page.
>> + */
>> +if ( stub_linear )
>> +{
>> +unsigned int other;
>> +
>> +for_each_online_cpu(other)
>> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
>> PAGE_SHIFT) )
>> +{
>> +stub_linear = 0;
>> +break;
>> +}
>> +}
>>  if ( stub_linear )
>>  {
>>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
> Tried this on-top of staging (fc5805daef) and I still get the same
> double fault.
 Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. 
 What
 size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
 are in the same page (and I'd never unmap anything here at all).
>>> 4 cores + HT, so 8 CPUs from Xen's PoV.
>> Can you try with the "x86/traps: Dump the instruction stream even for
>> double faults" patch I've just posted, and show the full #DF panic log
>> please?  (Its conceivable that there are multiple different issues here.)
> With Jan's and your patch:
>
> (XEN) mce_intel.c:782: MCA Capability: firstbank 0, extended MCE MSR 0, 
> BCAST, CMCI
> (XEN) CPU0 CMCI LVT vector (0xf2) already installed
> (XEN) Finishing wakeup from ACPI S3 state.
> (XEN) Enabling non-boot CPUs  ...
> (XEN) emul-priv-op.c:1166:d0v1 Domain attempted WRMSR 001b from 
> 0xfee00c00 to 0xfee0
> (XEN) emul-priv-op.c:1166:d0v1 Domain attempted WRMSR 001b from 
> 0xfee00c00 to 0xfee00800
> (XEN) emul-priv-op.c:1166:d0v2 Domain attempted WRMSR 001b from 
> 0xfee00c00 to 0xfee0
> (XEN) emul-priv-op.c:1166:d0v2 Domain attempted WRMSR 001b from 
> 0xfee00c00 to 0xfee00800
> (XEN) emul-priv-op.c:1166:d0v3 Domain attempted WRMSR 001b from 
> 0xfee00c00 to 0xfee0
> (XEN) emul-priv-op.c:1166:d0v3 Domain attempted WRMSR 001b from 
> 0xfee00c00 to 0xfee00800
> (XEN) emul-priv-op.c:1166:d0v4 Domain attempted WRMSR 001b from 
> 0xfee00c00 to 0xfee0
> (XEN) emul-priv-op.c:1166:d0v4 Domain attempted WRMSR 001b from 
> 0xfee00c00 to 0xfee00800

/sigh - Naughty Linux.  The PVOps really ought to know that they don't
have an APIC to play with, not that this related to the crash.

> (XEN) *** DOUBLE FAULT ***
> (XEN) [ Xen-4.11-rc  x86_64  debug=y   Not tainted ]
> (XEN) CPU:0
> (XEN) RIP:e008:[] handle_exception+0x9c/0xff
> (XEN) RFLAGS: 00010006   CONTEXT: hypervisor
> (XEN) rax: c90040ce40d8   rbx:    rcx: 0003
> (XEN) rdx:    rsi:    rdi: 
> (XEN) rbp: 36ffbf31bf07   rsp: c90040ce4000   r8:  
> (XEN) r9:     r10:    r11: 
> (XEN) r12:    r13:    r14: c90040ce7fff
> (XEN) r15:    cr0: 8005003b   cr4: 00042660
> (XEN) cr3: 00022200a000   cr2: c90040ce3ff8
> (XEN) fsb: 7fa9e7909740   gsb: 88021e74   gss: 
> (XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
> (XEN) Xen code around  (handle_exception+0x9c/0xff):
> (XEN)  00 f3 90 0f ae e8 eb f9  07 00 00 00 f3 90 0f ae e8 eb f9 83 e9 01 
> 75
> (XEN) Current stack base c90040ce differs from expected 
> 8300cec88000
> (XEN) Valid stack range: c90040ce6000-c90040ce8000, 
> sp=c90040ce4000, tss.rsp0=8300cec8ffa0
> (XEN) No stack overflow detected. Skipping stack trace.

Ok - this is the same as George's crash, and yes - I did misdiagnose the
stack we were on.  I presume this hardware doesn't have SMAP? (or we've
expected to take a #DF immediately at the head of the syscall hander.)

~Andrew

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Simon Gaiser

Andrew Cooper:
> On 24/05/18 15:14, Simon Gaiser wrote:
>> Jan Beulich:
>> On 24.05.18 at 16:00,  wrote:
 Jan Beulich:
> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
> I've failed to remember the fact that multiple CPUs share a stub
> mapping page. Therefore it is wrong to unconditionally zap the mapping
> when bringing down a CPU; it may only be unmapped when no other online
> CPU uses that same page.
>
> Reported-by: Simon Gaiser 
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>  
>  free_xen_pagetable(rpt);
>  
> -/* Also zap the stub mapping for this CPU. */
> +/*
> + * Also zap the stub mapping for this CPU, if no other online one 
> uses
> + * the same page.
> + */
> +if ( stub_linear )
> +{
> +unsigned int other;
> +
> +for_each_online_cpu(other)
> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
> PAGE_SHIFT) )
> +{
> +stub_linear = 0;
> +break;
> +}
> +}
>  if ( stub_linear )
>  {
>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
 Tried this on-top of staging (fc5805daef) and I still get the same
 double fault.
>>> Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. What
>>> size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
>>> are in the same page (and I'd never unmap anything here at all).
>> 4 cores + HT, so 8 CPUs from Xen's PoV.
> 
> Can you try with the "x86/traps: Dump the instruction stream even for
> double faults" patch I've just posted, and show the full #DF panic log
> please?  (Its conceivable that there are multiple different issues here.)

With Jan's and your patch:

(XEN) mce_intel.c:782: MCA Capability: firstbank 0, extended MCE MSR 0, BCAST, 
CMCI
(XEN) CPU0 CMCI LVT vector (0xf2) already installed
(XEN) Finishing wakeup from ACPI S3 state.
(XEN) Enabling non-boot CPUs  ...
(XEN) emul-priv-op.c:1166:d0v1 Domain attempted WRMSR 001b from 
0xfee00c00 to 0xfee0
(XEN) emul-priv-op.c:1166:d0v1 Domain attempted WRMSR 001b from 
0xfee00c00 to 0xfee00800
(XEN) emul-priv-op.c:1166:d0v2 Domain attempted WRMSR 001b from 
0xfee00c00 to 0xfee0
(XEN) emul-priv-op.c:1166:d0v2 Domain attempted WRMSR 001b from 
0xfee00c00 to 0xfee00800
(XEN) emul-priv-op.c:1166:d0v3 Domain attempted WRMSR 001b from 
0xfee00c00 to 0xfee0
(XEN) emul-priv-op.c:1166:d0v3 Domain attempted WRMSR 001b from 
0xfee00c00 to 0xfee00800
(XEN) emul-priv-op.c:1166:d0v4 Domain attempted WRMSR 001b from 
0xfee00c00 to 0xfee0
(XEN) emul-priv-op.c:1166:d0v4 Domain attempted WRMSR 001b from 
0xfee00c00 to 0xfee00800
(XEN) *** DOUBLE FAULT ***
(XEN) [ Xen-4.11-rc  x86_64  debug=y   Not tainted ]
(XEN) CPU:0
(XEN) RIP:e008:[] handle_exception+0x9c/0xff
(XEN) RFLAGS: 00010006   CONTEXT: hypervisor
(XEN) rax: c90040ce40d8   rbx:    rcx: 0003
(XEN) rdx:    rsi:    rdi: 
(XEN) rbp: 36ffbf31bf07   rsp: c90040ce4000   r8:  
(XEN) r9:     r10:    r11: 
(XEN) r12:    r13:    r14: c90040ce7fff
(XEN) r15:    cr0: 8005003b   cr4: 00042660
(XEN) cr3: 00022200a000   cr2: c90040ce3ff8
(XEN) fsb: 7fa9e7909740   gsb: 88021e74   gss: 
(XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
(XEN) Xen code around  (handle_exception+0x9c/0xff):
(XEN)  00 f3 90 0f ae e8 eb f9  07 00 00 00 f3 90 0f ae e8 eb f9 83 e9 01 75
(XEN) Current stack base c90040ce differs from expected 8300cec88000
(XEN) Valid stack range: c90040ce6000-c90040ce8000, 
sp=c90040ce4000, tss.rsp0=8300cec8ffa0
(XEN) No stack overflow detected. Skipping stack trace.
(XEN) 
(XEN) 
(XEN) Panic on CPU 0:
(XEN) DOUBLE FAULT -- system shutdown
(XEN) 
(XEN) 
(XEN) Reboot in five seconds...



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Jan Beulich

>>> On 24.05.18 at 16:24,  wrote:
> On 24/05/18 15:22, Jan Beulich wrote:
> On 24.05.18 at 16:18,  wrote:
>>> Can you try with the "x86/traps: Dump the instruction stream even for
>>> double faults" patch I've just posted, and show the full #DF panic log
>>> please?  (Its conceivable that there are multiple different issues here.)
>> Well, as long as we're on a guest kernel stack rather than our own, I
>> don't think the exact insn causing the #DF really matters. See earlier
>> mails I have sent in this regard.
> 
> In George's crash, we were in a weird place on the hypervisor stack, not
> a guest stack...

Go look again - %rsp pointed outside of hypervisor space in all cases that
I had looked at. And that's explained by the unmapping of the stubs: We'd
#PF right after first SYSCALL, and the handler would then run on the stack
that's still active from guest context.

Jan



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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Jan Beulich

>>> On 24.05.18 at 16:14,  wrote:
> Jan Beulich:
> On 24.05.18 at 16:00,  wrote:
>>> Jan Beulich:
 In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
 I've failed to remember the fact that multiple CPUs share a stub
 mapping page. Therefore it is wrong to unconditionally zap the mapping
 when bringing down a CPU; it may only be unmapped when no other online
 CPU uses that same page.

 Reported-by: Simon Gaiser 
 Signed-off-by: Jan Beulich 

 --- a/xen/arch/x86/smpboot.c
 +++ b/xen/arch/x86/smpboot.c
 @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
  
  free_xen_pagetable(rpt);
  
 -/* Also zap the stub mapping for this CPU. */
 +/*
 + * Also zap the stub mapping for this CPU, if no other online one uses
 + * the same page.
 + */
 +if ( stub_linear )
 +{
 +unsigned int other;
 +
 +for_each_online_cpu(other)
 +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
 PAGE_SHIFT) )
 +{
 +stub_linear = 0;
 +break;
 +}
 +}
  if ( stub_linear )
  {
  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>>>
>>> Tried this on-top of staging (fc5805daef) and I still get the same
>>> double fault.
>> 
>> Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. What
>> size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
>> are in the same page (and I'd never unmap anything here at all).
> 
> 4 cores + HT, so 8 CPUs from Xen's PoV.

May I ask you to do two things:
1) confirm that you can offline CPUs successfully using xen-hptool,
2) add a printk() to the code above making clear whether/when any
of the mappings actually get zapped?

Thanks, Jan



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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Andrew Cooper

On 24/05/18 15:22, Jan Beulich wrote:
 On 24.05.18 at 16:18,  wrote:
>> Can you try with the "x86/traps: Dump the instruction stream even for
>> double faults" patch I've just posted, and show the full #DF panic log
>> please?  (Its conceivable that there are multiple different issues here.)
> Well, as long as we're on a guest kernel stack rather than our own, I
> don't think the exact insn causing the #DF really matters. See earlier
> mails I have sent in this regard.

In George's crash, we were in a weird place on the hypervisor stack, not
a guest stack...

~Andrew

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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Andrew Cooper

On 24/05/18 15:14, Simon Gaiser wrote:
> Jan Beulich:
> On 24.05.18 at 16:00,  wrote:
>>> Jan Beulich:
 In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
 I've failed to remember the fact that multiple CPUs share a stub
 mapping page. Therefore it is wrong to unconditionally zap the mapping
 when bringing down a CPU; it may only be unmapped when no other online
 CPU uses that same page.

 Reported-by: Simon Gaiser 
 Signed-off-by: Jan Beulich 

 --- a/xen/arch/x86/smpboot.c
 +++ b/xen/arch/x86/smpboot.c
 @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
  
  free_xen_pagetable(rpt);
  
 -/* Also zap the stub mapping for this CPU. */
 +/*
 + * Also zap the stub mapping for this CPU, if no other online one uses
 + * the same page.
 + */
 +if ( stub_linear )
 +{
 +unsigned int other;
 +
 +for_each_online_cpu(other)
 +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
 PAGE_SHIFT) )
 +{
 +stub_linear = 0;
 +break;
 +}
 +}
  if ( stub_linear )
  {
  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>>> Tried this on-top of staging (fc5805daef) and I still get the same
>>> double fault.
>> Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. What
>> size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
>> are in the same page (and I'd never unmap anything here at all).
> 4 cores + HT, so 8 CPUs from Xen's PoV.

Can you try with the "x86/traps: Dump the instruction stream even for
double faults" patch I've just posted, and show the full #DF panic log
please?  (Its conceivable that there are multiple different issues here.)

~Andrew

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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Simon Gaiser

Jan Beulich:
 On 24.05.18 at 16:00,  wrote:
>> Jan Beulich:
>>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>>> I've failed to remember the fact that multiple CPUs share a stub
>>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>>> when bringing down a CPU; it may only be unmapped when no other online
>>> CPU uses that same page.
>>>
>>> Reported-by: Simon Gaiser 
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>>  
>>>  free_xen_pagetable(rpt);
>>>  
>>> -/* Also zap the stub mapping for this CPU. */
>>> +/*
>>> + * Also zap the stub mapping for this CPU, if no other online one uses
>>> + * the same page.
>>> + */
>>> +if ( stub_linear )
>>> +{
>>> +unsigned int other;
>>> +
>>> +for_each_online_cpu(other)
>>> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
>>> PAGE_SHIFT) )
>>> +{
>>> +stub_linear = 0;
>>> +break;
>>> +}
>>> +}
>>>  if ( stub_linear )
>>>  {
>>>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>>
>> Tried this on-top of staging (fc5805daef) and I still get the same
>> double fault.
> 
> Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. What
> size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
> are in the same page (and I'd never unmap anything here at all).

4 cores + HT, so 8 CPUs from Xen's PoV.



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Jan Beulich

>>> On 24.05.18 at 16:00,  wrote:
> Jan Beulich:
>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>> I've failed to remember the fact that multiple CPUs share a stub
>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>> when bringing down a CPU; it may only be unmapped when no other online
>> CPU uses that same page.
>> 
>> Reported-by: Simon Gaiser 
>> Signed-off-by: Jan Beulich 
>> 
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>  
>>  free_xen_pagetable(rpt);
>>  
>> -/* Also zap the stub mapping for this CPU. */
>> +/*
>> + * Also zap the stub mapping for this CPU, if no other online one uses
>> + * the same page.
>> + */
>> +if ( stub_linear )
>> +{
>> +unsigned int other;
>> +
>> +for_each_online_cpu(other)
>> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> 
>> PAGE_SHIFT) )
>> +{
>> +stub_linear = 0;
>> +break;
>> +}
>> +}
>>  if ( stub_linear )
>>  {
>>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
> 
> Tried this on-top of staging (fc5805daef) and I still get the same
> double fault.

Hmm, it worked for me offlining (and later re-onlining) several pCPU-s. What
size a system are you testing on? Mine has got only 12 CPUs, i.e. all stubs
are in the same page (and I'd never unmap anything here at all).

Jan



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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Jan Beulich

>>> On 24.05.18 at 15:48,  wrote:
> On 24/05/18 14:41, Jan Beulich wrote:
>> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
>> I've failed to remember the fact that multiple CPUs share a stub
>> mapping page. Therefore it is wrong to unconditionally zap the mapping
>> when bringing down a CPU; it may only be unmapped when no other online
>> CPU uses that same page.
>>
>> Reported-by: Simon Gaiser 
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>>  
>>  free_xen_pagetable(rpt);
>>  
>> -/* Also zap the stub mapping for this CPU. */
>> +/*
>> + * Also zap the stub mapping for this CPU, if no other online one uses
>> + * the same page.
>> + */
>> +if ( stub_linear )
>> +{
>> +unsigned int other;
>> +
>> +for_each_online_cpu(other)
> 
> Look over the code, it seems that with spaces is the more common style,
> but it is admittedly fairly mixed.

I'd prefer to leave it as is - personally I don't consider "for_each_online_cpu"
and alike keywords, which is what ./CODING_STYLE talks about. I accept
others taking a different position, i.e. I don't normally demand a particular
style to be used there, but in code I write I prefer to only apply spaces to
real keywords.

> Either way (as that's trivial to fix), Acked-by: Andrew Cooper
> 

Thanks, Jan



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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Andrew Cooper

On 24/05/18 14:41, Jan Beulich wrote:
> In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
> I've failed to remember the fact that multiple CPUs share a stub
> mapping page. Therefore it is wrong to unconditionally zap the mapping
> when bringing down a CPU; it may only be unmapped when no other online
> CPU uses that same page.
>
> Reported-by: Simon Gaiser 
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
>  
>  free_xen_pagetable(rpt);
>  
> -/* Also zap the stub mapping for this CPU. */
> +/*
> + * Also zap the stub mapping for this CPU, if no other online one uses
> + * the same page.
> + */
> +if ( stub_linear )
> +{
> +unsigned int other;
> +
> +for_each_online_cpu(other)

Look over the code, it seems that with spaces is the more common style,
but it is admittedly fairly mixed.

Either way (as that's trivial to fix), Acked-by: Andrew Cooper


> +if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> PAGE_SHIFT) 
> )
> +{
> +stub_linear = 0;
> +break;
> +}
> +}
>  if ( stub_linear )
>  {
>  l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);
>
>
>
>


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

[Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

2018-05-24 Thread Jan Beulich

In commit d1d6fc97d6 ("x86/xpti: really hide almost all of Xen image")
I've failed to remember the fact that multiple CPUs share a stub
mapping page. Therefore it is wrong to unconditionally zap the mapping
when bringing down a CPU; it may only be unmapped when no other online
CPU uses that same page.

Reported-by: Simon Gaiser 
Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -876,7 +876,21 @@ static void cleanup_cpu_root_pgt(unsigne
 
 free_xen_pagetable(rpt);
 
-/* Also zap the stub mapping for this CPU. */
+/*
+ * Also zap the stub mapping for this CPU, if no other online one uses
+ * the same page.
+ */
+if ( stub_linear )
+{
+unsigned int other;
+
+for_each_online_cpu(other)
+if ( !((per_cpu(stubs.addr, other) ^ stub_linear) >> PAGE_SHIFT) )
+{
+stub_linear = 0;
+break;
+}
+}
 if ( stub_linear )
 {
 l3_pgentry_t *l3t = l4e_to_l3e(common_pgt);





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

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

Re: [Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

[Xen-devel] [PATCH] x86/XPTI: fix S3 resume (and CPU offlining in general)

18 matches

Site Navigation

Mail list logo

Footer information