| Message ID | 0-v1-4991695894d8+211-vfio_iommufd_jgg@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Connect VFIO to IOMMUFD | expand |
On Tue, Oct 25, 2022 at 03:17:06PM -0300, Jason Gunthorpe wrote: > This series provides an alternative container layer for VFIO implemented > using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will > not be compiled in. > > At this point iommufd can be injected by passing in a iommfd FD to > VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd > to obtain the compat IOAS and then connect up all the VFIO drivers as > appropriate. > > This is temporary stopping point, a following series will provide a way to > directly open a VFIO device FD and directly connect it to IOMMUFD using > native ioctls that can expose the IOMMUFD features like hwpt, future > vPASID and dynamic attachment. > > This series, in compat mode, has passed all the qemu tests we have > available, including the test suites for the Intel GVT mdev. Aside from > the temporary limitation with P2P memory this is belived to be fully > compatible with VFIO. > > This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd Tested-by: Nicolin Chen <nicoleotsuka@nvidia.com> Tested this branch on ARM64+SMMUv3 with the iommufd selftest and QEMU passthrough sanity using noiommu and virtio-iommu setups by combining with both CONFIG_VFIO_CONTAINER=y and =n.
On Fri, Oct 28, 2022 at 04:53:21PM -0700, Nicolin Chen wrote: > On Tue, Oct 25, 2022 at 03:17:06PM -0300, Jason Gunthorpe wrote: > > This series provides an alternative container layer for VFIO implemented > > using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will > > not be compiled in. > > > > At this point iommufd can be injected by passing in a iommfd FD to > > VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd > > to obtain the compat IOAS and then connect up all the VFIO drivers as > > appropriate. > > > > This is temporary stopping point, a following series will provide a way to > > directly open a VFIO device FD and directly connect it to IOMMUFD using > > native ioctls that can expose the IOMMUFD features like hwpt, future > > vPASID and dynamic attachment. > > > > This series, in compat mode, has passed all the qemu tests we have > > available, including the test suites for the Intel GVT mdev. Aside from > > the temporary limitation with P2P memory this is belived to be fully > > compatible with VFIO. > > > > This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd > > Tested-by: Nicolin Chen <nicoleotsuka@nvidia.com> Sorry, wrong email -- should be: Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Tested this branch on ARM64+SMMUv3 with the iommufd selftest and > QEMU passthrough sanity using noiommu and virtio-iommu setups by > combining with both CONFIG_VFIO_CONTAINER=y and =n.
Hi Jason, On 2022/10/26 02:17, Jason Gunthorpe wrote: > This series provides an alternative container layer for VFIO implemented > using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will > not be compiled in. > > At this point iommufd can be injected by passing in a iommfd FD to > VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd > to obtain the compat IOAS and then connect up all the VFIO drivers as > appropriate. > > This is temporary stopping point, a following series will provide a way to > directly open a VFIO device FD and directly connect it to IOMMUFD using > native ioctls that can expose the IOMMUFD features like hwpt, future > vPASID and dynamic attachment. > > This series, in compat mode, has passed all the qemu tests we have > available, including the test suites for the Intel GVT mdev. Aside from > the temporary limitation with P2P memory this is belived to be fully > compatible with VFIO. > > This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd In our side, we found the gvt-g test failed. Guest i915 driver stuck at init phase. While with your former version (commit ID a249441ba6fd9d658f4a1b568453e3a742d12686), gvt-g test is passing. I noticed there a quite a few change in iommufd/pages.c from last version. We are internally tracing in the gvt-g side, may also good to have your attention. > It requires the iommufd series: > > https://lore.kernel.org/r/0-v3-402a7d6459de+24b-iommufd_jgg@nvidia.com > > Jason Gunthorpe (10): > vfio: Move vfio_device driver open/close code to a function > vfio: Move vfio_device_assign_container() into > vfio_device_first_open() > vfio: Rename vfio_device_assign/unassign_container() > vfio: Move storage of allow_unsafe_interrupts to vfio_main.c > vfio: Use IOMMU_CAP_ENFORCE_CACHE_COHERENCY for > vfio_file_enforced_coherent() > vfio-iommufd: Allow iommufd to be used in place of a container fd > vfio-iommufd: Support iommufd for physical VFIO devices > vfio-iommufd: Support iommufd for emulated VFIO devices > vfio: Make vfio_container optionally compiled > iommufd: Allow iommufd to supply /dev/vfio/vfio > > drivers/gpu/drm/i915/gvt/kvmgt.c | 3 + > drivers/iommu/iommufd/Kconfig | 12 + > drivers/iommu/iommufd/main.c | 35 +- > drivers/s390/cio/vfio_ccw_ops.c | 3 + > drivers/s390/crypto/vfio_ap_ops.c | 3 + > drivers/vfio/Kconfig | 38 ++- > drivers/vfio/Makefile | 5 +- > drivers/vfio/container.c | 136 ++------ > drivers/vfio/fsl-mc/vfio_fsl_mc.c | 3 + > drivers/vfio/iommufd.c | 161 +++++++++ > .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 6 + > drivers/vfio/pci/mlx5/main.c | 3 + > drivers/vfio/pci/vfio_pci.c | 3 + > drivers/vfio/platform/vfio_amba.c | 3 + > drivers/vfio/platform/vfio_platform.c | 3 + > drivers/vfio/vfio.h | 100 +++++- > drivers/vfio/vfio_iommu_type1.c | 5 +- > drivers/vfio/vfio_main.c | 318 ++++++++++++++---- > include/linux/vfio.h | 39 +++ > 19 files changed, 681 insertions(+), 198 deletions(-) > create mode 100644 drivers/vfio/iommufd.c > > > base-commit: 3bec937e94942a6aee8854be1c1f5cc2b92d15e2
On Mon, Oct 31, 2022 at 06:38:45PM +0800, Yi Liu wrote: > Hi Jason, > > On 2022/10/26 02:17, Jason Gunthorpe wrote: > > This series provides an alternative container layer for VFIO implemented > > using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will > > not be compiled in. > > > > At this point iommufd can be injected by passing in a iommfd FD to > > VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd > > to obtain the compat IOAS and then connect up all the VFIO drivers as > > appropriate. > > > > This is temporary stopping point, a following series will provide a way to > > directly open a VFIO device FD and directly connect it to IOMMUFD using > > native ioctls that can expose the IOMMUFD features like hwpt, future > > vPASID and dynamic attachment. > > > > This series, in compat mode, has passed all the qemu tests we have > > available, including the test suites for the Intel GVT mdev. Aside from > > the temporary limitation with P2P memory this is belived to be fully > > compatible with VFIO. > > > > This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd > > In our side, we found the gvt-g test failed. Guest i915 driver stuck at > init phase. While with your former version (commit ID > a249441ba6fd9d658f4a1b568453e3a742d12686), gvt-g test is passing. Oh, I didn't realize you grabbed such an older version for this testing.. > noticed there a quite a few change in iommufd/pages.c from last version. > We are internally tracing in the gvt-g side, may also good to have your > attention. syzkaller just ran into this that I was starting to investigate: @@ -1505,7 +1505,7 @@ int iopt_pages_fill_xarray(struct iopt_pages *pages, unsigned long start_index, int rc; pfn_reader_user_init(&user, pages); - user.upages_len = last_index - start_index + 1; + user.upages_len = (last_index - start_index + 1) * sizeof(*out_pages); interval_tree_for_each_double_span(&span, &pages->access_itree, It would certainly hit gvt - but you should get WARN_ON's not hangs There is something wrong with the test suite that it isn't covering the above, I'm going to look into that today. Jason
On 2022/10/31 20:18, Jason Gunthorpe wrote: > On Mon, Oct 31, 2022 at 06:38:45PM +0800, Yi Liu wrote: >> Hi Jason, >> >> On 2022/10/26 02:17, Jason Gunthorpe wrote: >>> This series provides an alternative container layer for VFIO implemented >>> using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will >>> not be compiled in. >>> >>> At this point iommufd can be injected by passing in a iommfd FD to >>> VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd >>> to obtain the compat IOAS and then connect up all the VFIO drivers as >>> appropriate. >>> >>> This is temporary stopping point, a following series will provide a way to >>> directly open a VFIO device FD and directly connect it to IOMMUFD using >>> native ioctls that can expose the IOMMUFD features like hwpt, future >>> vPASID and dynamic attachment. >>> >>> This series, in compat mode, has passed all the qemu tests we have >>> available, including the test suites for the Intel GVT mdev. Aside from >>> the temporary limitation with P2P memory this is belived to be fully >>> compatible with VFIO. >>> >>> This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd >> >> In our side, we found the gvt-g test failed. Guest i915 driver stuck at >> init phase. While with your former version (commit ID >> a249441ba6fd9d658f4a1b568453e3a742d12686), gvt-g test is passing. > > Oh, I didn't realize you grabbed such an older version for this testing.. yeah, this was grabbed before your posting. :-) >> noticed there a quite a few change in iommufd/pages.c from last version. >> We are internally tracing in the gvt-g side, may also good to have your >> attention. > > syzkaller just ran into this that I was starting to investigate: > > @@ -1505,7 +1505,7 @@ int iopt_pages_fill_xarray(struct iopt_pages *pages, unsigned long start_index, > int rc; > > pfn_reader_user_init(&user, pages); > - user.upages_len = last_index - start_index + 1; > + user.upages_len = (last_index - start_index + 1) * sizeof(*out_pages); > interval_tree_for_each_double_span(&span, &pages->access_itree, > > It would certainly hit gvt - but you should get WARN_ON's not hangs > > There is something wrong with the test suite that it isn't covering > the above, I'm going to look into that today. sounds to be the cause. I didn't see any significant change in vfio_main.c that may fail gvt. So should the iommufd changes. Then we will re-run the test after your update.:-)
On Mon, Oct 31, 2022 at 08:25:39PM +0800, Yi Liu wrote: > > There is something wrong with the test suite that it isn't covering > > the above, I'm going to look into that today. > > sounds to be the cause. I didn't see any significant change in vfio_main.c > that may fail gvt. So should the iommufd changes. Then we will re-run the > test after your update.:-) I updated the github with all the changes made so far, it is worth trying again! Thanks, Jason
On Tue, Nov 01, 2022 at 11:04:38AM +0800, Yi Liu wrote: > On 2022/11/1 07:24, Jason Gunthorpe wrote: > > On Mon, Oct 31, 2022 at 08:25:39PM +0800, Yi Liu wrote: > > > > There is something wrong with the test suite that it isn't covering > > > > the above, I'm going to look into that today. > > > > > > sounds to be the cause. I didn't see any significant change in vfio_main.c > > > that may fail gvt. So should the iommufd changes. Then we will re-run the > > > test after your update.:-) > > > > I updated the github with all the changes made so far, it is worth > > trying again! > > gvt is still failing with below call trace in host side. vfio_unpin_pages() > is still in problem. Any idea on it? > [ 206.464318] WARNING: CPU: 9 PID: 3362 at > drivers/iommu/iommufd/device.c:591 iommufd_access_pin_pages+0x337/0x360 Judging from this WARNING, and since gvt (mdev) needs pin_pages(), I assume this might be a fix, though Jason's latest change for the iova_alignment seems to be added for CONFIG_IOMMUFD_TEST only. ------ diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c index 72a289c5f8c9..185075528d5e 100644 --- a/drivers/vfio/iommufd.c +++ b/drivers/vfio/iommufd.c @@ -120,6 +120,7 @@ static void vfio_emulated_unmap(void *data, unsigned long iova, } static const struct iommufd_access_ops vfio_user_ops = { + .needs_pin_pages = 1, .unmap = vfio_emulated_unmap, }; ------ Perhaps you can try it first to see if we can test the rest part of the routine for now, till Jason acks tomorrow. Thanks Nic
On Tue, Nov 01, 2022 at 11:04:38AM +0800, Yi Liu wrote: > On 2022/11/1 07:24, Jason Gunthorpe wrote: > > On Mon, Oct 31, 2022 at 08:25:39PM +0800, Yi Liu wrote: > > > > There is something wrong with the test suite that it isn't covering > > > > the above, I'm going to look into that today. > > > > > > sounds to be the cause. I didn't see any significant change in vfio_main.c > > > that may fail gvt. So should the iommufd changes. Then we will re-run the > > > test after your update.:-) > > > > I updated the github with all the changes made so far, it is worth > > trying again! > > gvt is still failing with below call trace in host side. vfio_unpin_pages() > is still in problem. Any idea on it? Oh, this is my mistake, I rushed a bit getting updated branches: diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c index 40eb6931ab2321..29e7b1fdd0cd4a 100644 --- a/drivers/vfio/iommufd.c +++ b/drivers/vfio/iommufd.c @@ -118,6 +118,7 @@ static void vfio_emulated_unmap(void *data, unsigned long iova, } static const struct iommufd_access_ops vfio_user_ops = { + .needs_pin_pages = 1, .unmap = vfio_emulated_unmap, }; Thanks, Jason
On 2022/11/1 12:21, Nicolin Chen wrote: > On Tue, Nov 01, 2022 at 11:04:38AM +0800, Yi Liu wrote: >> On 2022/11/1 07:24, Jason Gunthorpe wrote: >>> On Mon, Oct 31, 2022 at 08:25:39PM +0800, Yi Liu wrote: >>>>> There is something wrong with the test suite that it isn't covering >>>>> the above, I'm going to look into that today. >>>> >>>> sounds to be the cause. I didn't see any significant change in vfio_main.c >>>> that may fail gvt. So should the iommufd changes. Then we will re-run the >>>> test after your update.:-) >>> >>> I updated the github with all the changes made so far, it is worth >>> trying again! >> >> gvt is still failing with below call trace in host side. vfio_unpin_pages() >> is still in problem. Any idea on it? > >> [ 206.464318] WARNING: CPU: 9 PID: 3362 at >> drivers/iommu/iommufd/device.c:591 iommufd_access_pin_pages+0x337/0x360 > > Judging from this WARNING, and since gvt (mdev) needs pin_pages(), > I assume this might be a fix, though Jason's latest change for the > iova_alignment seems to be added for CONFIG_IOMMUFD_TEST only. > > ------ > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c > index 72a289c5f8c9..185075528d5e 100644 > --- a/drivers/vfio/iommufd.c > +++ b/drivers/vfio/iommufd.c > @@ -120,6 +120,7 @@ static void vfio_emulated_unmap(void *data, unsigned long iova, > } > > static const struct iommufd_access_ops vfio_user_ops = { > + .needs_pin_pages = 1, > .unmap = vfio_emulated_unmap, > }; > > ------ > > Perhaps you can try it first to see if we can test the rest part of > the routine for now, till Jason acks tomorrow. fyi. it works so far. :-)
On 2022/11/1 19:41, Jason Gunthorpe wrote: > On Tue, Nov 01, 2022 at 11:04:38AM +0800, Yi Liu wrote: >> On 2022/11/1 07:24, Jason Gunthorpe wrote: >>> On Mon, Oct 31, 2022 at 08:25:39PM +0800, Yi Liu wrote: >>>>> There is something wrong with the test suite that it isn't covering >>>>> the above, I'm going to look into that today. >>>> >>>> sounds to be the cause. I didn't see any significant change in vfio_main.c >>>> that may fail gvt. So should the iommufd changes. Then we will re-run the >>>> test after your update.:-) >>> >>> I updated the github with all the changes made so far, it is worth >>> trying again! >> >> gvt is still failing with below call trace in host side. vfio_unpin_pages() >> is still in problem. Any idea on it? > > Oh, this is my mistake, I rushed a bit getting updated branches: > > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c > index 40eb6931ab2321..29e7b1fdd0cd4a 100644 > --- a/drivers/vfio/iommufd.c > +++ b/drivers/vfio/iommufd.c > @@ -118,6 +118,7 @@ static void vfio_emulated_unmap(void *data, unsigned long iova, > } > > static const struct iommufd_access_ops vfio_user_ops = { > + .needs_pin_pages = 1, > .unmap = vfio_emulated_unmap, > }; yes, it is. The call trace goes away. my colleague is running gvt full test now.
This series provides an alternative container layer for VFIO implemented using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will not be compiled in. At this point iommufd can be injected by passing in a iommfd FD to VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd to obtain the compat IOAS and then connect up all the VFIO drivers as appropriate. This is temporary stopping point, a following series will provide a way to directly open a VFIO device FD and directly connect it to IOMMUFD using native ioctls that can expose the IOMMUFD features like hwpt, future vPASID and dynamic attachment. This series, in compat mode, has passed all the qemu tests we have available, including the test suites for the Intel GVT mdev. Aside from the temporary limitation with P2P memory this is belived to be fully compatible with VFIO. This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd It requires the iommufd series: https://lore.kernel.org/r/0-v3-402a7d6459de+24b-iommufd_jgg@nvidia.com Jason Gunthorpe (10): vfio: Move vfio_device driver open/close code to a function vfio: Move vfio_device_assign_container() into vfio_device_first_open() vfio: Rename vfio_device_assign/unassign_container() vfio: Move storage of allow_unsafe_interrupts to vfio_main.c vfio: Use IOMMU_CAP_ENFORCE_CACHE_COHERENCY for vfio_file_enforced_coherent() vfio-iommufd: Allow iommufd to be used in place of a container fd vfio-iommufd: Support iommufd for physical VFIO devices vfio-iommufd: Support iommufd for emulated VFIO devices vfio: Make vfio_container optionally compiled iommufd: Allow iommufd to supply /dev/vfio/vfio drivers/gpu/drm/i915/gvt/kvmgt.c | 3 + drivers/iommu/iommufd/Kconfig | 12 + drivers/iommu/iommufd/main.c | 35 +- drivers/s390/cio/vfio_ccw_ops.c | 3 + drivers/s390/crypto/vfio_ap_ops.c | 3 + drivers/vfio/Kconfig | 38 ++- drivers/vfio/Makefile | 5 +- drivers/vfio/container.c | 136 ++------ drivers/vfio/fsl-mc/vfio_fsl_mc.c | 3 + drivers/vfio/iommufd.c | 161 +++++++++ .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 6 + drivers/vfio/pci/mlx5/main.c | 3 + drivers/vfio/pci/vfio_pci.c | 3 + drivers/vfio/platform/vfio_amba.c | 3 + drivers/vfio/platform/vfio_platform.c | 3 + drivers/vfio/vfio.h | 100 +++++- drivers/vfio/vfio_iommu_type1.c | 5 +- drivers/vfio/vfio_main.c | 318 ++++++++++++++---- include/linux/vfio.h | 39 +++ 19 files changed, 681 insertions(+), 198 deletions(-) create mode 100644 drivers/vfio/iommufd.c base-commit: 3bec937e94942a6aee8854be1c1f5cc2b92d15e2