Re: [PATCH] PCI: Fix devres regression in pci_intx()

From: Philipp Stanner
Date: Thu Jul 25 2024 - 11:21:53 EST


On Thu, 2024-07-25 at 07:26 -0700, Christoph Hellwig wrote:
> Can we please fix this to never silently redirect to a manager
> version

It is not the fix or the recent changes (which the fix is for) to PCI
devres that is doing that. pci_intx() has been "silently redirect[ing]
to a managed version" since 15 years.

The changes merged into v6.11 attempted to keep this behavior exactly
identical as a preparation for later cleanups. The fix here only
corrects the position of the redirection to where the "crazy devres
voodoo" had always been:

void pci_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) {
struct pci_devres *dr;

pci_write_config_word(pdev, PCI_COMMAND, new);

/* voodoo_begin */
dr = find_pci_dr(pdev);
if (dr && !dr->restore_intx) {
dr->restore_intx = 1;
dr->orig_intx = !enable;
}
/* voodoo_end */
}
}
EXPORT_SYMBOL_GPL(pci_intx);

> and add a proper pcim_intx instead 

That has already been done. pcim_intx() sits in drivers/pci/devres.c

> and use that where people actually
> want to use the crazy devres voodoo instead?  Doing this silently
> underneath will always create problems.

That's precisely what all my work is all about. The hybrid nature of
pci_intx(), pci_set_mwi() and all pci*request*() functions needs to be
removed.

However, that will take us some while, because the APIs are partly
ossificated and every user that relies on implicit crazy devres voodoo
has to be identified and then ported to *explicit* half-crazy devres
voodoo.

More details here:
https://lore.kernel.org/all/20240613115032.29098-1-pstanner@xxxxxxxxxx/

P.

>
>