Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v4 1/2] xen-pciback: prepare for the split for stub and PV
To:     Stefano Stabellini <sstabellini@kernel.org>
Cc:     Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>,
        Oleksandr Andrushchenko <andr2000@gmail.com>,
        "boris.ostrovsky@oracle.com" <boris.ostrovsky@oracle.com>,
        "julien@xen.org" <julien@xen.org>,
        "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Juergen Gross <jgross@suse.com>
References: <20210927065822.350973-1-andr2000@gmail.com>
 <e472468a-625e-6c4d-a9c2-85594e2ff908@suse.com>
 <accd0220-a9d7-145b-6632-9dee085ffc65@epam.com>
 <1cf5fbf3-6453-e258-3940-8b5bb96117b6@suse.com>
 <alpine.DEB.2.21.2109272112150.5022@sstabellini-ThinkPad-T480s>
From:   Jan Beulich <jbeulich@suse.com>
Message-ID: <0b952b8d-0ebd-1c8c-84d4-f02e05bc2a2b@suse.com>
Date:   Tue, 28 Sep 2021 08:42:07 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
In-Reply-To: <alpine.DEB.2.21.2109272112150.5022@sstabellini-ThinkPad-T480s>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VkNKRUszbE5vUlRwZ0NMMFdzbTh0Sm9QS0NWM3djc2pwS2F3Tk51YnZ0aEdP?=
 =?utf-8?B?Y1BDbmdkK29GdFhYRHpvSmFwWTJVSm1zVmpRNldWdFZkUG5na2d1ZmgzRHdR?=
 =?utf-8?B?Tmp5Z2JES0xNWUVwbUlpWnZKenJ0cEQ3M2pMNHIrL09LS2ZwL1dHam0ydWFX?=
 =?utf-8?B?QTJHZE82V3BYdzl1MFpGK3EzY3kzc3FiWUlZbW15MmVxbkZNb2hmM1ZHOVpa?=
 =?utf-8?B?dTZSV3UxSDJ2M1pjczF2ejM1ZzZRVk5KTjFURU1WRW5jdGpHTHdVd3loRlZZ?=
 =?utf-8?B?U3RUNTlrNnZ5M1g0cnZGNFlLM29PUkVYVERrUEFQaVh2RUgzMmh2Rm9DQi9t?=
 =?utf-8?B?VUkwK3c2d1pWNVZJNEV0Mm5NK21lcFJvYXg4UnBZdW9vUUFPT28zc255OXV2?=
 =?utf-8?B?elJtV2xxNjBnRnhSaGNlQmVGRFdObHdKUWU4NHU3bDVocTM5bnJRblllcndI?=
 =?utf-8?B?V3FrYy9tcy9aR3RSWG5QS2J1eUNjS052S0JIQ3hqWFpxYlBQVnBZQVMxU1JK?=
 =?utf-8?B?d0VRZmJEL3J1bGt5QWc3anZBSHFNMmdQREg4ai8zeE9TZWVPQklBSUhxSXQz?=
 =?utf-8?B?YmpWSldvazAzSGdLaDI4ajVmUUxHU2FmQzlpT0h0UUlXWGFVeVhIRG5qZ3NZ?=
 =?utf-8?B?WWU1aXJ3QmJrT1JvUnlVV25uUjF3Y0tiMUV5cFIrZHMxZjdlekhzQXFpQ0FE?=
 =?utf-8?B?YW9PZlFwemx4S2tCWXRTa1MvQ2R1RFZrdlNYR2VQUW5pR0Vaa0w3UmVUYVgy?=
 =?utf-8?B?eER1MzYxTmptK2dncmR0bElSK2ZjUVpVb3ErWHA3MHJGbE5EdmhDOVgrWncw?=
 =?utf-8?B?c3NPTk11R1F2a3A5WU41eWVvcU1QUzNwL3FNL3ZRVExrbDd6NlpnRmM4ZWJ2?=
 =?utf-8?B?TkRzT2VoSjd5NXExRS91WWdxWk9hWXNhRjN3QTBWek5jamZXZ05xNFlsczR1?=
 =?utf-8?B?Q1hYY3c2UjdtOUZwUHNDcFRRVElBL2Vwem40c0pZVHJRV2FsNEFVMDFsVmtq?=
 =?utf-8?B?RG1RUEJjVlphT1hTU1ozcERkNlJheTVEUmNwd2xXako2ckN6MFFpNlNZdlhX?=
 =?utf-8?B?bWcxSE1DenNLS0NVQVBaVndzcHkwcXNyaCtpQmZmTERmems1SUZRZVBDWjA5?=
 =?utf-8?B?b2hpOHZkSVFlTWM5UUNBRlgyN0gzcnBoR21wQUNRRlMrUVJmUzNKeWFmOG1W?=
 =?utf-8?B?STN0RlpjSGJuNGs5a2ZrWjJjVWlYY1lWbUlVZzhoUnNSWnJ2S0pIVmM1Mldj?=
 =?utf-8?B?TFRCQUhVSGV6TmVweGExRVJySll5RnROMy8xK2VyZFNPRUpiSzJiaWJNN2p4?=
 =?utf-8?B?bHNmbHdsdGpMTzR1bUNQK2VidkVjSjNyNDduT253ZUExclowenNQdGhLNC91?=
 =?utf-8?B?N2dMNFQzOStMUnVlVFprOVdFc1IwMUtLVlJubzZQc3RzWHRIS3BWWjhGSm00?=
 =?utf-8?B?emczbWxWZW94Q3N0VW9XMzdiaXdKMXkydXVETXdHMGZ1clpTNEg4OFVleWpw?=
 =?utf-8?B?SlRnTjBnZWRIT0daVWEvN0ZuaUx1Uzh2MkRZNHhFMmdER3Zoek9EYmZkSi9T?=
 =?utf-8?B?TTVWZmQ3RVliS2hJRlAydGxIUlhLMU90SEQvQTlmcVNGRW5BTUh1OXNjQ2M1?=
 =?utf-8?B?ZU1aQlkzQVlaaXJIZVpvTWtkOVZkZzlGaW84dWxiQVNIMnQ2SEFpakpqY3A5?=
 =?utf-8?B?ZmREWkFicm9VZTBiRTJiRnhuZ2Q5YjExNWN5MWNkbnM4a0lGYWMyTXljbkY1?=
 =?utf-8?Q?nhGLmG+RDd9nwXvxv8O1gFTlDZ2IpMs10N3gWZV?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b6bfc79-2216-4690-4c88-08d9824b187b
