linux-kernel - Re: [RFC PATCH v2 0/9] fpga: dfl: fix kernel warning on port release/assign for SRIOV

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <95bff99ee6c5bd37428544c72f5df233c7d602ad.camel@intel.com>
Date: Thu, 19 Sep 2024 21:52:35 +0000
From: "Colberg, Peter" <peter.colberg@...el.com>
To: "yilun.xu@...ux.intel.com" <yilun.xu@...ux.intel.com>
CC: "Xu, Yilun" <yilun.xu@...el.com>, "linux-fpga@...r.kernel.org"
	<linux-fpga@...r.kernel.org>, "mdf@...nel.org" <mdf@...nel.org>, "Wu, Hao"
	<hao.wu@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "russ.weight@...ux.dev"
	<russ.weight@...ux.dev>, "Pagani, Marco" <marpagan@...hat.com>,
	"trix@...hat.com" <trix@...hat.com>, "matthew.gerlach@...ux.intel.com"
	<matthew.gerlach@...ux.intel.com>
Subject: Re: [RFC PATCH v2 0/9] fpga: dfl: fix kernel warning on port
 release/assign for SRIOV

On Tue, 2024-04-23 at 16:27 +0800, Xu Yilun wrote:
> On Tue, Apr 09, 2024 at 07:39:33PM -0400, Peter Colberg wrote:
> > DFL ports are registered as platform devices in PF mode. The port device
> > should be removed from the host when the user wants to configure the
> > port as a VF and pass through to a virtual machine. The FME device
> > ioctls DFL_FPGA_FME_PORT_RELEASE/ASSIGN are designed for this purpose.
> > 
> > In the previous implementation, the port platform device is not completely
> > destroyed on port release: it is removed from the system by
> > platform_device_del(), but the platform device instance is retained.
> > When the port assign ioctl is called, the platform device is added back by
> > platform_device_add(), which conflicts with this comment of device_add():
> > "Do not call this routine more than once for any device structure", and
> > will cause a kernel warning at runtime.
> > 
> > This patch tries to completely unregister the port platform device on
> > release and registers a new one on assign. But the main work is to remove
> > the dependency on struct dfl_feature_platform_data for many internal DFL
> > APIs. This structure holds many DFL enumeration infos for feature devices.
> > Many DFL APIs are expected to work with these info even when the port
> > platform device is unregistered. But with the change the platform_data will
> > be freed in this case. So this patch introduces a new structure
> > dfl_feature_dev_data for these APIs, which acts similarly to the previous
> > dfl_feature_platform_data. The dfl_feature_platform_data then only needs a
> > pointer to dfl_feature_dev_data to make the feature device driver work.
> > 
> > The single monolithic v1 patch is split into multiple, smaller patches
> > at the request of the maintainer. The first patch adds temporary macros
> > that alias dfl_feature_dev_data ("fdata") to dfl_feature_platform_data
> > ("pdata") and associated functions from the "fdata" to the corresponding
> > "pdata" variants. Subsequent patches separate out most of the symbol
> > name changes required by this patch series, one patch per file. The last
> 
> One patch per file is not a requirement, simple replacement across
> multiple files won't cause trouble for reviewers. The important thing is
> that don't bury the key changes in these symbol replacement so that
> people can't get the point.

Thank you, in hindsight this should have been obvious. The v3 patch
series now breaks the v1 patch into logical, self-contained patches.

Thanks,
Peter