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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B196CC6FA82 for ; Wed, 14 Sep 2022 19:53:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=a9SkLfi3GF6nLae24pSRFUTBBLYZ7aUtznK2uuWHmGU=; b=ak9smnszZ6Ax6Y K1hlF7PNpwec30wl0qqFEntQ9PBH+S9OVIqU5l8EnM2eQLaF7Y89W2Vn+t1492VtnDH8SbnBTeL2p X1jXsz+swzoKNTvZQEUp193oO/OK+dsPCMsadXbfJQCj2khfQZ4LktGZCKX5AIkRlMRqO6cFW2+H6 LToLLEMcw0XKHjJF3UjjforHvR1e13hntkYsCymGNX6Gwbx4wtuiKErgKK3h2jN9eomTgGZY01CbV Knmgrek7/XQsekng4UlM7S22pVHtv/RjzeyGzMQLrZG9DRHAEB9maQ8dXWqSPUOJx+ThuQwhntQBS QTXc7B6NJOfeI0M4L7MQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oYYS6-008eB3-2o; Wed, 14 Sep 2022 19:53:42 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oYYRm-008dUo-B4; Wed, 14 Sep 2022 19:53:24 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 956841576; Wed, 14 Sep 2022 12:53:23 -0700 (PDT) Received: from [10.57.18.118] (unknown [10.57.18.118]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2CB443F73B; Wed, 14 Sep 2022 12:53:12 -0700 (PDT) Message-ID: <5f049bb8-76e6-901a-9f8e-b48d270bc4a0@arm.com> Date: Wed, 14 Sep 2022 20:53:07 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH 4/5] iommu: Regulate errno in ->attach_dev callback functions Content-Language: en-GB To: Nicolin Chen , Jason Gunthorpe , Jean-Philippe Brucker Cc: joro@8bytes.org, suravee.suthikulpanit@amd.com, will@kernel.org, robdclark@gmail.com, dwmw2@infradead.org, baolu.lu@linux.intel.com, agross@kernel.org, bjorn.andersson@linaro.org, konrad.dybcio@somainline.org, matthias.bgg@gmail.com, heiko@sntech.de, orsonzhai@gmail.com, baolin.wang@linux.alibaba.com, zhang.lyra@gmail.com, thierry.reding@gmail.com, sricharan@codeaurora.org, yong.wu@mediatek.com, vdumpa@nvidia.com, jonathanh@nvidia.com, tglx@linutronix.de, shameerali.kolothum.thodi@huawei.com, thunder.leizhen@huawei.com, christophe.jaillet@wanadoo.fr, yangyingliang@huawei.com, jon@solid-run.com, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-tegra@vger.kernel.org, virtualization@lists.linux-foundation.org References: <20220913082448.31120-1-nicolinc@nvidia.com> <20220913082448.31120-5-nicolinc@nvidia.com> From: Robin Murphy In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220914_125322_523583_7252DDD7 X-CRM114-Status: GOOD ( 23.89 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org On 2022-09-14 18:58, Nicolin Chen wrote: > On Wed, Sep 14, 2022 at 10:49:42AM +0100, Jean-Philippe Brucker wrote: >> External email: Use caution opening links or attachments >> >> >> On Wed, Sep 14, 2022 at 06:11:06AM -0300, Jason Gunthorpe wrote: >>> On Tue, Sep 13, 2022 at 01:27:03PM +0100, Jean-Philippe Brucker wrote: >>>> I think in the future it will be too easy to forget about the constrained >>>> return value of attach() while modifying some other part of the driver, >>>> and let an external helper return EINVAL. So I'd rather not propagate ret >>>> from outside of viommu_domain_attach() and finalise(). >>> >>> Fortunately, if -EINVAL is wrongly returned it only creates an >>> inefficiency, not a functional problem. So we do not need to be >>> precise here. >> >> Ah fair. In that case the attach_dev() documentation should indicate that >> EINVAL is a hint, so that callers don't rely on it (currently words "must" >> and "exclusively" indicate that returning EINVAL for anything other than >> device-domain incompatibility is unacceptable). The virtio-iommu >> implementation may well return EINVAL from the virtio stack or from the >> host response. > > How about this? > > + * * EINVAL - mainly, device and domain are incompatible, or something went > + * wrong with the domain. It's suggested to avoid kernel prints > + * along with this errno. And it's better to convert any EINVAL > + * returned from kAPIs to ENODEV if it is device-specific, or to > + * some other reasonable errno being listed below FWIW, I'd say something like: "The device and domain are incompatible. If this is due to some previous configuration of the domain, drivers should not log an error, since it is legitimate for callers to test reuse of an existing domain. Otherwise, it may still represent some fundamental problem." And then at the public interfaces state it from other angle: "The device and domain are incompatible. If the domain has already been used or configured in some way, attaching the same device to a different domain may be expected to succeed. Otherwise, it may still represent some fundamental problem." [ and to save another mail, I'm not sure copying the default comment for ENOSPC is all that helpful either - what is "space" for something that isn't a storage device? I'd guess limited hardware resources in some form, but in the IOMMU context, potential confusion with address space is maybe a little too close for comfort? ] >>>> Since we can't guarantee that APIs like virtio or ida won't ever return >>>> EINVAL, we should set all return values: >>> >>> I dislike this alot, it squashes all return codes to try to optimize >>> an obscure failure path :( > > Hmm...should I revert all the driver changes back to this version? Yeah, I don't think we need to go too mad here. Drivers shouldn't emit their *own* -EINVAL unless appropriate, but if it comes back from some external API then that implies something's gone unexpectedly wrong anyway - maybe it's a transient condition and a subsequent different attach might actually work out OK? We can't really say in general. Besides, if the driver sees an error which implies it's done something wrong itself, it probably shouldn't be trusted to try to reason about it further. The caller can handle any error as long as we set their expectations correctly. Thanks, Robin. _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip