* [PATCH 0/1] selftest: drivers: Add support its msi hwirq checking @ 2024-05-30 1:27 Joseph Jang 2024-05-30 1:27 ` [PATCH 1/1] " Joseph Jang 2024-07-30 20:59 ` [PATCH 0/1] " Shuah Khan 0 siblings, 2 replies; 6+ messages in thread From: Joseph Jang @ 2024-05-30 1:27 UTC (permalink / raw) To: shuah, jjang, mochs, linux-kernel, linux-kselftest; +Cc: linux-tegra 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 -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] selftest: drivers: Add support its msi hwirq checking 2024-05-30 1:27 [PATCH 0/1] selftest: drivers: Add support its msi hwirq checking Joseph Jang @ 2024-05-30 1:27 ` Joseph Jang 2024-07-31 19:24 ` Bjorn Helgaas 2024-07-30 20:59 ` [PATCH 0/1] " Shuah Khan 1 sibling, 1 reply; 6+ messages in thread From: Joseph Jang @ 2024-05-30 1:27 UTC (permalink / raw) To: shuah, jjang, mochs, linux-kernel, linux-kselftest; +Cc: linux-tegra 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 -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] selftest: drivers: Add support its msi hwirq checking 2024-05-30 1:27 ` [PATCH 1/1] " Joseph Jang @ 2024-07-31 19:24 ` Bjorn Helgaas 2024-07-31 20:42 ` Thomas Gleixner 0 siblings, 1 reply; 6+ messages in thread From: Bjorn Helgaas @ 2024-07-31 19:24 UTC (permalink / raw) To: Joseph Jang Cc: shuah, mochs, linux-kernel, linux-kselftest, linux-tegra, Thomas Gleixner [+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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] selftest: drivers: Add support its msi hwirq checking 2024-07-31 19:24 ` Bjorn Helgaas @ 2024-07-31 20:42 ` Thomas Gleixner 2024-07-31 23:12 ` Thomas Gleixner 0 siblings, 1 reply; 6+ messages in thread From: Thomas Gleixner @ 2024-07-31 20:42 UTC (permalink / raw) To: Bjorn Helgaas, Joseph Jang Cc: shuah, mochs, linux-kernel, linux-kselftest, linux-tegra 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] selftest: drivers: Add support its msi hwirq checking 2024-07-31 20:42 ` Thomas Gleixner @ 2024-07-31 23:12 ` Thomas Gleixner 0 siblings, 0 replies; 6+ messages in thread From: Thomas Gleixner @ 2024-07-31 23:12 UTC (permalink / raw) To: Bjorn Helgaas, Joseph Jang Cc: shuah, mochs, linux-kernel, linux-kselftest, linux-tegra 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/1] selftest: drivers: Add support its msi hwirq checking 2024-05-30 1:27 [PATCH 0/1] selftest: drivers: Add support its msi hwirq checking Joseph Jang 2024-05-30 1:27 ` [PATCH 1/1] " Joseph Jang @ 2024-07-30 20:59 ` Shuah Khan 1 sibling, 0 replies; 6+ messages in thread From: Shuah Khan @ 2024-07-30 20:59 UTC (permalink / raw) To: Joseph Jang, shuah, mochs, linux-kernel, linux-kselftest, Bjorn Helgaas Cc: linux-tegra, linux-pci@vger.kernel.org, Shuah Khan 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-31 23:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-05-30 1:27 [PATCH 0/1] selftest: drivers: Add support its msi hwirq checking Joseph Jang 2024-05-30 1:27 ` [PATCH 1/1] " Joseph Jang 2024-07-31 19:24 ` Bjorn Helgaas 2024-07-31 20:42 ` Thomas Gleixner 2024-07-31 23:12 ` Thomas Gleixner 2024-07-30 20:59 ` [PATCH 0/1] " Shuah Khan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox