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=-14.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 F2BDCC433F5 for ; Mon, 20 Sep 2021 23:18:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C9F7F61211 for ; Mon, 20 Sep 2021 23:18:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239763AbhITXTm (ORCPT ); Mon, 20 Sep 2021 19:19:42 -0400 Received: from mail.kernel.org ([198.145.29.99]:40316 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239487AbhITXRl (ORCPT ); Mon, 20 Sep 2021 19:17:41 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 07D2961211; Mon, 20 Sep 2021 23:16:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1632179774; bh=lWL8fnGNKTXM4Bi+yAVp1+gpNMDcF8Ek0Z+rLqzdBB8=; h=Date:From:To:cc:Subject:In-Reply-To:References:From; b=YEnp5FBXjtQsJausdghHKuLJItlSCUlpC2G+ai9iRGx9pGdDBniFNbpSBrobhC0Cz udviL2kOafQidCQaV1xpH53ha4GYfV8xaqKhTHHg3aM/g28kQEiFmht2+HiBCq5nFB GgqQa2vfu0+UORnW/8LvucewzMCrF3iKcRVzst9Jn+OHtBs8+6XhhDJtYnupvsaYhP CveIWnFLn9iU64Jv8J+Z3MLhIpBwWNSgNH2rkTgxkZYjz0PtV91H2V2cvyvvQoWGEm XBA5u+LZ0fERDYZguxuCh5HEbxpt2axAUFxdc7bj27chIwpKeS7uCQCUqleJgSI1CS dNpWHWgV3/m6g== Date: Mon, 20 Sep 2021 16:16:06 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-T480s To: Oleksandr Andrushchenko cc: Juergen Gross , Stefano Stabellini , "xen-devel@lists.xenproject.org" , "linux-kernel@vger.kernel.org" , "boris.ostrovsky@oracle.com" , "julien@xen.org" , "jbeulich@suse.com" , Anastasiia Lukianenko , Oleksandr Andrushchenko Subject: Re: [PATCH] xen-pciback: allow compiling on other archs than x86 In-Reply-To: <7f873e38-0362-1f60-7347-a490c9dc8572@epam.com> Message-ID: References: <20210917130123.1764493-1-andr2000@gmail.com> <35e2e36a-bade-d801-faa1-c9953678bb9d@suse.com> <7f873e38-0362-1f60-7347-a490c9dc8572@epam.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: multipart/mixed; BOUNDARY="8323329-1040504615-1632174404=:17979" Content-ID: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323329-1040504615-1632174404=:17979 Content-Type: text/plain; CHARSET=UTF-8 Content-Transfer-Encoding: 8BIT Content-ID: On Mon, 20 Sep 2021, Oleksandr Andrushchenko wrote: > On 20.09.21 14:30, Juergen Gross wrote: > > On 20.09.21 07:23, Oleksandr Andrushchenko wrote: > >> Hello, Stefano! > >> > >> On 18.09.21 00:45, Stefano Stabellini wrote: > >>> Hi Oleksandr, > >>> > >>> Why do you want to enable pciback on ARM? Is it only to "disable" a PCI > >>> device in Dom0 so that it can be safely assigned to a DomU? > >> Not only that > >>> > >>> I am asking because actually I don't think we want to enable the PV PCI > >>> backend feature of pciback on ARM, right? That would clash with the PCI > >>> assignment work you have been doing in Xen. They couldn't both work at > >>> the same time. > >> Correct, it is not used > >>> > >>> If we only need pciback to "park" a device in Dom0, wouldn't it be > >>> possible and better to use pci-stub instead? > >> > >> Not only that, so pci-stub is not enough > >> > >> The functionality which is implemented by the pciback and the toolstack > >> and which is relevant/missing/needed for ARM: > >> > >> 1. pciback is used as a database for assignable PCI devices, e.g. xl > >>      pci-assignable-{add|remove|list} manipulates that list. So, whenever the > >>      toolstack needs to know which PCI devices can be passed through it reads > >>      that from the relevant sysfs entries of the pciback. > >> > >> 2. pciback is used to hold the unbound PCI devices, e.g. when passing through > >>      a PCI device it needs to be unbound from the relevant device driver and bound > >>      to pciback (strictly speaking it is not required that the device is bound to > >>      pciback, but pciback is again used as a database of the passed through PCI > >>      devices, so we can re-bind the devices back to their original drivers when > >>      guest domain shuts down) > >> > >> 3. Device reset > >> > >> We have previously discussed on xen-devel ML possible solutions to that as from the > >> above we see that pciback functionality is going to be only partially used on Arm. > >> > >> Please see [1] and [2]: > >> > >> 1. It is not acceptable to manage the assignable list in Xen itself > >> > >> 2. pciback can be split into two parts: PCI assignable/bind/reset handling and > >> the rest like vPCI etc. > >> > >> 3. pcifront is not used on Arm > > > > It is neither in x86 PVH/HVM guests. > Didn't know that, thank you for pointing > > > >> So, limited use of the pciback is one of the bricks used to enable PCI passthrough > >> on Arm. It was enough to just re-structure the driver and have it run on Arm to achieve > >> all the goals above. > >> > >> If we still think it is desirable to break the pciback driver into "common" and "pcifront specific" > >> parts then it can be done, yet the patch is going to be the very first brick in that building. > > > > Doing this split should be done, as the pcifront specific part could be > > omitted on x86, too, in case no PV guests using PCI passthrough have to > > be supported. > Agree, that the final solution should have the driver split > > > >> So, I think this patch is still going to be needed besides which direction we take. > > > > Some kind of this patch, yes. It might look different in case the split > > is done first. > > > > I don't mind doing it in either sequence. > > > With this patch we have Arm on the same page as the above mentioned x86 guests, > > e.g. the driver has unused code, but yet allows Arm to function now. > > At this stage of PCI passthrough on Arm it is yet enough. Long term, when > > the driver gets split, Arm will benefit from that split too, but unfortunately I do not > > have enough bandwidth for that piece of work at the moment. That's fair and I don't want to scope-creep this simple patch asking for an enormous rework. At the same time I don't think we should enable the whole of pciback on ARM because it would be erroneous and confusing. I am wonder if there is a simple: if (!xen_pv_domain()) return; That we could add in a couple of places in pciback to stop it from initializing the parts we don't care about. Something along these lines (untested and probably incomplete). What do you guys think? diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c index da34ce85dc88..991ba0a9b359 100644 --- a/drivers/xen/xen-pciback/xenbus.c +++ b/drivers/xen/xen-pciback/xenbus.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "pciback.h" #define INVALID_EVTCHN_IRQ (-1) @@ -685,8 +686,12 @@ static int xen_pcibk_xenbus_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { int err = 0; - struct xen_pcibk_device *pdev = alloc_pdev(dev); + struct xen_pcibk_device *pdev; + + if (!xen_pv_domain()) + return 0; + pdev = alloc_pdev(dev); if (pdev == NULL) { err = -ENOMEM; xenbus_dev_fatal(dev, err, @@ -743,6 +748,9 @@ const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend; int __init xen_pcibk_xenbus_register(void) { + if (!xen_pv_domain()) + return 0; + xen_pcibk_backend = &xen_pcibk_vpci_backend; if (passthrough) xen_pcibk_backend = &xen_pcibk_passthrough_backend; @@ -752,5 +760,7 @@ int __init xen_pcibk_xenbus_register(void) void __exit xen_pcibk_xenbus_unregister(void) { + if (!xen_pv_domain()) + return; xenbus_unregister_driver(&xen_pcibk_driver); } --8323329-1040504615-1632174404=:17979--