linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v2 00/17] Add Nested Translation Support for SMMUv3
       [not found] <cover.1683688960.git.nicolinc@nvidia.com>
@ 2023-05-15 10:00 ` Zhangfei Gao
  2023-05-15 15:57   ` Nicolin Chen
  0 siblings, 1 reply; 6+ messages in thread
From: Zhangfei Gao @ 2023-05-15 10:00 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: jgg, robin.murphy, will, eric.auger, kevin.tian, baolu.lu, joro,
	shameerali.kolothum.thodi, jean-philippe, linux-arm-kernel, iommu,
	linux-kernel, kvm, alex.williamson, yi.l.liu

Hi, Nico

On Wed, 10 May 2023 at 11:34, Nicolin Chen <nicolinc@nvidia.com> wrote:
>
> [ This series is rebased on top of v6.4-rc1 merging Jason's iommu_hwpt
>   branch and Yi's vfio cdev v11 branch, then the replace v7 series and
>   the nesting v2 (candidate) series and Intel VT-d series. Note that
>   some of them are still getting finalized. So, there can be potential
>   minor API changes that would not be reflected in this series. Yet, we
>   can start the review at the SMMU driver specific things.
>
>   @robin, the hw_info patch still requires the errata patch that you
>   mentioned. Perhaps we can merge that separately or include it in v3.
>
>   Thanks! ]
>
> Changelog
> v2:
>  * Added arm_smmu_set_dev_data after the set_dev_data series.
>  * Added Jason's patch "vfio: Remove VFIO_TYPE1_NESTING_IOMMU"
>  * Replaced the iommu_get_unmanaged_domain() helper with Robin's patch.
>  * Reworked the code in arm_smmu_cmdq_build_cmd() to make NH_VA to be
>    a superset of NH_VAA.
>  * Added inline comments and a bug-report link to the patch unsetting
>    dst[2] and dst[3] of STE.
>  * Dropped the to_s2_cfg helper since only one place really needs it.
>  * Dropped the VMID (override) flag and s2vmid in iommu_hwpt_arm_smmuv3
>    structure, because it's expected for user space to use a shared S2
>    domain/hwpt for all devices, i.e. the VMID (allocated with the S2
>    domain is already unified. If there's some special case that still
>    needs a VMID unification, we should probably add it incrementally.
>  * Move the introduction of the "struct arm_smmu_domain *s2" function
>    parameter to the proper patch.
>  * Redefined "struct iommu_hwpt_arm_smmuv3" by adding ste_uptr/len and
>    out_event_uptr/len. Then added an arm_smmu_domain_finalise_nested()
>    function to read guest Stream Table Entry with a proper sanity.
>  * Reworked arm_smmu_cache_invalidate_user() by reading the guest CMDQ
>    directly, to support batching. Also, added return value feedback of
>    -ETIMEDOUT at CMD_SYNC, and reported CERROR_ILL errors via the CONS
>    in the user_data structure.
>  * Updated data/functions following the nesting infrastructure updates.
>  * Added/fixed multiple comments per v1 review inputs.
> v1:
>  https://lore.kernel.org/all/cover.1678348754.git.nicolinc@nvidia.com/
>
> --------------------------------------------------------------------------
>
> Hi all,
>
> This series of patches add nested translation support for ARM SMMUv3.
>
> Eric Auger made a huge effort previously with the VFIO uAPIs, and sent
> his v16 a year ago. Now, the nested translation should follow the new
> IOMMUFD uAPIs design. So, most of the key features are ported from the
> privous VFIO solution, and then rebuilt on top of the IOMMUFD nesting
> infrastructure.
>
> The essential parts in the driver to support a nested translation are
> ->hw_info, ->domain_alloc_user and ->cache_invalidate_user ops. So this
> series fundamentally adds these three functions in the SMMUv3 driver,
> along with several preparations and cleanups for them.
>
> One unique requirement for SMMUv3 nested translation support is the MSI
> doorbell address translation, which is a 2-stage translation too. And,
> to working with the ITS driver, an msi_cookie needs to be setup on the
> kernel-managed domain, the stage-2 domain of the nesting setup. And the
> same msi_cookie will be fetched, via iommu_dma_get_msi_mapping_domain(),
> in the iommu core to allocate and creates IOVA mappings for MSI doorbell
> page(s). However, with the nesting design, the device is attached to a
> user-managed domain, the stage-1 domain. So both the setup and fetching
> of the msi_cookie would not work at the level of stage-2 domain. Thus,
> on both sides, the msi_cookie setup and fetching require a redirection
> of the domain pointer. It's easy to do so in iommufd core, but needs a
> new op in the iommu core and driver.
>
> You can also find this series on the Github:
> https://github.com/nicolinc/iommufd/commits/iommufd_nesting-v2
>
> The kernel branch is tested with this QEMU branch:
> https://github.com/nicolinc/qemu/commits/wip/iommufd_rfcv4+nesting+smmuv3-v2
>

I rebased on these two branches and did some basic tests.

The basic functions work after backport
iommufd: Add IOMMU_PAGE_RESPONSE
iommufd: Add device fault handler support

https://github.com/Linaro/linux-kernel-warpdrive/tree/uacce-devel-6.4
https://github.com/Linaro/qemu/tree/iommufd-6.4-nesting-smmuv3-v2

However when debugging hotplug PCI device, it still does not work,
Segmentation fault same as 6.2.

guest kernel
CONFIG_HOTPLUG_PCI_PCIE=y

boot guest (this info does not appear in 6.2)
qemu-system-aarch64: -device
vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
Failed to set data -1
qemu-system-aarch64: -device
vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
failed to set device data

$ sudo nc -U /tmp/qmpm_1.socket
(qemu) info pci
(qemu) device_del acc1

guest:
qemu-system-aarch64: IOMMU_IOAS_UNMAP failed: No such file or directory
qemu-system-aarch64: vfio_container_dma_unmap(0xaaaae1fc0380,
0x8000000000, 0x10000) = -2 (No such file or directory)
qemu-system-aarch64: Failed to unset data -1
Segmentation fault (core dumped).  // also happened in 6.2

Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 00/17] Add Nested Translation Support for SMMUv3
  2023-05-15 10:00 ` [PATCH v2 00/17] Add Nested Translation Support for SMMUv3 Zhangfei Gao
@ 2023-05-15 15:57   ` Nicolin Chen
  2023-05-16  3:12     ` Zhangfei Gao
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolin Chen @ 2023-05-15 15:57 UTC (permalink / raw)
  To: Zhangfei Gao
  Cc: jgg, robin.murphy, will, eric.auger, kevin.tian, baolu.lu, joro,
	shameerali.kolothum.thodi, jean-philippe, linux-arm-kernel, iommu,
	linux-kernel, kvm, alex.williamson, yi.l.liu

Hi Zhangfei,

On Mon, May 15, 2023 at 06:00:26PM +0800, Zhangfei Gao wrote:
 
> I rebased on these two branches and did some basic tests.
> 
> The basic functions work after backport
> iommufd: Add IOMMU_PAGE_RESPONSE
> iommufd: Add device fault handler support
> 
> https://github.com/Linaro/linux-kernel-warpdrive/tree/uacce-devel-6.4
> https://github.com/Linaro/qemu/tree/iommufd-6.4-nesting-smmuv3-v2

Thanks for testing!

> However when debugging hotplug PCI device, it still does not work,
> Segmentation fault same as 6.2.
> 
> guest kernel
> CONFIG_HOTPLUG_PCI_PCIE=y
> 
> boot guest (this info does not appear in 6.2)
> qemu-system-aarch64: -device
> vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
> Failed to set data -1
> qemu-system-aarch64: -device
> vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
> failed to set device data

Hmm.. I wonder what fails the set_dev_data ioctl...

> $ sudo nc -U /tmp/qmpm_1.socket
> (qemu) info pci
> (qemu) device_del acc1
> 
> guest:
> qemu-system-aarch64: IOMMU_IOAS_UNMAP failed: No such file or directory
> qemu-system-aarch64: vfio_container_dma_unmap(0xaaaae1fc0380,
> 0x8000000000, 0x10000) = -2 (No such file or directory)

This is resulted from the following commit that we should
drop later:

commit c4fd2efd7c02dd30491adf676c1b0aed67656f36
Author: Yi Liu <yi.l.liu@intel.com>
Date:   Thu Apr 27 05:47:03 2023 -0700

    vfio/container: Skip readonly pages

    This is a temparary solution for Intel platform due to an errata in
    which readonly pages in second stage page table is exclusive with
    nested support.

    Signed-off-by: Yi Liu <yi.l.liu@intel.com>


> qemu-system-aarch64: Failed to unset data -1
> Segmentation fault (core dumped).  // also happened in 6.2

Hmm, would it be possible for you to run the test again by
adding the following tracers to your QEMU command?
    --trace "iommufd*" \
    --trace "smmu*" \
    --trace "vfio_*" \
    --trace "pci_*"

Thanks
Nic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 00/17] Add Nested Translation Support for SMMUv3
  2023-05-15 15:57   ` Nicolin Chen
