* [PATCH] VT-d: improve fault info logging
@ 2015-03-26 9:55 Jan Beulich
2015-03-26 10:08 ` Andrew Cooper
2015-03-27 0:35 ` Zhang, Yang Z
0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2015-03-26 9:55 UTC (permalink / raw)
To: xen-devel; +Cc: Yang Z Zhang, Kevin Tian
[-- Attachment #1: Type: text/plain, Size: 4314 bytes --]
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>
--- 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)
[-- Attachment #2: VT-d-fault-log.patch --]
[-- Type: text/plain, Size: 4346 bytes --]
VT-d: improve fault info logging
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>
--- 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)
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] VT-d: improve fault info logging
2015-03-26 9:55 [PATCH] VT-d: improve fault info logging Jan Beulich
@ 2015-03-26 10:08 ` Andrew Cooper
2015-03-27 0:35 ` Zhang, Yang Z
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2015-03-26 10:08 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Yang Z Zhang, Kevin Tian
[-- Attachment #1.1: Type: text/plain, Size: 4740 bytes --]
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
[-- Attachment #1.2: Type: text/html, Size: 5590 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] VT-d: improve fault info logging
2015-03-26 9:55 [PATCH] VT-d: improve fault info logging Jan Beulich
2015-03-26 10:08 ` Andrew Cooper
@ 2015-03-27 0:35 ` Zhang, Yang Z
1 sibling, 0 replies; 3+ messages in thread
From: Zhang, Yang Z @ 2015-03-27 0:35 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Tian, Kevin
Jan Beulich wrote on 2015-03-26:
> 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>
Acked-by: Yang Zhang <yang.z.zhang@intel.com>
>
> --- 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)
Best regards,
Yang
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-03-27 0:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-26 9:55 [PATCH] VT-d: improve fault info logging Jan Beulich
2015-03-26 10:08 ` Andrew Cooper
2015-03-27 0:35 ` Zhang, Yang Z
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.