[PATCH 0/3 v2] Add kdump support for the SEV enabled guest

[PATCH 0/3 v2] Add kdump support for the SEV enabled guest

[Lianbo Jiang]

Just like the physical machines support kdump, the virtual machines
also need kdump. When a virtual machine panic, we also need to dump
its memory for analysis.

For the SEV virtual machine, the memory is also encrypted. When SEV
is enabled, the first kernel is loaded into the encrypted area.
Unlike the SME, the first kernel is loaded into the decrypted area.

Because of this difference between SME and SEV, we need to properly
map the kexec memory area in order to correctly access it.

Test tools:
makedumpfile[v1.6.5]:
git://git.code.sf.net/p/makedumpfile/code
commit <d222b01e516b> ("Add support for AMD Secure Memory Encryption")
Note: This patch was merged into the devel branch.

crash-7.2.5: https://github.com/crash-utility/crash.git
commit <942d813cda35> ("Fix for the "kmem -i" option on Linux 5.0")

kexec-tools-2.0.19:
git://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git
commit <942d813cda35> ("Fix for the kmem '-i' option on Linux 5.0")
http://lists.infradead.org/pipermail/kexec/2019-March/022576.html
Note: The second kernel cann't boot without this patch.

kernel:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
commit <f261c4e529da> ("Merge branch 'akpm' (patches from Andrew)")

Test steps:
[1] load the vmlinux and initrd for kdump
# kexec -p /boot/vmlinuz-5.0.0+ --initrd=/boot/initramfs-5.0.0+kdump.img --command-line="BOOT_IMAGE=(hd0,gpt2)/vmlinuz-5.0.0+ ro resume=UUID=126c5e95-fc8b-48d6-a23b-28409198a52e console=ttyS0,115200 earlyprintk=serial irqpoll nr_cpus=1 reset_devices cgroup_disable=memory mce=off numa=off udev.children-max=2 panic=10 rootflags=nofail acpi_no_memhotplug transparent_hugepage=never disable_cpu_apicid=0"

[2] trigger panic
# echo 1 > /proc/sys/kernel/sysrq
# echo c > /proc/sysrq-trigger

[3] check and parse the vmcore
# crash vmlinux /var/crash/127.0.0.1-2019-03-15-05\:03\:42/vmcore

Changes since v1:
1. Modify the patch subject prefixes.
2. Improve patch log: add parentheses at the end of the function names.
3. Fix the multiple confusing checks.
4. Add comment in the arch_kexec_post_alloc_pages().

Lianbo Jiang (3):
  x86/kexec: Do not map the kexec area as decrypted when SEV is active
  x86/kexec: Set the C-bit in the identity map page table when SEV is
    active
  kdump,proc/vmcore: Enable kdumping encrypted memory when SEV was
    active

 arch/x86/kernel/machine_kexec_64.c | 27 ++++++++++++++++++++++++++-
 fs/proc/vmcore.c                   |  6 +++---
 2 files changed, 29 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH 1/3 v2] x86/kexec: Do not map the kexec area as decrypted when SEV is active

[Lianbo Jiang]

Currently, the arch_kexec_post_{alloc,free}_pages() unconditionally
maps the kexec area as decrypted. This works fine when SME is active.
Because in SME, the first kernel is loaded in decrypted area by the
BIOS, so the second kernel must be also loaded into the decrypted
memory.

When SEV is active, the first kernel is loaded into the encrypted
area, so the second kernel must be also loaded into the encrypted
memory. Lets make sure that arch_kexec_post_{alloc,free}_pages()
does not clear the memory encryption mask from the kexec area when
SEV is active.

Co-developed-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
---
 arch/x86/kernel/machine_kexec_64.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index ceba408ea982..f60611531d17 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -559,18 +559,33 @@ void arch_kexec_unprotect_crashkres(void)
 	kexec_mark_crashkres(false);
 }
 
