kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Catangiu, Adrian Costin" <acatan@amazon.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"Graf (AWS), Alexander" <graf@amazon.de>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"ebiederm@xmission.com" <ebiederm@xmission.com>,
	"rppt@kernel.org" <rppt@kernel.org>,
	"0x7f454c46@gmail.com" <0x7f454c46@gmail.com>,
	"borntraeger@de.ibm.com" <borntraeger@de.ibm.com>,
	"Jason@zx2c4.com" <Jason@zx2c4.com>,
	"jannh@google.com" <jannh@google.com>, "w@1wt.eu" <w@1wt.eu>,
	"MacCarthaigh, Colm" <colmmacc@amazon.com>,
	"luto@kernel.org" <luto@kernel.org>,
	"tytso@mit.edu" <tytso@mit.edu>,
	"ebiggers@kernel.org" <ebiggers@kernel.org>,
	"Woodhouse, David" <dwmw@amazon.co.uk>,
	"bonzini@gnu.org" <bonzini@gnu.org>,
	"Singh, Balbir" <sblbir@amazon.com>,
	"Weiss, Radu" <raduweis@amazon.com>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"mhocko@kernel.org" <mhocko@kernel.org>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"pavel@ucw.cz" <pavel@ucw.cz>,
	"mpe@ellerman.id.au" <mpe@ellerman.id.au>,
	"areber@redhat.com" <areber@redhat.com>,
	"ovzxemul@gmail.com" <ovzxemul@gmail.com>,
	"avagin@gmail.com" <avagin@gmail.com>,
	"ptikhomirov@virtuozzo.com" <ptikhomirov@virtuozzo.com>,
	"gil@azul.com" <gil@azul.com>,
	"asmehra@redhat.com" <asmehra@redhat.com>,
	"dgunigun@redhat.com" <dgunigun@redhat.com>,
	"vijaysun@ca.ibm.com" <vijaysun@ca.ibm.com>,
	"oridgar@gmail.com" <oridgar@gmail.com>,
	"ghammer@redhat.com" <ghammer@redhat.com>
Subject: Re: [PATCH v4 1/2] drivers/misc: sysgenid: add system generation id driver
Date: Thu, 21 Jan 2021 12:48:23 +0000	[thread overview]
Message-ID: <3159F7DB-C72B-4727-9C83-7E7619FC7D98@amazon.com> (raw)
In-Reply-To: <20210112080427-mutt-send-email-mst@kernel.org>

