All of lore.kernel.org
 help / color / mirror / Atom feed
From: mst@redhat.com (Michael S. Tsirkin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] virtio: Try to untangle DMA coherency
Date: Thu, 2 Feb 2017 18:16:38 +0200	[thread overview]
Message-ID: <20170202181357-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <bb041fbe-3fde-0dc1-ae28-cca5acfe9dc5@arm.com>

On Thu, Feb 02, 2017 at 01:34:03PM +0000, Robin Murphy wrote:
> On 02/02/17 11:26, Will Deacon wrote:
> > On Wed, Feb 01, 2017 at 09:19:22PM +0200, Michael S. Tsirkin wrote:
> >> On Wed, Feb 01, 2017 at 06:27:09PM +0000, Will Deacon wrote:
> >>> On Wed, Feb 01, 2017 at 08:09:21PM +0200, Michael S. Tsirkin wrote:
> >>>> I'd like to do that instead. It's fastboot doing the unreasonable thing
> >>>> here and deviating from what every other legacy device without exception
> >>>> did for years. If this means fastboot will need to update to virtio 1,
> >>>> all the better.
> >>>
> >>> The problem still exists with virtio 1, unless we require that the
> >>> "dma-coherent" property is set/unset correctly when VIRTIO_F_IOMMU_PLATFORM
> >>> is advertised by the device (which is what I suggested in my reply).
> >>
> >> I'm not ignoring that, but I need to understand that part a bit better.
> >> I'll reply to that patch in a day or two after looking at how _CCA is
> >> supposed to work.
> > 
> > Thanks. I do think that whatever solution we come up with for virtio 1
> > should influence what we do for legacy.
> > 
> >>> We can't detect the fastmodel,
> >>
> >> Surely, it puts a hardware id somewhere? I think you mean
> >> fastmodel isn't always affected, right?
> > 
> > I don't think there's a hardware ID. The thing is, the fastmodel is a
> > toolkit for building all sorts of platforms: you can chop and change
> > the CPUs, the peripherals, the memory, the interrupt controller, the
> > interconnect etc. Pretty much everything can be customised. So, for
> > any fastmodel configuration that places virtio upstream of the SMMU
> > (which is common, because virtio is one of the few DMA-capable peripherals
> > that the fastmodel supports), we need to do something special.
> > 
> >> I'd like to see basically
> >>
> >> if (fastmodel)
> >> 	a pile of special work-arounds
> >> else
> >> 	not less hacky but more common virtio work-arounds
> >>
> >> :)
> >>
> >> And then I can apply whatever comes from @arm.com and not
> >> worry about breaking actual hardware.
> > 
> > What we could do is call iommu_group_get(&vdev->dev) for legacy
> 
> Actually, that should be vdev->dev.parent - I'm now not sure quite what
> I managed to successfully test yesterday, but apparently it wasn't this
> patch :(
> 
> > devices if CONFIG_ARM64. If that returns non-NULL, then we know that
> > the device is upstream of an SMMU, which means it must be the fastmodel.
> 
> We can boot 32-bit kernels on models, so I'd be inclined to keep
> CONFIG_ARM included, but I do tend to agree that explicitly checking for
> an IOMMU is probably the cleanest approach if we reposition this as a
> more specific quirk. I'll split apart the "Fast Models are wacky" vs.
> "uh-oh device coherency" aspects and spin a v2 so that we can
> (hopefully) sort the regression out ASAP.
> 
> Robin.
> 
> > 
> > Will
> > 

I still think the right thing to do for this platform is to add an API
for driver to say "disable protection for this device and allow
this device 1:1 access to all memory".  This
would make both platforms which bypass the iommu and platforms that
don't happy as PA == dma address.

-- 
MST

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org
Subject: Re: [PATCH] virtio: Try to untangle DMA coherency
Date: Thu, 2 Feb 2017 18:16:38 +0200	[thread overview]
Message-ID: <20170202181357-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <bb041fbe-3fde-0dc1-ae28-cca5acfe9dc5-5wv7dgnIgG8@public.gmane.org>

On Thu, Feb 02, 2017 at 01:34:03PM +0000, Robin Murphy wrote:
> On 02/02/17 11:26, Will Deacon wrote:
> > On Wed, Feb 01, 2017 at 09:19:22PM +0200, Michael S. Tsirkin wrote:
> >> On Wed, Feb 01, 2017 at 06:27:09PM +0000, Will Deacon wrote:
> >>> On Wed, Feb 01, 2017 at 08:09:21PM +0200, Michael S. Tsirkin wrote:
> >>>> I'd like to do that instead. It's fastboot doing the unreasonable thing
> >>>> here and deviating from what every other legacy device without exception
> >>>> did for years. If this means fastboot will need to update to virtio 1,
> >>>> all the better.
> >>>
> >>> The problem still exists with virtio 1, unless we require that the
> >>> "dma-coherent" property is set/unset correctly when VIRTIO_F_IOMMU_PLATFORM
> >>> is advertised by the device (which is what I suggested in my reply).
> >>
> >> I'm not ignoring that, but I need to understand that part a bit better.
> >> I'll reply to that patch in a day or two after looking at how _CCA is
> >> supposed to work.
> > 
> > Thanks. I do think that whatever solution we come up with for virtio 1
> > should influence what we do for legacy.
> > 
> >>> We can't detect the fastmodel,
> >>
> >> Surely, it puts a hardware id somewhere? I think you mean
> >> fastmodel isn't always affected, right?
> > 
> > I don't think there's a hardware ID. The thing is, the fastmodel is a
> > toolkit for building all sorts of platforms: you can chop and change
> > the CPUs, the peripherals, the memory, the interrupt controller, the
> > interconnect etc. Pretty much everything can be customised. So, for
> > any fastmodel configuration that places virtio upstream of the SMMU
> > (which is common, because virtio is one of the few DMA-capable peripherals
> > that the fastmodel supports), we need to do something special.
> > 
> >> I'd like to see basically
> >>
> >> if (fastmodel)
> >> 	a pile of special work-arounds
> >> else
> >> 	not less hacky but more common virtio work-arounds
> >>
> >> :)
> >>
> >> And then I can apply whatever comes from @arm.com and not
> >> worry about breaking actual hardware.
> > 
> > What we could do is call iommu_group_get(&vdev->dev) for legacy
> 
> Actually, that should be vdev->dev.parent - I'm now not sure quite what
> I managed to successfully test yesterday, but apparently it wasn't this
> patch :(
> 
> > devices if CONFIG_ARM64. If that returns non-NULL, then we know that
> > the device is upstream of an SMMU, which means it must be the fastmodel.
> 
> We can boot 32-bit kernels on models, so I'd be inclined to keep
> CONFIG_ARM included, but I do tend to agree that explicitly checking for
> an IOMMU is probably the cleanest approach if we reposition this as a
> more specific quirk. I'll split apart the "Fast Models are wacky" vs.
> "uh-oh device coherency" aspects and spin a v2 so that we can
> (hopefully) sort the regression out ASAP.
> 
> Robin.
> 
> > 
> > Will
> > 

I still think the right thing to do for this platform is to add an API
for driver to say "disable protection for this device and allow
this device 1:1 access to all memory".  This
would make both platforms which bypass the iommu and platforms that
don't happy as PA == dma address.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2017-02-02 16:16 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-01 12:25 [PATCH] virtio: Try to untangle DMA coherency Robin Murphy
2017-02-01 12:25 ` Robin Murphy
2017-02-01 14:57 ` Will Deacon
2017-02-01 14:57   ` Will Deacon
2017-02-01 17:58   ` Rob Herring
2017-02-01 17:58     ` Rob Herring
2017-02-01 17:58   ` Rob Herring
2017-02-01 18:09 ` Michael S. Tsirkin
2017-02-01 18:09   ` Michael S. Tsirkin
2017-02-01 18:27   ` Will Deacon
2017-02-01 18:27     ` Will Deacon
2017-02-01 18:27     ` Will Deacon
2017-02-01 19:19     ` Michael S. Tsirkin
2017-02-01 19:19       ` Michael S. Tsirkin
2017-02-02 11:26       ` Will Deacon
2017-02-02 11:26       ` Will Deacon
2017-02-02 11:26         ` Will Deacon
2017-02-02 13:34         ` Robin Murphy
2017-02-02 13:34           ` Robin Murphy
2017-02-02 16:16           ` Michael S. Tsirkin [this message]
2017-02-02 16:16             ` Michael S. Tsirkin
2017-02-02 16:39             ` Marc Zyngier
2017-02-02 16:39             ` Marc Zyngier
2017-02-02 16:39               ` Marc Zyngier
2017-02-02 16:44               ` Michael S. Tsirkin
2017-02-02 16:44               ` Michael S. Tsirkin
2017-02-02 16:44                 ` Michael S. Tsirkin
2017-02-02 16:16           ` Michael S. Tsirkin
2017-02-02 16:30           ` Michael S. Tsirkin
2017-02-02 16:30             ` Michael S. Tsirkin
2017-02-02 16:40             ` Will Deacon
2017-02-02 16:40               ` Will Deacon
2017-02-02 16:45               ` Michael S. Tsirkin
2017-02-02 16:45                 ` Michael S. Tsirkin
2017-02-02 16:45               ` Michael S. Tsirkin
2017-02-09 18:17               ` Michael S. Tsirkin
2017-02-09 18:17                 ` Michael S. Tsirkin
2017-02-09 18:31                 ` Will Deacon
2017-02-09 18:31                   ` Will Deacon
2017-02-09 18:49                   ` Michael S. Tsirkin
2017-02-09 18:49                     ` Michael S. Tsirkin
2017-02-09 18:54                     ` Ard Biesheuvel
2017-02-09 18:54                     ` Ard Biesheuvel
2017-02-09 18:54                       ` Ard Biesheuvel
2017-02-09 18:56                     ` Will Deacon
2017-02-09 18:56                       ` Will Deacon
2017-02-10 17:16                   ` Michael S. Tsirkin
2017-02-10 17:16                     ` Michael S. Tsirkin
2017-02-13 11:57                     ` Will Deacon
2017-02-13 11:57                       ` Will Deacon
2017-02-02 16:30           ` Michael S. Tsirkin
2017-02-08 12:58       ` Alexander Graf
2017-02-08 12:58         ` Alexander Graf
2017-02-09 20:57         ` Michael S. Tsirkin
2017-02-09 20:57           ` Michael S. Tsirkin
2017-02-09 22:20           ` Alexander Graf
2017-02-09 22:20             ` Alexander Graf
2017-02-01 19:19     ` Michael S. Tsirkin
2017-02-01 18:09 ` Michael S. Tsirkin

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=20170202181357-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.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.