All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Cc: leedom-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org,
	herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org,
	David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Harsh-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org
Subject: Re: [PATCH] iommu/vt-d: Fix scatterlist offset handling
Date: Mon, 6 Nov 2017 10:47:09 -0800	[thread overview]
Message-ID: <20171106104709.06b38f7c@jacob-builder> (raw)
In-Reply-To: <20171006144309.GA30803-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>

On Fri, 6 Oct 2017 16:43:09 +0200
Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote:

> On Tue, Oct 03, 2017 at 07:05:17PM +0100, Robin Murphy wrote:
> > Now, there are indeed plenty of drivers and subsystems which do
> > work on lists of explicitly single pages - anything doing some
> > variant of "addr = kmap_atomic(sg_page(sg)) + sg->offset;" is easy
> > to spot - but I don't think DMA API implementations are in a
> > position to make any kind of assumption; nearly all of them just
> > shut up and handle sg->length bytes from sg_phys(sg) without
> > questioning the caller, and I reckon that's exactly what they
> > should be doing.  
> 
> I agree with that, it is not explicitly forbidden to have an
> sg->offset > PAGE_SIZE and most IOMMU drivers handle this case.
> 
> So this is a problem I'd like to see resolved in the VT-d driver too.
> If nobody comes up with a correct fix soon I'll apply this one and
> rip out the large-page support from __domain_mapping() to make it
> work.
> 
Hi All,

Just to give an update on the offline debugging of this issue. With
Robin's patch applied, I was able to reproduce the failure with
similar configuration that Jain helped to set up.

I added trace prints just to see the map/unmap activities leading to
the DMAR fault. When fault occurs, the trace shows there is an unmap to
the offending iova pfn. So I think this is a separate problem than
Robin's patch is fixing. I think we should move forward to merge this
patch upstream and stable. The remaining problem is likely a race
condition between unmap and DMA activities.

Here a brief extracted log, ee3d7 is the iova pfn in question.
#1. map sg pfn ee3d7
          <idle>-0     [076] 74124.154254: bprint:               __domain_mapping: vpfn:ee3d7, pgoff=2126, np:1, da:ee3d784e, len:1464
, ppfn:1849c9c                                                                                                                        

#2. unmap ee3d7000
         <idle>-0     [054] 74124.154301: bprint:               intel_unmap: Device 0000:18:00.4 unmapping: pfn ee3d7-ee3d7          
         <idle>-0     [076] 74124.154301: bprint:               __domain_mapping: lvlpg:1, nrpg 0, vpfn:ec2ff, ppfn:183221a, sg_res:0
         <idle>-0     [059] 74124.154302: bprint:               __domain_mapping: lvlpg:1, nrpg 0, vpfn:ee719, ppfn:c3e4dd, sg_res:0 
         <idle>-0     [076] 74124.154302: bprint:               __domain_mapping: vpfn:f183b, pgoff=78, np:1, da:f183b04e, len:1464, 

#3. DMA to unmapped address ee3d7000, DMAR fault raised.
  +2.952861] dmar_fault: 6 callbacks suppressed                                                                                   
  +0.000002] DMAR: DRHD: handling fault status reg 2                                                                              
  +0.005588] turning tracing off                                                                                                  
  +0.003592] DMAR: [DMA Write] Request device [18:00.4] fault addr ee3d7000 [fault reason 05] PTE Write access is not set         
                                                                                                                                  
                                                                                                                                                                                                                                                               
         <idle>-0     [000] 74124.156906: bputs:
         0xffffffffb259916bs: turning tracing off     


Thanks,

Jacob

> Speaking of __domain_mapping(), this function is a big unmaintainable
> mess which should be split and rewritten. A clean and maintainable
> rewrite can alse re-add the large-page support.
> 
> 
> Regards,
> 
> 	Joerg
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

[Jacob Pan]

WARNING: multiple messages have this Message-ID (diff)
From: Jacob Pan <jacob.jun.pan@linux.intel.com>
To: Joerg Roedel <joro@8bytes.org>
Cc: Robin Murphy <robin.murphy@arm.com>,
	leedom@chelsio.com, herbert@gondor.apana.org.au,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org,
	David Woodhouse <dwmw2@infradead.org>,
	Harsh@chelsio.com, jacob.jun.pan@linux.intel.com,
	Alex Williamson <alex.williamson@redhat.com>
Subject: Re: [PATCH] iommu/vt-d: Fix scatterlist offset handling
Date: Mon, 6 Nov 2017 10:47:09 -0800	[thread overview]
Message-ID: <20171106104709.06b38f7c@jacob-builder> (raw)
In-Reply-To: <20171006144309.GA30803@8bytes.org>

On Fri, 6 Oct 2017 16:43:09 +0200
Joerg Roedel <joro@8bytes.org> wrote:

> On Tue, Oct 03, 2017 at 07:05:17PM +0100, Robin Murphy wrote:
> > Now, there are indeed plenty of drivers and subsystems which do
> > work on lists of explicitly single pages - anything doing some
> > variant of "addr = kmap_atomic(sg_page(sg)) + sg->offset;" is easy
> > to spot - but I don't think DMA API implementations are in a
> > position to make any kind of assumption; nearly all of them just
> > shut up and handle sg->length bytes from sg_phys(sg) without
> > questioning the caller, and I reckon that's exactly what they
> > should be doing.  
> 
> I agree with that, it is not explicitly forbidden to have an
> sg->offset > PAGE_SIZE and most IOMMU drivers handle this case.
> 
> So this is a problem I'd like to see resolved in the VT-d driver too.
> If nobody comes up with a correct fix soon I'll apply this one and
> rip out the large-page support from __domain_mapping() to make it
> work.
> 
Hi All,

