On Fri, Sep 07, 2018 at 12:57:28PM -0500, Brijesh Singh wrote:
> kvmclock defines few static variables which are shared with the
> hypervisor during the kvmclock initialization.
>
> When SEV is active, memory is encrypted with a guest-specific key, and
> if guest OS wants to share the memory region with hypervisor then it must
"... if *the* guest OS ... with *the* hypervisor ..."
> clear the C-bit before sharing it.
<---- newline here.
> Currently, we use
> kernel_physical_mapping_init() to split large pages before clearing the
> C-bit on shared pages. But it fails when called from the kvmclock
> initialization (mainly because memblock allocator is not ready that early
... because *the* memblock allocator...
hmm, those definite articles are kinda all lost... :)
> during boot).
>
> Add a __decrypted section attribute which can be used when defining
> such shared variable. The so-defined variables will be placed in the
> .data..decrypted section. This section is mapped with C=0 early
> during boot, we also ensure that the initialized values are updated
> to match with C=0 (i.e perform an in-place decryption). The
> .data..decrypted section is PMD-aligned and sized so that we avoid
> the need to split the large pages when mapping the section.
>
> The sme_encrypt_kernel() was used to perform the in-place encryption
Here, of all things, you don't need a definite article :)
"sme_encrypt_kernel() performs the in-place encryption...
> of the Linux kernel and initrd when SME is active. The routine has been
> enhanced to decrypt the .data..decrypted section for both SME and SEV
> cases.
Otherwise, that's a much better commit message!
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> Reviewed-by: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: k...@vger.kernel.org
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: Borislav Petkov <b...@suse.de>
> Cc: "H. Peter Anvin" <h...@zytor.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: Paolo Bonzini <pbonz...@redhat.com>
> Cc: Sean Christopherson <sean.j.christopher...@intel.com>
> Cc: k...@vger.kernel.org
> Cc: "Radim Krčmář" <rkrc...@redhat.com>
> ---
> arch/x86/include/asm/mem_encrypt.h | 6 +++
> arch/x86/kernel/head64.c | 11 +++++
> arch/x86/kernel/vmlinux.lds.S | 17 +++++++
> arch/x86/mm/mem_encrypt_identity.c | 94
> ++++++++++++++++++++++++++++++++------
> 4 files changed, 113 insertions(+), 15 deletions(-)
...
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 8bde0a4..4cb1064 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -65,6 +65,21 @@ jiffies_64 = jiffies;
> #define ALIGN_ENTRY_TEXT_BEGIN . = ALIGN(PMD_SIZE);
> #define ALIGN_ENTRY_TEXT_END . = ALIGN(PMD_SIZE);
>
> +/*
> + * This section contains data which will be mapped as decrypted. Memory
> + * encryption operates on a page basis. Make this section PMD-aligned
> + * to avoid spliting the pages while mapping the section early.
"splitting" - damn, that spell checker of yours is still not working... ;-\
> + *
> + * Note: We use a separate section so that only this section gets
> + * decrypted to avoid exposing more than we wish.
> + */
> +#define DATA_DECRYPTED \
> + . = ALIGN(PMD_SIZE); \
> + __start_data_decrypted = .; \
> + *(.data..decrypted); \
> + . = ALIGN(PMD_SIZE); \
> + __end_data_decrypted = .; \
> +
> #else
>
> #define X86_ALIGN_RODATA_BEGIN
...
> @@ -487,28 +510,69 @@ static void __init teardown_workarea_map(struct
> sme_workarea_data *wa,
> native_write_cr3(__native_read_cr3());
> }
>
> +static void __init decrypt_shared_data(struct sme_workarea_data *wa,
> + struct sme_populate_pgd_data *ppd)
> +{
> + unsigned long decrypted_start, decrypted_end, decrypted_len;
> +
> + /* Physical addresses of decrypted data section */
> + decrypted_start = __pa_symbol(__start_data_decrypted);
> + decrypted_end = ALIGN(__pa_symbol(__end_data_decrypted), PMD_PAGE_SIZE);
Why?
You have:
+ . = ALIGN(PMD_SIZE); \
+ __end_data_decrypted = .; \
It is already aligned by definition?!?
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
--
