Re: [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Allow BAR assignment via configfs

[Frank Li]

On Tue, Jun 03, 2025 at 07:03:40PM +0200, Jerome Brunet wrote:
> The current BAR configuration for the PCI vNTB endpoint function allocates
> BARs in order, which lacks flexibility and does not account for
> platform-specific quirks. This is problematic on Renesas platforms, where
> BAR_4 is a fixed 256B region that ends up being used for MW1, despite being
> better suited for doorbells.
>
> Add new configfs attributes to allow users to specify arbitrary BAR
> assignments. If no configuration is provided, the driver retains its
> original behavior of sequential BAR allocation, preserving compatibility
> with existing userspace setups.
>
> This enables use cases such as assigning BAR_2 for MW1 and using the
> limited BAR_4 for doorbells on Renesas platforms.
>
> Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> ---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 131 ++++++++++++++++++++++++--
>  1 file changed, 124 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 2198282a80a40774047502a37f0288ca396bdb0e..7475d87659b1c70aa41b0999eabfa661f4ceed39 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -73,6 +73,8 @@ enum epf_ntb_bar {
>  	BAR_MW1,
>  	BAR_MW2,
>  	BAR_MW3,
> +	BAR_MW4,
> +	VNTB_BAR_NUM,
>  };
>
>  /*
> @@ -132,7 +134,7 @@ struct epf_ntb {
>  	bool linkup;
>  	u32 spad_size;
>
> -	enum pci_barno epf_ntb_bar[6];
> +	enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
>
>  	struct epf_ntb_ctrl *reg;
>
> @@ -654,6 +656,62 @@ static void epf_ntb_epc_destroy(struct epf_ntb *ntb)
>  	pci_epc_put(ntb->epf->epc);
>  }
>
> +
> +/**
> + * epf_ntb_is_bar_used() - Check if a bar is used in the ntb configuration
> + * @ntb: NTB device that facilitates communication between HOST and VHOST
> + * @barno: Checked bar number
> + *
> + * Returns: true if used, false if free.
> + */
> +static bool epf_ntb_is_bar_used(struct epf_ntb *ntb,
> +				enum pci_barno barno)
> +{
> +	int i;
> +
> +	for (i = 0; i < VNTB_BAR_NUM; i++) {
> +		if (ntb->epf_ntb_bar[i] == barno)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + * epf_ntb_find_bar() - Assign BAR number when no configuration is provided
> + * @epc_features: The features provided by the EPC specific to this EPF
> + * @ntb: NTB device that facilitates communication between HOST and VHOST
> + * @barno: Bar start index
> + *
> + * When the BAR configuration was not provided through the userspace
> + * configuration, automatically assign BAR as it has been historically
> + * done by this endpoint function.
> + *
> + * Returns: the BAR number found, if any. -1 otherwise
> + */
> +static int epf_ntb_find_bar(struct epf_ntb *ntb,
> +			    const struct pci_epc_features *epc_features,
> +			    enum epf_ntb_bar bar,
> +			    enum pci_barno barno)
> +{
> +	while (ntb->epf_ntb_bar[bar] < 0) {
> +		barno = pci_epc_get_next_free_bar(epc_features, barno);
> +		if (barno < 0)
> +			break; /* No more BAR available */
> +
> +		/*
> +		 * Verify if the BAR found is not already assigned
> +		 * through the provided configuration
> +		 */
> +		if (!epf_ntb_is_bar_used(ntb, barno))
> +			ntb->epf_ntb_bar[bar] = barno;
> +
> +		barno += 1;
> +	}
> +
> +	return barno;
> +}
> +
>  /**
>   * epf_ntb_init_epc_bar() - Identify BARs to be used for each of the NTB
>   * constructs (scratchpad region, doorbell, memorywindow)
> @@ -676,23 +734,21 @@ static int epf_ntb_init_epc_bar(struct epf_ntb *ntb)
>  	epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
>
>  	/* These are required BARs which are mandatory for NTB functionality */
> -	for (bar = BAR_CONFIG; bar <= BAR_MW1; bar++, barno++) {
> -		barno = pci_epc_get_next_free_bar(epc_features, barno);
> +	for (bar = BAR_CONFIG; bar <= BAR_MW1; bar++) {
> +		barno = epf_ntb_find_bar(ntb, epc_features, bar, barno);
>  		if (barno < 0) {
>  			dev_err(dev, "Fail to get NTB function BAR\n");
>  			return -EINVAL;
>  		}
> -		ntb->epf_ntb_bar[bar] = barno;
>  	}
>
>  	/* These are optional BARs which don't impact NTB functionality */
> -	for (bar = BAR_MW1, i = 1; i < num_mws; bar++, barno++, i++) {
> -		barno = pci_epc_get_next_free_bar(epc_features, barno);
> +	for (bar = BAR_MW1, i = 1; i < num_mws; bar++, i++) {
> +		barno = epf_ntb_find_bar(ntb, epc_features, bar, barno);
>  		if (barno < 0) {
>  			ntb->num_mws = i;
>  			dev_dbg(dev, "BAR not available for > MW%d\n", i + 1);
>  		}
> -		ntb->epf_ntb_bar[bar] = barno;
>  	}
>
>  	return 0;
> @@ -860,6 +916,37 @@ static ssize_t epf_ntb_##_name##_store(struct config_item *item,	\
>  	return len;							\
>  }
>
> +#define EPF_NTB_BAR_R(_name, _id)					\
> +	static ssize_t epf_ntb_##_name##_show(struct config_item *item,	\
> +					      char *page)		\
> +	{								\
> +		struct config_group *group = to_config_group(item);	\
> +		struct epf_ntb *ntb = to_epf_ntb(group);		\
> +									\
> +		return sprintf(page, "%d\n", ntb->epf_ntb_bar[_id]);	\
> +	}
> +
> +#define EPF_NTB_BAR_W(_name, _id)					\
> +	static ssize_t epf_ntb_##_name##_store(struct config_item *item, \
> +					       const char *page, size_t len) \
> +	{								\
> +	struct config_group *group = to_config_group(item);		\
> +	struct epf_ntb *ntb = to_epf_ntb(group);			\
> +	int val;							\
> +	int ret;							\
> +									\
> +	ret = kstrtoint(page, 0, &val);					\
> +	if (ret)							\
> +		return ret;						\
> +									\
> +	if (val < NO_BAR || val > BAR_5)				\
> +		return -EINVAL;						\
> +									\
> +	ntb->epf_ntb_bar[_id] = val;					\
> +									\
> +	return len;							\
> +	}
> +
>  static ssize_t epf_ntb_num_mws_store(struct config_item *item,
>  				     const char *page, size_t len)
>  {
> @@ -899,6 +986,18 @@ EPF_NTB_MW_R(mw3)
>  EPF_NTB_MW_W(mw3)
>  EPF_NTB_MW_R(mw4)
>  EPF_NTB_MW_W(mw4)
> +EPF_NTB_BAR_R(ctrl_bar, BAR_CONFIG)
> +EPF_NTB_BAR_W(ctrl_bar, BAR_CONFIG)
> +EPF_NTB_BAR_R(db_bar, BAR_DB)
> +EPF_NTB_BAR_W(db_bar, BAR_DB)
> +EPF_NTB_BAR_R(mw1_bar, BAR_MW1)
> +EPF_NTB_BAR_W(mw1_bar, BAR_MW1)
> +EPF_NTB_BAR_R(mw2_bar, BAR_MW1)
> +EPF_NTB_BAR_W(mw2_bar, BAR_MW1)
> +EPF_NTB_BAR_R(mw3_bar, BAR_MW3)
> +EPF_NTB_BAR_W(mw3_bar, BAR_MW3)
> +EPF_NTB_BAR_R(mw4_bar, BAR_MW4)
> +EPF_NTB_BAR_W(mw4_bar, BAR_MW4)
>
>  CONFIGFS_ATTR(epf_ntb_, spad_count);
>  CONFIGFS_ATTR(epf_ntb_, db_count);
> @@ -910,6 +1009,12 @@ CONFIGFS_ATTR(epf_ntb_, mw4);
>  CONFIGFS_ATTR(epf_ntb_, vbus_number);
>  CONFIGFS_ATTR(epf_ntb_, vntb_pid);
>  CONFIGFS_ATTR(epf_ntb_, vntb_vid);
> +CONFIGFS_ATTR(epf_ntb_, ctrl_bar);
> +CONFIGFS_ATTR(epf_ntb_, db_bar);
> +CONFIGFS_ATTR(epf_ntb_, mw1_bar);
> +CONFIGFS_ATTR(epf_ntb_, mw2_bar);
> +CONFIGFS_ATTR(epf_ntb_, mw3_bar);
> +CONFIGFS_ATTR(epf_ntb_, mw4_bar);
>
>  static struct configfs_attribute *epf_ntb_attrs[] = {
>  	&epf_ntb_attr_spad_count,
> @@ -922,6 +1027,12 @@ static struct configfs_attribute *epf_ntb_attrs[] = {
>  	&epf_ntb_attr_vbus_number,
>  	&epf_ntb_attr_vntb_pid,
>  	&epf_ntb_attr_vntb_vid,
> +	&epf_ntb_attr_ctrl_bar,
> +	&epf_ntb_attr_db_bar,
> +	&epf_ntb_attr_mw1_bar,
> +	&epf_ntb_attr_mw2_bar,
> +	&epf_ntb_attr_mw3_bar,
> +	&epf_ntb_attr_mw4_bar,
>  	NULL,
>  };
>
> @@ -1379,6 +1490,7 @@ static int epf_ntb_probe(struct pci_epf *epf,
>  {
>  	struct epf_ntb *ntb;
>  	struct device *dev;
> +	int i;
>
>  	dev = &epf->dev;
>
> @@ -1389,6 +1501,11 @@ static int epf_ntb_probe(struct pci_epf *epf,
>  	epf->header = &epf_ntb_header;
>  	ntb->epf = epf;
>  	ntb->vbus_number = 0xff;
> +
> +	/* Initially, no bar is assigned */
> +	for (i = 0; i < VNTB_BAR_NUM; i++)
> +		ntb->epf_ntb_bar[i] = NO_BAR;
> +
>  	epf_set_drvdata(epf, ntb);
>
>  	dev_info(dev, "pci-ep epf driver loaded\n");
>
> --
> 2.47.2
>