+/*
+ * During a traditional boot under SME, SME will encrypt the kernel,
+ * so the SME kexec kernel also needs to be un-encrypted in order to
+ * replicate a normal SME boot.
+ * During a traditional boot under SEV, the kernel has already been
+ * loaded encrypted, so the SEV kexec kernel needs to be encrypted in
+ * order to replicate a normal SEV boot.
+ */
 int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
 {
+	if (sev_active())
+		return 0;
+
 	/*
 	 * If SME is active we need to be sure that kexec pages are
 	 * not encrypted because when we boot to the new kernel the
 	 * pages won't be accessed encrypted (initially).
 	 */
 	return set_memory_decrypted((unsigned long)vaddr, pages);
+
 }
 
 void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages)
 {
+	if (sev_active())
+		return;
+
 	/*
 	 * If SME is active we need to reset the pages back to being
 	 * an encrypted mapping before freeing them.
-- 
2.17.1

Re: [PATCH 1/3 v2] x86/kexec: Do not map the kexec area as decrypted when SEV is active

[Borislav Petkov]

On Wed, Mar 27, 2019 at 01:36:27PM +0800, Lianbo Jiang wrote:
> Currently, the arch_kexec_post_{alloc,free}_pages() unconditionally
> maps the kexec area as decrypted. This works fine when SME is active.
> Because in SME, the first kernel is loaded in decrypted area by the
> BIOS, so the second kernel must be also loaded into the decrypted
> memory.
> 
> When SEV is active, the first kernel is loaded into the encrypted
> area, so the second kernel must be also loaded into the encrypted
> memory. Lets make sure that arch_kexec_post_{alloc,free}_pages()
> does not clear the memory encryption mask from the kexec area when
> SEV is active.

This commit message still doesn't explain the big picture why you want
this change.

And it must explain it because it might be all clear in your head now
but months from now, you, we, all would've forgotten why this change was
needed.

So pls add blurb that this whole effort is being done so that SEV VMs
can kdump too. I.e., the 10000ft picture.

Anyone must be able to figure out *why* a change has been done just by
doing git archeology. So make sure you explain it properly.

If unsure, try to put yourself in the shoes of some future kernel
developer who is trying to find out why this change has been done. Now
read the commit message you've written. Does it make any sense to him? I
think not.

Do you catch my drift?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 1/3 v2] x86/kexec: Do not map the kexec area as decrypted when SEV is active

[lijiang]

在 2019年04月02日 18:32, Borislav Petkov 写道:
> On Wed, Mar 27, 2019 at 01:36:27PM +0800, Lianbo Jiang wrote:
>> Currently, the arch_kexec_post_{alloc,free}_pages() unconditionally
>> maps the kexec area as decrypted. This works fine when SME is active.
>> Because in SME, the first kernel is loaded in decrypted area by the
>> BIOS, so the second kernel must be also loaded into the decrypted
>> memory.
>>
>> When SEV is active, the first kernel is loaded into the encrypted
>> area, so the second kernel must be also loaded into the encrypted
>> memory. Lets make sure that arch_kexec_post_{alloc,free}_pages()
>> does not clear the memory encryption mask from the kexec area when
>> SEV is active.
> 
> This commit message still doesn't explain the big picture why you want
> this change.
> 

When a virtual machine panic, we also need to dump its memory for analysis.
But, for the SEV virtual machine, the memory is encrypted. To support the
SEV kdump, these changes would be necessary, otherwise, it will not work.

Lets consider the following situations:

[1] How to load the images(kernel and initrd) when SEV is enabled in the
    first kernel?

    Based on the amd-memory-encryption.txt and SEV's patch series, the boot
    images must be encrypted before guest(VM) can be booted(Please see Secure
    Encrypted Virutualization Key Management 'Launching a guest(usage flow)').
    Naturally use the similar way to load the images(kernel and initrd) to the
    crash reserved areas, and these areas are encrypted when SEV is active.

    That is to say, when SEV is active in the first kernel, need to load the
    kernel and initrd to the encrypted areas, so i did the following changes:

    [a] Do not map the kexec area as decrypted when SEV is active.
        Currently, the arch_kexec_post_{alloc,free}_pages() unconditionally
        maps the kexec areas as decrypted. Obviously, for the SEV case, it can
        not work well, need to improve them. Please refer to the first patch
        in this patch series.

    [b] Set the C-bit in the identity map page table when SEV is active.
        Because the second kernel images(kernel and initrd) are loaded to the
        encrypted areas, in order to correctly access these encrypted memory(
        pages), need to set the C-bit in the identity mapping page table when
        kexec builds the identity mapping page table.

[2] How to dump the old memory in the second kernel?

    Here, it is similar to the SME kdump, if SEV was enabled in the first kernel,
    the old memory is also encrypted, the old memory has to be remapped with
    memory encryption mask in order to access it properly.

    [a] The ioremap_encrypted() is still necessary.
        Used to remap the old memory with memory encryption mask.

    [b] Enable dumping encrypted memory when SEV was active.
        Because the whole memory is encrypted in the first kernel when SEV is
        enabled, that is to say, the notes and elfcorehdr are also encrypted,
        and they are also saved to the encrypted memory. Following commit
        992b649a3f01 ("kdump, proc/vmcore: Enable kdumping encrypted memory with
        SME enabled"), both SME and SEV cases need to be considered and modified
        correctly. Please refer to the third patch in this patch series.


Hope this help. Thanks.

> And it must explain it because it might be all clear in your head now
> but months from now, you, we, all would've forgotten why this change was
> needed.
> 
> So pls add blurb that this whole effort is being done so that SEV VMs
> can kdump too. I.e., the 10000ft picture.
> 
> Anyone must be able to figure out *why* a change has been done just by
> doing git archeology. So make sure you explain it properly.
> 
> If unsure, try to put yourself in the shoes of some future kernel
> developer who is trying to find out why this change has been done. Now
> read the commit message you've written. Does it make any sense to him? I
> think not.
> 
> Do you catch my drift?
> 

Yes, understood, thank you.

So sorry for the delay, i am trying my best to explain it in detail.

Thanks.
Lianbo

Re: [PATCH 1/3 v2] x86/kexec: Do not map the kexec area as decrypted when SEV is active

[Borislav Petkov]

On Fri, Apr 26, 2019 at 09:59:54AM +0800, lijiang wrote:
> Hope this help. Thanks.

It does help, yes. When this explanation above is part of the commit
message, it helps immensely!

:-)

> So sorry for the delay, i am trying my best to explain it in detail.

I don't care about the delay as long as the commit messages properly
explain why the change is needed.

So thanks for doing that.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 1/3 v2] x86/kexec: Do not map the kexec area as decrypted when SEV is active

[lijiang]

在 2019年04月26日 21:02, Borislav Petkov 写道:
> On Fri, Apr 26, 2019 at 09:59:54AM +0800, lijiang wrote:
>> Hope this help. Thanks.
> 
> It does help, yes. When this explanation above is part of the commit
> message, it helps immensely!

OK, i will add them to the commit message and post again.

> 
> :-)
> 
>> So sorry for the delay, i am trying my best to explain it in detail.
> 
> I don't care about the delay as long as the commit messages properly
> explain why the change is needed.
> 
> So thanks for doing that.
> 

It's my pleasure. Thanks.

Lianbo

[PATCH 2/3 v2] x86/kexec: Set the C-bit in the identity map page table when SEV is active

[Lianbo Jiang]

When SEV is active, the second kernel image is loaded into the
encrypted memory. Lets make sure that when kexec builds the
identity mapping page table it adds the memory encryption mask(C-bit).

Co-developed-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
---
 arch/x86/kernel/machine_kexec_64.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index f60611531d17..11fe352f7344 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -56,6 +56,7 @@ static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
 	pte_t *pte;
 	unsigned long vaddr, paddr;
 	int result = -ENOMEM;
+	pgprot_t prot = PAGE_KERNEL_EXEC_NOENC;
 
 	vaddr = (unsigned long)relocate_kernel;
 	paddr = __pa(page_address(image->control_code_page)+PAGE_SIZE);
@@ -92,7 +93,11 @@ static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
 		set_pmd(pmd, __pmd(__pa(pte) | _KERNPG_TABLE));
 	}
 	pte = pte_offset_kernel(pmd, vaddr);
-	set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, PAGE_KERNEL_EXEC_NOENC));
+
+	if (sev_active())
+		prot = PAGE_KERNEL_EXEC;
+
+	set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, prot));
 	return 0;
 err:
 	return result;
@@ -129,6 +134,11 @@ static int init_pgtable(struct kimage *image, unsigned long start_pgtable)
 	level4p = (pgd_t *)__va(start_pgtable);
 	clear_page(level4p);
 
+	if (sev_active()) {
+		info.page_flag |= _PAGE_ENC;
+		info.kernpg_flag = _KERNPG_TABLE;
+	}
+
 	if (direct_gbpages)
 		info.direct_gbpages = true;
 
-- 
2.17.1

[PATCH 3/3 v2] kdump,proc/vmcore: Enable kdumping encrypted memory when SEV was active

[Lianbo Jiang]

In the kdump kernel, the memory of first kernel needs to be dumped
into the vmcore file.

It is similar to the SME, if SEV is enabled in the first kernel, the
old memory has to be remapped with memory encryption mask in order to
access it properly. Following commit 992b649a3f01 ("kdump, proc/vmcore:
Enable kdumping encrypted memory with SME enabled") took care of the
SME case but it uses sme_active() which checks for SME only. Lets use
the mem_encrypt_active() which returns true when either of them are
active.

Unlike the SME, the first kernel is loaded into the encrypted memory
when SEV was enabled, hence the kernel elf header must be remapped as
encrypted in order to access it properly.

Co-developed-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
---
 fs/proc/vmcore.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 3fe90443c1bb..cda6c1922e4f 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -165,7 +165,7 @@ void __weak elfcorehdr_free(unsigned long long addr)
  */
 ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
 {
-	return read_from_oldmem(buf, count, ppos, 0, false);
+	return read_from_oldmem(buf, count, ppos, 0, sev_active());
 }
 
 /*
@@ -173,7 +173,7 @@ ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
  */
 ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos)
 {
-	return read_from_oldmem(buf, count, ppos, 0, sme_active());
+	return read_from_oldmem(buf, count, ppos, 0, mem_encrypt_active());
 }
 
 /*
@@ -373,7 +373,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos,
 					    buflen);
 			start = m->paddr + *fpos - m->offset;
 			tmp = read_from_oldmem(buffer, tsz, &start,
-					       userbuf, sme_active());
+					       userbuf, mem_encrypt_active());
 			if (tmp < 0)
 				return tmp;
 			buflen -= tsz;
-- 
2.17.1