Re: [PATCH V5 2/5] PCI/TPH: Add Steering Tag support

Simon Horman
2024-09-17T07:32:22Z

On Mon, Sep 16, 2024 at 03:51:00PM -0500, Wei Huang wrote:
> pcie_tph_get_cpu_st() is added to allow a caller to retrieve Steering Tags
> for a target memory that is associated with a specific CPU. The ST tag is
> retrieved by invoking ACPI _DSM of the device's Root Port device.
>
> pcie_tph_set_st_entry() is added to support updating the device's Steering
> Tags. The tags will be written into the device's MSI-X table or the ST
> table located in the TPH Extended Capability space.
>
> Co-developed-by: Eric Van Tassell <Eric.VanTassell@amd.com>
> Signed-off-by: Eric Van Tassell <Eric.VanTassell@amd.com>
> Signed-off-by: Wei Huang <wei.huang2@amd.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>

...

> @@ -45,6 +201,163 @@ static u8 get_rp_completer_type(struct pci_dev *pdev)
> return FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK, reg);
> }
>
> +/* Write ST to MSI-X vector control reg - Return 0 if OK, otherwise -errno */
> +static int write_tag_to_msix(struct pci_dev *pdev, int msix_idx, u16 tag)
> +{
> + struct msi_desc *msi_desc = NULL;
> + void __iomem *vec_ctrl;
> + u32 val, mask;
> + int err = 0;
> +
> + msi_lock_descs(&pdev->dev);
> +
> + /* Find the msi_desc entry with matching msix_idx */
> + msi_for_each_desc(msi_desc, &pdev->dev, MSI_DESC_ASSOCIATED) {
> + if (msi_desc->msi_index == msix_idx)
> + break;
> + }
> +
> + if (!msi_desc) {
> + err = -ENXIO;
> + goto err_out;
> + }
> +
> + /* Get the vector control register (offset 0xc) pointed by msix_idx */
> + vec_ctrl = pdev->msix_base + msix_idx * PCI_MSIX_ENTRY_SIZE;
> + vec_ctrl += PCI_MSIX_ENTRY_VECTOR_CTRL;
> +
> + val = readl(vec_ctrl);
> + mask = PCI_MSIX_ENTRY_CTRL_ST_LOWER | PCI_MSIX_ENTRY_CTRL_ST_UPPER;
> + val &= ~mask;
> + val |= FIELD_PREP(mask, (u32)tag);

Hi Wei Huang,

Unfortunately clang-18 (x86_64, allmodconfig, W=1, when applied to net-next)
complains about this. I think it is because it expects FIELD_PREP to be
used with a mask that is a built-in constant.

drivers/pci/pcie/tph.c:232:9: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((mask), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (mask)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
232 | val |= FIELD_PREP(mask, (u32)tag);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/bitfield.h:115:3: note: expanded from macro 'FIELD_PREP'
115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/bitfield.h:72:53: note: expanded from macro '__BF_FIELD_CHECK'
72 | BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
73 | __bf_cast_unsigned(_reg, ~0ull), \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
74 | _pfx "type of reg too small for mask"); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
././include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert'
498 | __compiletime_assert(condition, msg, prefix, suffix)
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert'
490 | if (!(condition)) \
| ^~~~~~~~~
1 warning generated.

> + writel(val, vec_ctrl);
> +
> + /* Read back to flush the update */
> + val = readl(vec_ctrl);
> +
> +err_out:
> + msi_unlock_descs(&pdev->dev);
> + return err;
> +}

...