[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/8] domain: GADDR based shared guest area registration alternative - teardown
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 27 Sep 2023 12:42:11 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=SRi5VjQgfxrYjczCVYIBrMc33L8M0BLiOCDQ6a1hbKc=; b=JUrCcm+TTXFchOHe8zdogbGBXokrdnwXPV1lWhrZ90IC411RiLbCx54ip7sDBvn/9lwDnFmILWdaHm8AqKK1pLBQWo0HHj6d58nlK85iDJFLofwnsxc4f4jt9RnjSyhghaHP63iK1wf4sAFkKl2TwOV5pQZs0Pau27NliEEjNvq9GwVhEJZ45klPfTMYZvJF9kAdSnVwQccYEZTO/o4X3hTJsQbR64S5nP7w5Xtjw6d33xW0+z9ztI7pFf54XOOkbPBRE3OMp0BXnwDdOdzgj5X0dKo2h6c+uJoZzukSKZesjiZtrtZn31+EdNfrMIzEdiYqIcBfvpfiLOWTuA3O0g==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=B68sFV+rlF170qPeoy/8TBvAQm/dboRJbXNdIbSfllkh/7Eu5rxQy53T2LA/8e4+DdZlxwrTpNNfNxiZwe2iZ3JtUvFb2fP7+hqo+FHGz/3yP0uX9upk4W5BFG2fvxkoJlQjVtDfAVTMQudG+YCn4DHazprH7DS5DMmP/S+alWX/bJ7FHTx4z1qRurLTyNI/E8FJhOCBZ2ZYFVMCZJgxTz29Jklt3DCzqEEUSXcyGOghWgmsxWJEdqwtxJvzG+b7u0y/Z9EUHxxAz9RMWri03t547BqL+meNLkOwVm1Wuy8NX3dL7edRQZxLRPWXqIJASMU83a9sZhfrfSqhdHw2rw==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
- Delivery-date: Wed, 27 Sep 2023 10:42:31 +0000
- Ironport-data: A9a23:Lqwy064mQLg0PE8j5GZ0wAxRtPbGchMFZxGqfqrLsTDasY5as4F+v jZLXGuHaPmCYWameNpxb9+y9xgF7JHdm9JmSQdprCkwHi5G8cbLO4+Ufxz6V8+wwm8vb2o8t plDNYOQRCwQZiWBzvt4GuG59RGQ7YnRG/ykTraCY3gtLeNdYH9JoQp5nOIkiZJfj9G8Agec0 fv/uMSaM1K+s9JOGjt8B5mr9lU355wehBtC5gZlPaER4weH/5UoJMl3yZ+ZfiOQrrZ8RoZWd 86bpJml82XQ+QsaC9/Nut4XpWVTH9Y+lSDX4pZnc/DKbipq/0Te4Y5iXBYoUm9Fii3hojxE4 I4lWapc6+seFvakdOw1C3G0GszlVEFM0OevzXOX6aR/w6BaGpdFLjoH4EweZOUlFuhL7W5m9 8JJGQsdT1O6m+OV0ev4V+gzufsxI5y+VG8fkikIITDxK98DGMqGb4CUoNhS0XE3m9xEGuvYa 4wBcz1zYR/cYhpJfFAKFJY5m+TujX76G9FagAvN+exrvC6OnUooj+WF3Nn9I7RmQe1PmUmVv CTe9nnRCRAGLt2PjzGC9xpAg8eWx32lCNJOTO3QGvhCrE2S3khNLRsqWFriqN+L0RHnW/ZWA hlBksYphe1onKCxdfH/VRClpH+PvjYHRsFdVeY97Wml2qfSpgqUGGUAZjpAc8A98t87QyQw0 V2ElM+vAiZg2JWXVHac+7G8vT60fy8PIgcqfjQYRAEI593ipoAbjR/VSNtnVqmvgbXdBjXY0 z2M6i8kiN0uYdUj0qy6+RXLhmyqr52QFAotvFyIAySi8x9zY5Oja8qw81/H4P1cLYGfCF6co HwDnMvY5+cLZX2QqBGwrCw2NOnBz5643Pf02DaDw7FJG+yRxkOe
- Ironport-hdrordr: A9a23:aLnUj66M4N0M/4tShAPXwNvXdLJyesId70hD6qkRc203TiX2ra qTdZgguCMc6wxwZJhDo7690cC7KBu2yXcS2+Us1NyZPTUO1lHGEGmnhrGSpgEJ3EbFh4xg6Z s=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Wed, Sep 27, 2023 at 11:55:19AM +0200, Jan Beulich wrote:
> On 27.09.2023 10:51, Roger Pau Monné wrote:
> > On Wed, May 03, 2023 at 05:54:47PM +0200, Jan Beulich wrote:
> > I guess we should also zap the runstate handlers, or else we might
> > corrupt guest state.
>
> So you think the guest might re-register a different area post resume?
> I can certainly add another patch to zap the handles; I don't think it
> should be done right here, not the least because if we see room for
> such behavior, that change may even want backporting.
For correctness it would be good to zap them, but there's no rush, as
I do think guests will set to the same address on resume.
> >> @@ -1568,6 +1572,19 @@ void unmap_vcpu_info(struct vcpu *v)
> >> put_page_and_type(mfn_to_page(mfn));
> >> }
> >>
> >> +/*
> >> + * This is only intended to be used for domain cleanup (or more generally
> >> only
> >> + * with at least the respective vCPU, if it's not the current one,
> >> reliably
> >> + * paused).
> >> + */
> >> +void unmap_guest_area(struct vcpu *v, struct guest_area *area)
> >
> > vcpu param could be const given the current code, but I assume this
> > will be changed by future patches. Same could be said about
> > guest_area but I'm confident that will change.
>
> True for both.
>
> >> +{
> >> + struct domain *d = v->domain;
> >> +
> >> + if ( v != current )
> >> + ASSERT(atomic_read(&v->pause_count) |
> >> atomic_read(&d->pause_count));
> >
> > Isn't this racy?
>
> It is, yes.
>
> > What guarantees that the vcpu won't be kicked just
> > after the check has been performed?
>
> Nothing. This check isn't any better than assertions towards an ordinary
> spinlock being held. I assume you realize that we've got a number of such
> assertions elsewhere already.
Right, but different from spinlock assertions, the code here could be
made safe just by pausing the vCPU?
Thanks, Roger.
|
