From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joerg Roedel Subject: Re: [PATCH v2 08/11] PCI: Clean up ATS error handling Date: Mon, 27 Jul 2015 14:56:32 +0200 Message-ID: <20150727125632.GD27614@suse.de> References: <20150721001243.28145.81610.stgit@bhelgaas-glaptop2.roam.corp.google.com> <20150721001454.28145.35617.stgit@bhelgaas-glaptop2.roam.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20150721001454.28145.35617.stgit@bhelgaas-glaptop2.roam.corp.google.com> Sender: linux-pci-owner@vger.kernel.org To: Bjorn Helgaas Cc: linux-pci@vger.kernel.org, David Woodhouse , iommu@lists.linux-foundation.org, Gregor Dick List-Id: iommu@lists.linux-foundation.org On Mon, Jul 20, 2015 at 07:14:54PM -0500, Bjorn Helgaas wrote: > There's no need to BUG() if we enable ATS when it's already enabled. We > don't need to BUG() when disabling ATS on a device that doesn't support ATS > or if it's already disabled. If ATS is enabled, certainly we found an ATS > capability in the past, so it should still be there now. > > Clean up these error paths. > > Signed-off-by: Bjorn Helgaas > --- > drivers/pci/ats.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > index 0b5b0ed..67524a7 100644 > --- a/drivers/pci/ats.c > +++ b/drivers/pci/ats.c > @@ -44,11 +44,12 @@ int pci_enable_ats(struct pci_dev *dev, int ps) > u16 ctrl; > struct pci_dev *pdev; > > - BUG_ON(dev->ats_cap && dev->ats_enabled); > - > if (!dev->ats_cap) > return -EINVAL; > > + if (pci_ats_enabled(dev)) > + return -EINVAL; > + Better return -EBUSY here? It would be good to find out if we are trying to enable ATS while it is already enabled. A WARN_ON around this would also be good, so that we spot bugs in the iommu drivers early when they use this interface. Joerg