From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] VT-d: improve fault info logging Date: Thu, 26 Mar 2015 10:08:13 +0000 Message-ID: <5513DA8D.7060108@citrix.com> References: <5513E597020000780006DCBE@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1142068884107916728==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Yb4i4-0006WM-Pn for xen-devel@lists.xenproject.org; Thu, 26 Mar 2015 10:08:21 +0000 In-Reply-To: <5513E597020000780006DCBE@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Yang Z Zhang , Kevin Tian List-Id: xen-devel@lists.xenproject.org --===============1142068884107916728== Content-Type: multipart/alternative; boundary="------------010305020203040305090407" --------------010305020203040305090407 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit On 26/03/15 09:55, Jan Beulich wrote: > I got repeatedly annoyed by there not getting anything logged by > default on VT-d faults (and hence having to tell people to add extra > command line options), and hence I think it is time to redo this code: > Log basic fault information at guest-warning level (rate limited by > default), and show the page walk in verbose rather than only in debug > mode. Break up multi-line message so that each gets a proper log level > attached, at once splitting out the common part. Also don't log > "unknown" faults as interrupt-remapping ones. > > As a minor cleanup fix the type of the involved "fault_type" variables. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper (I have had a similar wish to improve this logging in some copious free time) > > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -800,7 +800,8 @@ static const char *intr_remap_fault_reas > "Blocked an interrupt request due to source-id verification failure", > }; > > -static const char *iommu_get_fault_reason(u8 fault_reason, int *fault_type) > +static const char *iommu_get_fault_reason(u8 fault_reason, > + enum faulttype *fault_type) > { > if ( fault_reason >= 0x20 && ( fault_reason < 0x20 + > ARRAY_SIZE(intr_remap_fault_reasons)) ) > @@ -823,35 +824,48 @@ static const char *iommu_get_fault_reaso > static int iommu_page_fault_do_one(struct iommu *iommu, int type, > u8 fault_reason, u16 source_id, u64 addr) > { > - const char *reason; > - int fault_type; > + const char *reason, *kind; > + enum faulttype fault_type; > u16 seg = iommu->intel->drhd->segment; > - reason = iommu_get_fault_reason(fault_reason, &fault_type); > > - if ( fault_type == DMA_REMAP ) > + reason = iommu_get_fault_reason(fault_reason, &fault_type); > + switch ( fault_type ) > { > - INTEL_IOMMU_DEBUG( > - "DMAR:[%s] Request device [%04x:%02x:%02x.%u] " > - "fault addr %"PRIx64", iommu reg = %p\n" > - "DMAR:[fault reason %02xh] %s\n", > - (type ? "DMA Read" : "DMA Write"), > - seg, (source_id >> 8), PCI_SLOT(source_id & 0xFF), > - PCI_FUNC(source_id & 0xFF), addr, iommu->reg, > - fault_reason, reason); > - if (iommu_debug) > - print_vtd_entries(iommu, (source_id >> 8), > - (source_id & 0xff), (addr >> PAGE_SHIFT)); > + case DMA_REMAP: > + printk(XENLOG_G_WARNING VTDPREFIX > + "DMAR:[%s] Request device [%04x:%02x:%02x.%u] " > + "fault addr %"PRIx64", iommu reg = %p\n", > + (type ? "DMA Read" : "DMA Write"), > + seg, PCI_BUS(source_id), PCI_SLOT(source_id), > + PCI_FUNC(source_id), addr, iommu->reg); > + kind = "DMAR"; > + break; > + case INTR_REMAP: > + printk(XENLOG_G_WARNING VTDPREFIX > + "INTR-REMAP: Request device [%04x:%02x:%02x.%u] " > + "fault index %"PRIx64", iommu reg = %p\n", > + seg, PCI_BUS(source_id), PCI_SLOT(source_id), > + PCI_FUNC(source_id), addr >> 48, iommu->reg); > + kind = "INTR-REMAP"; > + break; > + default: > + printk(XENLOG_G_WARNING VTDPREFIX > + "UNKNOWN: Request device [%04x:%02x:%02x.%u] " > + "fault addr %"PRIx64", iommu reg = %p\n", > + seg, PCI_BUS(source_id), PCI_SLOT(source_id), > + PCI_FUNC(source_id), addr, iommu->reg); > + kind = "UNKNOWN"; > + break; > } > - else > - INTEL_IOMMU_DEBUG( > - "INTR-REMAP: Request device [%04x:%02x:%02x.%u] " > - "fault index %"PRIx64", iommu reg = %p\n" > - "INTR-REMAP:[fault reason %02xh] %s\n", > - seg, (source_id >> 8), PCI_SLOT(source_id & 0xFF), > - PCI_FUNC(source_id & 0xFF), addr >> 48, iommu->reg, > - fault_reason, reason); > - return 0; > > + gprintk(XENLOG_G_WARNING VTDPREFIX, "%s: reason %02x - %s\n", > + kind, fault_reason, reason); > + > + if ( iommu_verbose && fault_type == DMA_REMAP ) > + print_vtd_entries(iommu, PCI_BUS(source_id), PCI_DEVFN2(source_id), > + addr >> PAGE_SHIFT); > + > + return 0; > } > > static void iommu_fault_status(u32 fault_status) > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------010305020203040305090407 Content-Type: text/html; charset="windows-1252" Content-Transfer-Encoding: 7bit
On 26/03/15 09:55, Jan Beulich wrote:
I got repeatedly annoyed by there not getting anything logged by
default on VT-d faults (and hence having to tell people to add extra
command line options), and hence I think it is time to redo this code:
Log basic fault information at guest-warning level (rate limited by
default), and show the page walk in verbose rather than only in debug
mode. Break up multi-line message so that each gets a proper log level
attached, at once splitting out the common part. Also don't log
"unknown" faults as interrupt-remapping ones.

