Re: [PATCH v5 1/8] x86/hygon: Add Family 0x18 DF node enumeration

[Borislav Petkov]

On Thu, Sep 10, 2026 at 04:27:27PM +0800, Lin Wang wrote:
> +/*
> + * Cached identity for one DF instance.  After sorting, CDDs occupy
> + * nodes[0..num_cdd-1].
> + *
> + * The PCI BDF is the access point for a DF node, not its identity.
> + * socket_id and dfid, read from DF registers, together identify the
> + * node in hardware.
> + */
> +struct hygon_node {
> +	struct pci_dev	*misc;		/* DF function 3 */
> +	struct pci_dev	*link;		/* DF function 4 */
> +	u8		socket_id;	/* F1x200[30:28] */
> +	u8		dfid;		/* model-dependent DFID */
> +	bool		is_cdd;		/* DFID >= 4 */
> +};
> +
> +struct hygon_node_cache {
> +	struct hygon_node	*nodes;		/* sorted: CDD first, then IOD */
> +	u16			num_nodes;	/* CDD + IOD = total */
> +	u16			num_cdd;	/* CDD only */
> +	u16			num_sockets;
> +
> +	/* Set after DF node collection, sorting and validation complete. */
> +	bool			ready;
> +};

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:76 [+	struct pci_dev	*misc;		/* DF function 3 */]

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:77 [+	struct pci_dev	*link;		/* DF function 4 */]

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:78 [+	u8		socket_id;	/* F1x200[30:28] */]

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:79 [+	u8		dfid;		/* model-dependent DFID */]

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:80 [+	bool		is_cdd;		/* DFID >= 4 */]

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:84 [+	struct hygon_node	*nodes;		/* sorted: CDD first, then IOD */]

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:85 [+	u16			num_nodes;	/* CDD + IOD = total */]

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:86 [+	u16			num_cdd;	/* CDD only */]

verify_comment_style: WARNING: No tail comments please:
 arch/x86/kernel/hygon_node.c:101 [+	u16	f5_id;			/* 0 = not available */]

From: Documentation/process/maintainer-tip.rst

Please peruse that doc.

No tail comments (see below):
       
  Please refrain from using tail comments. Tail comments disturb the
  reading flow in almost all contexts, but especially in code::

        if (somecondition_is_true) /* Don't put a comment here */
                dostuff(); /* Neither here */
       
        seed = MAGIC_CONSTANT; /* Nor here */

  Use freestanding comments instead::
       
        /* This condition is not obvious without a comment */
        if (somecondition_is_true) {
                /* This really needs to be documented */
                dostuff();
        }

        /* This magic initialization needs a comment. Maybe not? */
        seed = MAGIC_CONSTANT;

  Use C++ style, tail comments when documenting structs in headers to
  achieve a more compact layout and better readability::

        // eax
        u32     x2apic_shift    :  5, // Number of bits to shift APIC ID right
                                      // for the topology ID at the next level
                                : 27; // Reserved
        // ebx
        u32     num_processors  : 16, // Number of processors at current level
                                : 16; // Reserved

  versus::

        /* eax */
                /*
                 * Number of bits to shift APIC ID right for the topology ID
                 * at the next level
                 */
         u32     x2apic_shift    :  5,
                 /* Reserved */
                                 : 27;

        /* ebx */
                /* Number of processors at current level */
        u32     num_processors  : 16,
                /* Reserved */
                                : 16;

...

> +static int __init hygon_collect_nodes(struct hygon_node_cache *cache)
> +{
> +	struct hygon_node *nodes;
> +	struct pci_dev *misc = NULL;
> +	struct pci_dev *link = NULL;
> +	u16 capacity = 0, count = 0;
> +	u8 observed_socket_mask = 0;
> +	u8 socket_present_mask = 0;
> +	int ret;

The tip-tree preferred ordering of variable declarations at the
beginning of a function is reverse fir tree order::

	struct long_struct_name *descriptive_name;
	unsigned long foo, bar;
	unsigned int tmp;
	int ret;

The above is faster to parse than the reverse ordering::

	int ret;
	unsigned int tmp;
	unsigned long foo, bar;
	struct long_struct_name *descriptive_name;

And even more so than random ordering::

	unsigned long foo, bar;
	int ret;
	struct long_struct_name *descriptive_name;
	unsigned int tmp;

Please audit all your functions in all patches.

> +
> +	while ((misc = next_hygon_dev(misc, hygon_nb_misc_ids)))
> +		capacity++;
> +
> +	if (!capacity)
> +		return -ENODEV;
> +

...

> +int hygon_pci_dev_to_df_node(struct pci_dev *pdev)
> +{
> +	u16 i;
> +
> +	if (!pdev)
> +		return -EINVAL;
> +
> +	if (!hygon_cache.ready)
> +		return -ENODEV;
> +
> +	for (i = 0; i < hygon_cache.num_nodes; i++) {
> +		struct pci_dev *misc = hygon_cache.nodes[i].misc;
> +
> +		if (pci_domain_nr(misc->bus) == pci_domain_nr(pdev->bus) &&
> +		    misc->bus->number       == pdev->bus->number      &&
> +		    PCI_SLOT(misc->devfn)   == PCI_SLOT(pdev->devfn))
> +			return i;

Something happened here to that whitespace.

:)

> +	}
> +
> +	return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(hygon_pci_dev_to_df_node);
> +
> +/*
> + * Build the Hygon DF node cache at fs_initcall.
> + */
> +static int __init hygon_node_init(void)
> +{
> +	int ret;
> +
> +	if (!hygon_get_df_cfg())
> +		return 0;
> +
> +	ret = hygon_build_cache();
> +	if (ret) {
> +		pr_warn("DF node cache build failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	pr_info("%u DF nodes (%u CDDs, %u IODs) across %u sockets\n",
> +		hygon_cache.num_nodes, hygon_cache.num_cdd,
> +		hygon_cache.num_nodes - hygon_cache.num_cdd,
> +		hygon_cache.num_sockets);
> +
> +	return 0;
> +}
> +fs_initcall(hygon_node_init);
> -- 

Sure looks complex... I hope you know what you're doing...

:-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette