[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 10/12] xen-pcifront: this module is PV-only


  • To: Bjorn Helgaas <helgaas@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 7 Sep 2021 18:14:16 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=LcbSHEuJ/n74+zmRLZEo33hTo1f5VMgflTIa8T0fZDo=; b=hlECaybXcC4+Gqa0ZwV4tIXbyv1K0tcfdO0905bXclPl03sBWJcmqDkXofYLTXOiRZCev3CIIUjEWvaCGMgUx5itykjLU1t4uj3MH88agT3otTrO5HMq/hdDybDiIXoUQxsff0qlygti68A/W6WFnfr9lsEIZi49N9RTKQBnGzzAf9I49ITsZt6TEfTSz2Iqg6KeazRxE908KkzLP3V5XHt4K9ftHh2t1d2UfM3okwqUjBWq75LEtJ5GD2rneVlrOrX1il9gcbm9f9+sd9QVY0VI83KkeC4CMydYFbKyXn+pwb6uSJt/Eav8rwAixcpaJidFN+DKIEBIGHS7D2amOA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=O3x2M6ZNPBorl0v/ebyO/SUSa6mf94oeLXF8VSaWjisitGFKx+VnZOSNKsjdeK+OC8gssgIH3Ttl1UQcogKgGCqqQ55wppPCifo4NAZnBe0QovFu1L4bnMkC8+jjgaSK4uedNlZxj/ZJvcXPadD99Vwb0BMvUhtYksdm4XWeb2QaVHHDKq5GZcvT1/r+yV8YUw+nd7lokoJZQRl6TU5JKZ1zQ61NrPIMct5rp+oiHMqRODHvkrnG/Na2W6D/5Nm4RXfA6urjYvf5+wLnNMVOw6l1Q05eqGnagKOVCza2VWG8wmSWLsMbxMfVUEanRhaGuIvQcYQPogs5UpES9zJszA==
  • Authentication-results: vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=none action=none header.from=suse.com;
  • Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>, "linux-pci@xxxxxxxxxxxxxxx" <linux-pci@xxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 07 Sep 2021 16:14:27 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 07.09.2021 17:30, Bjorn Helgaas wrote:
> Update subject to follow conventions (use "git log --oneline
> drivers/pci/Kconfig").  Should say what this patch does.

I can change that; I don't think it'll carry any different information.

> Commit log below should also say what this patch does.  Currently it's
> part of the rationale for the change, but doesn't say what the patch
> does.

"There's no point building ..." to me is as good as "Don't build ...".
But oh well, I can adjust ...

> On Tue, Sep 07, 2021 at 02:10:41PM +0200, Jan Beulich wrote:
>> It's module init function does a xen_pv_domain() check first thing.
>> Hence there's no point building it in non-PV configurations.
> 
> s/It's/<name of function that calls xen_pv_domain()/   # pcifront_init()?

I don't understand this - how is "module init function" not clear
enough?

> s/building it/building <name of module>/               # xen-pcifront.o?

The driver name is already part of the subject; I didn't think I
need to repeat that one here.

> I see that CONFIG_XEN_PV is only mentioned in arch/x86, so
> CONFIG_XEN_PV=y cannot be set on other arches.  Is the current
> "depends on X86" just a reflection of that, or is it because of some
> other x86 dependency in the code?
> 
> The connection between xen_pv_domain() and CONFIG_XEN_PV is not
> completely obvious.
> 
> If you only build xen-pcifront.o when CONFIG_XEN_PV=y, and
> xen_pv_domain() is true if and only if CONFIG_XEN_PV=y, why bother
> calling xen_pv_domain() at all?

Because XEN_PV=y only _enables_ the kernel to run in PV mode. It
may be enabled to also run in HVM and/or PVH modes. And it may
then _run_ in any of the enabled modes. IOW xen_pv_domain() will
always return false when !XEN_PV; no other implication is valid.
I don't think this basic concept needs explaining in a simple
patch like this. Instead I think the config option in question,
despite living in drivers/pci/Kconfig, should be under "XEN
HYPERVISOR INTERFACE" maintainership. I realize that's not even
expressable in ./MAINTAINERS. I wonder why the option was put
there in the first place, rather than in drivers/xen/Kconfig.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.