As a minor cleanup fix the type of the involved "fault_type" variables.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

(I have had a similar wish to improve this logging in some copious free time)


--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -800,7 +800,8 @@ static const char *intr_remap_fault_reas
     "Blocked an interrupt request due to source-id verification failure",
 };
 
-static const char *iommu_get_fault_reason(u8 fault_reason, int *fault_type)
+static const char *iommu_get_fault_reason(u8 fault_reason,
+                                          enum faulttype *fault_type)
 {
     if ( fault_reason >= 0x20 && ( fault_reason < 0x20 +
                 ARRAY_SIZE(intr_remap_fault_reasons)) )
@@ -823,35 +824,48 @@ static const char *iommu_get_fault_reaso
 static int iommu_page_fault_do_one(struct iommu *iommu, int type,
                                    u8 fault_reason, u16 source_id, u64 addr)
 {
-    const char *reason;
-    int fault_type;
+    const char *reason, *kind;
+    enum faulttype fault_type;
     u16 seg = iommu->intel->drhd->segment;
-    reason = iommu_get_fault_reason(fault_reason, &fault_type);
 
-    if ( fault_type == DMA_REMAP )
+    reason = iommu_get_fault_reason(fault_reason, &fault_type);
+    switch ( fault_type )
     {
-        INTEL_IOMMU_DEBUG(
-                "DMAR:[%s] Request device [%04x:%02x:%02x.%u] "
-                "fault addr %"PRIx64", iommu reg = %p\n"
-                "DMAR:[fault reason %02xh] %s\n",
-                (type ? "DMA Read" : "DMA Write"),
-                seg, (source_id >> 8), PCI_SLOT(source_id & 0xFF),
-                PCI_FUNC(source_id & 0xFF), addr, iommu->reg,
-                fault_reason, reason);
-	if (iommu_debug)
-            print_vtd_entries(iommu, (source_id >> 8),
-                          (source_id & 0xff), (addr >> PAGE_SHIFT));
+    case DMA_REMAP:
+        printk(XENLOG_G_WARNING VTDPREFIX
+               "DMAR:[%s] Request device [%04x:%02x:%02x.%u] "
+               "fault addr %"PRIx64", iommu reg = %p\n",
+               (type ? "DMA Read" : "DMA Write"),
+               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
+               PCI_FUNC(source_id), addr, iommu->reg);
+        kind = "DMAR";
+        break;
+    case INTR_REMAP:
+        printk(XENLOG_G_WARNING VTDPREFIX
+               "INTR-REMAP: Request device [%04x:%02x:%02x.%u] "
+               "fault index %"PRIx64", iommu reg = %p\n",
+               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
+               PCI_FUNC(source_id), addr >> 48, iommu->reg);
+        kind = "INTR-REMAP";
+        break;
+    default:
+        printk(XENLOG_G_WARNING VTDPREFIX
+               "UNKNOWN: Request device [%04x:%02x:%02x.%u] "
+               "fault addr %"PRIx64", iommu reg = %p\n",
+               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
+               PCI_FUNC(source_id), addr, iommu->reg);
+        kind = "UNKNOWN";
+        break;
     }
-    else
-        INTEL_IOMMU_DEBUG(
-                "INTR-REMAP: Request device [%04x:%02x:%02x.%u] "
-                "fault index %"PRIx64", iommu reg = %p\n"
-                "INTR-REMAP:[fault reason %02xh] %s\n",
-                seg, (source_id >> 8), PCI_SLOT(source_id & 0xFF),
-                PCI_FUNC(source_id & 0xFF), addr >> 48, iommu->reg,
-                fault_reason, reason);
-    return 0;
 
+    gprintk(XENLOG_G_WARNING VTDPREFIX, "%s: reason %02x - %s\n",
+            kind, fault_reason, reason);
+
+    if ( iommu_verbose && fault_type == DMA_REMAP )
+        print_vtd_entries(iommu, PCI_BUS(source_id), PCI_DEVFN2(source_id),
+                          addr >> PAGE_SHIFT);
+
+    return 0;
 }
 
 static void iommu_fault_status(u32 fault_status)





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------010305020203040305090407-- --===============1142068884107916728== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============1142068884107916728==--