| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87plizdcxd.ffs@tglx>
Date: Sun, 02 Mar 2025 10:01:02 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Hans Zhang <18255117159@....com>
Cc: manivannan.sadhasivam@...aro.org, kw@...ux.com, kwilczynski@...nel.org,
bhelgaas@...gle.com, Frank.Li@....com, cassel@...nel.org,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org, Hans Zhang
<18255117159@....com>
Subject: Re: [v2] genirq/msi: Add the address and data that show MSI/MSIX
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@...t:/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@...t:/sys/kernel/debug/irq/irqs#
> root@...t:/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@...01000:its@...50000-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
Powered by blists - more mailing lists