X-MS-Exchange-CrossTenant-AuthSource: AM0PR04MB5587.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Sep 2021 06:42:10.2703
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: aMudJ5KM71Sf+5MfV+5mF0+1ALQRHAkYY9mZOKS+1cDAXiSaLMlNYieBztoYS/9xmSTQ3WjlOUy2ZDMKROz9zA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB6002
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 28.09.2021 06:18, Stefano Stabellini wrote:
> On Mon, 27 Sep 2021, Juergen Gross wrote:
>> On 27.09.21 09:35, Oleksandr Andrushchenko wrote:
>>>
>>> On 27.09.21 10:26, Jan Beulich wrote:
>>>> On 27.09.2021 08:58, Oleksandr Andrushchenko wrote:
>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>>
>>>>> Currently PCI backend implements multiple functionalities at a time.
>>>>> To name a few:
>>>>> 1. It 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. It is used to hold the unbound PCI devices list, 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 for the devices being passed through
>>>>> 4. Para-virtualised use-cases support
>>>>>
>>>>> The para-virtualised part of the driver is not always needed as some
>>>>> architectures, e.g. Arm or x86 PVH Dom0, are not using backend-frontend
>>>>> model for PCI device passthrough. For such use-cases make the very
>>>>> first step in splitting the xen-pciback driver into two parts: Xen
>>>>> PCI stub and PCI PV backend drivers.
>>>>>
>>>>> Signed-off-by: Oleksandr Andrushchenko
>>>>> <oleksandr_andrushchenko@epam.com>
>>>>>
>>>>> ---
>>>>> Changes since v3:
>>>>> - Move CONFIG_XEN_PCIDEV_STUB to the second patch
>>>> I'm afraid this wasn't fully done:
>>>>
>>>>> --- a/drivers/xen/xen-pciback/Makefile
>>>>> +++ b/drivers/xen/xen-pciback/Makefile
>>>>> @@ -1,5 +1,6 @@
>>>>>    # SPDX-License-Identifier: GPL-2.0
>>>>>    obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback.o
>>>>> +obj-$(CONFIG_XEN_PCIDEV_STUB) += xen-pciback.o
>>>> While benign when added here, this addition still doesn't seem to
>>>> belong here.
>>>
>>> My bad. So, it seems without CONFIG_XEN_PCIDEV_STUB the change seems
>>>
>>> to be non-functional. With CONFIG_XEN_PCIDEV_STUB we fail to build on 32-bit
>>>
>>> architectures...
>>>
>>> What would be the preference here? Stefano suggested that we still define
>>>
>>> CONFIG_XEN_PCIDEV_STUB, but in disabled state, e.g. we add tristate to it
>>>
>>> in the second patch
>>>
>>> Another option is just to squash the two patches.
>>
>> Squashing would be fine for me.
>  
> It is fine for me to squash the two patches.
> 
> But in any case, wouldn't it be better to modify that specific change to:
> 
> diff --git a/drivers/xen/xen-pciback/Makefile b/drivers/xen/xen-pciback/Makefile
> index e2cb376444a6..e23c758b85ae 100644
> --- a/drivers/xen/xen-pciback/Makefile
> +++ b/drivers/xen/xen-pciback/Makefile
> @@ -1,6 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback.o
> -obj-$(CONFIG_XEN_PCIDEV_STUB) += xen-pciback.o
> +obj-$(CONFIG_XEN_PCI_STUB) += xen-pciback.o

But that wouldn't allow the driver to be a module anymore, would it?

Jan