Just to give an update on the offline debugging of this issue. With
Robin's patch applied, I was able to reproduce the failure with
similar configuration that Jain helped to set up.

I added trace prints just to see the map/unmap activities leading to
the DMAR fault. When fault occurs, the trace shows there is an unmap to
the offending iova pfn. So I think this is a separate problem than
Robin's patch is fixing. I think we should move forward to merge this
patch upstream and stable. The remaining problem is likely a race
condition between unmap and DMA activities.

Here a brief extracted log, ee3d7 is the iova pfn in question.
#1. map sg pfn ee3d7
          <idle>-0     [076] 74124.154254: bprint:               __domain_mapping: vpfn:ee3d7, pgoff=2126, np:1, da:ee3d784e, len:1464
, ppfn:1849c9c                                                                                                                        

#2. unmap ee3d7000
         <idle>-0     [054] 74124.154301: bprint:               intel_unmap: Device 0000:18:00.4 unmapping: pfn ee3d7-ee3d7          
         <idle>-0     [076] 74124.154301: bprint:               __domain_mapping: lvlpg:1, nrpg 0, vpfn:ec2ff, ppfn:183221a, sg_res:0
         <idle>-0     [059] 74124.154302: bprint:               __domain_mapping: lvlpg:1, nrpg 0, vpfn:ee719, ppfn:c3e4dd, sg_res:0 
         <idle>-0     [076] 74124.154302: bprint:               __domain_mapping: vpfn:f183b, pgoff=78, np:1, da:f183b04e, len:1464, 

#3. DMA to unmapped address ee3d7000, DMAR fault raised.
  +2.952861] dmar_fault: 6 callbacks suppressed                                                                                   
  +0.000002] DMAR: DRHD: handling fault status reg 2                                                                              
  +0.005588] turning tracing off                                                                                                  
  +0.003592] DMAR: [DMA Write] Request device [18:00.4] fault addr ee3d7000 [fault reason 05] PTE Write access is not set         
                                                                                                                                  
                                                                                                                                                                                                                                                               
         <idle>-0     [000] 74124.156906: bputs:
         0xffffffffb259916bs: turning tracing off     


Thanks,

Jacob

> Speaking of __domain_mapping(), this function is a big unmaintainable
> mess which should be split and rewritten. A clean and maintainable
> rewrite can alse re-add the large-page support.
> 
> 
> Regards,
> 
> 	Joerg
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

[Jacob Pan]

  parent reply	other threads:[~2017-11-06 18:47 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-28 14:14 [PATCH] iommu/vt-d: Fix scatterlist offset handling Robin Murphy
2017-09-28 16:17 ` Casey Leedom
2017-09-28 13:29   ` Raj, Ashok
2017-09-28 16:59     ` Robin Murphy
2017-09-28 15:43       ` Raj, Ashok
2017-10-03 19:36         ` Raj, Ashok
2017-09-29  8:14 ` Harsh Jain
     [not found]   ` <fe25071a-18bf-e468-01e7-36515f2110e2-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
2017-09-29 16:18     ` Casey Leedom
2017-09-29 16:18       ` Casey Leedom
     [not found]       ` <MWHPR12MB160034E91A834504FE85C07BC87E0-Gy0DoCVfaSVsWITs4OkDoAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-10-03 12:22         ` Harsh Jain
2017-10-03 22:22           ` Casey Leedom
     [not found] ` <644c3e01654f8bd48d669c36e424959d6ef0e27e.1506607370.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2017-10-03 12:55   ` David Woodhouse
2017-10-03 12:55     ` David Woodhouse
2017-10-03 18:05     ` Robin Murphy
2017-10-03 22:16       ` David Woodhouse
2017-10-04 11:18         ` Robin Murphy
2017-10-06 14:43       ` Joerg Roedel
2017-10-06 12:54         ` Raj, Ashok
     [not found]         ` <20171006144309.GA30803-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2017-11-06 18:47           ` Jacob Pan [this message]
2017-11-06 18:47             ` Jacob Pan
2017-11-15 23:54             ` Jacob Pan
2017-11-15 23:54               ` Jacob Pan
2017-11-16 21:32               ` Alex Williamson
     [not found]                 ` <20171116143244.2583d044-1yVPhWWZRC1BDLzU/O5InQ@public.gmane.org>
2017-11-16 21:09                   ` Raj, Ashok
2017-11-16 21:09                     ` Raj, Ashok
2017-11-17 16:18                     ` Alex Williamson
2017-11-17 16:18                       ` Alex Williamson
2017-11-17 15:48                       ` Raj, Ashok
2017-11-17 17:44                         ` Casey Leedom
2017-11-17 17:44                           ` Casey Leedom
2017-11-17 17:44                           ` Casey Leedom
     [not found]                           ` <SN1PR12MB035214EF471935B6F4220E36C82F0-z7L1TMIYDg4e2a8M8f4RFAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-11-17 18:09                             ` Jacob Pan
2017-11-17 18:09                               ` Jacob Pan

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=20171106104709.06b38f7c@jacob-builder \
    --to=jacob.jun.pan-vuqaysv1563yd54fqh9/ca@public.gmane.org \
    --cc=Harsh-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org \
    --cc=dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org \
    --cc=leedom-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org \
    --cc=linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /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.