From: Jason Gunthorpe <jgg@nvidia.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: kvm@vger.kernel.org, David Airlie <airlied@linux.ie>,
dri-devel@lists.freedesktop.org,
Vineeth Vijayan <vneethv@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
Christoph Hellwig <hch@lst.de>,
linux-s390@vger.kernel.org,
Matthew Rosato <mjrosato@linux.ibm.com>,
Halil Pasic <pasic@linux.ibm.com>,
Nicolin Chen <nicolinc@nvidia.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
intel-gfx@lists.freedesktop.org,
Tony Krowiak <akrowiak@linux.ibm.com>,
Eric Farman <farman@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Harald Freudenberger <freude@linux.ibm.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>,
intel-gvt-dev@lists.freedesktop.org,
Jason Herne <jjherne@linux.ibm.com>,
Cornelia Huck <cohuck@redhat.com>,
Peter Oberparleiter <oberpar@linux.ibm.com>,
Sven Schnelle <svens@linux.ibm.com>
Subject: Re: [Intel-gfx] [PATCH v4 1/2] vfio: Replace the DMA unmapping notifier with a callback
Date: Wed, 20 Jul 2022 17:08:29 -0300 [thread overview]
Message-ID: <20220720200829.GW4609@nvidia.com> (raw)
In-Reply-To: <20220720134113.4225f9d6.alex.williamson@redhat.com>
On Wed, Jul 20, 2022 at 01:41:13PM -0600, Alex Williamson wrote:
> ie. we don't need the gfn, we only need the iova.
Right, that makes sense
> However then I start to wonder why we're passing in 1 for the number of
> pages because this previously notifier, now callback is called for the
> entire vfio_dma range when we find any pinned pages.
Well, it is doing this because it only ever pins one page.
The drivers are confused about what the contract is. vfio is calling
the notifier with the entire IOVA range that is being unmapped and the
drivers are expecting to receive notifications only for the IOVA they
have actually pinned.
> Should ap and ccw implementations of .dma_unmap just be replaced with a
> BUG_ON(1)?
The point of these callbacks is to halt concurrent DMA, and ccw does
that today. It looks like AP is missing a call to ap_aqic(), so it is
probably double wrong.
What I'd suggest is adding a WARN_ON that the dma->pfn_list is not
empty and leave these functions alone.
Most likely AP should be fixed to call vfio_ap_irq_disable() and to
check the q->saved_pfn against the IOVA.
But I'm inclined to leave this as-is for this series given we are at
rc7.
Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>,
David Airlie <airlied@linux.ie>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
Cornelia Huck <cohuck@redhat.com>,
Daniel Vetter <daniel@ffwll.ch>,
dri-devel@lists.freedesktop.org,
Harald Freudenberger <freude@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org,
Jani Nikula <jani.nikula@linux.intel.com>,
Jason Herne <jjherne@linux.ibm.com>,
Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
kvm@vger.kernel.org, linux-s390@vger.kernel.org,
Matthew Rosato <mjrosato@linux.ibm.com>,
Peter Oberparleiter <oberpar@linux.ibm.com>,
Halil Pasic <pasic@linux.ibm.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>,
Sven Schnelle <svens@linux.ibm.com>,
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
Vineeth Vijayan <vneethv@linux.ibm.com>,
Zhi Wang <zhi.a.wang@intel.com>,
Tony Krowiak <akrowiak@linux.ibm.com>,
Eric Farman <farman@linux.ibm.com>,
Christoph Hellwig <hch@lst.de>, Kevin Tian <kevin.tian@intel.com>,
Zhenyu Wang <zhenyuw@linux.intel.com>,
Nicolin Chen <nicolinc@nvidia.com>
Subject: Re: [PATCH v4 1/2] vfio: Replace the DMA unmapping notifier with a callback
Date: Wed, 20 Jul 2022 17:08:29 -0300 [thread overview]
Message-ID: <20220720200829.GW4609@nvidia.com> (raw)
In-Reply-To: <20220720134113.4225f9d6.alex.williamson@redhat.com>
On Wed, Jul 20, 2022 at 01:41:13PM -0600, Alex Williamson wrote:
> ie. we don't need the gfn, we only need the iova.
Right, that makes sense
> However then I start to wonder why we're passing in 1 for the number of
> pages because this previously notifier, now callback is called for the
> entire vfio_dma range when we find any pinned pages.
Well, it is doing this because it only ever pins one page.
The drivers are confused about what the contract is. vfio is calling
the notifier with the entire IOVA range that is being unmapped and the
drivers are expecting to receive notifications only for the IOVA they
have actually pinned.
> Should ap and ccw implementations of .dma_unmap just be replaced with a
> BUG_ON(1)?
The point of these callbacks is to halt concurrent DMA, and ccw does
that today. It looks like AP is missing a call to ap_aqic(), so it is
probably double wrong.
What I'd suggest is adding a WARN_ON that the dma->pfn_list is not
empty and leave these functions alone.
Most likely AP should be fixed to call vfio_ap_irq_disable() and to
check the q->saved_pfn against the IOVA.
But I'm inclined to leave this as-is for this series given we are at
rc7.
Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: kvm@vger.kernel.org, David Airlie <airlied@linux.ie>,
Kevin Tian <kevin.tian@intel.com>,
dri-devel@lists.freedesktop.org,
Vineeth Vijayan <vneethv@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
Christoph Hellwig <hch@lst.de>,
linux-s390@vger.kernel.org,
Matthew Rosato <mjrosato@linux.ibm.com>,
Halil Pasic <pasic@linux.ibm.com>,
Nicolin Chen <nicolinc@nvidia.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
intel-gfx@lists.freedesktop.org, Zhi Wang <zhi.a.wang@intel.com>,
Tony Krowiak <akrowiak@linux.ibm.com>,
Eric Farman <farman@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Harald Freudenberger <freude@linux.ibm.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>,
intel-gvt-dev@lists.freedesktop.org,
Jason Herne <jjherne@linux.ibm.com>,
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
Cornelia Huck <cohuck@redhat.com>,
Peter Oberparleiter <oberpar@linux.ibm.com>,
Sven Schnelle <svens@linux.ibm.com>
Subject: Re: [PATCH v4 1/2] vfio: Replace the DMA unmapping notifier with a callback
Date: Wed, 20 Jul 2022 17:08:29 -0300 [thread overview]
Message-ID: <20220720200829.GW4609@nvidia.com> (raw)
In-Reply-To: <20220720134113.4225f9d6.alex.williamson@redhat.com>
On Wed, Jul 20, 2022 at 01:41:13PM -0600, Alex Williamson wrote:
> ie. we don't need the gfn, we only need the iova.
Right, that makes sense
> However then I start to wonder why we're passing in 1 for the number of
> pages because this previously notifier, now callback is called for the
> entire vfio_dma range when we find any pinned pages.
Well, it is doing this because it only ever pins one page.
The drivers are confused about what the contract is. vfio is calling
the notifier with the entire IOVA range that is being unmapped and the
drivers are expecting to receive notifications only for the IOVA they
have actually pinned.
> Should ap and ccw implementations of .dma_unmap just be replaced with a
> BUG_ON(1)?
The point of these callbacks is to halt concurrent DMA, and ccw does
that today. It looks like AP is missing a call to ap_aqic(), so it is
probably double wrong.
What I'd suggest is adding a WARN_ON that the dma->pfn_list is not
empty and leave these functions alone.
Most likely AP should be fixed to call vfio_ap_irq_disable() and to
check the q->saved_pfn against the IOVA.
But I'm inclined to leave this as-is for this series given we are at
rc7.
Jason
next prev parent reply other threads:[~2022-07-20 20:08 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-20 0:02 [Intel-gfx] [PATCH v4 0/2] Remove the VFIO_IOMMU_NOTIFY_DMA_UNMAP notifier Jason Gunthorpe
2022-07-20 0:02 ` Jason Gunthorpe
2022-07-20 0:02 ` Jason Gunthorpe
2022-07-20 0:02 ` [Intel-gfx] [PATCH v4 1/2] vfio: Replace the DMA unmapping notifier with a callback Jason Gunthorpe
2022-07-20 0:02 ` Jason Gunthorpe
2022-07-20 0:02 ` Jason Gunthorpe
2022-07-20 19:41 ` [Intel-gfx] " Alex Williamson
2022-07-20 19:41 ` Alex Williamson
2022-07-20 19:41 ` Alex Williamson
2022-07-20 20:08 ` Jason Gunthorpe [this message]
2022-07-20 20:08 ` Jason Gunthorpe
2022-07-20 20:08 ` Jason Gunthorpe
2022-07-20 23:04 ` [Intel-gfx] " Alex Williamson
2022-07-20 23:04 ` Alex Williamson
2022-07-20 23:04 ` Alex Williamson
2022-07-21 16:01 ` [Intel-gfx] " Eric Farman
2022-07-21 16:01 ` Eric Farman
2022-07-21 16:01 ` Eric Farman
2022-07-21 16:41 ` [Intel-gfx] " Alex Williamson
2022-07-21 16:41 ` Alex Williamson
2022-07-21 16:41 ` Alex Williamson
2022-07-20 0:02 ` [Intel-gfx] [PATCH v4 2/2] vfio: Replace the iommu notifier with a device list Jason Gunthorpe
2022-07-20 0:02 ` Jason Gunthorpe
2022-07-20 0:02 ` Jason Gunthorpe
2022-07-20 0:29 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for Remove the VFIO_IOMMU_NOTIFY_DMA_UNMAP notifier Patchwork
2022-07-20 20:07 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for Remove the VFIO_IOMMU_NOTIFY_DMA_UNMAP notifier (rev2) Patchwork
2022-07-22 22:50 ` [Intel-gfx] [PATCH v4 0/2] Remove the VFIO_IOMMU_NOTIFY_DMA_UNMAP notifier Alex Williamson
2022-07-22 22:50 ` Alex Williamson
2022-07-22 22:50 ` Alex Williamson
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=20220720200829.GW4609@nvidia.com \
--to=jgg@nvidia.com \
--cc=agordeev@linux.ibm.com \
--cc=airlied@linux.ie \
--cc=akrowiak@linux.ibm.com \
--cc=alex.williamson@redhat.com \
--cc=borntraeger@linux.ibm.com \
--cc=cohuck@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=farman@linux.ibm.com \
--cc=freude@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=hch@lst.de \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-gvt-dev@lists.freedesktop.org \
--cc=jjherne@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=mjrosato@linux.ibm.com \
--cc=nicolinc@nvidia.com \
--cc=oberpar@linux.ibm.com \
--cc=pasic@linux.ibm.com \
--cc=rodrigo.vivi@intel.com \
--cc=svens@linux.ibm.com \
--cc=vneethv@linux.ibm.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.