@ 2023-05-16  3:12     ` Zhangfei Gao
  2023-05-25 23:42       ` Nicolin Chen
  0 siblings, 1 reply; 6+ messages in thread
From: Zhangfei Gao @ 2023-05-16  3:12 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: jgg, robin.murphy, will, eric.auger, kevin.tian, baolu.lu, joro,
	shameerali.kolothum.thodi, jean-philippe, linux-arm-kernel, iommu,
	linux-kernel, kvm, alex.williamson, yi.l.liu

On Mon, 15 May 2023 at 23:58, Nicolin Chen <nicolinc@nvidia.com> wrote:
>
> Hi Zhangfei,
>
> On Mon, May 15, 2023 at 06:00:26PM +0800, Zhangfei Gao wrote:
>
> > I rebased on these two branches and did some basic tests.
> >
> > The basic functions work after backport
> > iommufd: Add IOMMU_PAGE_RESPONSE
> > iommufd: Add device fault handler support
> >
> > https://github.com/Linaro/linux-kernel-warpdrive/tree/uacce-devel-6.4
> > https://github.com/Linaro/qemu/tree/iommufd-6.4-nesting-smmuv3-v2
>
> Thanks for testing!
>
> > However when debugging hotplug PCI device, it still does not work,
> > Segmentation fault same as 6.2.
> >
> > guest kernel
> > CONFIG_HOTPLUG_PCI_PCIE=y
> >
> > boot guest (this info does not appear in 6.2)
> > qemu-system-aarch64: -device
> > vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
> > Failed to set data -1
> > qemu-system-aarch64: -device
> > vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
> > failed to set device data
>
> Hmm.. I wonder what fails the set_dev_data ioctl...
Simply debug, it is because dev_data.sid=0, causing
arm_smmu_set_dev_user_data fail

