Re: [vfio-users] semantics of VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP

2021-06-01 Thread Thanos Makatos



> -Original Message-
> From: Alex Williamson 
> Sent: 01 June 2021 15:14
> To: Thanos Makatos 
> Cc: vfio-users@redhat.com; John Levon ; Swapnil
> Ingle ; linux-ker...@vger.kernel.org;
> k...@vger.kernel.org
> Subject: Re: semantics of VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP
> 
> On Tue, 1 Jun 2021 13:48:22 +
> Thanos Makatos  wrote:
> 
> > (sending here as I can't find a relevant list in
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_vg
> > er-
> 2Dlists.html=DwICAg=s883GpUCOChKOHiocYtGcg=XTpYsh5Ps2zJvtw
> 6og
> >
> tti46atk736SI4vgsJiUKIyDE=E6G0G_Z_M2cIQvruwQk6NRrha3NkW8gdO11
> pPUm8vg
> > k=-7KcTuEYFphAcU1aya0t_Jh4aP9jVPq2N2YxVu9Lu84= )
> 
> $ ./scripts/get_maintainer.pl include/uapi/linux/vfio.h Alex Williamson
>  (maintainer:VFIO DRIVER) Cornelia Huck
>  (reviewer:VFIO DRIVER) k...@vger.kernel.org (open
> list:VFIO DRIVER) linux-ker...@vger.kernel.org (open list)
> 
> > I'm trying to understand the semantics of
> > VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP. My (very rough)
> understanding
> > so far is that once a page gets pinned then it's considered dirty and
> > if the page is still pinned then it remains dirty even after we're
> > done serving VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP. Is my
> > understanding correct?
> 
> This is the current type1 implementation, but the semantics only require that
> a page is reported dirty if it's actually been written.
> Without support for tracking DMA writes, we assume that any page
> accessible to the device is constantly dirty.  This will be refined over time 
> as
> software and hardware support improves, but we currently error on the side
> of assuming all pinned pages are always dirty.
> Thanks,   

Makes sense, thanks.

> 
> Alex


___
vfio-users mailing list
vfio-users@redhat.com
https://listman.redhat.com/mailman/listinfo/vfio-users



[vfio-users] semantics of VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP

