On 2022-04-21 15:43, Shameerali Kolothum Thodi wrote:
-----Original Message-----
From: Steven Price [mailto:steven.price@xxxxxxx]
Sent: 21 April 2022 13:59
To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>;
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx;
iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
Cc: Linuxarm <linuxarm@xxxxxxxxxx>; lorenzo.pieralisi@xxxxxxx;
joro@xxxxxxxxxx; robin.murphy@xxxxxxx; will@xxxxxxxxxx; wanghuiqiang
<wanghuiqiang@xxxxxxxxxx>; Guohanjun (Hanjun Guo)
<guohanjun@xxxxxxxxxx>; Sami.Mujawar@xxxxxxx; jon@xxxxxxxxxxxxx;
eric.auger@xxxxxxxxxx; laurentiu.tudor@xxxxxxx; hch@xxxxxxxxxxxxx
Subject: Re: [PATCH v10 0/9] ACPI/IORT: Support for IORT RMR node
On 20/04/2022 17:48, Shameer Kolothum wrote:
Hi
v9 --> v10
- Dropped patch #1 ("Add temporary RMR node flag definitions") since
the ACPICA header updates patch is now in the mailing list[1]
- Based on the suggestion from Christoph, introduced a
resv_region_free_fw_data() callback in struct iommu_resv_region and
used that to free RMR specific memory allocations.
Though there is a small change from v9 with respect to how we free up
the FW specific data, I have taken the liberty to pick up the R-by and
T-by tags from Lorenzo, Steve and Laurentiu. But please do take a look
again and let me know.
I've given this a go and it works fine on my Juno setup. So do keep my
T-by tag.
Many thanks for that.
Sami has been kind enough to give me an updated firmware which also
fixes the RMR node in the IORT. Although as mentioned before the details
of the RMR node are currently being ignored so this doesn't change the
functionality but silences the warning.
Strictly they're not ignored, you just won't be getting past the point
where they're not entirely not ignored. It'll appear to work because
arm_smmu_rmr_install_bypass_smr() just bypasses the whole stream until
the actual device turns up to join up to the StreamID and the "real"
processing of RMRs happens via iommu_create_device_direct_mappings() -
if there's no actual HDLCD device described in the DSDT, that will never
happen, and even if there is, chances are that things will currently
happen in the wrong order such we'd end up waiting to replay
iommu_probe_device() from acpi_iommu_configure_id() once a driver binds,
and *that* definitely can't happen without teaching the HDLCD driver
about ACPI.
My concern is that with the RMR region effectively ignored we may see
more broken firmware, and while a length of zero produces a warning, an
otherwise incorrect length will currently "silently work" but mean that
any future tightening would cause problems. For example if the SMMU
driver were to recreate the mappings to only cover the region specified
in the RMR it may not be large enough if the RMR base/length are not
correct.
Not sure how we can further validate the RMR if the firmware provides an
incorrect one. I see your point of future tightening causing problems
with broken firmware. But then it is indeed a "broken firmware"...
It's up to the maintainers as to whether they see this as a
problem or not.
Hi Robin,
Any thoughts on this?
In general we can't second-guess firmware. Even a zero-length RMR should
have ample opportunity to blow up outside this one corner case where
Linux never gets to associate the StreamID with a corresponding device.
Robin.