hw/arm/smmu-common.c
smmu_dev_set_iommu_device
.sid = smmu_get_sid(sdev)
smmu_dev_set_iommu_device dev_data.sid=0

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
arm_smmu_set_dev_user_data
u32 sid_user = user->sid;
if (!sid_user) return -EINVAL;

>
> > $ sudo nc -U /tmp/qmpm_1.socket
> > (qemu) info pci
> > (qemu) device_del acc1
> >
> > guest:
> > qemu-system-aarch64: IOMMU_IOAS_UNMAP failed: No such file or directory
> > qemu-system-aarch64: vfio_container_dma_unmap(0xaaaae1fc0380,
> > 0x8000000000, 0x10000) = -2 (No such file or directory)
>
From ex-email reply
(Eric) In qemu arm virt machine 0x8000000000 matches the PCI MMIO region.
(Yi) Currently, iommufd kernel part doesn't support mapping device BAR MMIO.
This is a known gap.

> This is resulted from the following commit that we should
> drop later:
>
> commit c4fd2efd7c02dd30491adf676c1b0aed67656f36
> Author: Yi Liu <yi.l.liu@intel.com>
> Date:   Thu Apr 27 05:47:03 2023 -0700
>
>     vfio/container: Skip readonly pages
>
>     This is a temparary solution for Intel platform due to an errata in
>     which readonly pages in second stage page table is exclusive with
>     nested support.
>
>     Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>
>
> > qemu-system-aarch64: Failed to unset data -1
> > Segmentation fault (core dumped).  // also happened in 6.2
>
> Hmm, would it be possible for you to run the test again by
> adding the following tracers to your QEMU command?
>     --trace "iommufd*" \
>     --trace "smmu*" \
>     --trace "vfio_*" \
>     --trace "pci_*"
>

