Re: [v2] genirq/msi: Add the address and data that show MSI/MSIX

[Thomas Gleixner <tglx@linutronix.de>]

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