| Message ID | 20250227162821.253020-1-18255117159@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | genirq/msi: Add the address and data that show MSI/MSIX | expand |
On Fri, Feb 28, 2025 at 12:28:21AM +0800, Hans Zhang wrote: > Add to view the addresses and data stored in the MSI capability or the > addresses and data stored in the MSIX vector table. > > e.g. > root@root:/sys/bus/pci/devices/<dev>/msi_irqs# ls > 86 87 88 89 > root@root:/sys/bus/pci/devices/<dev>/msi_irqs# cat * > msix > address_hi: 0x00000000 > address_lo: 0x0e060040 > msg_data: 0x00000000 > msix > address_hi: 0x00000000 > address_lo: 0x0e060040 > msg_data: 0x00000001 > msix > address_hi: 0x00000000 > address_lo: 0x0e060040 > msg_data: 0x00000002 > msix > address_hi: 0x00000000 > address_lo: 0x0e060040 > msg_data: 0x00000003 > > Signed-off-by: Hans Zhang <18255117159@163.com> > --- > kernel/irq/msi.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c > index 396a067a8a56..a37a3e535fb8 100644 > --- a/kernel/irq/msi.c > +++ b/kernel/irq/msi.c > @@ -503,8 +503,18 @@ static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr, > { > /* MSI vs. MSIX is per device not per interrupt */ > bool is_msix = dev_is_pci(dev) ? to_pci_dev(dev)->msix_enabled : false; > + struct msi_desc *desc; > + u32 irq; > + > + if (kstrtoint(attr->attr.name, 10, &irq) < 0) > + return 0; > > - return sysfs_emit(buf, "%s\n", is_msix ? "msix" : "msi"); > + desc = irq_get_msi_desc(irq); > + return sysfs_emit( > + buf, > + "%s\n address_hi: 0x%08x\n address_lo: 0x%08x\n msg_data: 0x%08x\n", > + is_msix ? "msix" : "msi", desc->msg.address_hi, > + desc->msg.address_lo, desc->msg.data); Sysfs is an ABI. You cannot change the semantics of an attribute. - Mani
On 2025/2/28 00:39, Manivannan Sadhasivam wrote: > On Fri, Feb 28, 2025 at 12:28:21AM +0800, Hans Zhang wrote: >> Add to view the addresses and data stored in the MSI capability or the >> addresses and data stored in the MSIX vector table. >> >> e.g. >> root@root:/sys/bus/pci/devices/<dev>/msi_irqs# ls >> 86 87 88 89 >> root@root:/sys/bus/pci/devices/<dev>/msi_irqs# cat * >> msix >> address_hi: 0x00000000 >> address_lo: 0x0e060040 >> msg_data: 0x00000000 >> msix >> address_hi: 0x00000000 >> address_lo: 0x0e060040 >> msg_data: 0x00000001 >> msix >> address_hi: 0x00000000 >> address_lo: 0x0e060040 >> msg_data: 0x00000002 >> msix >> address_hi: 0x00000000 >> address_lo: 0x0e060040 >> msg_data: 0x00000003 >> >> Signed-off-by: Hans Zhang <18255117159@163.com> >> --- >> kernel/irq/msi.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c >> index 396a067a8a56..a37a3e535fb8 100644 >> --- a/kernel/irq/msi.c >> +++ b/kernel/irq/msi.c >> @@ -503,8 +503,18 @@ static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr, >> { >> /* MSI vs. MSIX is per device not per interrupt */ >> bool is_msix = dev_is_pci(dev) ? to_pci_dev(dev)->msix_enabled : false; >> + struct msi_desc *desc; >> + u32 irq; >> + >> + if (kstrtoint(attr->attr.name, 10, &irq) < 0) >> + return 0; >> >> - return sysfs_emit(buf, "%s\n", is_msix ? "msix" : "msi"); >> + desc = irq_get_msi_desc(irq); >> + return sysfs_emit( >> + buf, >> + "%s\n address_hi: 0x%08x\n address_lo: 0x%08x\n msg_data: 0x%08x\n", >> + is_msix ? "msix" : "msi", desc->msg.address_hi, >> + desc->msg.address_lo, desc->msg.data); > > Sysfs is an ABI. You cannot change the semantics of an attribute. > Thanks Mani for the tip. If I want to implement similar functionality, where should I add it? Since this sysfs node is the only one that displays the MSI/MSIX interrupt number, I don't know where to implement similar debug functionality at this time. Do you have any suggestions? Or it shouldn't have a similar function. Best regards Hans
On Thu, Feb 27 2025 at 22:09, Manivannan Sadhasivam wrote: > On Fri, Feb 28, 2025 at 12:28:21AM +0800, Hans Zhang wrote: >> + return sysfs_emit( >> + buf, >> + "%s\n address_hi: 0x%08x\n address_lo: 0x%08x\n msg_data: 0x%08x\n", >> + is_msix ? "msix" : "msi", desc->msg.address_hi, >> + desc->msg.address_lo, desc->msg.data); > > Sysfs is an ABI. You cannot change the semantics of an attribute. Correct. Aside of that this is debug information and has no business in sysfs. The obvious place to expose this is via the existing debugfs irq/* mechanism. All it requires is to implement a debug_show() callback in the MSI core code and assign it to domain ops::debug_show() on domain creation, if it does not provide its own callback. Thanks, tglx
On Fri, Feb 28, 2025 at 12:49:38AM +0800, Hans Zhang wrote: > > > On 2025/2/28 00:39, Manivannan Sadhasivam wrote: > > On Fri, Feb 28, 2025 at 12:28:21AM +0800, Hans Zhang wrote: > > > Add to view the addresses and data stored in the MSI capability or the > > > addresses and data stored in the MSIX vector table. > > > > > > e.g. > > > root@root:/sys/bus/pci/devices/<dev>/msi_irqs# ls > > > 86 87 88 89 > > > root@root:/sys/bus/pci/devices/<dev>/msi_irqs# cat * > > > msix > > > address_hi: 0x00000000 > > > address_lo: 0x0e060040 > > > msg_data: 0x00000000 > > > msix > > > address_hi: 0x00000000 > > > address_lo: 0x0e060040 > > > msg_data: 0x00000001 > > > msix > > > address_hi: 0x00000000 > > > address_lo: 0x0e060040 > > > msg_data: 0x00000002 > > > msix > > > address_hi: 0x00000000 > > > address_lo: 0x0e060040 > > > msg_data: 0x00000003 > > > > > > Signed-off-by: Hans Zhang <18255117159@163.com> > > > --- > > > kernel/irq/msi.c | 12 +++++++++++- > > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > > > diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c > > > index 396a067a8a56..a37a3e535fb8 100644 > > > --- a/kernel/irq/msi.c > > > +++ b/kernel/irq/msi.c > > > @@ -503,8 +503,18 @@ static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr, > > > { > > > /* MSI vs. MSIX is per device not per interrupt */ > > > bool is_msix = dev_is_pci(dev) ? to_pci_dev(dev)->msix_enabled : false; > > > + struct msi_desc *desc; > > > + u32 irq; > > > + > > > + if (kstrtoint(attr->attr.name, 10, &irq) < 0) > > > + return 0; > > > - return sysfs_emit(buf, "%s\n", is_msix ? "msix" : "msi"); > > > + desc = irq_get_msi_desc(irq); > > > + return sysfs_emit( > > > + buf, > > > + "%s\n address_hi: 0x%08x\n address_lo: 0x%08x\n msg_data: 0x%08x\n", > > > + is_msix ? "msix" : "msi", desc->msg.address_hi, > > > + desc->msg.address_lo, desc->msg.data); > > > > Sysfs is an ABI. You cannot change the semantics of an attribute. > > > > Thanks Mani for the tip. > > If I want to implement similar functionality, where should I add it? Since > this sysfs node is the only one that displays the MSI/MSIX interrupt number, > I don't know where to implement similar debug functionality at this time. Do > you have any suggestions? Or it shouldn't have a similar function. I think it is useful feature to help debug. Generally only one property for one sys file. A possible create 3 files under /sys/kernel/irq/26/ address_hi, address_lo, msg_data. cat address_hi, only show 0x00000000. ... ABI doc need update also. Thomas(tglx) may provide better suggestions. Frank > > Best regards > Hans >
On 2025/2/28 02:03, Frank Li wrote: > On Fri, Feb 28, 2025 at 12:49:38AM +0800, Hans Zhang wrote: >> Thanks Mani for the tip. >> >> If I want to implement similar functionality, where should I add it? Since >> this sysfs node is the only one that displays the MSI/MSIX interrupt number, >> I don't know where to implement similar debug functionality at this time. Do >> you have any suggestions? Or it shouldn't have a similar function. > > I think it is useful feature to help debug. Generally only one property > for one sys file. > > A possible create 3 files under > > /sys/kernel/irq/26/ > address_hi, address_lo, msg_data. > > cat address_hi, only show 0x00000000. ... ABI doc need update also. > > Thomas(tglx) may provide better suggestions. Thank you very much for Frank's opinion and agreeing with my idea. I also think this is a good debug method. I will reply to Thomas(tglx) later, and then please review whether the patch I provided is OK? Best regards Hans
On 2025/2/28 01:51, Thomas Gleixner wrote: > On Thu, Feb 27 2025 at 22:09, Manivannan Sadhasivam wrote: >> On Fri, Feb 28, 2025 at 12:28:21AM +0800, Hans Zhang wrote: >>> + return sysfs_emit( >>> + buf, >>> + "%s\n address_hi: 0x%08x\n address_lo: 0x%08x\n msg_data: 0x%08x\n", >>> + is_msix ? "msix" : "msi", desc->msg.address_hi, >>> + desc->msg.address_lo, desc->msg.data); >> >> Sysfs is an ABI. You cannot change the semantics of an attribute. > > Correct. Aside of that this is debug information and has no business in > sysfs. > > The obvious place to expose this is via the existing debugfs irq/* > mechanism. All it requires is to implement a debug_show() callback in > the MSI core code and assign it to domain ops::debug_show() on domain > creation, if it does not provide its own callback. Hi Thomas(tglx), Is the following patch OK? Please give me some advice. Thank you very much. Best regards Hans patch: diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c index ca142b9a4db3..447fa24520f4 100644 --- a/kernel/irq/debugfs.c +++ b/kernel/irq/debugfs.c @@ -3,6 +3,7 @@ #include <linux/irqdomain.h> #include <linux/irq.h> +#include <linux/msi.h> #include <linux/uaccess.h> #include "internals.h" @@ -56,6 +57,26 @@ static const struct irq_bit_descr irqchip_flags[] = { BIT_MASK_DESCR(IRQCHIP_MOVE_DEFERRED), }; +static void irq_debug_show_msi_msix(struct seq_file *m, struct irq_data *data, + int ind) +{ + struct msi_desc *desc; + bool is_msix; + + desc = irq_get_msi_desc(data->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 void irq_debug_show_chip(struct seq_file *m, struct irq_data *data, int ind) { @@ -178,6 +199,7 @@ static int irq_debug_show(struct seq_file *m, void *p) seq_printf(m, "node: %d\n", irq_data_get_node(data)); irq_debug_show_masks(m, desc); irq_debug_show_data(m, data, 0); + irq_debug_show_msi_msix(m, data, 0); raw_spin_unlock_irq(&desc->lock); return 0; } 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: 682 0 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021761 Edge nvme0q1 88: 0 400 0 0 0 0 0 0 0 0 0 0 ITS-MSI 76021762 Edge nvme0q2 89: 0 0 246 0 0 0 0 0 0 0 0 0 ITS-MSI 76021763 Edge nvme0q3 90: 0 0 0 141 0 0 0 0 0 0 0 0 ITS-MSI 76021764 Edge nvme0q4 91: 0 0 0 0 177 0 0 0 0 0 0 0 ITS-MSI 76021765 Edge nvme0q5 92: 0 0 0 0 0 173 0 0 0 0 0 0 ITS-MSI 76021766 Edge nvme0q6 93: 0 0 0 0 0 0 374 0 0 0 0 0 ITS-MSI 76021767 Edge nvme0q7 94: 0 0 0 0 0 0 0 62 0 0 0 0 ITS-MSI 76021768 Edge nvme0q8 95: 0 0 0 0 0 0 0 0 137 0 0 0 ITS-MSI 76021769 Edge nvme0q9 96: 0 0 0 0 0 0 0 0 0 177 0 0 ITS-MSI 76021770 Edge nvme0q10 97: 0 0 0 0 0 0 0 0 0 0 403 0 ITS-MSI 76021771 Edge nvme0q11 98: 0 0 0 0 0 0 0 0 0 0 0 246 ITS-MSI 76021772 Edge nvme0q12 root@root:/sys/kernel/debug/irq/irqs# cat 86 handler: handle_fasteoi_irq device: 0000:91:00.0 status: 0x00000000 istate: 0x00004000 ddepth: 0 wdepth: 0 dstate: 0x31401200 IRQD_ACTIVATED IRQD_IRQ_STARTED IRQD_SINGLE_TARGET IRQD_AFFINITY_SET IRQD_AFFINITY_ON_ACTIVATE IRQD_HANDLE_ENFORCE_IRQCTX node: 0 affinity: 6 effectiv: 6 domain: :soc@0:interrupt-controller@0e001000:its@0e050000-3 hwirq: 0x4880000 chip: ITS-MSI flags: 0x20 IRQCHIP_ONESHOT_SAFE parent: domain: :soc@0:interrupt-controller@0e001000:its@0e050000-5 hwirq: 0x2001 chip: ITS flags: 0x0 parent: domain: :soc@0:interrupt-controller@0e001000-1 hwirq: 0x2001 chip: GICv3 flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE msix: address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000000 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 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 msix: address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000001 root@root:/sys/kernel/debug/irq/irqs# root@root:/sys/kernel/debug/irq/irqs# cat 88 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: 1 effectiv: 1 domain: :soc@0:interrupt-controller@0e001000:its@0e050000-3 hwirq: 0x4880002 chip: ITS-MSI flags: 0x20 IRQCHIP_ONESHOT_SAFE parent: domain: :soc@0:interrupt-controller@0e001000:its@0e050000-5 hwirq: 0x2003 chip: ITS flags: 0x0 parent: domain: :soc@0:interrupt-controller@0e001000-1 hwirq: 0x2003 chip: GICv3 flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE msix: address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000002 root@root:/sys/kernel/debug/irq/irqs# cat 89 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: 2 effectiv: 2 domain: :soc@0:interrupt-controller@0e001000:its@0e050000-3 hwirq: 0x4880003 chip: ITS-MSI flags: 0x20 IRQCHIP_ONESHOT_SAFE parent: domain: :soc@0:interrupt-controller@0e001000:its@0e050000-5 hwirq: 0x2004 chip: ITS flags: 0x0 parent: domain: :soc@0:interrupt-controller@0e001000-1 hwirq: 0x2004 chip: GICv3 flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE msix: address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000003
On Fri, Feb 28 2025 at 17:04, Hans Zhang wrote: > Is the following patch OK? No. > static void > irq_debug_show_chip(struct seq_file *m, struct irq_data *data, int ind) > { > @@ -178,6 +199,7 @@ static int irq_debug_show(struct seq_file *m, void *p) > seq_printf(m, "node: %d\n", irq_data_get_node(data)); > irq_debug_show_masks(m, desc); > irq_debug_show_data(m, data, 0); > + irq_debug_show_msi_msix(m, data, 0); > raw_spin_unlock_irq(&desc->lock); > return 0; > } This is just violating the layering and I told you what to do: "implement a debug_show() callback in the MSI core code and assign it to domain ops::debug_show() on domain creation, if it does not provide its own callback." If you don't understand what I tell you, then please ask instead of going off and hacking up something completely different. Here is another hint: Look at msi_domain_ops_default and at msi_domain_update_dom_ops() If you still have questions, feel free to ask. Thanks, tglx
On 2025/2/28 19:26, Thomas Gleixner wrote: > On Fri, Feb 28 2025 at 17:04, Hans Zhang wrote: >> Is the following patch OK? > > No. > >> static void >> irq_debug_show_chip(struct seq_file *m, struct irq_data *data, int ind) >> { >> @@ -178,6 +199,7 @@ static int irq_debug_show(struct seq_file *m, void *p) >> seq_printf(m, "node: %d\n", irq_data_get_node(data)); >> irq_debug_show_masks(m, desc); >> irq_debug_show_data(m, data, 0); >> + irq_debug_show_msi_msix(m, data, 0); >> raw_spin_unlock_irq(&desc->lock); >> return 0; >> } > > This is just violating the layering and I told you what to do: > > "implement a debug_show() callback in the MSI core code and assign > it to domain ops::debug_show() on domain creation, if it does not > provide its own callback." > > If you don't understand what I tell you, then please ask instead of > going off and hacking up something completely different. > > Here is another hint: > > Look at msi_domain_ops_default and at msi_domain_update_dom_ops() > > If you still have questions, feel free to ask. > Hi Thomas(tglx), I'm very sorry that I didn't understand what you meant at the beginning. Thank you very much for your patient guidance. Now, how about this patch? If you agree, I will resubmit the next version. Best regards Hans patch: diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index 396a067a8a56..98771a0b7d70 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -756,12 +756,33 @@ 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, 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
On Fri, Feb 28 2025 at 23:17, Hans Zhang wrote: > I'm very sorry that I didn't understand what you meant at the > beginning. No problem. > > +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); No need for these line breaks. You have 100 characters available. > + 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, > }; Looks about right. Thanks, tglx
On 2025/3/1 02:14, Thomas Gleixner wrote: > On Fri, Feb 28 2025 at 23:17, Hans Zhang wrote: >> I'm very sorry that I didn't understand what you meant at the >> beginning. > > No problem. > >> >> +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); > > No need for these line breaks. You have 100 characters available. > Will change. Best regards Hans >> + 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, >> }; > > Looks about right. > > Thanks, > > tglx
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index 396a067a8a56..a37a3e535fb8 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -503,8 +503,18 @@ static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr, { /* MSI vs. MSIX is per device not per interrupt */ bool is_msix = dev_is_pci(dev) ? to_pci_dev(dev)->msix_enabled : false; + struct msi_desc *desc; + u32 irq; + + if (kstrtoint(attr->attr.name, 10, &irq) < 0) + return 0; - return sysfs_emit(buf, "%s\n", is_msix ? "msix" : "msi"); + desc = irq_get_msi_desc(irq); + return sysfs_emit( + buf, + "%s\n address_hi: 0x%08x\n address_lo: 0x%08x\n msg_data: 0x%08x\n", + is_msix ? "msix" : "msi", desc->msg.address_hi, + desc->msg.address_lo, desc->msg.data); } static void msi_sysfs_remove_desc(struct device *dev, struct msi_desc *desc)
Add to view the addresses and data stored in the MSI capability or the addresses and data stored in the MSIX vector table. e.g. root@root:/sys/bus/pci/devices/<dev>/msi_irqs# ls 86 87 88 89 root@root:/sys/bus/pci/devices/<dev>/msi_irqs# cat * msix address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000000 msix address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000001 msix address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000002 msix address_hi: 0x00000000 address_lo: 0x0e060040 msg_data: 0x00000003 Signed-off-by: Hans Zhang <18255117159@163.com> --- kernel/irq/msi.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) base-commit: dd83757f6e686a2188997cb58b5975f744bb7786