qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Graf <graf@amazon.de>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>, Jann Horn <jannh@google.com>
Cc: Willy Tarreau <w@1wt.eu>, Colm MacCarthaigh <colmmacc@amazon.com>,
	"Catangiu, Adrian Costin" <acatan@amazon.com>,
	Andy Lutomirski <luto@kernel.org>,
	"Theodore Y. Ts'o" <tytso@mit.edu>,
	Eric Biggers <ebiggers@kernel.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	 kernel list <linux-kernel@vger.kernel.org>,
	"open list:VIRTIO GPU DRIVER"
	<virtualization@lists.linux-foundation.org>,
	"Woodhouse, David" <dwmw@amazon.co.uk>, <bonzini@gnu.org>,
	"Singh, Balbir" <sblbir@amazon.com>,
	"Weiss, Radu" <raduweis@amazon.com>, <oridgar@gmail.com>,
	<ghammer@redhat.com>, Jonathan Corbet <corbet@lwn.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Qemu Developers" <qemu-devel@nongnu.org>,
	KVM list <kvm@vger.kernel.org>,
	"Michal Hocko" <mhocko@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Pavel Machek" <pavel@ucw.cz>,
	Linux API <linux-api@vger.kernel.org>,
	"Christian Borntraeger" <borntraeger@de.ibm.com>,
	<mpe@ellerman.id.au>
Subject: Re: [PATCH] drivers/virt: vmgenid: add vm generation id driver
Date: Sat, 17 Oct 2020 20:09:06 +0200	[thread overview]
Message-ID: <aacdff7a-2af1-4f46-6ab2-2a9d5b865d35@amazon.de> (raw)
In-Reply-To: <CAHmME9qHGSF8w3DoyCP+ud_N0MAJ5_8zsUWx=rxQB1mFnGcu9w@mail.gmail.com>

Hi Jason,

On 17.10.20 15:24, Jason A. Donenfeld wrote:
> 
> After discussing this offline with Jann a bit, I have a few general
> comments on the design of this.
> 
> First, the UUID communicated by the hypervisor should be consumed by
> the kernel -- added as another input to the rng -- and then userspace

We definitely want a kernel internal notifier as well, yes :).

> should be notified that it should reseed any userspace RNGs that it
> may have, without actually communicating that UUID to userspace. IOW,

I also tend to agree that it makes sense to disconnect the actual UUID 
we receive from the notification to user space. This would allow us to 
create a generic mechanism for VM save/restore cycles across different 
hypervisors. Let me add PPC and s390x people to the CC list to see 
whether they have anything remotely similar to the VmGenID mechanism. 
For x86 and aarch64, the ACPI and memory based VmGenID implemented here 
is the most obvious option to implement IMHO. It's also already 
implemented in all major hypervisors.

> I agree with Jann there. Then, it's the functioning of this
> notification mechanism to userspace that is interesting to me.

Absolutely! Please have a look at the previous discussion here:

 
https://lore.kernel.org/linux-pm/B7793B7A-3660-4769-9B9A-FFCF250728BB@amazon.com/

The user space interface is absolutely what this is about.

> There are a few design goals of notifying userspace: it should be
> fast, because people who are using userspace RNGs are usually doing so
> in the first place to completely avoid syscall overhead for whatever
> high performance application they have - e.g. I recall conversations
> with Colm about his TLS implementation needing to make random IVs
> _really_ fast. It should also happen as early as possible, with no
> race or as minimal as possible race window, so that userspace doesn't
> begin using old randomness and then switch over after the damage is
> already done.

There are multiple facets and different types of consumers here. For a 
user space RNG, I agree that fast and as race free as possible is key. 
That's what the mmap interface is there for.

There are applications way beyond that though. What do you do with 
applications that already consumed randomness? For example a cached pool 
of SSL keys. Or a higher level language primitive that consumes 
randomness and caches its seed somewhere in an internal data structure. 
Or even worse: your system's host ssh key.

For those types of events, an mmap (or vDSO) interface does not work. We 
need to actively allow user space applications to readjust to the new 
environment - either internally (the language primitive case) or through 
a system event, maybe even as systemd trigger (the ssh host key case).

