From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755139AbdFWURt (ORCPT ); Fri, 23 Jun 2017 16:17:49 -0400 Received: from mga03.intel.com ([134.134.136.65]:63581 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755122AbdFWURr (ORCPT ); Fri, 23 Jun 2017 16:17:47 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,380,1493708400"; d="scan'208";a="118179241" Date: Fri, 23 Jun 2017 13:19:23 -0700 From: Jacob Pan To: Alex Williamson Cc: iommu@lists.linux-foundation.org, LKML , Joerg Roedel , David Woodhouse , "Liu, Yi L" , Lan Tianyu , "Tian, Kevin" , Raj Ashok , Jean Delvare , jacob.jun.pan@linux.intel.com Subject: Re: [RFC 7/9] iommu/dmar: notify unrecoverable faults Message-ID: <20170623131923.2457d642@jacob-builder> In-Reply-To: <20170622165416.6ea718f1@w520.home> References: <1497478983-77580-1-git-send-email-jacob.jun.pan@linux.intel.com> <1497478983-77580-8-git-send-email-jacob.jun.pan@linux.intel.com> <20170622165416.6ea718f1@w520.home> Organization: OTC X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 22 Jun 2017 16:54:16 -0600 Alex Williamson wrote: > On Wed, 14 Jun 2017 15:23:01 -0700 > Jacob Pan wrote: > > > Currently, when device DMA faults are detected by IOMMU the fault > > reasons are printed but the offending device is not notified. > > This patch allows device drivers to be optionally notified for fault > > conditions when device specific handling is needed for more subtle > > processing, e.g. request with PASID transactions. > > > > Signed-off-by: Jacob Pan > > Signed-off-by: Ashok Raj > > --- > > drivers/iommu/dmar.c | 37 ++++++++++++++++++++++++++++++++++++- > > 1 file changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c > > index cbf7763..2c0b80d464 100644 > > --- a/drivers/iommu/dmar.c > > +++ b/drivers/iommu/dmar.c > > @@ -1587,11 +1587,43 @@ void dmar_msi_read(int irq, struct msi_msg > > *msg) raw_spin_unlock_irqrestore(&iommu->register_lock, flag); > > } > > > > +static int dmar_unrecov_fault_notify(u8 fault_reason, u16 > > source_id, > > + unsigned long long addr) > > +{ > > + int ret; > > + struct pci_dev *pdev; > > + struct iommu_fault_event *event; > > + > > + pdev = pci_get_bus_and_slot(source_id >> 8, source_id & > > 0xFF); > > + if (!pdev) > > + return -ENODEV; > > + pr_debug("Notify PCI device fault [%02x:%02x.%d]\n", > > + source_id >> 8, PCI_SLOT(source_id & 0xff), > > + PCI_FUNC(source_id & 0xff)); > > + event = kzalloc(sizeof(*event) + sizeof(fault_reason), > > GFP_KERNEL); > > + if (!event) > > + return -ENOMEM; > > Leaks pdev reference. > same as before. pci_get_bus_and_slot() does not do ref counting. > > + > > + pci_dev_get(pdev); > > + event->dev = &pdev->dev; > > + event->buf[0] = fault_reason; > > + event->addr = addr; > > + event->length = sizeof(fault_reason); > > + event->flags = IOMMU_FAULT_UNRECOV; > > + ret = iommu_fault_notifier_call_chain(event); > > + > > + pci_dev_put(pdev); > > + kfree(event); > > + > > + return ret; > > +} > > + > > static int dmar_fault_do_one(struct intel_iommu *iommu, int type, > > u8 fault_reason, u16 source_id, unsigned long long > > addr) { > > const char *reason; > > int fault_type; > > + int ret = 0; > > > > reason = dmar_get_fault_reason(fault_reason, &fault_type); > > > > @@ -1600,11 +1632,14 @@ static int dmar_fault_do_one(struct > > intel_iommu *iommu, int type, source_id >> 8, PCI_SLOT(source_id & > > 0xFF), PCI_FUNC(source_id & 0xFF), addr >> 48, > > fault_reason, reason); > > - else > > + else { > > pr_err("[%s] Request device [%02x:%02x.%d] fault > > addr %llx [fault reason %02d] %s\n", type ? "DMA Read" : "DMA > > Write", source_id >> 8, PCI_SLOT(source_id & 0xFF), > > PCI_FUNC(source_id & 0xFF), addr, > > fault_reason, reason); > > + ret = dmar_unrecov_fault_notify(fault_reason, > > source_id, addr); > > For what purpose are we recording this return code? > good catch. I will drop the return code. > > + } > > + > > return 0; > > } > > > [Jacob Pan]