Have sent you the log directly, since it is too big.

Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 00/17] Add Nested Translation Support for SMMUv3
  2023-05-16  3:12     ` Zhangfei Gao
@ 2023-05-25 23:42       ` Nicolin Chen
  2023-05-26  1:58         ` zhangfei gao
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolin Chen @ 2023-05-25 23:42 UTC (permalink / raw)
  To: Zhangfei Gao
  Cc: jgg, robin.murphy, will, eric.auger, kevin.tian, baolu.lu, joro,
	shameerali.kolothum.thodi, jean-philippe, linux-arm-kernel, iommu,
	linux-kernel, kvm, alex.williamson, yi.l.liu

On Tue, May 16, 2023 at 11:12:44AM +0800, Zhangfei Gao wrote:

> > > However when debugging hotplug PCI device, it still does not work,
> > > Segmentation fault same as 6.2.
> > >
> > > guest kernel
> > > CONFIG_HOTPLUG_PCI_PCIE=y
> > >
> > > boot guest (this info does not appear in 6.2)
> > > qemu-system-aarch64: -device
> > > vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
> > > Failed to set data -1
> > > qemu-system-aarch64: -device
> > > vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
> > > failed to set device data
> >
> > Hmm.. I wonder what fails the set_dev_data ioctl...
> Simply debug, it is because dev_data.sid=0, causing
> arm_smmu_set_dev_user_data fail

I found that too. The input pci bus number is 1, yet the in
the context of set_dev_data, the pci bus number is 0, which
resulted in a 0-valued sid. I will take another look to get
why.

> > > $ sudo nc -U /tmp/qmpm_1.socket
> > > (qemu) info pci
> > > (qemu) device_del acc1
> > >
> > > guest:
> > > qemu-system-aarch64: IOMMU_IOAS_UNMAP failed: No such file or directory
> > > qemu-system-aarch64: vfio_container_dma_unmap(0xaaaae1fc0380,
> > > 0x8000000000, 0x10000) = -2 (No such file or directory)
> >
> From ex-email reply
> (Eric) In qemu arm virt machine 0x8000000000 matches the PCI MMIO region.
> (Yi) Currently, iommufd kernel part doesn't support mapping device BAR MMIO.
> This is a known gap.

OK.

> > > qemu-system-aarch64: Failed to unset data -1
> > > Segmentation fault (core dumped).  // also happened in 6.2
> >
> > Hmm, would it be possible for you to run the test again by
> > adding the following tracers to your QEMU command?
> >     --trace "iommufd*" \
> >     --trace "smmu*" \
> >     --trace "vfio_*" \
> >     --trace "pci_*"
> >
> 
> Have sent you the log directly, since it is too big.

I have found two missing pieces in the device detach routine.
Applying the following should fix the crash at hotplug path.

----------------------------------------------------------------------------
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index 89a256efa999..2344307523cb 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -151,8 +151,10 @@ void vfio_container_destroy(VFIOContainer *container)
     }

     QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, tmp) {
-        memory_region_unregister_iommu_notifier(
-                MEMORY_REGION(giommu->iommu_mr), &giommu->n);
+        if (giommu->n.notifier_flags) {
+            memory_region_unregister_iommu_notifier(
+                    MEMORY_REGION(giommu->iommu_mr), &giommu->n);
+        }
         QLIST_REMOVE(giommu, giommu_next);
         g_free(giommu);
     }
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 844c60892db2..35d31480390d 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -652,6 +652,9 @@ found:
      */
     if (QLIST_EMPTY(&container->hwpt_list)) {
         vfio_as_del_container(space, bcontainer);
+        if (bcontainer->nested) {
+            memory_listener_unregister(& bcontainer->prereg_listener);
+        }
     }
     __vfio_device_detach_container(vbasedev, container, &err);
     if (err) {
----------------------------------------------------------------------------

Would you please try your case with it?

Thanks
Nic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 00/17] Add Nested Translation Support for SMMUv3
  2023-05-25 23:42       ` Nicolin Chen
@ 2023-05-26  1:58         ` zhangfei gao
  2023-05-26  5:10           ` Nicolin Chen
  0 siblings, 1 reply; 6+ messages in thread
From: zhangfei gao @ 2023-05-26  1:58 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: jgg, robin.murphy, will, eric.auger, kevin.tian, baolu.lu, joro,
	shameerali.kolothum.thodi, jean-philippe, linux-arm-kernel, iommu,
	linux-kernel, kvm, alex.williamson, yi.l.liu


