linux-kernel - Re: [EXTERNAL] [PATCH] vdpa/octeon_ep: Use non-hybrid PCI devres API

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <a57d5f6f47ec8ba2860df853aca324e96320eba5.camel@mailbox.org>
Date: Thu, 17 Apr 2025 12:39:03 +0200
From: Philipp Stanner <phasta@...lbox.org>
To: Vamsi Krishna Attunuru <vattunuru@...vell.com>, Philipp Stanner
 <phasta@...nel.org>, Srujana Challa <schalla@...vell.com>, "Michael S.
 Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>, Xuan Zhuo
 <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
 <eperezma@...hat.com>, Shijith Thotton <sthotton@...vell.com>, Dan
 Carpenter <dan.carpenter@...aro.org>, Satha Koteswara Rao Kottidi
 <skoteshwar@...vell.com>
Cc: "virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>, 
 "linux-kernel@...r.kernel.org"
	 <linux-kernel@...r.kernel.org>
Subject: Re: [EXTERNAL] [PATCH] vdpa/octeon_ep: Use non-hybrid PCI devres API

On Thu, 2025-04-17 at 09:02 +0000, Vamsi Krishna Attunuru wrote:
> 
> 
> > -----Original Message-----
> > From: Philipp Stanner <phasta@...nel.org>
> > Sent: Thursday, April 17, 2025 1:32 PM
> > To: Srujana Challa <schalla@...vell.com>; Vamsi Krishna Attunuru
> > <vattunuru@...vell.com>; Michael S. Tsirkin <mst@...hat.com>; Jason
> > Wang <jasowang@...hat.com>; Xuan Zhuo <xuanzhuo@...ux.alibaba.com>;
> > Eugenio Pérez <eperezma@...hat.com>; Shijith Thotton
> > <sthotton@...vell.com>; Dan Carpenter <dan.carpenter@...aro.org>;
> > Philipp Stanner <phasta@...nel.org>; Satha Koteswara Rao Kottidi
> > <skoteshwar@...vell.com>
> > Cc: virtualization@...ts.linux.dev; linux-kernel@...r.kernel.org
> > Subject: [EXTERNAL] [PATCH] vdpa/octeon_ep: Use non-hybrid PCI
> > devres
> > API
> > 
> > octeon enables its PCI device with pcim_enable_device(). This,
> > implicitly,
> > switches the function pci_request_region() into managed mode, where
> > it
> > becomes a devres function. The PCI subsystem wants to remove this
> > hybrid
> > nature from its interfaces. 
> > octeon enables its PCI device with pcim_enable_device(). This,
> > implicitly,
> > switches the function pci_request_region() into managed mode, where
> > it
> > becomes a devres function.
> > 
> > The PCI subsystem wants to remove this hybrid nature from its
> > interfaces. To
> > do so, users of the aforementioned combination of functions must be
> > ported
> > to non-hybrid functions.
> > 
> > Moreover, since both functions are already managed in this driver,
> > the calls to
> > pci_release_region() are unnecessary.
> > 
> > Remove the calls to pci_release_region().
> > 
> > Replace the call to sometimes-managed pci_request_region() with one
> > to the
> > always-managed pcim_request_region().
> 

Hi,

> Thanks you, Philipps, for the patch. The Octeon EP driver does not
> use managed calls for handling resource regions. 
> This is because the PCIe PF function reduces its resources to share
> them with its VFs and later restores them to original size
> once the VFs are released. Due to this resource sharing requirement,
> the driver cannot use the always-managed request
> regions calls.

so this would mean that the driver is already broken.
pci_request_region() in your driver is always-managed since you call
pcim_enable_device(). Or am I missing something?

The only way for you to, currently, have non-managed versions is by
using pci_enable_device() instead and then doing pci_disable_device()
manually in remove() and the other appropriate places.

This patch should not change behavior in any way.

If you're sure that having no management is not a problem, then we
could simply drop this patch and I later remove the hybrid feature from
PCI. Then your call to pci_request_region() will become unmanaged, even
if you keep the pcim_enable_device().

Tell me if you have a preference.

P.

> 
> > 
> > Signed-off-by: Philipp Stanner <phasta@...nel.org>
> > ---
> > drivers/vdpa/octeon_ep/octep_vdpa_main.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> > b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> > index f3d4dda4e04c..e0da6367661e 100644
> > --- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> > +++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> > @@ -391,7 +391,7 @@ static int octep_iomap_region(struct pci_dev
> > *pdev,
> > u8 __iomem **tbl, u8 bar)  {
> > 	int ret;
> > 
> > -	ret = pci_request_region(pdev, bar,
> > OCTEP_VDPA_DRIVER_NAME);
> > +	ret = pcim_request_region(pdev, bar,
> > OCTEP_VDPA_DRIVER_NAME);
> > 	if (ret) {
> > 		dev_err(&pdev->dev, "Failed to request BAR:%u
> > region\n",
> > bar);
> > 		return ret;
> > @@ -400,7 +400,6 @@ static int octep_iomap_region(struct pci_dev
> > *pdev,
> > u8 __iomem **tbl, u8 bar)
> > 	tbl[bar] = pci_iomap(pdev, bar, pci_resource_len(pdev,
> > bar));
> > 	if (!tbl[bar]) {
> > 		dev_err(&pdev->dev, "Failed to iomap BAR:%u\n",
> > bar);
> > -		pci_release_region(pdev, bar);
> > 		ret = -ENOMEM;
> > 	}
> > 
> > @@ -410,7 +409,6 @@ static int octep_iomap_region(struct pci_dev
> > *pdev,
> > u8 __iomem **tbl, u8 bar)  static void octep_iounmap_region(struct
> > pci_dev
> > *pdev, u8 __iomem **tbl, u8 bar)  {
> > 	pci_iounmap(pdev, tbl[bar]);
> > -	pci_release_region(pdev, bar);
> > }
> > 
> > static void octep_vdpa_pf_bar_shrink(struct octep_pf *octpf)
> > --
> > 2.48.1
>