In order to validate ITS-MSI hwirq entry in the /proc/interrupts, we have created a shell script to check is there any duplicated ITS-MSI hwirq entry.
Joseph Jang (1): selftest: drivers: Add support its msi hwirq checking
tools/testing/selftests/drivers/irq/Makefile | 5 +++++ .../selftests/drivers/irq/its-msi-irq-test.sh | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tools/testing/selftests/drivers/irq/Makefile create mode 100755 tools/testing/selftests/drivers/irq/its-msi-irq-test.sh
Validate there are no duplicate ITS-MSI hwirqs from the /sys/kernel/irq/*/hwirq.
One example log show 2 duplicated MSI entries in the /proc/interrupts.
150: 0 ... ITS-MSI 3355443200 Edge pciehp 152: 0 ... ITS-MSI 3355443200 Edge pciehp
Kernel patch ("PCI/MSI: Fix MSI hwirq truncation") [1] fix above issue. [1]: https://lore.kernel.org/all/20240115135649.708536-1-vidyas@nvidia.com/
Reviewed-by: Matthew R. Ochs mochs@nvidia.com Signed-off-by: Joseph Jang jjang@nvidia.com --- tools/testing/selftests/drivers/irq/Makefile | 5 +++++ .../selftests/drivers/irq/its-msi-irq-test.sh | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tools/testing/selftests/drivers/irq/Makefile create mode 100755 tools/testing/selftests/drivers/irq/its-msi-irq-test.sh
diff --git a/tools/testing/selftests/drivers/irq/Makefile b/tools/testing/selftests/drivers/irq/Makefile new file mode 100644 index 000000000000..569df5de22ee --- /dev/null +++ b/tools/testing/selftests/drivers/irq/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 + +TEST_PROGS := its-msi-irq-test.sh + +include ../../lib.mk diff --git a/tools/testing/selftests/drivers/irq/its-msi-irq-test.sh b/tools/testing/selftests/drivers/irq/its-msi-irq-test.sh new file mode 100755 index 000000000000..87c88674903f --- /dev/null +++ b/tools/testing/selftests/drivers/irq/its-msi-irq-test.sh @@ -0,0 +1,20 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +if [ -z "$(grep "ITS-MSI" /proc/interrupts)" ]; then + echo "SKIP: no ITS-MSI irq." + exit 4 +fi + +# Get ITS-MSI hwirq list from /sys/kernel/irq/*/hwirq. +its_msi_irq_list=$(grep "ITS-MSI" /sys/kernel/irq/*/chip_name | + awk -F ':' '{print $1}' | + xargs -I {} sh -c 'cat $(dirname {})/hwirq' | sort -V) + +# Check whether could find duplicated its-msi hwirq or not. +if [ -n "$(echo "$its_msi_irq_list" | uniq -cd)" ]; then + echo "ERROR: find duplicated its-msi hwirq." + exit 1 +fi + +exit 0
[+cc Thomas]
On Wed, May 29, 2024 at 06:27:27PM -0700, Joseph Jang wrote:
Validate there are no duplicate ITS-MSI hwirqs from the /sys/kernel/irq/*/hwirq.
One example log show 2 duplicated MSI entries in the /proc/interrupts.
150: 0 ... ITS-MSI 3355443200 Edge pciehp 152: 0 ... ITS-MSI 3355443200 Edge pciehp
I don't know how ITS-MSI works, so I don't know whether it's an error that both entries mention 3355443200.
3355443200 == 0xc8000000, which looks like it could be an address or address/data pair or something, and it does make sense to me that if two devices write the same MSI address/data, it should result in the same IRQ.
It seems like maybe this is a generic issue, i.e., if this is a problem, maybe it would affect *other* kinds of MSI too, not just ITS-MSI?
Kernel patch ("PCI/MSI: Fix MSI hwirq truncation") [1] fix above issue. [1]: https://lore.kernel.org/all/20240115135649.708536-1-vidyas@nvidia.com/
Reviewed-by: Matthew R. Ochs mochs@nvidia.com Signed-off-by: Joseph Jang jjang@nvidia.com
tools/testing/selftests/drivers/irq/Makefile | 5 +++++ .../selftests/drivers/irq/its-msi-irq-test.sh | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tools/testing/selftests/drivers/irq/Makefile create mode 100755 tools/testing/selftests/drivers/irq/its-msi-irq-test.sh
diff --git a/tools/testing/selftests/drivers/irq/Makefile b/tools/testing/selftests/drivers/irq/Makefile new file mode 100644 index 000000000000..569df5de22ee --- /dev/null +++ b/tools/testing/selftests/drivers/irq/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0
+TEST_PROGS := its-msi-irq-test.sh
+include ../../lib.mk diff --git a/tools/testing/selftests/drivers/irq/its-msi-irq-test.sh b/tools/testing/selftests/drivers/irq/its-msi-irq-test.sh new file mode 100755 index 000000000000..87c88674903f --- /dev/null +++ b/tools/testing/selftests/drivers/irq/its-msi-irq-test.sh @@ -0,0 +1,20 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0
+if [ -z "$(grep "ITS-MSI" /proc/interrupts)" ]; then
- echo "SKIP: no ITS-MSI irq."
- exit 4
+fi
+# Get ITS-MSI hwirq list from /sys/kernel/irq/*/hwirq. +its_msi_irq_list=$(grep "ITS-MSI" /sys/kernel/irq/*/chip_name |
Is there a limit on the size of the "*" expansion here?
awk -F ':' '{print $1}' |xargs -I {} sh -c 'cat $(dirname {})/hwirq' | sort -V)+# Check whether could find duplicated its-msi hwirq or not. +if [ -n "$(echo "$its_msi_irq_list" | uniq -cd)" ]; then
- echo "ERROR: find duplicated its-msi hwirq."
- exit 1
+fi
+exit 0
2.34.1
On Wed, Jul 31 2024 at 14:24, Bjorn Helgaas wrote:
On Wed, May 29, 2024 at 06:27:27PM -0700, Joseph Jang wrote:
Validate there are no duplicate ITS-MSI hwirqs from the /sys/kernel/irq/*/hwirq.
One example log show 2 duplicated MSI entries in the /proc/interrupts.
150: 0 ... ITS-MSI 3355443200 Edge pciehp 152: 0 ... ITS-MSI 3355443200 Edge pciehp
I don't know how ITS-MSI works, so I don't know whether it's an error that both entries mention 3355443200.
3355443200 == 0xc8000000, which looks like it could be an address or address/data pair or something, and it does make sense to me that if two devices write the same MSI address/data, it should result in the same IRQ.
That was an issue with truncation which got fixed some time ago:
https://lore.kernel.org/all/20240115135649.708536-1-vidyas@nvidia.com/
It seems like maybe this is a generic issue, i.e., if this is a problem, maybe it would affect *other* kinds of MSI too, not just ITS-MSI?
It's the same for ALL interrupts whether MSI or not.
The requirement is that for any interrupt chip all hardware interrupt numbers related to a particular chip must be unique.
Adding a ITS-MSI specific parser is just wrong. It's a generic problem and has absolutely nothing to do with ITS or MSI.
Aside of that the proposed parser does not even work anymore on 6.11 because we switched ARM[64] over to per device domains during the merge window.
So if we want a selftest for the correctness of the hardware interrupt numbers then it should grab the per interrupt sysfs entry 'chip_name' and 'hwirq' pairs and do an analysis per 'chip_name' whether all hardware interrupt numbers for a chip are unique.
Thanks,
tglx
On Wed, Jul 31 2024 at 22:42, Thomas Gleixner wrote:
Aside of that the proposed parser does not even work anymore on 6.11 because we switched ARM[64] over to per device domains during the merge window.
So if we want a selftest for the correctness of the hardware interrupt numbers then it should grab the per interrupt sysfs entry 'chip_name' and 'hwirq' pairs and do an analysis per 'chip_name' whether all hardware interrupt numbers for a chip are unique.
I just hacked up a 20 lines snake script to analyze it and indeed that produces duplicates because some interrupt chips do not have unique chip names as they are shared between interrupt domains and the chip names are constant.
There are several ways to handle this:
1) Amend /sys/kernel/irq/$N/chip_name with the irq domain name
2) Expose the irq domain name in /sys/kernel/irq/$N/domain_name
3) Utilize the existing /sys/kernel/debug/irq/ mechanism
#1 Does change the output of chip_name, but that is a kernel internal detail anyway so there is no real UABI concern.
#2 has the advantage that it does not change the output of chip_name but it consumes more memory for a dubious value.
#3 has the downside that it requires CONFIG_GENERIC_IRQ_DEBUGFS=y and is root only, but that should be not a problem for testing. We have other selftests which have Kconfig dependencies and root requirements. The upside is that it does not require kernel changes.
No real strong opinion either way, but all of that is better than a ITS specific parser which fails to work on the next kernel version.
Thanks,
tglx
On 5/29/24 19:27, Joseph Jang wrote:
In order to validate ITS-MSI hwirq entry in the /proc/interrupts, we have created a shell script to check is there any duplicated ITS-MSI hwirq entry.
Joseph Jang (1): selftest: drivers: Add support its msi hwirq checking
tools/testing/selftests/drivers/irq/Makefile | 5 +++++ .../selftests/drivers/irq/its-msi-irq-test.sh | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tools/testing/selftests/drivers/irq/Makefile create mode 100755 tools/testing/selftests/drivers/irq/its-msi-irq-test.sh
Sorry for the delay on this. It has gotten lost in my Inbox. Since this is related to PCIe and MSI, I would like PCIe maintainers to review this first.
Adding Bjorn Helgaas and linux-pci@vger.kernel.org list
Bjorn, please review - I can take this through kselftest tree once I get your okay.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Bjorn Helgaas -
Joseph Jang -
Shuah Khan -
Thomas Gleixner