On Mon, Jul 20, 2015 at 03:55:16PM +0200, Joerg Roedel wrote:
> Hi Bjorn,
>
> On Fri, Jul 17, 2015 at 04:31:52PM -0500, Bjorn Helgaas wrote:
> > -static int ats_alloc_one(struct pci_dev *dev, int ps)
> > +static void ats_alloc_one(struct pci_dev *dev)
> > {
> > int pos;
> > u16 cap;
> > @@ -25,20 +25,17 @@ static int ats_alloc_one(struct pci_dev *dev, int ps)
> >
> > pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ATS);
> > if (!pos)
> > - return -ENODEV;
> > + return;
> >
> > ats = kzalloc(sizeof(*ats), GFP_KERNEL);
> > if (!ats)
> > - return -ENOMEM;
> > + return;
>
> I think we should print a warning here when the allocation fails.
> Otherwise the user wonders why ATS can't be used despite the available
> capability.
Good idea, done (although it gets removed right away in the next patch :))
>
> > - if (dev->is_physfn || dev->is_virtfn) {
> > - struct pci_dev *pdev = dev->is_physfn ? dev : dev->physfn;
> > -
> > - mutex_lock(&pdev->sriov->lock);
> > - if (pdev->ats)
> > - rc = pdev->ats->stu == ps ? 0 : -EINVAL;
> > - else
> > - rc = ats_alloc_one(pdev, ps);
> > + if (ps < PCI_ATS_MIN_STU)
> > + return -EINVAL;
> >
> > - if (!rc)
> > - pdev->ats->ref_cnt++;
> > - mutex_unlock(&pdev->sriov->lock);
> > - if (rc)
> > - return rc;
> > - }
> > + ctrl = PCI_ATS_CTRL_ENABLE;
> > + if (dev->is_virtfn) {
> > + struct pci_dev *pdev = dev->physfn;
> >
> > - if (!dev->is_physfn) {
> > - rc = ats_alloc_one(dev, ps);
> > - if (rc)
> > - return rc;
> > + if (pdev->ats->stu != ps)
> > + return -EINVAL;
> > + } else {
> > + dev->ats->stu = ps;
> > + ctrl |= PCI_ATS_CTRL_STU(dev->ats->stu - PCI_ATS_MIN_STU);
> > }
>
> Okay, enabling ATS for the virt_fn will now fail when it is not
> already enabled for the phys_fn. The drivers probe function, which might
> enable SR-IOV, runs after BUS_NOTIFY_ADD_DEVICE has finished, so this
> should be safe.
>
> But I think a comment explaining these dependencies would be good here.
Comment added, thanks!
Bjorn
--
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
