kernel/irq/msi.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
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);
}
static void msi_sysfs_remove_desc(struct device *dev, struct msi_desc *desc)
base-commit: dd83757f6e686a2188997cb58b5975f744bb7786
--
2.25.1On 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 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 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
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 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
© 2016 - 2025 Red Hat, Inc.