在 2023/5/26 07:42, Nicolin Chen 写道:
> On Tue, May 16, 2023 at 11:12:44AM +0800, Zhangfei Gao wrote:
>
>>>> However when debugging hotplug PCI device, it still does not work,
>>>> Segmentation fault same as 6.2.
>>>>
>>>> guest kernel
>>>> CONFIG_HOTPLUG_PCI_PCIE=y
>>>>
>>>> boot guest (this info does not appear in 6.2)
>>>> qemu-system-aarch64: -device
>>>> vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
>>>> Failed to set data -1
>>>> qemu-system-aarch64: -device
>>>> vfio-pci,host=0000:76:00.1,bus=pci.1,addr=0x0,id=acc1,iommufd=iommufd0:
>>>> failed to set device data
>>> Hmm.. I wonder what fails the set_dev_data ioctl...
>> Simply debug, it is because dev_data.sid=0, causing
>> arm_smmu_set_dev_user_data fail
> I found that too. The input pci bus number is 1, yet the in
> the context of set_dev_data, the pci bus number is 0, which
> resulted in a 0-valued sid. I will take another look to get
> why.
>
>>>> $ sudo nc -U /tmp/qmpm_1.socket
>>>> (qemu) info pci
>>>> (qemu) device_del acc1
>>>>
>>>> guest:
>>>> qemu-system-aarch64: IOMMU_IOAS_UNMAP failed: No such file or directory
>>>> qemu-system-aarch64: vfio_container_dma_unmap(0xaaaae1fc0380,
>>>> 0x8000000000, 0x10000) = -2 (No such file or directory)
>>  From ex-email reply
>> (Eric) In qemu arm virt machine 0x8000000000 matches the PCI MMIO region.
>> (Yi) Currently, iommufd kernel part doesn't support mapping device BAR MMIO.
>> This is a known gap.
> OK.
>
>>>> qemu-system-aarch64: Failed to unset data -1
>>>> Segmentation fault (core dumped).  // also happened in 6.2
>>> Hmm, would it be possible for you to run the test again by
>>> adding the following tracers to your QEMU command?
>>>      --trace "iommufd*" \
>>>      --trace "smmu*" \
>>>      --trace "vfio_*" \
>>>      --trace "pci_*"
>>>
>> Have sent you the log directly, since it is too big.
> I have found two missing pieces in the device detach routine.
> Applying the following should fix the crash at hotplug path.
>
> ----------------------------------------------------------------------------
> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> index 89a256efa999..2344307523cb 100644
> --- a/hw/vfio/container-base.c
> +++ b/hw/vfio/container-base.c
> @@ -151,8 +151,10 @@ void vfio_container_destroy(VFIOContainer *container)
>       }
>
>       QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, tmp) {
> -        memory_region_unregister_iommu_notifier(
> -                MEMORY_REGION(giommu->iommu_mr), &giommu->n);
> +        if (giommu->n.notifier_flags) {
> +            memory_region_unregister_iommu_notifier(
> +                    MEMORY_REGION(giommu->iommu_mr), &giommu->n);
> +        }
>           QLIST_REMOVE(giommu, giommu_next);
>           g_free(giommu);
>       }
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 844c60892db2..35d31480390d 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -652,6 +652,9 @@ found:
>        */
>       if (QLIST_EMPTY(&container->hwpt_list)) {
>           vfio_as_del_container(space, bcontainer);
> +        if (bcontainer->nested) {
> +            memory_listener_unregister(& bcontainer->prereg_listener);
> +        }
>       }
>       __vfio_device_detach_container(vbasedev, container, &err);
>       if (err) {
> ----------------------------------------------------------------------------
>
> Would you please try your case with it?


Yes, this solve the hotplug segmentation fault

Still report

qemu-system-aarch64: IOMMU_IOAS_UNMAP failed: No such file or directory
qemu-system-aarch64: vfio_container_dma_unmap(0xaaaae622e300, 
0x8000000000, 0x10000) = -2 (No such file or directory)
qemu-system-aarch64: Failed to unset data -1 (only the first time of 
device_del)

Test with device_del and device_add

Thanks.




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 00/17] Add Nested Translation Support for SMMUv3
  2023-05-26  1:58         ` zhangfei gao
@ 2023-05-26  5:10           ` Nicolin Chen
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2023-05-26  5:10 UTC (permalink / raw)
  To: zhangfei gao
  Cc: jgg, robin.murphy, will, eric.auger, kevin.tian, baolu.lu, joro,
	shameerali.kolothum.thodi, jean-philippe, linux-arm-kernel, iommu,
	linux-kernel, kvm, alex.williamson, yi.l.liu

