From: John Garry <john.garry@huawei.com>
To: Robin Murphy <robin.murphy@arm.com>,
Marc Zyngier <maz@kernel.org>, "Will Deacon" <will@kernel.org>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Sudeep Holla <sudeep.holla@arm.com>,
"Guohanjun (Hanjun Guo)" <guohanjun@huawei.com>
Cc: Saravana Kannan <saravanak@google.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Alex Williamson <alex.williamson@redhat.com>,
Linuxarm <linuxarm@huawei.com>,
iommu <iommu@lists.linux-foundation.org>,
Bjorn Helgaas <bhelgaas@google.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: arm64 iommu groups issue
Date: Mon, 17 Feb 2020 12:08:48 +0000 [thread overview]
Message-ID: <828ec7b3-27af-f0b9-b4a6-0886b0c24b5a@huawei.com> (raw)
In-Reply-To: <68643b18-c920-f997-a6d4-a5d9177c0f4e@arm.com>
>>
>> Right, and even worse is that it relies on the port driver even
>> existing at all.
>>
>> All this iommu group assignment should be taken outside device driver
>> probe paths.
>>
>> However we could still consider device links for sync'ing the SMMU and
>> each device probing.
>
> Yes, we should get that for DT now thanks to the of_devlink stuff, but
> cooking up some equivalent for IORT might be worthwhile.
It doesn't solve this problem, but at least we could remove the
iommu_ops check in iort_iommu_xlate().
We would need to carve out a path from pci_device_add() or even
device_add() to solve all cases.
>
>>> Another thought that crosses my mind is that when pci_device_group()
>>> walks up to the point of ACS isolation and doesn't find an existing
>>> group, it can still infer that everything it walked past *should* be put
>>> in the same group it's then eventually going to return. Unfortunately I
>>> can't see an obvious way for it to act on that knowledge, though, since
>>> recursive iommu_probe_device() is unlikely to end well.
>>
[...]
>> And this looks to be the reason for which current
>> iommu_bus_init()->bus_for_each_device(..., add_iommu_group) fails also.
>
> Of course, just adding a 'correct' add_device replay without the
> of_xlate process doesn't help at all. No wonder this looked suspiciously
> simpler than where the first idea left off...
>
> (on reflection, the core of this idea seems to be recycling the existing
> iommu_bus_init walk rather than building up a separate "waiting list",
> while forgetting that that wasn't the difficult part of the original
> idea anyway)
We could still use a bus walk to add the group per iommu, but we would
need an additional check to ensure the device is associated with the IOMMU.
>
>> On this current code mentioned, the principle of this seems wrong to
>> me - we call bus_for_each_device(..., add_iommu_group) for the first
>> SMMU in the system which probes, but we attempt to add_iommu_group()
>> for all devices on the bus, even though the SMMU for that device may
>> yet to have probed.
>
> Yes, iommu_bus_init() is one of the places still holding a
> deeply-ingrained assumption that the ops go live for all IOMMU instances
> at once, which is what warranted the further replay in
> of_iommu_configure() originally. Moving that out of
> of_platform_device_create() to support probe deferral is where the
> trouble really started.
I'm not too familiar with the history here, but could this be reverted
now with the introduction of of_devlink stuff?
Cheers,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-02-17 12:08 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-19 8:43 arm64 iommu groups issue John Garry
2019-09-19 13:25 ` Robin Murphy
2019-09-19 14:35 ` John Garry
2019-11-04 12:18 ` John Garry
2020-02-13 15:49 ` John Garry
2020-02-13 19:40 ` Robin Murphy
2020-02-14 14:09 ` John Garry
2020-02-14 18:35 ` Robin Murphy
2020-02-17 12:08 ` John Garry [this message]
2020-06-12 14:30 ` Lorenzo Pieralisi
2020-06-15 7:35 ` John Garry
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=828ec7b3-27af-f0b9-b4a6-0886b0c24b5a@huawei.com \
--to=john.garry@huawei.com \
--cc=alex.williamson@redhat.com \
--cc=bhelgaas@google.com \
--cc=guohanjun@huawei.com \
--cc=iommu@lists.linux-foundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=lorenzo.pieralisi@arm.com \
--cc=maz@kernel.org \
--cc=robin.murphy@arm.com \
--cc=saravanak@google.com \
--cc=sudeep.holla@arm.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).