To give everyone enough time before we consider a system as "updated to 
the new environment", we have the callback logic with the "Orchestrator" 
that can check whether all listeners to system wide updates confirms 
they adjusted themselves.

That's what the rest of the logic is there for: A read+poll interface 
and all of the orchestration logic. It's not for the user space RNG 
case, it's for all of its downstream users.

> I'm also not wedded to using Microsoft's proprietary hypervisor design
> for this. If we come up with a better interface, I don't think it's
> asking too much to implement that and reasonably expect for Microsoft
> to catch up. Maybe someone here will find that controversial, but
> whatever -- discussing ideal designs does not seem out of place or
> inappropriate for how we usually approach things in the kernel, and a
> closed source hypervisor coming along shouldn't disrupt that.

The main bonus point on this interface is that Hyper-V, VMware and QEMU 
implement it already. It would be a very natural for into the ecosystem. 
I agree though that we shouldn't have our user space interface 
necessarily dictated by it: Other hypervisors may implement different 
ways such as a simple edge IRQ that gets triggered whenever the VM gets 
resumed.

> So, anyway, here are a few options with some pros and cons for the
> kernel notifying userspace that its RNG should reseed.

I can only stress again that we should not be laser focused on the RNG 
case. In a lot of cases, data has already been generated by the RNG 
before the snapshot and needs to be reinitialized after the snapshot. In 
other cases such as system UUIDs, it's completely orthogonal to the RNG.

> 
> 1. SIGRND - a new signal. Lol.

Doable, but a lot of plumbing in user space. It's also not necessarily a 
good for for event notification in most user space applications.

> 
> 2. Userspace opens a file descriptor that it can epoll on. Pros are
> that many notification mechanisms already use this. Cons is that this
> requires syscall and might be more racy than we want. Another con is
> that this a new thing for userspace programs to do.

That's part of what this patch does, right? This patch implements 
read+poll as well as mmap() for high speed reads.

> 3. We stick an atomic counter in the vDSO, Jann's suggestion. Pros are
> that this is extremely fast, and also simple to use and implement.
> There are enough sequence points in typical crypto programs that
> checking to see whether this counter has changed before doing whatever
> operation seems easy enough. Cons are that typically we've been
> conservative about adding things to the vDSO, and this is also a new
> thing for userspace programs to do.

The big con is that its use is going to be super limited to applications 
that can be adapted to check their "vm generation" through a vDSO call / 
read every time they consume data that may potentially need to be 
regenerated.

This probably works for the pure RNG case. It falls apart for more 
sophisticated things such as "redo my ssh host keys and restart the 
service" or "regenerate my samba machine uuid".

> 4. We already have a mechanism for this kind of thing, because the
> same issue comes up when fork()ing. The solution was MADV_WIPEONFORK,
> where userspace marks a page to be zeroed when forking, for the
> purposes of the RNG being notified when its world gets split in two.
> This is basically the same thing as we're discussing here with guest
> snapshots, except it's on the system level rather than the process
> level, and a system has many processes. But the problem space is still
> almost the same, and we could simply reuse that same mechanism. There
> are a few implementation strategies for that:

Yup, that's where we started from :). And then we ran into resistance by 
the mm people (on CC here). And then we looked at the problem more in 
depth and checked what it would take to for example implement this for 
user space RNGs in Java. It's ... more complicated than one may think at 
first.

> 4a. We mess with the PTEs of all processes' pages that are
> MADV_WIPEONFORK, like fork does now, when the hypervisor notifies us
> to do so. Then we wind up reusing the already existing logic for
> userspace RNGs. Cons might be that this usually requires semaphores,
> and we're in irq context, so we'd have to hoist to a workqueue, which
> means either more wake up latency, or a larger race window.
> 
> 4b. We just memzero all processes' pages that are MADV_WIPEONFORK,
> when the hypervisor notifies us to do so. Then we wind up reusing the
> already existing logic for userspace RNGs.
> 
> 4c. The guest kernel maintains an array of physical addresses that are
> MADV_WIPEONFORK. The hypervisor knows about this array and its
> location through whatever protocol, and before resuming a
> moved/snapshotted/duplicated VM, it takes the responsibility for
> memzeroing this memory. The huge pro here would be that this
> eliminates all races, and reduces complexity quite a bit, because the
> hypervisor can perfectly synchronize its bringup (and SMP bringup)
> with this, and it can even optimize things like on-disk memory
> snapshots to simply not write out those pages to disk.
> 
> A 4c-like approach seems like it'd be a lot of bang for the buck -- we
> reuse the existing mechanism (MADV_WIPEONFORK), so there's no new
> userspace API to deal with, and it'd be race free, and eliminate a lot
> of kernel complexity.
> 
> But 4b and 3 don't seem too bad either.
> 
> Any thoughts on 4c? Is that utterly insane, or does that actually get
> us somewhere close to what we want?

