Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <cover.1581555616.git.ashish.kalra@amd.com>
In-Reply-To: <cover.1581555616.git.ashish.kalra@amd.com>
From:   Andy Lutomirski <luto@kernel.org>
Date:   Wed, 12 Feb 2020 21:43:41 -0800
Message-ID: <CALCETrXE9cWd3TbBZMsAwmSwWpDYFsicLZ=amHLWsvE0burQSw@mail.gmail.com>
Subject: Re: [PATCH 00/12] SEV Live Migration Patchset.
To:     Ashish Kalra <Ashish.Kalra@amd.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Radim Krcmar <rkrcmar@redhat.com>,
        Joerg Roedel <joro@8bytes.org>, Borislav Petkov <bp@suse.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        David Rientjes <rientjes@google.com>, X86 ML <x86@kernel.org>,
        kvm list <kvm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Dave Hansen <dave.hansen@intel.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Feb 12, 2020 at 5:14 PM Ashish Kalra <Ashish.Kalra@amd.com> wrote:
>
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> This patchset adds support for SEV Live Migration on KVM/QEMU.

I skimmed this all and I don't see any description of how this all works.

Does any of this address the mess in svm_register_enc_region()?  Right
now, when QEMU (or a QEMU alternative) wants to allocate some memory
to be used for guest encrypted pages, it mmap()s some memory and the
kernel does get_user_pages_fast() on it.  The pages are kept pinned
for the lifetime of the mapping.  This is not at all okay.  Let's see:

 - The memory is pinned and it doesn't play well with the Linux memory
management code.  You just wrote a big patch set to migrate the pages
to a whole different machines, but we apparently can't even migrate
them to a different NUMA node or even just a different address.  And
good luck swapping it out.

 - The memory is still mapped in the QEMU process, and that mapping is
incoherent with actual guest access to the memory.  It's nice that KVM
clflushes it so that, in principle, everything might actually work,
but this is gross.  We should not be exposing incoherent mappings to
userspace.

Perhaps all this fancy infrastructure you're writing for migration and
all this new API surface could also teach the kernel how to migrate
pages from a guest *to the same guest* so we don't need to pin pages
forever.  And perhaps you could put some thought into how to improve
the API so that it doesn't involve nonsensical incoherent mappings.

(To be blunt: if I had noticed how the SEV code worked before it was
merged, I would have NAKed it.  It's too late now to retroactively
remove it from the kernel, but perhaps we could try not to pile more
complexity on top of the unfortunate foundation we have.)