2021-06-01 Thread Thanos Makatos
(sending here as I can't find a relevant list in 
http://vger.kernel.org/vger-lists.html)

I'm trying to understand the semantics of 
VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP. My (very rough) understanding so far is 
that once a page gets pinned then it's considered dirty and if the page is 
still pinned then it remains dirty even after we're done serving 
VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP. Is my understanding correct?


___
vfio-users mailing list
vfio-users@redhat.com
https://listman.redhat.com/mailman/listinfo/vfio-users



Re: [vfio-users] location of regions VFIO_REGION_TYPE_MIGRATION/VFIO_REGION_SUBTYPE_MIGRATION

2020-10-02 Thread Thanos Makatos
> -Original Message-
> From: Auger Eric 
> Sent: 02 October 2020 14:56
> To: Thanos Makatos ; vfio-
> us...@redhat.com
> Cc: John G Johnson 
> Subject: Re: [vfio-users] location of regions
> VFIO_REGION_TYPE_MIGRATION/VFIO_REGION_SUBTYPE_MIGRATION
> 
> Hi Thanos,
> 
> On 10/2/20 2:54 PM, Thanos Makatos wrote:
> > According to linux/include/uapi/linux/vfio.h, for a device to support
> migration
> > it must provide a VFIO capability of type VFIO_REGION_INFO_CAP_TYPE
> and set
> > .type/.subtype to
> VFIO_REGION_TYPE_MIGRATION/VFIO_REGION_TYPE_MIGRATION.
> >
> > What confuses me is the following:
> >
> > "The structure vfio_device_migration_info is placed at the 0th offset of
> > the VFIO_REGION_SUBTYPE_MIGRATION region to get and set VFIO
> device related
> > migration information."
> >
> > Where do regions VFIO_REGION_TYPE_MIGRATION and
> VFIO_REGION_SUBTYPE_MIGRATION
> > live on the device's address space? Is just like any other region, except 
> > that
> > in the case of a PCI device it's index must be equal to, or larger than,
> > VFIO_PCI_NUM_REGIONS, and set arbitrarily by the device
> implementation? If
> > so, then I suppose this index must appear in struct
> vfio_device_info.num_regions?
> 
> You need to call VFIO_DEVICE_GET_REGION_INFO with the region
> type/subtype. The index of the region is stored in struct
> vfio_region_info index.

Are you talking from perspective of the user or the driver? I want to know how
to support migration in the driver. VFIO_DEVICE_GET_REGION_INFO doesn't take a
type/subtype argument, it only takes the region index and the driver fills in
the rest of the information.

___
vfio-users mailing list
vfio-users@redhat.com
https://www.redhat.com/mailman/listinfo/vfio-users



[vfio-users] location of regions VFIO_REGION_TYPE_MIGRATION/VFIO_REGION_SUBTYPE_MIGRATION

2020-10-02 Thread Thanos Makatos
According to linux/include/uapi/linux/vfio.h, for a device to support migration
it must provide a VFIO capability of type VFIO_REGION_INFO_CAP_TYPE and set
.type/.subtype to VFIO_REGION_TYPE_MIGRATION/VFIO_REGION_TYPE_MIGRATION.

What confuses me is the following:

"The structure vfio_device_migration_info is placed at the 0th offset of
the VFIO_REGION_SUBTYPE_MIGRATION region to get and set VFIO device related
migration information."

Where do regions VFIO_REGION_TYPE_MIGRATION and VFIO_REGION_SUBTYPE_MIGRATION
live on the device's address space? Is just like any other region, except that
in the case of a PCI device it's index must be equal to, or larger than,
VFIO_PCI_NUM_REGIONS, and set arbitrarily by the device implementation? If
so, then I suppose this index must appear in struct 
vfio_device_info.num_regions?


___
vfio-users mailing list
vfio-users@redhat.com
https://www.redhat.com/mailman/listinfo/vfio-users



Re: [vfio-users] missing VFIO_IOMMU_NOTIFY_DMA_UNMAP event when driver hasn't called vfio_pin_pages

2020-02-28 Thread Thanos Makatos
> Drivers that handle DMA region registration events without having to call
> vfio_pin_pages (e.g. in muser we inject the fd backing that VMA to a
> userspace
> process and then ask it to memory map that fd) aren't notified at all when
> that
> region is unmapped.  Because of this, we get duplicate/overlapping DMA
> regions
> that can be problematic to properly handle (e.g. we can implicitly unmap the
> existing region etc.). Would it make sense for VFIO to always send the DMA
> unregistration event to a driver (or at least conditionally, if the driver
> requests it with some flag during driver registration time), even if it 
> doesn't
> currently have any pages pinned? I think this could be beneficial for drivers
> other than muser, e.g. some driver set up some bookkeeping data structure
> in
> response to the DMA_MAP event but it didn't happen to have to pin any
> page. By
> receiving the DMA_UNMAP event it could release that data
> structure/memory.
> 
> I've experimented with a proof of concept which seems to work:
> 
> # git diff --cached
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c
> index d864277ea16f..2aaa88f64c67 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -875,6 +875,7 @@ static int vfio_dma_do_unmap(struct vfio_iommu
> *iommu,
> struct vfio_dma *dma, *dma_last = NULL;
> size_t unmapped = 0;
> int ret = 0, retries = 0;
> +   bool advised = false;
> 
> mask = ((uint64_t)1 << __ffs(vfio_pgsize_bitmap(iommu))) - 1;
> 
> @@ -944,9 +945,11 @@ static int vfio_dma_do_unmap(struct vfio_iommu
> *iommu,
> if (dma->task->mm != current->mm)
> break;
> 
> -   if (!RB_EMPTY_ROOT(>pfn_list)) {
> +   if (!RB_EMPTY_ROOT(>pfn_list) || !advised) {
> struct vfio_iommu_type1_dma_unmap nb_unmap;
> 
> +   advised = true;
> +
> if (dma_last == dma) {
> BUG_ON(++retries > 10);
> } else {

I have also experimented with sending two overlapping DMA regions to VFIO and
vfio_dma_do_map explicitly fails this operation with -EEXIST. Therefore I could
assume that if my driver receives two overlapping DMA regions then the existing
one can be safely unmapped. However, there is still a possibility of resource
leak since there is no guarantee that at least part of an unmapped DMA region
will be clobbered by another one. This could be partially mitigated by
introducing some timeout to unmap the fd of a DMA region that hasn't been
accessed for some time (and then mmap it on demand if necessary), but it's still
not ideal.

I still think giving the option to mdev drivers to request to be notified
for DMA unmap events is the best way to solve this problem. Are there other
alternatives?


___
vfio-users mailing list
vfio-users@redhat.com
https://www.redhat.com/mailman/listinfo/vfio-users



[vfio-users] missing VFIO_IOMMU_NOTIFY_DMA_UNMAP event when driver hasn't called vfio_pin_pages

2020-02-25 Thread Thanos Makatos
Drivers that handle DMA region registration events without having to call
vfio_pin_pages (e.g. in muser we inject the fd backing that VMA to a userspace
process and then ask it to memory map that fd) aren't notified at all when that
region is unmapped.  Because of this, we get duplicate/overlapping DMA regions
that can be problematic to properly handle (e.g. we can implicitly unmap the
existing region etc.). Would it make sense for VFIO to always send the DMA
unregistration event to a driver (or at least conditionally, if the driver
requests it with some flag during driver registration time), even if it doesn't
currently have any pages pinned? I think this could be beneficial for drivers
other than muser, e.g. some driver set up some bookkeeping data structure in
response to the DMA_MAP event but it didn't happen to have to pin any page. By
receiving the DMA_UNMAP event it could release that data structure/memory.

I've experimented with a proof of concept which seems to work:

# git diff --cached
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index d864277ea16f..2aaa88f64c67 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -875,6 +875,7 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
struct vfio_dma *dma, *dma_last = NULL;
size_t unmapped = 0;
int ret = 0, retries = 0;
+   bool advised = false;

mask = ((uint64_t)1 << __ffs(vfio_pgsize_bitmap(iommu))) - 1;

@@ -944,9 +945,11 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
if (dma->task->mm != current->mm)
break;

-   if (!RB_EMPTY_ROOT(>pfn_list)) {
+   if (!RB_EMPTY_ROOT(>pfn_list) || !advised) {
struct vfio_iommu_type1_dma_unmap nb_unmap;

+   advised = true;
+
if (dma_last == dma) {
BUG_ON(++retries > 10);
} else {


___
vfio-users mailing list
vfio-users@redhat.com
https://www.redhat.com/mailman/listinfo/vfio-users