linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: lijiang <lijiang@redhat.com>
Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org,
	x86@kernel.org, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, akpm@linux-foundation.org, dyoung@redhat.com
Subject: Re: [PATCH 2/2 v5] x86/kexec_file: add reserved e820 ranges to kdump kernel e820 table
Date: Fri, 9 Nov 2018 14:51:46 +0800	[thread overview]
Message-ID: <20181109065146.GD31064@MiWiFi-R3L-srv> (raw)
In-Reply-To: <cc579d02-19e9-8749-ea48-3ff0fbf6461d@redhat.com>

On 11/07/18 at 05:10pm, lijiang wrote:
> In fact, these patches are really simple. As the subject mentioned, this patch
> [PATCH 2/2] adds the reserved e820 ranges to kdump kernel e820 table, and the
> first patch [PATCH 1/2] helps to exactly add the e820(E820_TYPE_RESERVED) type
> to kdump kernel e820 table, that is to say, it will filter out some unnecessary
> type(E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820_TYPE_RESERVED_KERN).
> 
> At present, when we use the kexec to load the kernel image and initramfs(for
> example: kexec -s -p xxxx), the latest kernel does not pass the e820 reserved
> ranges to the second kernel, which might produce two problems:
> 
> The first one is the MMCONFIG issue, although which does not make the system
> crash or hang, this issue is still a potential risk, because my test can't
> cover all cases due to resource constraints(Machine), and i'm not sure what
> it will happen on other machine.
> 
> The second issue is that the e820 reserved ranges do not setup in kdump kernel,
> which will cause some functions which are related to the e820 reserved ranges
> to become invalid. For example:
> 
> early_memremap()->
> early_memremap_pgprot_adjust()->
> memremap_should_map_decrypted()->
> e820__get_entry_type()
> 
> Please focus on these functions, early_memremap_pgprot_adjust() and memremap_should_map_decrypted().
> 
> In the first kernel, these ranges sit in e820 reserved ranges, so the memremap_should_map_decrypted()
> will return true, that is to say, the reserved memory is decrypted, then the early_memremap_pgprot_adjust()
> will call the pgprot_decrypted() to clear the memory encryption mask.
> 
> In the second kernel, because the e820 reserved ranges are not passed to the second kernel, these ranges
> don't sit in the e820 reserved ranges, so the the memremap_should_map_decrypted() will return false, that
> is to say, the reserved memory is encrypted, and then the early_memremap_pgprot_adjust() will also call the
> pgprot_encrypted() to set the memory encryption mask.
> 
> Obviously, in the second kernel, the e820 reserved memory is still decrypted, it has gone wrong. So, if
> don't fix, kdump won't work when we use the command(kexec -s -p xxx) to load the kernel image and initramfs.

Yes, this looks better. In fact, I searched cover-letter and both patch
lg, didn't find anything to tell what will happen without these
patchset. We should at least tell the patch is for cleanup/improvement/fix/feature.
This "if don't fix, kdump won't work ..." is the sentence I first saw
telling the sequence or the problem.

Please do not misunderstand those questions from me with unfriendly tone,
words are cold and can't express emotions exactly, and appearance. Those
questions are just meaning that people may get confusion from those
places.

What: describe the problem you met.
Why:  tell the root cause
How:  how you fix it

This is what I usually use to write patch log. Just for your reference.

Thanks
Baoquan

  parent reply	other threads:[~2018-11-09  6:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07  5:00 [PATCH 0/2 v5] add reserved e820 ranges to the kdump kernel e820 table Lianbo Jiang
2018-11-07  5:00 ` [PATCH 1/2 v5] x86/kexec_file: add e820 entry in case e820 type string matches to io resource name Lianbo Jiang
2018-11-07  5:17   ` Baoquan He
2018-11-07  5:00 ` [PATCH 2/2 v5] x86/kexec_file: add reserved e820 ranges to kdump kernel e820 table Lianbo Jiang
2018-11-07  5:23   ` Baoquan He
2018-11-07  9:10     ` lijiang
2018-11-07  9:35       ` Dave Young
2018-11-09  6:51       ` Baoquan He [this message]
2018-11-09  7:13         ` lijiang

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=20181109065146.GD31064@MiWiFi-R3L-srv \
    --to=bhe@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=dyoung@redhat.com \
    --cc=kexec@lists.infradead.org \
    --cc=lijiang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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).