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=-7.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 65E8DC433FE for ; Wed, 9 Dec 2020 23:35:22 +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 22E5E2313E for ; Wed, 9 Dec 2020 23:35:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 22E5E2313E 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.48866.86453 (Exim 4.92) (envelope-from ) id 1kn8zE-0001y3-RW; Wed, 09 Dec 2020 23:35:08 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 48866.86453; Wed, 09 Dec 2020 23:35:08 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kn8zE-0001xw-OQ; Wed, 09 Dec 2020 23:35:08 +0000 Received: by outflank-mailman (input) for mailman id 48866; Wed, 09 Dec 2020 23:35:07 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kn8zD-0001xr-Mw for xen-devel@lists.xenproject.org; Wed, 09 Dec 2020 23:35:07 +0000 Received: from mail.kernel.org (unknown [198.145.29.99]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id b6ff657a-6697-45ce-9138-c05a936da4ef; Wed, 09 Dec 2020 23:35:07 +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: b6ff657a-6697-45ce-9138-c05a936da4ef Date: Wed, 9 Dec 2020 15:35:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1607556906; bh=YoVLcOmgtgdXh5DDh0RvYgaGpz4Ay9q2qHzeVVTt8vg=; h=From:To:cc:Subject:In-Reply-To:References:From; b=afrq+N/w2XDVFCf4axW1W9X5d/bV4XE2CgoKQbVvj7wjO3JxR3DpMcTtFEZzvfASE OfORAofcNOZQeLCQX2E5VHdjvrHmF3mbjeg7BxenYzWbuZiECFlvAOAChJfM9Y9RKB bzWyWRTTEYc1Qui6waJga0IPUD1F3BoYgwQPap4bPPU51bRcvoTfmohiMA6fNpZVOo upm701jDTarenQfmqUtXrQfHk9kbKKauR5DNTp3SfcpYT5+81sWzbuPwMMtnFEHfS6 tgdMTkOCsvDALI/ohl6JrJlw9fj1ZctGtcUtrHHiKsGD1ev4BVS+imha1OciqwubhU v1N3INidYbJ1g== From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-T480s To: Stefano Stabellini cc: Oleksandr Tyshchenko , xen-devel@lists.xenproject.org, Oleksandr Tyshchenko , Julien Grall , Volodymyr Babchuk , Julien Grall Subject: Re: [PATCH V3 15/23] xen/arm: Stick around in leave_hypervisor_to_guest until I/O has completed In-Reply-To: Message-ID: References: <1606732298-22107-1-git-send-email-olekstysh@gmail.com> <1606732298-22107-16-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 Wed, 9 Dec 2020, Stefano Stabellini wrote: > On Mon, 30 Nov 2020, Oleksandr Tyshchenko wrote: > > From: Oleksandr Tyshchenko > > > > This patch adds proper handling of return value of > > vcpu_ioreq_handle_completion() which involves using a loop > > in leave_hypervisor_to_guest(). > > > > The reason to use an unbounded loop here is the fact that vCPU > > shouldn't continue until an I/O has completed. In Xen case, if an I/O > > never completes then it most likely means that something went horribly > > wrong with the Device Emulator. And it is most likely not safe to > > continue. So letting the vCPU to spin forever if I/O never completes > > is a safer action than letting it continue and leaving the guest in > > unclear state and is the best what we can do for now. > > > > This wouldn't be an issue for Xen as do_softirq() would be called at > > every loop. In case of failure, the guest will crash and the vCPU > > will be unscheduled. > > Imagine that we have two guests: one that requires an ioreq server and > one that doesn't. If I am not mistaken this loop could potentially spin > forever on a pcpu, thus preventing any other guest being scheduled, even > if the other guest doesn't need any ioreq servers. > > > My other concern is that we are busy-looping. Could we call something > like wfi() or do_idle() instead? The ioreq server event notification of > completion should wake us up? > > Following this line of thinking, I am wondering if instead of the > busy-loop we should call vcpu_block_unless_event_pending(current) in > try_handle_mmio if IO_RETRY. Then when the emulation is done, QEMU (or > equivalent) calls xenevtchn_notify which ends up waking up the domU > vcpu. Would that work? I read now Julien's reply: we are already doing something similar to what I suggested with the following call chain: check_for_vcpu_work -> vcpu_ioreq_handle_completion -> wait_for_io -> wait_on_xen_event_channel So the busy-loop here is only a safety-belt in cause of a spurious wake-up, in which case we are going to call again check_for_vcpu_work, potentially causing a guest reschedule. Then, this is fine and addresses both my concerns. Maybe let's add a note in the commit message about it. I am also wondering if there is any benefit in calling wait_for_io() earlier, maybe from try_handle_mmio if IO_RETRY? leave_hypervisor_to_guest is very late for that. In any case, it is not an important optimization (if it is even an optimization at all) so it is fine to leave it as is.