Re: [PATCH 1/5] pci: Add PCI DMA source ID quirk

From: Alex Williamson
Date: Wed May 30 2012 - 14:05:44 EST


On Wed, 2012-05-30 at 21:27 +0800, Jiang Liu wrote:
> Hi Alex,
> pci_get_slot() will hold a reference count on the returned
> pci device, so need to release the reference count for balance.
> According to patches "[PATCH 5/7] amd_iommu: Make use of DMA quirks
> and ACS checks in IOMMU groups" and "[PATCH 6/7] intel-iommu: Make
> use of DMA quirks and ACS checks in IOMMU groups", current patches
> failed to release the reference count gained by pci_get_slot().

Thanks for pointing this out Gerry. I think I'll rename
pci_dma_source() to pci_get_dma_source() which will then increment the
reference of the device it returns, whether it's a quirk or the original
device. The caller will then be responsible for doing a pci_dev_put().
Thanks,

Alex

> On 05/30/2012 12:48 PM, Alex Williamson wrote:
> > DMA transactions are tagged with the source ID of the device making
> > the request. Occasionally hardware screws this up and uses the
> > source ID of a different device (often the wrong function number of
> > a multifunction device). A specific Ricoh multifunction device is
> > a prime example of this problem and included in this patch. The
> > purpose of this function is that given a pci_dev, return the pci_dev
> > to use as the source ID for DMA. When hardware works correctly,
> > this returns the input device. For the components of the Ricoh
> > multifunction device, return the pci_dev for function 0.
> >
> > This will be used by IOMMU drivers for determining the boundaries
> > of IOMMU groups as multiple devices using the same source ID must
> > be contained within the same group. This can also be used by
> > existing streaming DMA paths for the same purpose.
> >
> > Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > ---
> >
> > drivers/pci/quirks.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > include/linux/pci.h | 5 +++++
> > 2 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 2a75216..004e167 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -3179,3 +3179,43 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
> >
> > return -ENOTTY;
> > }
> > +
> > +static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
> > +{
> > + return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
> > +}
> > +
> > +static const struct pci_dev_dma_source {
> > + u16 vendor;
> > + u16 device;
> > + struct pci_dev *(*dma_source)(struct pci_dev *dev);
> > +} pci_dev_dma_source[] = {
> > + /*
> > + * https://bugzilla.redhat.com/show_bug.cgi?id=605888
> > + *
> > + * Some Ricoh devices use the function 0 source ID for DMA on
> > + * other functions of a multifunction device. The DMA devices
> > + * is therefore function 0, which will have implications of the
> > + * iommu grouping of these devices.
> > + */
> > + { PCI_VENDOR_ID_RICOH, 0xe822, pci_func_0_dma_source },
> > + { PCI_VENDOR_ID_RICOH, 0xe230, pci_func_0_dma_source },
> > + { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
> > + { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
> > + { 0 }
> > +};
> > +
> > +struct pci_dev *pci_dma_source(struct pci_dev *dev)
> > +{
> > + const struct pci_dev_dma_source *i;
> > +
> > + for (i = pci_dev_dma_source; i->dma_source; i++) {
> > + if ((i->vendor == dev->vendor ||
> > + i->vendor == (u16)PCI_ANY_ID) &&
> > + (i->device == dev->device ||
> > + i->device == (u16)PCI_ANY_ID))
> > + return i->dma_source(dev);
> > + }
> > +
> > + return dev;
> > +}
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index d8c379d..5bc7502 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1486,9 +1486,14 @@ enum pci_fixup_pass {
> >
> > #ifdef CONFIG_PCI_QUIRKS
> > void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
> > +struct pci_dev *pci_dma_source(struct pci_dev *dev);
> > #else
> > static inline void pci_fixup_device(enum pci_fixup_pass pass,
> > struct pci_dev *dev) {}
> > +static inline struct pci_dev *pci_dma_source(struct pci_dev *dev)
> > +{
> > + return dev;
> > +}
> > #endif
> >
> > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/