Re: [PATCH, DEBUG] x86/32: Add small delay after resume

Srinivas Pandruvada
2015-06-15T16:12:38Z

On Sat, 2015-06-13 at 09:15 +0200, Ingo Molnar wrote:
> * Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>
> > > Also, could you please describe how the failure triggers in your system: how
> > > many times do you have to suspend/resume to trigger the segfaults, and is
> > > there anything that makes the failures less or more likely?
> >
> > It is very random. Sometimes only few hundred trys reproduce this issue. Some
> > other times it requires thousands of trys (sometimes not reproducible at all for
> > days) It is very time sensitive.
>
> So the very same kernel image will produce different crash patterns depending on
> the time of day? That suggests heat/hardware problems.
>
> > [...] A small delay or some debug code in resume path prevents this to crash.
>
> Fun...
>
> > The BIOS folks created special version to check if they are corrupting any DS,
> > but they were not able to catch any corruption. [...]
>
> So is it true that we always execute wakeup_pmode_return first after we return
> from the BIOS?
>
> If so then the BIOS touching DS cannot be an issue, as we re-initialize all
> segment selectors, which reloads the descriptors:
>
> ENTRY(wakeup_pmode_return)
> wakeup_pmode_return:
> movw $__KERNEL_DS, %ax
> movw %ax, %ss
> movw %ax, %ds
> movw %ax, %es
> movw %ax, %fs
> movw %ax, %gs
>
> # reload the gdt, as we need the full 32 bit address
> lidt saved_idt
> lldt saved_ldt
> ljmp $(__KERNEL_CS), $1f
>
> > [...] Since these are special deployed systems running critical application,
> > need to request the tests again with your changes. May take long time.
>
> So my second patch is clearly broken as per Brian Gerst's comments.
>
> What I would suggest is to try a patch that adds just 100 NOPs or so - attached
> below. This patch will add a small delay without any side effects (other than
> changing the kernel image layout).
>
> If that makes the crash go away, then I'd say the likelihood that it's hardware
> related increases substantially: maybe a PLL has not stabilized yet sufficiently
> after resume, or there's some latent heat sensitivity and the fan has not started
> up yet, etc.

> ( You can then use this simple delay generating patch in production systems as
> well, to work around the problem. Maybe convince the BIOS folks to add a delay
> like this to their resume path before they call Linux. )
This was already experimented. They added delay in BIOS before handing
over to OS, the crash still occurred.
We were thinking that BIOS SMI handler responsible for suspend/wake up
corrupted the DS even after control passed over to OS. But couldn't
prove it.
Thanks for your valuable debugging suggestions.

Thanks,
Srinivas

>
> Thanks,
>
> Ingo
>
> =================>
>
> arch/x86/kernel/acpi/wakeup_32.S | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kernel/acpi/wakeup_32.S b/arch/x86/kernel/acpi/wakeup_32.S
> index 665c6b7d2ea9..ef26999da80a 100644
> --- a/arch/x86/kernel/acpi/wakeup_32.S
> +++ b/arch/x86/kernel/acpi/wakeup_32.S
> @@ -10,6 +10,12 @@
>
> ENTRY(wakeup_pmode_return)
> wakeup_pmode_return:
> +
> + /* Timing delay of a few dozen cycles: give the hardware some time to recover */
> + .rept 100
> + nop
> + .endr
> +
> movw $__KERNEL_DS, %ax
> movw %ax, %ss
> movw %ax, %ds