| 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
| ||
|
Message-ID: <914de38f0b6b3d2ddf98c16e427085732bebfd4a.camel@redhat.com>
Date: Fri, 11 Oct 2024 14:03:39 +0200
From: Philipp Stanner <pstanner@...hat.com>
To: Alex Williamson <alex.williamson@...hat.com>
Cc: Damien Le Moal <dlemoal@...nel.org>, Niklas Cassel <cassel@...nel.org>,
Sergey Shtylyov <s.shtylyov@....ru>, Basavaraj Natikar
<basavaraj.natikar@....com>, Jiri Kosina <jikos@...nel.org>, Benjamin
Tissoires <bentiss@...nel.org>, Arnd Bergmann <arnd@...db.de>, Greg
Kroah-Hartman <gregkh@...uxfoundation.org>, Alex Dubov <oakad@...oo.com>,
Sudarsana Kalluru <skalluru@...vell.com>, Manish Chopra
<manishc@...vell.com>, "David S. Miller" <davem@...emloft.net>, Eric
Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo
Abeni <pabeni@...hat.com>, Rasesh Mody <rmody@...vell.com>,
GR-Linux-NIC-Dev@...vell.com, Igor Mitsyanko <imitsyanko@...ntenna.com>,
Sergey Matyukevich <geomatsi@...il.com>, Kalle Valo <kvalo@...nel.org>,
Sanjay R Mehta <sanju.mehta@....com>, Shyam Sundar S K
<Shyam-sundar.S-k@....com>, Jon Mason <jdmason@...zu.us>, Dave Jiang
<dave.jiang@...el.com>, Allen Hubbe <allenbh@...il.com>, Bjorn Helgaas
<bhelgaas@...gle.com>, Juergen Gross <jgross@...e.com>, Stefano Stabellini
<sstabellini@...nel.org>, Oleksandr Tyshchenko
<oleksandr_tyshchenko@...m.com>, Jaroslav Kysela <perex@...ex.cz>, Takashi
Iwai <tiwai@...e.com>, Mario Limonciello <mario.limonciello@....com>, Chen
Ni <nichen@...as.ac.cn>, Ricky Wu <ricky_wu@...ltek.com>, Al Viro
<viro@...iv.linux.org.uk>, Breno Leitao <leitao@...ian.org>, Kevin Tian
<kevin.tian@...el.com>, Thomas Gleixner <tglx@...utronix.de>, Ilpo
Järvinen <ilpo.jarvinen@...ux.intel.com>, Mostafa Saleh
<smostafa@...gle.com>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Hannes Reinecke <hare@...e.de>, John Garry <john.g.garry@...cle.com>,
Soumya Negi <soumya.negi97@...il.com>, Jason Gunthorpe <jgg@...pe.ca>, Yi
Liu <yi.l.liu@...el.com>, "Dr. David Alan Gilbert" <linux@...blig.org>,
Christian Brauner <brauner@...nel.org>, Ankit Agrawal <ankita@...dia.com>,
Reinette Chatre <reinette.chatre@...el.com>, Eric Auger
<eric.auger@...hat.com>, Ye Bin <yebin10@...wei.com>, Marek
Marczykowski-Górecki <marmarek@...isiblethingslab.com>,
Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Kai Vehmanen
<kai.vehmanen@...ux.intel.com>, Peter Ujfalusi
<peter.ujfalusi@...ux.intel.com>, Rui Salvaterra <rsalvaterra@...il.com>,
Marc Zyngier <maz@...nel.org>, linux-ide@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
ntb@...ts.linux.dev, linux-pci@...r.kernel.org,
linux-staging@...ts.linux.dev, kvm@...r.kernel.org,
xen-devel@...ts.xenproject.org, linux-sound@...r.kernel.org
Subject: Re: [RFC PATCH 01/13] PCI: Prepare removing devres from pci_intx()
On Thu, 2024-10-10 at 11:43 -0600, Alex Williamson wrote:
> On Wed, 9 Oct 2024 10:35:07 +0200
> Philipp Stanner <pstanner@...hat.com> wrote:
>
> > pci_intx() is a hybrid function which sometimes performs devres
> > operations, depending on whether pcim_enable_device() has been used
> > to
> > enable the pci_dev. This sometimes-managed nature of the function
> > is
> > problematic. Notably, it causes the function to allocate under some
> > circumstances which makes it unusable from interrupt context.
> >
> > To, ultimately, remove the hybrid nature from pci_intx(), it is
> > first
> > necessary to provide an always-managed and a never-managed version
> > of that function. Then, all callers of pci_intx() can be ported to
> > the
> > version they need, depending whether they use pci_enable_device()
> > or
> > pcim_enable_device().
> >
> > An always-managed function exists, namely pcim_intx(), for which
> > __pcim_intx(), a never-managed version of pci_intx() had been
> > implemented.
> >
> > Make __pcim_intx() a public function under the name
> > pci_intx_unmanaged(). Make pcim_intx() a public function.
> >
> > Signed-off-by: Philipp Stanner <pstanner@...hat.com>
> > ---
> > drivers/pci/devres.c | 24 +++---------------------
> > drivers/pci/pci.c | 26 ++++++++++++++++++++++++++
> > include/linux/pci.h | 2 ++
> > 3 files changed, 31 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
> > index b133967faef8..475a3ae5c33f 100644
> > --- a/drivers/pci/devres.c
> > +++ b/drivers/pci/devres.c
> > @@ -411,31 +411,12 @@ static inline bool mask_contains_bar(int
> > mask, int bar)
> > return mask & BIT(bar);
> > }
> >
> > -/*
> > - * This is a copy of pci_intx() used to bypass the problem of
> > recursive
> > - * function calls due to the hybrid nature of pci_intx().
> > - */
> > -static void __pcim_intx(struct pci_dev *pdev, int enable)
> > -{
> > - u16 pci_command, new;
> > -
> > - pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
> > -
> > - if (enable)
> > - new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
> > - else
> > - new = pci_command | PCI_COMMAND_INTX_DISABLE;
> > -
> > - if (new != pci_command)
> > - pci_write_config_word(pdev, PCI_COMMAND, new);
> > -}
> > -
> > static void pcim_intx_restore(struct device *dev, void *data)
> > {
> > struct pci_dev *pdev = to_pci_dev(dev);
> > struct pcim_intx_devres *res = data;
> >
> > - __pcim_intx(pdev, res->orig_intx);
> > + pci_intx_unmanaged(pdev, res->orig_intx);
> > }
> >
> > static struct pcim_intx_devres *get_or_create_intx_devres(struct
> > device *dev)
> > @@ -472,10 +453,11 @@ int pcim_intx(struct pci_dev *pdev, int
> > enable)
> > return -ENOMEM;
> >
> > res->orig_intx = !enable;
> > - __pcim_intx(pdev, enable);
> > + pci_intx_unmanaged(pdev, enable);
> >
> > return 0;
> > }
> > +EXPORT_SYMBOL(pcim_intx);
>
> What precludes this from _GPL? Also note that this is now calling a
> GPL symbol, so by default I'd assume it should also be GPL. Thanks,
Ah right, I overlooked that pci_intx() also has the _GPL version.
Will make consistent.
Thx,
P.
>
> Alex
>
> >
> > static void pcim_disable_device(void *pdev_raw)
> > {
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 7d85c04fbba2..318cfb5b5e15 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -4476,6 +4476,32 @@ void pci_disable_parity(struct pci_dev *dev)
> > }
> > }
> >
> > +/**
> > + * pci_intx - enables/disables PCI INTx for device dev, unmanaged
> > version
> > + * @pdev: the PCI device to operate on
> > + * @enable: boolean: whether to enable or disable PCI INTx
> > + *
> > + * Enables/disables PCI INTx for device @pdev
> > + *
> > + * This function behavios identically to pci_intx(), but is never
> > managed with
> > + * devres.
> > + */
> > +void pci_intx_unmanaged(struct pci_dev *pdev, int enable)
> > +{
> > + u16 pci_command, new;
> > +
> > + pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
> > +
> > + if (enable)
> > + new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
> > + else
> > + new = pci_command | PCI_COMMAND_INTX_DISABLE;
> > +
> > + if (new != pci_command)
> > + pci_write_config_word(pdev, PCI_COMMAND, new);
> > +}
> > +EXPORT_SYMBOL_GPL(pci_intx_unmanaged);
> > +
> > /**
> > * pci_intx - enables/disables PCI INTx for device dev
> > * @pdev: the PCI device to operate on
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 573b4c4c2be6..6b8cde76d564 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1353,6 +1353,7 @@ int __must_check pcim_set_mwi(struct pci_dev
> > *dev);
> > int pci_try_set_mwi(struct pci_dev *dev);
> > void pci_clear_mwi(struct pci_dev *dev);
> > void pci_disable_parity(struct pci_dev *dev);
> > +void pci_intx_unmanaged(struct pci_dev *pdev, int enable);
> > void pci_intx(struct pci_dev *dev, int enable);
> > bool pci_check_and_mask_intx(struct pci_dev *dev);
> > bool pci_check_and_unmask_intx(struct pci_dev *dev);
> > @@ -2293,6 +2294,7 @@ static inline void pci_fixup_device(enum
> > pci_fixup_pass pass,
> > struct pci_dev *dev) { }
> > #endif
> >
> > +int pcim_intx(struct pci_dev *pdev, int enabled);
> > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned
> > long maxlen);
> > void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar,
> > const char *name);
>
Powered by blists - more mailing lists