All of the options for "4" are possible and have an RFC out. Please 
check out the discussion linked above :).

The problem with anything that relies on close loop reads (options 3+4) 
is not going to work well with the more sophisticated use case of 
derived data.

IMHO it will boil down to "both". We will need a high-speed interface 
that with close-to-0 overhead tells you either the generation ID or 
clears pages (options 3+4) as well as something that is bigger for 
applications that can either intrinsically (sshd) or by system design 
(Java) not adopt the mechanisms above easily.

That said, we need to start somewhere. I don't mind which angle we start 
from. But this is a real world problem and one that will only become 
more prevalent over time as VMs are used for more than only your 
traditional enterprise hardware consolidation.


Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879



  parent reply	other threads:[~2020-10-17 18:36 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <AQHWo8lIfZnFKGe8nkGmhTCXwq5R3w==>
2020-10-16 14:33 ` [PATCH] drivers/virt: vmgenid: add vm generation id driver Catangiu, Adrian Costin
2020-10-16 15:00   ` Catangiu, Adrian Costin
2020-10-16 15:14   ` gregkh
2020-10-17  1:40   ` Jann Horn
2020-10-17  3:36     ` Willy Tarreau
2020-10-17  4:02       ` Jann Horn
2020-10-17  4:34         ` Colm MacCarthaigh
2020-10-17  5:01           ` Jann Horn
2020-10-17  5:29             ` Colm MacCarthaigh
2020-10-17  5:37             ` Willy Tarreau
2020-10-17  5:52               ` Jann Horn
2020-10-17  6:44                 ` Willy Tarreau
2020-10-17  6:55                   ` Jann Horn
2020-10-17  7:17                     ` Willy Tarreau
2020-10-17 13:24                     ` Jason A. Donenfeld
2020-10-17 18:06                       ` Catangiu, Adrian Costin
2020-10-17 18:09                       ` Alexander Graf [this message]
2020-10-18  2:08                         ` Jann Horn
2020-10-20  9:35                         ` Christian Borntraeger
2020-10-20  9:54                           ` Alexander Graf
2020-10-20 16:54                         ` Catangiu, Adrian Costin
2020-10-18  3:14                       ` Colm MacCarthaigh
2020-10-18 15:52                       ` Michael S. Tsirkin
2020-10-18 15:54                         ` Andy Lutomirski
2020-10-18 15:59                           ` Michael S. Tsirkin
2020-10-18 16:14                             ` Andy Lutomirski
2020-10-19 15:00                               ` Michael S. Tsirkin
2020-10-17 18:10     ` Andy Lutomirski
2020-10-19 17:15       ` Mathieu Desnoyers
2020-10-20 10:00         ` Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aacdff7a-2af1-4f46-6ab2-2a9d5b865d35@amazon.de \
    --to=graf@amazon.de \
    --cc=Jason@zx2c4.com \
    --cc=acatan@amazon.com \
    --cc=bonzini@gnu.org \
    --cc=borntraeger@de.ibm.com \
    --cc=colmmacc@amazon.com \
    --cc=corbet@lwn.net \
    --cc=dwmw@amazon.co.uk \
    --cc=ebiggers@kernel.org \
    --cc=ghammer@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=mst@redhat.com \
    --cc=oridgar@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=qemu-devel@nongnu.org \
    --cc=raduweis@amazon.com \
    --cc=rafael@kernel.org \
    --cc=sblbir@amazon.com \
    --cc=tytso@mit.edu \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).