Date:   Fri, 26 Apr 2019 09:59:54 +0800
From:   lijiang <lijiang@...hat.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-kernel@...r.kernel.org, kexec@...ts.infradead.org,
        tglx@...utronix.de, mingo@...hat.com, akpm@...ux-foundation.org,
        x86@...nel.org, hpa@...or.com, dyoung@...hat.com, bhe@...hat.com,
        Thomas.Lendacky@....com, brijesh.singh@....com
Subject: Re: [PATCH 1/3 v2] x86/kexec: Do not map the kexec area as decrypted
 when SEV is active

在 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

Powered by blists - more mailing lists