Re: [PATCH v3] x86/sev: Make enc_dec_hypercall() accept a size instead of npages

[Ben Hillier]

On Thu, Aug 24, 2023 at 3:37 PM Steve Rutherford <srutherford@xxxxxxxxxx> wrote:
>
> enc_dec_hypercall() accepted a page count instead of a size, which
> forced its callers to round up. As a result, non-page aligned
> vaddrs caused pages to be spuriously marked as decrypted via the
> encryption status hypercall, which in turn caused consistent
> corruption of pages during live migration. Live migration requires
> accurate encryption status information to avoid migrating pages
> from the wrong perspective.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 064ce6c550a0 ("mm: x86: Invoke hypercall when page encryption status is changed")
> Signed-off-by: Steve Rutherford <srutherford@xxxxxxxxxx>
> Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Reviewed-by: Pankaj Gupta <pankaj.gupta@xxxxxxx>
Ran test comparing the c-bit status in the guest page tables to the
host perspective. Before the patch, there was a c-bit status mismatch.
Adding the patch fixed these mismatched c-bits.
Tested-by: Ben Hillier <bhillier@xxxxxxxxxx>
> ---
>  arch/x86/include/asm/mem_encrypt.h |  6 +++---
>  arch/x86/kernel/kvm.c              |  4 +---
>  arch/x86/mm/mem_encrypt_amd.c      | 13 ++++++-------
>  3 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 7f97a8a97e24..473b16d73b47 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -50,8 +50,8 @@ void __init sme_enable(struct boot_params *bp);
>
>  int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
>  int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
> -void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages,
> -                                           bool enc);
> +void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr,
> +                                           unsigned long size, bool enc);
>
>  void __init mem_encrypt_free_decrypted_mem(void);
>
> @@ -85,7 +85,7 @@ early_set_memory_decrypted(unsigned long vaddr, unsigned long size) { return 0;
>  static inline int __init
>  early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0; }
>  static inline void __init
> -early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages, bool enc) {}
> +early_set_mem_enc_dec_hypercall(unsigned long vaddr, unsigned long size, bool enc) {}
>
>  static inline void mem_encrypt_free_decrypted_mem(void) { }
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 6a36db4f79fd..b8ab9ee5896c 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -966,10 +966,8 @@ static void __init kvm_init_platform(void)
>                  * Ensure that _bss_decrypted section is marked as decrypted in the
>                  * shared pages list.
>                  */
> -               nr_pages = DIV_ROUND_UP(__end_bss_decrypted - __start_bss_decrypted,
> -                                       PAGE_SIZE);
>                 early_set_mem_enc_dec_hypercall((unsigned long)__start_bss_decrypted,
> -                                               nr_pages, 0);
> +                                               __end_bss_decrypted - __start_bss_decrypted, 0);
>
>                 /*
>                  * If not booted using EFI, enable Live migration support.
> diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
> index 54bbd5163e8d..6faea41e99b6 100644
> --- a/arch/x86/mm/mem_encrypt_amd.c
> +++ b/arch/x86/mm/mem_encrypt_amd.c
> @@ -288,11 +288,10 @@ static bool amd_enc_cache_flush_required(void)
>         return !cpu_feature_enabled(X86_FEATURE_SME_COHERENT);
>  }
>
> -static void enc_dec_hypercall(unsigned long vaddr, int npages, bool enc)
> +static void enc_dec_hypercall(unsigned long vaddr, unsigned long size, bool enc)
>  {
>  #ifdef CONFIG_PARAVIRT
> -       unsigned long sz = npages << PAGE_SHIFT;
> -       unsigned long vaddr_end = vaddr + sz;
> +       unsigned long vaddr_end = vaddr + size;
>
>         while (vaddr < vaddr_end) {
>                 int psize, pmask, level;
> @@ -342,7 +341,7 @@ static bool amd_enc_status_change_finish(unsigned long vaddr, int npages, bool e
>                 snp_set_memory_private(vaddr, npages);
>
>         if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
> -               enc_dec_hypercall(vaddr, npages, enc);
> +               enc_dec_hypercall(vaddr, npages << PAGE_SHIFT, enc);
>
>         return true;
>  }
> @@ -466,7 +465,7 @@ static int __init early_set_memory_enc_dec(unsigned long vaddr,
>
>         ret = 0;
>
> -       early_set_mem_enc_dec_hypercall(start, PAGE_ALIGN(size) >> PAGE_SHIFT, enc);
> +       early_set_mem_enc_dec_hypercall(start, size, enc);
>  out:
>         __flush_tlb_all();
>         return ret;
> @@ -482,9 +481,9 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
>         return early_set_memory_enc_dec(vaddr, size, true);
>  }
>
> -void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages, bool enc)
> +void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, unsigned long size, bool enc)
>  {
> -       enc_dec_hypercall(vaddr, npages, enc);
> +       enc_dec_hypercall(vaddr, size, enc);
>  }
>
>  void __init sme_early_init(void)
> --
> 2.42.0.rc1.204.g551eb34607-goog
>