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

[Simon Horman <horms@kernel.org>]

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;
> +}

...