On Sat, Jan 30, 2010 at 12:54:20PM -0800, Yinghai Lu wrote:
> On 01/17/2010 01:01 PM, Matthew Wilcox wrote:
> > Make pci_scan_slot more robust
> >
> > Yinghai pointed out that the new pci_scan_slot() crashes when called
> > on an ARI-capable slot that is empty. Fix this by exiting early from
> > pci_scan_slot if there is no device in the slot.
> >
> > Also make next_ari_func() robust against devices not existing in case
> > the ARI capability is corrupt. ARI also requires that the devices be
> > listed in order, so if we find a function listed that is out of order,
> > stop scanning to prevent loops.
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 11824d7..a06887f 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1222,11 +1222,19 @@ EXPORT_SYMBOL(pci_scan_single_device);
> > static unsigned next_ari_fn(struct pci_dev *dev, unsigned fn)
> > {
> > u16 cap;
> > - unsigned pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
> > + unsigned pos, next_fn;
> > +
> > + if (!dev)
> > + return 0;
>
> this patch should work too because of checking here.
This should never trigger for your case, because it would be caught ...
> > +
> > + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
> > if (!pos)
> > return 0;
> > pci_read_config_word(dev, pos + 4, &cap);
> > - return cap >> 8;
> > + next_fn = cap >> 8;
> > + if (next_fn <= fn)
> > + return 0;
> > + return next_fn;
> > }
> >
> > static unsigned next_trad_fn(struct pci_dev *dev, unsigned fn)
> > @@ -1271,12 +1279,14 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
> > return 0; /* Already scanned the entire slot */
> >
> > dev = pci_scan_single_device(bus, devfn);
> > - if (dev && !dev->is_added) /* new device? */
> > + if (!dev)
> > + return 0;
... down here. The check up there is for the case where a function is
mistakenly listed in the ARI capability that doesn't exist.
> > + if (!dev->is_added)
> > nr++;
> >
> > if (pci_ari_enabled(bus))
> > next_fn = next_ari_fn;
> > - else if (dev && dev->multifunction)
> > + else if (dev->multifunction)
> > next_fn = next_trad_fn;
> >
> > for (fn = next_fn(dev, 0); fn > 0; fn = next_fn(dev, fn)) {
> >
>
> YH
> --
> 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
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
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
