From: Lu Baolu <baolu.lu@linux.intel.com>
To: eric.auger@redhat.com, Robin Murphy <robin.murphy@arm.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Joerg Roedel <joro@8bytes.org>,
Alex Williamson <alex.williamson@redhat.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Jason Gunthorpe <jgg@nvidia.com>,
Christoph Hellwig <hch@infradead.org>,
Kevin Tian <kevin.tian@intel.com>,
Ashok Raj <ashok.raj@intel.com>
Cc: baolu.lu@linux.intel.com, Chaitanya Kulkarni <kch@nvidia.com>,
kvm@vger.kernel.org, Stuart Yoder <stuyoder@gmail.com>,
rafael@kernel.org, David Airlie <airlied@linux.ie>,
linux-pci@vger.kernel.org, Cornelia Huck <cohuck@redhat.com>,
linux-kernel@vger.kernel.org,
Jonathan Hunter <jonathanh@nvidia.com>,
iommu@lists.linux-foundation.org,
Thierry Reding <thierry.reding@gmail.com>,
Jacob jun Pan <jacob.jun.pan@intel.com>,
Daniel Vetter <daniel@ffwll.ch>,
Diana Craciun <diana.craciun@oss.nxp.com>,
Dan Williams <dan.j.williams@intel.com>,
Li Yang <leoyang.li@nxp.com>, Will Deacon <will@kernel.org>,
Dmitry Osipenko <digetx@gmail.com>
Subject: Re: [PATCH v7 01/11] iommu: Add DMA ownership management interfaces
Date: Tue, 8 Mar 2022 08:31:14 +0800 [thread overview]
Message-ID: <5fb85230-466c-9615-0867-bb17cab34be5@linux.intel.com> (raw)
In-Reply-To: <ac75c521-fb13-8414-a81b-9178cbed3471@redhat.com>
On 2022/3/7 20:42, Eric Auger wrote:
> Hi Lu,
>
> On 3/7/22 4:27 AM, Lu Baolu wrote:
>> Hi Robin,
>>
>> On 3/4/22 10:10 PM, Robin Murphy wrote:
>>> On 2022-03-04 13:55, Eric Auger wrote:
>>>> Hi Robin,
>>>>
>>>> On 3/4/22 1:22 PM, Robin Murphy wrote:
>>>>> On 2022-03-04 10:43, Lu Baolu wrote:
>>>>>> Hi Eric,
>>>>>>
>>>>>> On 2022/3/4 18:34, Eric Auger wrote:
>>>>>>> I hit a WARN_ON() when unbinding an e1000e driver just after boot:
>>>>>>>
>>>>>>> sudo modprobe -v vfio-pci
>>>>>>> echo vfio-pci | sudo tee -a
>>>>>>> /sys/bus/pci/devices/0004:01:00.0/driver_override
>>>>>>> vfio-pci
>>>>>>> echo 0004:01:00.0 | sudo tee -a /sys/bus/pci/drivers/e1000e/unbind
>>>>>>>
>>>>>>>
>>>>>>> [ 390.042811] ------------[ cut here ]------------
>>>>>>> [ 390.046468] WARNING: CPU: 42 PID: 5589 at
>>>>>>> drivers/iommu/iommu.c:3123
>>>>>>> iommu_device_unuse_default_domain+0x68/0x100
>>>>>>> [ 390.056710] Modules linked in: vfio_pci vfio_pci_core vfio_virqfd
>>>>>>> vfio_iommu_type1 vfio xt_CHECKSUM xt_MASQUERADE xt_conntrack
>>>>>>> ipt_REJECT
>>>>>>> nf_reject_ipv4 nft_compat nft_chain_nat nf_nat nf_conntrack
>>>>>>> nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink bridge stp llc
>>>>>>> rfkill
>>>>>>> sunrpc vfat fat mlx5_ib ib_uverbs ib_core acpi_ipmi ipmi_ssif
>>>>>>> ipmi_devintf ipmi_msghandler cppc_cpufreq drm xfs libcrc32c
>>>>>>> mlx5_core sg
>>>>>>> mlxfw crct10dif_ce tls ghash_ce sha2_ce sha256_arm64 sha1_ce
>>>>>>> sbsa_gwdt
>>>>>>> e1000e psample sdhci_acpi ahci_platform sdhci libahci_platform
>>>>>>> qcom_emac
>>>>>>> mmc_core hdma hdma_mgmt dm_mirror dm_region_hash dm_log dm_mod fuse
>>>>>>> [ 390.110618] CPU: 42 PID: 5589 Comm: tee Kdump: loaded Not tainted
>>>>>>> 5.17.0-rc4-lu-v7-official+ #24
>>>>>>> [ 390.119384] Hardware name: WIWYNN QDF2400 Reference Evaluation
>>>>>>> Platform CV90-LA115-P120/QDF2400 Customer Reference Board, BIOS
>>>>>>> 0ACJA570
>>>>>>> 11/05/2018
>>>>>>> [ 390.132492] pstate: a0400005 (NzCv daif +PAN -UAO -TCO -DIT -SSBS
>>>>>>> BTYPE=--)
>>>>>>> [ 390.139436] pc : iommu_device_unuse_default_domain+0x68/0x100
>>>>>>> [ 390.145165] lr : iommu_device_unuse_default_domain+0x38/0x100
>>>>>>> [ 390.150894] sp : ffff80000fbb3bc0
>>>>>>> [ 390.154193] x29: ffff80000fbb3bc0 x28: ffff03c0cf6b2400 x27:
>>>>>>> 0000000000000000
>>>>>>> [ 390.161311] x26: 0000000000000000 x25: 0000000000000000 x24:
>>>>>>> ffff03c0c7cc5720
>>>>>>> [ 390.168429] x23: ffff03c0c2b9d150 x22: ffffb4e61df223f8 x21:
>>>>>>> ffffb4e61df223f8
>>>>>>> [ 390.175547] x20: ffff03c7c03c3758 x19: ffff03c7c03c3700 x18:
>>>>>>> 0000000000000000
>>>>>>> [ 390.182665] x17: 0000000000000000 x16: 0000000000000000 x15:
>>>>>>> 0000000000000000
>>>>>>> [ 390.189783] x14: 0000000000000000 x13: 0000000000000030 x12:
>>>>>>> ffff03c0d519cd80
>>>>>>> [ 390.196901] x11: 7f7f7f7f7f7f7f7f x10: 0000000000000dc0 x9 :
>>>>>>> ffffb4e620b54f8c
>>>>>>> [ 390.204019] x8 : ffff03c0cf6b3220 x7 : ffff4ef132bba000 x6 :
>>>>>>> 00000000000000ff
>>>>>>> [ 390.211137] x5 : ffff03c0c2b9f108 x4 : ffff03c0d51f6438 x3 :
>>>>>>> 0000000000000000
>>>>>>> [ 390.218255] x2 : ffff03c0cf6b2400 x1 : 0000000000000000 x0 :
>>>>>>> 0000000000000000
>>>>>>> [ 390.225374] Call trace:
>>>>>>> [ 390.227804] iommu_device_unuse_default_domain+0x68/0x100
>>>>>>> [ 390.233187] pci_dma_cleanup+0x38/0x44
>>>>>>> [ 390.236919] __device_release_driver+0x1a8/0x260
>>>>>>> [ 390.241519] device_driver_detach+0x50/0xd0
>>>>>>> [ 390.245686] unbind_store+0xf8/0x120
>>>>>>> [ 390.249245] drv_attr_store+0x30/0x44
>>>>>>> [ 390.252891] sysfs_kf_write+0x50/0x60
>>>>>>> [ 390.256537] kernfs_fop_write_iter+0x134/0x1cc
>>>>>>> [ 390.260964] new_sync_write+0xf0/0x18c
>>>>>>> [ 390.264696] vfs_write+0x230/0x2d0
>>>>>>> [ 390.268082] ksys_write+0x74/0x100
>>>>>>> [ 390.271467] __arm64_sys_write+0x28/0x3c
>>>>>>> [ 390.275373] invoke_syscall.constprop.0+0x58/0xf0
>>>>>>> [ 390.280061] el0_svc_common.constprop.0+0x160/0x164
>>>>>>> [ 390.284922] do_el0_svc+0x34/0xcc
>>>>>>> [ 390.288221] el0_svc+0x30/0x140
>>>>>>> [ 390.291346] el0t_64_sync_handler+0xa4/0x130
>>>>>>> [ 390.295599] el0t_64_sync+0x1a0/0x1a4
>>>>>>> [ 390.299245] ---[ end trace 0000000000000000 ]---
>>>>>>>
>>>>>>>
>>>>>>> I put some traces in the code and I can see that
>>>>>>> iommu_device_use_default_domain() effectively is called on
>>>>>>> 0004:01:00.0 e1000e device on pci_dma_configure() but at that time
>>>>>>> the iommu group is NULL:
>>>>>>> [ 10.569427] e1000e 0004:01:00.0: ------ ENTRY pci_dma_configure
>>>>>>> driver_managed_area=0
>>>>>>> [ 10.569431] e1000e 0004:01:00.0: ****
>>>>>>> iommu_device_use_default_domain ENTRY
>>>>>>> [ 10.569433] e1000e 0004:01:00.0: ****
>>>>>>> iommu_device_use_default_domain no group
>>>>>>> [ 10.569435] e1000e 0004:01:00.0: pci_dma_configure
>>>>>>> iommu_device_use_default_domain returned 0
>>>>>>> [ 10.569492] e1000e 0004:01:00.0: Adding to iommu group 3
>>>>>>>
>>>>>>> ^^^the group is added after the
>>>>>>> iommu_device_use_default_domain() call
>>>>>>> So the group->owner_cnt is not incremented as expected.
>>>>>>
>>>>>> Thank you for reporting this. Do you have any idea why the driver is
>>>>>> loaded before iommu_probe_device()?
>>>>>
>>>>> Urgh, this is the horrible firmware-data-ordering thing again. The
>>>>> stuff I've been saying about having to rework the whole .dma_configure
>>>>> mechanism in the near future is to fix this properly.
>>>>>
>>>>> The summary is that in patch #4, calling
>>>>> iommu_device_use_default_domain() *before* {of,acpi}_dma_configure is
>>>>> currently a problem. As things stand, the IOMMU driver ignored the
>>>>> initial iommu_probe_device() call when the device was added, since at
>>>>> that point it had no fwspec yet. In this situation,
>>>>> {of,acpi}_iommu_configure() are retriggering iommu_probe_device()
>>>>> after the IOMMU driver has seen the firmware data via .of_xlate to
>>>>> learn that it it actually responsible for the given device.
>>>>
>>>> thank you for providing the info. Hope this is something Lu can work
>>>> around.
>>>
>>> Hopefully it's just a case of flipping the calls around, so that
>>> iommu_use_default_domain() goes at the end, and calls
>>> arch_teardown_dma_ops() if it fails. From a quick skim I *think* that
>>> should still work out to the desired behaviour (or at least close
>>> enough that we can move forward without a circular dependency between
>>> fixes...)
>>
>> This is a reasonable solution to me. Thank you for the information and
>> suggestion.
>>
>> Eric, I have updated the patch #4 and uploaded a new version here:
>>
>> https://github.com/LuBaolu/intel-iommu/commits/iommu-dma-ownership-v8
>
> with v8 I do not hit the warning anymore and the owner accounting seems
> to work as expected.
Thank you, Eric! I will post the v8 soon.
Best regards,
baolu
next prev parent reply other threads:[~2022-03-08 0:31 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-28 0:50 [PATCH v7 00/11] Fix BUG_ON in vfio_iommu_group_notifier() Lu Baolu
2022-02-28 0:50 ` [PATCH v7 01/11] iommu: Add DMA ownership management interfaces Lu Baolu
2022-03-04 10:34 ` Eric Auger
2022-03-04 10:43 ` Lu Baolu
2022-03-04 12:22 ` Robin Murphy
2022-03-04 13:55 ` Eric Auger
2022-03-04 14:10 ` Robin Murphy
2022-03-07 3:27 ` Lu Baolu
2022-03-07 12:42 ` Eric Auger
2022-03-08 0:31 ` Lu Baolu [this message]
2022-02-28 0:50 ` [PATCH v7 02/11] driver core: Add dma_cleanup callback in bus_type Lu Baolu
2022-02-28 0:50 ` [PATCH v7 03/11] amba: Stop sharing platform_dma_configure() Lu Baolu
2022-02-28 0:50 ` [PATCH v7 04/11] bus: platform,amba,fsl-mc,PCI: Add device DMA ownership management Lu Baolu
2022-02-28 0:50 ` [PATCH v7 05/11] PCI: pci_stub: Set driver_managed_dma Lu Baolu
2022-02-28 19:54 ` Bjorn Helgaas
2022-02-28 0:50 ` [PATCH v7 06/11] PCI: portdrv: " Lu Baolu
2022-02-28 19:56 ` Bjorn Helgaas
2022-03-01 2:54 ` Lu Baolu
2022-02-28 0:50 ` [PATCH v7 07/11] vfio: Set DMA ownership for VFIO devices Lu Baolu
2022-02-28 22:06 ` Alex Williamson
2022-02-28 0:50 ` [PATCH v7 08/11] vfio: Remove use of vfio_group_viable() Lu Baolu
2022-02-28 22:06 ` Alex Williamson
2022-02-28 0:50 ` [PATCH v7 09/11] vfio: Delete the unbound_list Lu Baolu
2022-02-28 22:06 ` Alex Williamson
2022-02-28 0:50 ` [PATCH v7 10/11] vfio: Remove iommu group notifier Lu Baolu
2022-02-28 22:06 ` Alex Williamson
2022-02-28 0:50 ` [PATCH v7 11/11] iommu: Remove iommu group changes notifier Lu Baolu
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=5fb85230-466c-9615-0867-bb17cab34be5@linux.intel.com \
--to=baolu.lu@linux.intel.com \
--cc=airlied@linux.ie \
--cc=alex.williamson@redhat.com \
--cc=ashok.raj@intel.com \
--cc=bhelgaas@google.com \
--cc=cohuck@redhat.com \
--cc=dan.j.williams@intel.com \
--cc=daniel@ffwll.ch \
--cc=diana.craciun@oss.nxp.com \
--cc=digetx@gmail.com \
--cc=eric.auger@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=hch@infradead.org \
--cc=iommu@lists.linux-foundation.org \
--cc=jacob.jun.pan@intel.com \
--cc=jgg@nvidia.com \
--cc=jonathanh@nvidia.com \
--cc=joro@8bytes.org \
--cc=kch@nvidia.com \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=leoyang.li@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=robin.murphy@arm.com \
--cc=stuyoder@gmail.com \
--cc=thierry.reding@gmail.com \
--cc=will@kernel.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 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).