All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Cc: Yang Z Zhang <yang.z.zhang@intel.com>, Kevin Tian <kevin.tian@intel.com>
Subject: Re: [PATCH] VT-d: improve fault info logging
Date: Thu, 26 Mar 2015 10:08:13 +0000	[thread overview]
Message-ID: <5513DA8D.7060108@citrix.com> (raw)
In-Reply-To: <5513E597020000780006DCBE@mail.emea.novell.com>


[-- 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

  reply	other threads:[~2015-03-26 10:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-26  9:55 [PATCH] VT-d: improve fault info logging Jan Beulich
2015-03-26 10:08 ` Andrew Cooper [this message]
2015-03-27  0:35 ` Zhang, Yang Z

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5513DA8D.7060108@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=kevin.tian@intel.com \
    --cc=xen-devel@lists.xenproject.org \
    --cc=yang.z.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.