On Fri, May 26, 2023 at 09:58:52AM +0800, zhangfei gao wrote:

> > I have found two missing pieces in the device detach routine.
> > Applying the following should fix the crash at hotplug path.
> > 
> > ----------------------------------------------------------------------------
> > diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> > index 89a256efa999..2344307523cb 100644
> > --- a/hw/vfio/container-base.c
> > +++ b/hw/vfio/container-base.c
> > @@ -151,8 +151,10 @@ void vfio_container_destroy(VFIOContainer *container)
> >       }
> > 
> >       QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, tmp) {
> > -        memory_region_unregister_iommu_notifier(
> > -                MEMORY_REGION(giommu->iommu_mr), &giommu->n);
> > +        if (giommu->n.notifier_flags) {
> > +            memory_region_unregister_iommu_notifier(
> > +                    MEMORY_REGION(giommu->iommu_mr), &giommu->n);
> > +        }
> >           QLIST_REMOVE(giommu, giommu_next);
> >           g_free(giommu);
> >       }
> > diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> > index 844c60892db2..35d31480390d 100644
> > --- a/hw/vfio/iommufd.c
> > +++ b/hw/vfio/iommufd.c
> > @@ -652,6 +652,9 @@ found:
> >        */
> >       if (QLIST_EMPTY(&container->hwpt_list)) {
> >           vfio_as_del_container(space, bcontainer);
> > +        if (bcontainer->nested) {
> > +            memory_listener_unregister(& bcontainer->prereg_listener);
> > +        }
> >       }
> >       __vfio_device_detach_container(vbasedev, container, &err);
> >       if (err) {
> > ----------------------------------------------------------------------------
> > 
> > Would you please try your case with it?
> 
> 
> Yes, this solve the hotplug segmentation fault

Nice. Thanks!

> Still report
> 
> qemu-system-aarch64: IOMMU_IOAS_UNMAP failed: No such file or directory
> qemu-system-aarch64: vfio_container_dma_unmap(0xaaaae622e300,
> 0x8000000000, 0x10000) = -2 (No such file or directory)
> qemu-system-aarch64: Failed to unset data -1 (only the first time of
> device_del)
> 
> Test with device_del and device_add

I found the "pci.1" has secondary bus number 0 when VM inits:

(qemu) info pci
  [...]
  Bus  0, device   2, function 0:
    PCI bridge: PCI device 1b36:000c
      IRQ 0, pin A
      BUS 0.
      secondary bus 0.
      subordinate bus 0.
      IO range [0xf000, 0x0fff]
      memory range [0xfff00000, 0x000fffff]
      prefetchable memory range [0xfff00000, 0x000fffff]
      BAR0: 32 bit memory at 0xffffffffffffffff [0x00000ffe].
      id "pci.1"

Then it changes later during the guest OS boots:

(qemu) info pci
  [...]
  Bus  0, device   2, function 0:
    PCI bridge: PCI device 1b36:000c
      IRQ 255, pin A
      BUS 0.
      secondary bus 1.
      subordinate bus 1.
      IO range [0x0000, 0x0fff]
      memory range [0x10000000, 0x101fffff]
      prefetchable memory range [0x8000000000, 0x80000fffff]
      BAR0: 32 bit memory at 0x10240000 [0x10240fff].
      id "pci.1"

This must be related the PCI bus init thing, since it doesn't
fully assign correct the bus numbers and ranges being listed
above, in the first dump.

I will try figuring out what's going on, because this doesn't
make too much sense for our ->set_iommu_device callback if a
PCIBus isn't fully ready.

Alternatively, I could move the set_dev_data ioctl out of the
->set_iommu_device callback to a later stage.

Overall, this should be fixed in the next version.

Thank you
Nicolin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-05-26  5:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1683688960.git.nicolinc@nvidia.com>
2023-05-15 10:00 ` [PATCH v2 00/17] Add Nested Translation Support for SMMUv3 Zhangfei Gao
2023-05-15 15:57   ` Nicolin Chen
2023-05-16  3:12     ` Zhangfei Gao
2023-05-25 23:42       ` Nicolin Chen
2023-05-26  1:58         ` zhangfei gao
2023-05-26  5:10           ` Nicolin Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).