On Mon, Mar 23, 2026 at 05:44:44PM -0700, Jakub Kicinski wrote:
> On Thu, 19 Mar 2026 00:09:13 -0700 Erni Sri Satya Vennela wrote:
> > Add debugfs entries to expose hardware configuration and diagnostic
> > information that aids in debugging driver initialization and runtime
> > operations without adding noise to dmesg.
> >
> > The debugfs directory creation and removal for each PCI device is
> > integrated into mana_gd_setup() and mana_gd_cleanup_device()
> > respectively, so that all callers (probe, remove, suspend, resume,
> > shutdown) share a single code path.
> >
> > Device-level entries (under /sys/kernel/debug/mana/<slot>/):
> > - num_msix_usable, max_num_queues: Max resources from hardware
> > - gdma_protocol_ver, pf_cap_flags1: VF version negotiation results
> > - num_vports, bm_hostmode: Device configuration
> >
> > Per-vPort entries (under /sys/kernel/debug/mana/<slot>/vportN/):
> > - port_handle: Hardware vPort handle
> > - max_sq, max_rq: Max queues from vPort config
> > - indir_table_sz: Indirection table size
> > - steer_rx, steer_rss, steer_update_tab, steer_cqe_coalescing:
> > Last applied steering configuration parameters
>
> AI says:
>
> > @@ -1918,15 +1930,23 @@ static int mana_gd_setup(struct pci_dev *pdev)
> > struct gdma_context *gc = pci_get_drvdata(pdev);
> > int err;
> >
> > + if (gc->is_pf)
> > + gc->mana_pci_debugfs = debugfs_create_dir("0",
> > mana_debugfs_root);
> > + else
> > + gc->mana_pci_debugfs =
> > debugfs_create_dir(pci_slot_name(pdev->slot),
> > + mana_debugfs_root);
>
> If pdev->slot is NULL (which can happen for VFs in environments like generic
> VFIO passthrough or nested KVM), will pci_slot_name(pdev->slot) cause a
> NULL pointer dereference?
>
> Also, could this naming scheme cause name collisions? If multiple PFs are
> present, they would all try to use "0". Similarly, VFs across different
> PCI domains or buses might share the same physical slot identifier, leading
> to -EEXIST errors. Would it be safer to use the unique PCI BDF via
> pci_name(pdev) instead?
Yes. that is a better way to handle it. I will use pci_name. We can
remove if-else and use only one for both the cases.
>
> > @@ -3141,6 +3149,24 @@ static int mana_init_port(struct net_device *ndev)
> > eth_hw_addr_set(ndev, apc->mac_addr);
> > sprintf(vport, "vport%d", port_idx);
> > apc->mana_port_debugfs = debugfs_create_dir(vport,
> > gc->mana_pci_debugfs);
> > +
> > + debugfs_create_u64("port_handle", 0400, apc->mana_port_debugfs,
> > + &apc->port_handle);
>
> When operations like changing the MTU or setting an XDP program trigger a
> detach/attach cycle, mana_detach() invokes mana_cleanup_port_context(),
> which recursively removes the apc->mana_port_debugfs directory.
> During re-attachment, mana_attach() calls mana_init_port(), which
> recreates the directory and the new files added in this patch. However, the
> pre-existing current_speed file (created in mana_probe_port()) is not
> recreated here.
>
> Does this cause the current_speed file to be permanently lost after a
> detach/attach cycle? Should the creation of current_speed be moved to
> mana_init_port() so it survives the cycle?
Yes that is correct.
Since these issues are pre-existing and not introduced from my patch.
I'll plan to send them as different patch with fixes tag.
> --
> pw-bot: cr
