linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Borislav Petkov <bp@alien8.de>, Chen Yu <yu.c.chen@intel.com>,
	Linux PM <linux-pm@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Lee@nazgul.tnic, Chun-Yi <jlee@suse.com>
Subject: Re: [PATCH][v8] PM / hibernate: Verify the consistent of e820 memory map by md5 value
Date: Tue, 30 Aug 2016 21:53:23 +0200	[thread overview]
Message-ID: <20160830195323.GA9937@amd> (raw)
In-Reply-To: <7552351.MYV7GZcH0A@vostro.rjw.lan>


> > > I would warn and try to continue regardless (which was the original
> > > plan here AFAICS), or we change a possible data loss into a guaranteed
> > > one.
> > > 
> > > IMO it is sufficient to give up when a PFN we have image data for is
> > > not pfn_valid() during resume, which we do already.
> > 
> > Well... can you guarantee what will be effect of resuming with
> > different memory map?
> > 
> > Because there's big difference between panic and trying to continue
> > with corrupted memory.
> 
> If all of the page frames the image kernel used before hibernation are
> available during resume as well, memory won't really get corrupted, at least
> not right away.
> 
> There may be problems going forward, but whether or not they actually happen
> depends on what the differences are.  So while an e820 mismatch indicates that
> things may go wrong, it doesn't necessarily mean that they will.

Well "memory won't get corrupted right away" seems like good reason to
panic the machine ASAP.

You can flip some bits in memory, and it may not cause problems. Still
if you know some bits in memory were flipped, you'd better panic the
machine. Continuing is unsafe.

If you could guarantee that machine will panic down the line, and not
something worse, you'd be right.

But at least the case where there is _less_ memory available after
resume, kernel will write into BIOS reserved memory and bad things
will happen. Yes, it usually panics, but it is quite clear it could
corrupt memory, too.

So I believe we should take the patch, and let users update their
BIOSes. [And I believe it is not too widespread, either.]

If you want to try to cook a patch that determines if new e820 map is
superset of the old one... well... I believe the resulting complexity
will be obviously unreasonable but I guess you (or some interested
person) can try.

> Also, that panic() may cause hibernation to stop working in a sort of hard and
> nasty way where it used to work flawlessly previously and that would be a
> regression, so not really acceptable.

Well, turning memory corruption bug into panic is an improvement, not
a regression.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

  reply	other threads:[~2016-08-30 19:53 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-28 16:35 [PATCH][v8] PM / hibernate: Verify the consistent of e820 memory map by md5 value Chen Yu
2016-08-28 16:36 ` Pavel Machek
2016-08-29  4:59 ` Borislav Petkov
2016-08-29  7:15   ` Pavel Machek
2016-08-29 13:41     ` Borislav Petkov
2016-08-30  8:35       ` joeyli
2016-08-30 11:54         ` Rafael J. Wysocki
2016-09-08 21:15           ` Pavel Machek
2016-09-09  7:36             ` Chen Yu
2016-09-09  7:33               ` Pavel Machek
2016-08-30 11:51       ` Rafael J. Wysocki
2016-08-29 13:41   ` Rafael J. Wysocki
2016-08-29 15:13     ` Pavel Machek
2016-08-30 12:04       ` Rafael J. Wysocki
2016-08-30 19:53         ` Pavel Machek [this message]
2016-08-30 21:50           ` Rafael J. Wysocki
2016-08-31 11:03             ` Pavel Machek
2016-08-31  0:27 ` Rafael J. Wysocki
2016-08-31 11:07   ` Pavel Machek
2016-08-31 11:43     ` Pavel Machek
2016-08-31 11:54       ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160830195323.GA9937@amd \
    --to=pavel@ucw.cz \
    --cc=Lee@nazgul.tnic \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jlee@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rafael@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yu.c.chen@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).