From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A6770C4361B for ; Wed, 9 Dec 2020 21:33:04 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 504BB23A02 for ; Wed, 9 Dec 2020 21:33:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 504BB23A02 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.48784.86290 (Exim 4.92) (envelope-from ) id 1kn74s-0005yX-6q; Wed, 09 Dec 2020 21:32:50 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 48784.86290; Wed, 09 Dec 2020 21:32:50 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kn74s-0005yQ-3d; Wed, 09 Dec 2020 21:32:50 +0000 Received: by outflank-mailman (input) for mailman id 48784; Wed, 09 Dec 2020 21:32:48 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kn74q-0005yL-PI for xen-devel@lists.xenproject.org; Wed, 09 Dec 2020 21:32:48 +0000 Received: from mail.kernel.org (unknown [198.145.29.99]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 9bca2c42-c755-443d-9530-0ffb5b08eeb9; Wed, 09 Dec 2020 21:32:48 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 9bca2c42-c755-443d-9530-0ffb5b08eeb9 Date: Wed, 9 Dec 2020 13:32:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1607549567; bh=5CVtPUFtiMxACabaGtBvf6XMIWIWvn7TVRc7ElyN5X8=; h=From:To:cc:Subject:In-Reply-To:References:From; b=fQZQiimmrHmRpECbf3+ldOiRcQMOI5BAEOnbMUsx5eB/c/XYhcpQORv92uS3bA0Wz 5Vx1WF7sflBexLwWIribrYfG7fR+ai3YB5QApxDDvwWVqKpNsMogAWaoexRqLNWAXL IX475PS7tLXCRCJ3A/V6i5SuyCPcIE+uQizdSfhzdSqS5xrHtsUIppV/Z8sNZkmj4A vkiX8XON7B0BrOOb3vjJK0Rwgj1lvVLjlj4ebiJJCzG8pe6NkJ5Tr3KmJxJQXCJXW6 yHKrzo+1kwoD+Ezj6Xr/R9UgKDlB5xUT7DIxXuGLenrATeSBUT01akhF/iof7VIkwA 3ipsRT5cxDIuA== From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-T480s To: Oleksandr Tyshchenko cc: xen-devel@lists.xenproject.org, Oleksandr Tyshchenko , Stefano Stabellini , Julien Grall , Volodymyr Babchuk , Paul Durrant , Julien Grall Subject: Re: [PATCH V3 13/23] xen/ioreq: Use guest_cmpxchg64() instead of cmpxchg() In-Reply-To: <1606732298-22107-14-git-send-email-olekstysh@gmail.com> Message-ID: References: <1606732298-22107-1-git-send-email-olekstysh@gmail.com> <1606732298-22107-14-git-send-email-olekstysh@gmail.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Mon, 30 Nov 2020, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko > > The cmpxchg() in ioreq_send_buffered() operates on memory shared > with the emulator domain (and the target domain if the legacy > interface is used). > > In order to be on the safe side we need to switch > to guest_cmpxchg64() to prevent a domain to DoS Xen on Arm. > > As there is no plan to support the legacy interface on Arm, > we will have a page to be mapped in a single domain at the time, > so we can use s->emulator in guest_cmpxchg64() safely. > > Thankfully the only user of the legacy interface is x86 so far > and there is not concern regarding the atomics operations. > > Please note, that the legacy interface *must* not be used on Arm > without revisiting the code. > > Signed-off-by: Oleksandr Tyshchenko > CC: Julien Grall > > --- > Please note, this is a split/cleanup/hardening of Julien's PoC: > "Add support for Guest IO forwarding to a device emulator" > > Changes RFC -> V1: > - new patch > > Changes V1 -> V2: > - move earlier to avoid breaking arm32 compilation > - add an explanation to commit description and hvm_allow_set_param() > - pass s->emulator > > Changes V2 -> V3: > - update patch description > --- > --- > xen/arch/arm/hvm.c | 4 ++++ > xen/common/ioreq.c | 3 ++- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c > index 8951b34..9694e5a 100644 > --- a/xen/arch/arm/hvm.c > +++ b/xen/arch/arm/hvm.c > @@ -31,6 +31,10 @@ > > #include > > +/* > + * The legacy interface (which involves magic IOREQ pages) *must* not be used > + * without revisiting the code. > + */ This is a NIT, but I'd prefer if you moved the comment a few lines below, maybe just before the existing comment starting with "The following parameters". The reason is that as it is now it is not clear which set_params interfaces should not be used without revisiting the code. With that: Acked-by: Stefano Stabellini > static int hvm_allow_set_param(const struct domain *d, unsigned int param) > { > switch ( param ) > diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c > index 3ca5b96..4855dd8 100644 > --- a/xen/common/ioreq.c > +++ b/xen/common/ioreq.c > @@ -29,6 +29,7 @@ > #include > #include > > +#include > #include > > #include > @@ -1182,7 +1183,7 @@ static int ioreq_send_buffered(struct ioreq_server *s, ioreq_t *p) > > new.read_pointer = old.read_pointer - n * IOREQ_BUFFER_SLOT_NUM; > new.write_pointer = old.write_pointer - n * IOREQ_BUFFER_SLOT_NUM; > - cmpxchg(&pg->ptrs.full, old.full, new.full); > + guest_cmpxchg64(s->emulator, &pg->ptrs.full, old.full, new.full); > } > > notify_via_xen_event_channel(d, s->bufioreq_evtchn); > -- > 2.7.4 >