| Message ID | 20250301123953.291675-1-18255117159@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v2] genirq/msi: Add the address and data that show MSI/MSIX | expand |
Hi Hans,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 76544811c850a1f4c055aa182b513b7a843868ea]
url: https://github.com/intel-lab-lkp/linux/commits/Hans-Zhang/genirq-msi-Add-the-address-and-data-that-show-MSI-MSIX/20250301-204332
base: 76544811c850a1f4c055aa182b513b7a843868ea
patch link: https://lore.kernel.org/r/20250301123953.291675-1-18255117159%40163.com
patch subject: [v2] genirq/msi: Add the address and data that show MSI/MSIX
config: arm-randconfig-001-20250302 (https://download.01.org/0day-ci/archive/20250302/202503020812.PKZf7JBa-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250302/202503020812.PKZf7JBa-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503020812.PKZf7JBa-lkp@intel.com/
All warnings (new ones prefixed by >>):
kernel/irq/msi.c: In function 'msi_domain_debug_show':
kernel/irq/msi.c:770:9: error: implicit declaration of function 'seq_printf'; did you mean 'bstr_printf'? [-Wimplicit-function-declaration]
770 | seq_printf(m, "%*s%s:", ind, "", is_msix ? "msix" : "msi");
| ^~~~~~~~~~
| bstr_printf
kernel/irq/msi.c: At top level:
kernel/irq/msi.c:782:10: error: 'const struct irq_domain_ops' has no member named 'debug_show'
782 | .debug_show = msi_domain_debug_show,
| ^~~~~~~~~~
>> kernel/irq/msi.c:782:27: warning: excess elements in struct initializer
782 | .debug_show = msi_domain_debug_show,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/irq/msi.c:782:27: note: (near initialization for 'msi_domain_ops')
vim +782 kernel/irq/msi.c
775
776 static const struct irq_domain_ops msi_domain_ops = {
777 .alloc = msi_domain_alloc,
778 .free = msi_domain_free,
779 .activate = msi_domain_activate,
780 .deactivate = msi_domain_deactivate,
781 .translate = msi_domain_translate,
> 782 .debug_show = msi_domain_debug_show,
783 };
784
Hi Hans, kernel test robot noticed the following build errors: [auto build test ERROR on 76544811c850a1f4c055aa182b513b7a843868ea] url: https://github.com/intel-lab-lkp/linux/commits/Hans-Zhang/genirq-msi-Add-the-address-and-data-that-show-MSI-MSIX/20250301-204332 base: 76544811c850a1f4c055aa182b513b7a843868ea patch link: https://lore.kernel.org/r/20250301123953.291675-1-18255117159%40163.com patch subject: [v2] genirq/msi: Add the address and data that show MSI/MSIX config: x86_64-buildonly-randconfig-003-20250302 (https://download.01.org/0day-ci/archive/20250302/202503020807.c3MhmbJh-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250302/202503020807.c3MhmbJh-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202503020807.c3MhmbJh-lkp@intel.com/ All errors (new ones prefixed by >>): kernel/irq/msi.c: In function 'msi_domain_debug_show': >> kernel/irq/msi.c:770:9: error: implicit declaration of function 'seq_printf'; did you mean 'bstr_printf'? [-Werror=implicit-function-declaration] 770 | seq_printf(m, "%*s%s:", ind, "", is_msix ? "msix" : "msi"); | ^~~~~~~~~~ | bstr_printf kernel/irq/msi.c: At top level: >> kernel/irq/msi.c:782:10: error: 'const struct irq_domain_ops' has no member named 'debug_show' 782 | .debug_show = msi_domain_debug_show, | ^~~~~~~~~~ >> kernel/irq/msi.c:782:27: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init] 782 | .debug_show = msi_domain_debug_show, | ^~~~~~~~~~~~~~~~~~~~~ kernel/irq/msi.c:782:27: note: (near initialization for 'msi_domain_ops') kernel/irq/msi.c:782:27: error: initialization of 'int (*)(struct irq_domain *, unsigned int, unsigned int, void *)' from incompatible pointer type 'void (*)(struct seq_file *, struct irq_domain *, struct irq_data *, int)' [-Werror=incompatible-pointer-types] kernel/irq/msi.c:782:27: note: (near initialization for 'msi_domain_ops.alloc') kernel/irq/msi.c:782:27: warning: initialized field overwritten [-Woverride-init] kernel/irq/msi.c:782:27: note: (near initialization for 'msi_domain_ops.alloc') cc1: some warnings being treated as errors vim +770 kernel/irq/msi.c 758 759 static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d, 760 struct irq_data *irqd, int ind) 761 { 762 struct msi_desc *desc; 763 bool is_msix; 764 765 desc = irq_get_msi_desc(irqd->irq); 766 if (!desc) 767 return; 768 769 is_msix = desc->pci.msi_attrib.is_msix; > 770 seq_printf(m, "%*s%s:", ind, "", is_msix ? "msix" : "msi"); 771 seq_printf(m, "\n%*saddress_hi: 0x%08x", ind + 1, "", desc->msg.address_hi); 772 seq_printf(m, "\n%*saddress_lo: 0x%08x", ind + 1, "", desc->msg.address_lo); 773 seq_printf(m, "\n%*smsg_data: 0x%08x\n", ind + 1, "", desc->msg.data); 774 } 775 776 static const struct irq_domain_ops msi_domain_ops = { 777 .alloc = msi_domain_alloc, 778 .free = msi_domain_free, 779 .activate = msi_domain_activate, 780 .deactivate = msi_domain_deactivate, 781 .translate = msi_domain_translate, > 782 .debug_show = msi_domain_debug_show, 783 }; 784
Hans! On Sat, Mar 01 2025 at 20:39, Hans Zhang wrote: > The debug_show() callback function is implemented in the MSI core code. > And assign it to the domain ops::debug_show() creation. > > cat /sys/kernel/debug/irq/irqs/msi_irq_num, the address and data stored > in the MSI capability or the address and data stored in the MSIX vector > table will be displayed. So this explains what the patch is doing and what the output is. But it fails to explain the _why_. Documentation gives proper guidance: https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-changes > e.g. > root@root:/sys/kernel/debug/irq/irqs# cat /proc/interrupts | grep ITS > 85: 0 0 0 0 0 0 0 0 0 0 0 0 ITS-MSI 75497472 Edge PCIe PME, aerdrv > 86: 0 30 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021760 Edge nvme0q0 > 87: 287 0 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021761 Edge nvme0q1 > 88: 0 265 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021762 Edge nvme0q2 > 89: 0 0 177 0 0 0 0 0 0 0 0 0 ITS-MSI 76021763 Edge nvme0q3 > 90: 0 0 0 76 0 0 0 0 0 0 0 0 ITS-MSI 76021764 Edge nvme0q4 > 91: 0 0 0 0 161 0 0 0 0 0 0 0 ITS-MSI 76021765 Edge nvme0q5 > 92: 0 0 0 0 0 991 0 0 0 0 0 0 ITS-MSI 76021766 Edge nvme0q6 > 93: 0 0 0 0 0 0 194 0 0 0 0 0 ITS-MSI 76021767 Edge nvme0q7 > 94: 0 0 0 0 0 0 0 94 0 0 0 0 ITS-MSI 76021768 Edge nvme0q8 > 95: 0 0 0 0 0 0 0 0 148 0 0 0 ITS-MSI 76021769 Edge nvme0q9 > 96: 0 0 0 0 0 0 0 0 0 261 0 0 ITS-MSI 76021770 Edge nvme0q10 > 97: 0 0 0 0 0 0 0 0 0 0 127 0 ITS-MSI 76021771 Edge nvme0q11 > 98: 0 0 0 0 0 0 0 0 0 0 0 317 ITS-MSI 76021772 Edge nvme0q12 How is this relevant to describe the patch? > root@root:/sys/kernel/debug/irq/irqs# > root@root:/sys/kernel/debug/irq/irqs# cat 87 > handler: handle_fasteoi_irq > device: 0000:91:00.0 > status: 0x00000000 > istate: 0x00004000 > ddepth: 0 > wdepth: 0 > dstate: 0x31600200 > IRQD_ACTIVATED > IRQD_IRQ_STARTED > IRQD_SINGLE_TARGET > IRQD_AFFINITY_MANAGED > IRQD_AFFINITY_ON_ACTIVATE > IRQD_HANDLE_ENFORCE_IRQCTX > node: 0 > affinity: 0 > effectiv: 0 > domain: :soc@0:interrupt-controller@0e001000:its@0e050000-3 > hwirq: 0x4880001 > chip: ITS-MSI This output is from a pre 6.11 kernel... > flags: 0x20 > IRQCHIP_ONESHOT_SAFE > msix: > address_hi: 0x00000000 > address_lo: 0x0e060040 > msg_data: 0x00000001 For demonstration it's enough to stop here, no? > +static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d, > + struct irq_data *irqd, int ind) > +{ > + struct msi_desc *desc; > + bool is_msix; > + > + desc = irq_get_msi_desc(irqd->irq); Move this up to the declaration. > + if (!desc) > + return; > + > + is_msix = desc->pci.msi_attrib.is_msix; That's not valid for non PCI MSI interrupts. This function is used for all types of MSI interrupts. So for non PCI MSI interrupts this will output random garbage. Just print the address and be done with it. The MSI variant is visible from the chip name on current kernels. It's either ITS-PCI-MSI or ITS-PCI-MSIX and not ITS-MSI. > + seq_printf(m, "%*s%s:", ind, "", is_msix ? "msix" : "msi"); > + seq_printf(m, "\n%*saddress_hi: 0x%08x", ind + 1, "", desc->msg.address_hi); > + seq_printf(m, "\n%*saddress_lo: 0x%08x", ind + 1, "", desc->msg.address_lo); > + seq_printf(m, "\n%*smsg_data: 0x%08x\n", ind + 1, "", desc->msg.data); > +} > + > static const struct irq_domain_ops msi_domain_ops = { > .alloc = msi_domain_alloc, > .free = msi_domain_free, > .activate = msi_domain_activate, > .deactivate = msi_domain_deactivate, > .translate = msi_domain_translate, > + .debug_show = msi_domain_debug_show, This does not build when CONFIG_GENERIC_IRQ_DEBUGFS=n. Thanks, tglx
Hi Thomas(tglx), On 2025/3/2 17:01, Thomas Gleixner wrote: > Hans! > > On Sat, Mar 01 2025 at 20:39, Hans Zhang wrote: >> The debug_show() callback function is implemented in the MSI core code. >> And assign it to the domain ops::debug_show() creation. >> >> cat /sys/kernel/debug/irq/irqs/msi_irq_num, the address and data stored >> in the MSI capability or the address and data stored in the MSIX vector >> table will be displayed. > > So this explains what the patch is doing and what the output is. But it > fails to explain the _why_. Documentation gives proper guidance: > > https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-changes Is the following explanation OK? Displaying the address and data of the MSI/MSIX interrupt in the debugfs helps with debugging. >> e.g. >> root@root:/sys/kernel/debug/irq/irqs# cat /proc/interrupts | grep ITS >> 85: 0 0 0 0 0 0 0 0 0 0 0 0 ITS-MSI 75497472 Edge PCIe PME, aerdrv >> 86: 0 30 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021760 Edge nvme0q0 >> 87: 287 0 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021761 Edge nvme0q1 >> 88: 0 265 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021762 Edge nvme0q2 >> 89: 0 0 177 0 0 0 0 0 0 0 0 0 ITS-MSI 76021763 Edge nvme0q3 >> 90: 0 0 0 76 0 0 0 0 0 0 0 0 ITS-MSI 76021764 Edge nvme0q4 >> 91: 0 0 0 0 161 0 0 0 0 0 0 0 ITS-MSI 76021765 Edge nvme0q5 >> 92: 0 0 0 0 0 991 0 0 0 0 0 0 ITS-MSI 76021766 Edge nvme0q6 >> 93: 0 0 0 0 0 0 194 0 0 0 0 0 ITS-MSI 76021767 Edge nvme0q7 >> 94: 0 0 0 0 0 0 0 94 0 0 0 0 ITS-MSI 76021768 Edge nvme0q8 >> 95: 0 0 0 0 0 0 0 0 148 0 0 0 ITS-MSI 76021769 Edge nvme0q9 >> 96: 0 0 0 0 0 0 0 0 0 261 0 0 ITS-MSI 76021770 Edge nvme0q10 >> 97: 0 0 0 0 0 0 0 0 0 0 127 0 ITS-MSI 76021771 Edge nvme0q11 >> 98: 0 0 0 0 0 0 0 0 0 0 0 317 ITS-MSI 76021772 Edge nvme0q12 > > How is this relevant to describe the patch? The preceding information will be deleted in v4 patch. > >> root@root:/sys/kernel/debug/irq/irqs# >> root@root:/sys/kernel/debug/irq/irqs# cat 87 >> handler: handle_fasteoi_irq >> device: 0000:91:00.0 >> status: 0x00000000 >> istate: 0x00004000 >> ddepth: 0 >> wdepth: 0 >> dstate: 0x31600200 >> IRQD_ACTIVATED >> IRQD_IRQ_STARTED >> IRQD_SINGLE_TARGET >> IRQD_AFFINITY_MANAGED >> IRQD_AFFINITY_ON_ACTIVATE >> IRQD_HANDLE_ENFORCE_IRQCTX >> node: 0 >> affinity: 0 >> effectiv: 0 >> domain: :soc@0:interrupt-controller@0e001000:its@0e050000-3 >> hwirq: 0x4880001 >> chip: ITS-MSI > > This output is from a pre 6.11 kernel... I will delete all other information except for what my patch will display. > >> flags: 0x20 >> IRQCHIP_ONESHOT_SAFE >> msix: >> address_hi: 0x00000000 >> address_lo: 0x0e060040 >> msg_data: 0x00000001 > > For demonstration it's enough to stop here, no? Yes, I will change it to the following: msix: address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000001 > >> +static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d, >> + struct irq_data *irqd, int ind) >> +{ >> + struct msi_desc *desc; >> + bool is_msix; >> + >> + desc = irq_get_msi_desc(irqd->irq); > > Move this up to the declaration. > >> + if (!desc) >> + return; >> + >> + is_msix = desc->pci.msi_attrib.is_msix; > > That's not valid for non PCI MSI interrupts. Do you mean to remove the following two lines of code? is_msix = desc->pci.msi_attrib.is_msix; seq_printf(m, "%*s%s:", ind, "", is_msix ? "msix" : "msi"); > This function is used for all types of MSI interrupts. So for non PCI > MSI interrupts this will output random garbage. Just print the address > and be done with it. The MSI variant is visible from the chip name on > current kernels. It's either ITS-PCI-MSI or ITS-PCI-MSIX and not > ITS-MSI. > >> + seq_printf(m, "%*s%s:", ind, "", is_msix ? "msix" : "msi"); >> + seq_printf(m, "\n%*saddress_hi: 0x%08x", ind + 1, "", desc->msg.address_hi); >> + seq_printf(m, "\n%*saddress_lo: 0x%08x", ind + 1, "", desc->msg.address_lo); >> + seq_printf(m, "\n%*smsg_data: 0x%08x\n", ind + 1, "", desc->msg.data); >> +} >> + >> static const struct irq_domain_ops msi_domain_ops = { >> .alloc = msi_domain_alloc, >> .free = msi_domain_free, >> .activate = msi_domain_activate, >> .deactivate = msi_domain_deactivate, >> .translate = msi_domain_translate, >> + .debug_show = msi_domain_debug_show, > > This does not build when CONFIG_GENERIC_IRQ_DEBUGFS=n. > Kernel test robot has reported a compilation error, and I have submitted v3 patch to solve this problem. I will fix all your questions in v4 patch. Finally, thank you very much for all your comments. Best regards Hans
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index 396a067a8a56..7dc786360172 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -756,12 +756,30 @@ static int msi_domain_translate(struct irq_domain *domain, struct irq_fwspec *fw return info->ops->msi_translate(domain, fwspec, hwirq, type); } +static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d, + struct irq_data *irqd, int ind) +{ + struct msi_desc *desc; + bool is_msix; + + desc = irq_get_msi_desc(irqd->irq); + if (!desc) + return; + + is_msix = desc->pci.msi_attrib.is_msix; + seq_printf(m, "%*s%s:", ind, "", is_msix ? "msix" : "msi"); + seq_printf(m, "\n%*saddress_hi: 0x%08x", ind + 1, "", desc->msg.address_hi); + seq_printf(m, "\n%*saddress_lo: 0x%08x", ind + 1, "", desc->msg.address_lo); + seq_printf(m, "\n%*smsg_data: 0x%08x\n", ind + 1, "", desc->msg.data); +} + static const struct irq_domain_ops msi_domain_ops = { .alloc = msi_domain_alloc, .free = msi_domain_free, .activate = msi_domain_activate, .deactivate = msi_domain_deactivate, .translate = msi_domain_translate, + .debug_show = msi_domain_debug_show, }; static irq_hw_number_t msi_domain_ops_get_hwirq(struct msi_domain_info *info,
The debug_show() callback function is implemented in the MSI core code. And assign it to the domain ops::debug_show() creation. cat /sys/kernel/debug/irq/irqs/msi_irq_num, the address and data stored in the MSI capability or the address and data stored in the MSIX vector table will be displayed. e.g. root@root:/sys/kernel/debug/irq/irqs# cat /proc/interrupts | grep ITS 85: 0 0 0 0 0 0 0 0 0 0 0 0 ITS-MSI 75497472 Edge PCIe PME, aerdrv 86: 0 30 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021760 Edge nvme0q0 87: 287 0 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021761 Edge nvme0q1 88: 0 265 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021762 Edge nvme0q2 89: 0 0 177 0 0 0 0 0 0 0 0 0 ITS-MSI 76021763 Edge nvme0q3 90: 0 0 0 76 0 0 0 0 0 0 0 0 ITS-MSI 76021764 Edge nvme0q4 91: 0 0 0 0 161 0 0 0 0 0 0 0 ITS-MSI 76021765 Edge nvme0q5 92: 0 0 0 0 0 991 0 0 0 0 0 0 ITS-MSI 76021766 Edge nvme0q6 93: 0 0 0 0 0 0 194 0 0 0 0 0 ITS-MSI 76021767 Edge nvme0q7 94: 0 0 0 0 0 0 0 94 0 0 0 0 ITS-MSI 76021768 Edge nvme0q8 95: 0 0 0 0 0 0 0 0 148 0 0 0 ITS-MSI 76021769 Edge nvme0q9 96: 0 0 0 0 0 0 0 0 0 261 0 0 ITS-MSI 76021770 Edge nvme0q10 97: 0 0 0 0 0 0 0 0 0 0 127 0 ITS-MSI 76021771 Edge nvme0q11 98: 0 0 0 0 0 0 0 0 0 0 0 317 ITS-MSI 76021772 Edge nvme0q12 root@root:/sys/kernel/debug/irq/irqs# root@root:/sys/kernel/debug/irq/irqs# cat 87 handler: handle_fasteoi_irq device: 0000:91:00.0 status: 0x00000000 istate: 0x00004000 ddepth: 0 wdepth: 0 dstate: 0x31600200 IRQD_ACTIVATED IRQD_IRQ_STARTED IRQD_SINGLE_TARGET IRQD_AFFINITY_MANAGED IRQD_AFFINITY_ON_ACTIVATE IRQD_HANDLE_ENFORCE_IRQCTX node: 0 affinity: 0 effectiv: 0 domain: :soc@0:interrupt-controller@0e001000:its@0e050000-3 hwirq: 0x4880001 chip: ITS-MSI flags: 0x20 IRQCHIP_ONESHOT_SAFE msix: address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000001 parent: domain: :soc@0:interrupt-controller@0e001000:its@0e050000-5 hwirq: 0x2002 chip: ITS flags: 0x0 parent: domain: :soc@0:interrupt-controller@0e001000-1 hwirq: 0x2002 chip: GICv3 flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE Signed-off-by: Hans Zhang <18255117159@163.com> --- Changes since v1: https://lore.kernel.org/linux-pci/20250227162821.253020-1-18255117159@163.com/ - According to Thomas(tglx), the debug_show() callback should be added to the MSI core code. --- kernel/irq/msi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) base-commit: 76544811c850a1f4c055aa182b513b7a843868ea