On 12/01/2021, 15:09, "Michael S. Tsirkin" <mst@redhat.com> wrote:

    On Tue, Jan 12, 2021 at 02:15:59PM +0200, Adrian Catangiu wrote:
    > +3) Mapped memory polling simplified example::
    > +
    > +     /*
    > +      * app/library function that provides cached secrets
    > +      */
    > +     char * safe_cached_secret(app_data_t *app)
    > +     {
    > +             char *secret;
    > +             volatile unsigned *const genid_ptr = get_sysgenid_mapping(app);
    > +     again:
    > +             secret = __cached_secret(app);
    > +
    > +             if (unlikely(*genid_ptr != app->cached_genid)) {
    > +                     app->cached_genid = *genid_ptr;
    > +                     barrier();
    > +
    > +                     // rebuild world then confirm the genid update (thru write)
    > +                     rebuild_caches(app);
    > +
    > +                     ack_sysgenid_update(app);
    > +
    > +                     goto again;
    > +             }
    > +
    > +             return secret;

    Hmm. This seems to rely on the assumption that if you have
    read the ID and it did not change, then all is well.

    Problem is, in the interrupt driven implementation
    this is not a safe assumption to make: ID
    from hypervisor might have changed but interrupt
    could be delayed.

    If we map the hypervisor ID to userspace then we won't
    have this race ... worth worry about? why not?

This is a very good point! Unfortunately, there is no immediate solution here.
The current patch-set makes this trade-off in order to gain the genericity of
a system-level generation ID which is not limited to VMs usecases, but can also
be used with things like CRIU.

Directly mapping the vmgenid UUID to userspace was the initial design of this
patch-set (see v1), but it was argued against and evolved into current state.

I would personally rather enhance the HW support (vmgenid device for example)
to also expose a configurable u32 Vm Gen Counter alongside the 128-bit UUID;
and add support in SysGenID to offload the counter to HW when applicable.

The broader view is we need to strike the right balance between a functional,
safe mechanism with today's technology, but also design a future-proof generic

Fixing the race you mentioned in SysGenID only moves the race a bit further up
the stack - you generate the secret race-free but the secret can still be duplicated
in the next layer. To make any mechanism completely safe we need to conceptually
disconnect ourselves from believing that a restored snapshot is immediately available.
There needs to be some entity that moves the restored VM/container/system
from a transient state to "available". That entity can be a process inside the VM,
but it can also be something outside the VM, in the hypervisor or in the stack
surrounding it. That entity is responsible for managing the transition of the VM
or container from transient -> available. It is responsible for not allowing the VM/
container to be used or usable until that transition is complete.

In the first generations of VM clones and CRIU production deployments, I expect
this entity to be a stack-specific component with intimate knowledge of the system
components, transient states, lifecycle, etc. Which this patch-set enables.


Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.

  reply	other threads:[~2021-01-21 12:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-12 12:15 [PATCH v4 0/2] System Generation ID driver and VMGENID backend Adrian Catangiu
2021-01-12 12:15 ` [PATCH v4 1/2] drivers/misc: sysgenid: add system generation id driver Adrian Catangiu
2021-01-12 13:08   ` Greg KH
2021-01-21  9:54     ` Catangiu, Adrian Costin
2021-01-12 13:08   ` Michael S. Tsirkin
2021-01-21 12:48     ` Catangiu, Adrian Costin [this message]
2021-01-19 22:34   ` Randy Dunlap
2021-01-27 22:15   ` Pavel Machek
2021-02-02 14:32     ` Michael S. Tsirkin
2021-01-12 12:16 ` [PATCH v4 2/2] drivers/virt: vmgenid: add vm " Adrian Catangiu
2021-01-12 12:48 ` [PATCH v4 0/2] System Generation ID driver and VMGENID backend Michael S. Tsirkin
2021-01-21 10:28   ` Catangiu, Adrian Costin
2021-01-27 12:47     ` Michael S. Tsirkin
2021-01-28 12:58       ` Alexander Graf
2021-02-02 14:34         ` Michael S. Tsirkin

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:

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

  git send-email \
    --in-reply-to=3159F7DB-C72B-4727-9C83-7E7619FC7D98@amazon.com \
    --to=acatan@amazon.com \
    --cc=0x7f454c46@gmail.com \
    --cc=Jason@zx2c4.com \
    --cc=areber@redhat.com \
    --cc=arnd@arndb.de \
    --cc=asmehra@redhat.com \
    --cc=avagin@gmail.com \
    --cc=bonzini@gnu.org \
    --cc=borntraeger@de.ibm.com \
    --cc=colmmacc@amazon.com \
    --cc=corbet@lwn.net \
    --cc=dgunigun@redhat.com \
    --cc=dwmw@amazon.co.uk \
    --cc=ebiederm@xmission.com \
    --cc=ebiggers@kernel.org \
    --cc=ghammer@redhat.com \
    --cc=gil@azul.com \
    --cc=graf@amazon.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@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=ovzxemul@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=ptikhomirov@virtuozzo.com \
    --cc=qemu-devel@nongnu.org \
    --cc=raduweis@amazon.com \
    --cc=rafael@kernel.org \
    --cc=rppt@kernel.org \
    --cc=sblbir@amazon.com \
    --cc=tytso@mit.edu \
    --cc=vijaysun@ca.ibm.com \
    --cc=w@1wt.eu \


* 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).