Re: [PATCH v11 1/7] PCI: endpoint: Add auxiliary resource query API

On Thu, Mar 26, 2026 at 05:47:56PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Mar 24, 2026 at 05:37:22PM +0900, Koichiro Den wrote:
> > Endpoint controller drivers may integrate auxiliary blocks (e.g. DMA
> > engines) whose register windows and descriptor memories metadata need to
> > be exposed to a remote peer. Endpoint function drivers need a generic
> > way to discover such resources without hard-coding controller-specific
> > helpers.
> > 
> > Add pci_epc_get_aux_resources() and the corresponding pci_epc_ops
> > get_aux_resources() callback. The API returns a list of resources
> > described by type, physical address and size, plus type-specific
> > metadata.
> > 
> > Passing resources == NULL (or num_resources == 0) returns the required
> > number of entries.
> > 
> > Signed-off-by: Koichiro Den <[email protected]>
> > ---
> > Changes in v11:
> >   - Drop PCI_EPC_AUX_DMA_CTRL_MMIO and PCI_EPC_AUX_DMA_CHAN_DESC from
> >     the generic aux-resource API for now; keep only the DOORBELL_MMIO
> >     resource used by this series.
> > 
> >  drivers/pci/endpoint/pci-epc-core.c | 41 ++++++++++++++++++++++++
> >  include/linux/pci-epc.h             | 48 +++++++++++++++++++++++++++++
> >  2 files changed, 89 insertions(+)
> > 
> > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> > index 6c3c58185fc5..dc6d6ab4ea1e 100644
> > --- a/drivers/pci/endpoint/pci-epc-core.c
> > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > @@ -156,6 +156,47 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
> >  }
> >  EXPORT_SYMBOL_GPL(pci_epc_get_features);
> >  
> > +/**
> > + * pci_epc_get_aux_resources() - query EPC-provided auxiliary resources
> > + * @epc: EPC device
> > + * @func_no: function number
> > + * @vfunc_no: virtual function number
> > + * @resources: output array (may be NULL to query required count)
> > + * @num_resources: size of @resources array in entries (0 when querying count)
> > + *
> > + * Some EPC backends integrate auxiliary blocks (e.g. DMA engines) whose control
> > + * registers and/or descriptor memories can be exposed to the host by mapping
> > + * them into BAR space. This helper queries the backend for such resources.
> > + *
> > + * Return:
> > + *   * >= 0: number of resources returned (or required, if @resources is NULL)
> 
> This feels like an awkward API design. This API returns 0 as a failure apart
> other error nos and wants the callers to do:
> 
> 	if (ret)
> 		return ret;
> 	else if (ret == 0)
> 		return -ENODEV;
> 
> I think this design comes due to the fact you have a single API doing both
> resource count and resource get. This also evident in patch 7.
> 
> You should just split this API into two. One for returning resource count and
> another for resource get.

Thanks for the review.

That makes sense. I'll rework and introduce pci_epc_count_aux_resources().

Best regards,
Koichiro

> 
> - Mani
> 
> > + *   * -EOPNOTSUPP: backend does not support auxiliary resource queries
> > + *   * other -errno on failure
> > + */
> > +int pci_epc_get_aux_resources(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > +			      struct pci_epc_aux_resource *resources,
> > +			      int num_resources)
> > +{
> > +	int ret;
> > +
> > +	if (!epc || !epc->ops)
> > +		return -EINVAL;
> > +
> > +	if (func_no >= epc->max_functions)
> > +		return -EINVAL;
> > +
> > +	if (!epc->ops->get_aux_resources)
> > +		return -EOPNOTSUPP;
> > +
> > +	mutex_lock(&epc->lock);
> > +	ret = epc->ops->get_aux_resources(epc, func_no, vfunc_no, resources,
> > +					  num_resources);
> > +	mutex_unlock(&epc->lock);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_epc_get_aux_resources);
> > +
> >  /**
> >   * pci_epc_stop() - stop the PCI link
> >   * @epc: the link of the EPC device that has to be stopped
> > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> > index 334c2b7578d0..a6131f1636b0 100644
> > --- a/include/linux/pci-epc.h
> > +++ b/include/linux/pci-epc.h
> > @@ -61,6 +61,47 @@ struct pci_epc_map {
> >  	void __iomem	*virt_addr;
> >  };
> >  
> > +/**
> > + * enum pci_epc_aux_resource_type - auxiliary resource type identifiers
> > + * @PCI_EPC_AUX_DOORBELL_MMIO: Doorbell MMIO, that might be outside the DMA
> > + *                             controller register window
> > + *
> > + * EPC backends may expose auxiliary blocks (e.g. DMA engines) by mapping their
> > + * register windows and descriptor memories into BAR space. This enum
> > + * identifies the type of each exposable resource.
> > + */
> > +enum pci_epc_aux_resource_type {
> > +	PCI_EPC_AUX_DOORBELL_MMIO,
> > +};
> > +
> > +/**
> > + * struct pci_epc_aux_resource - a physical auxiliary resource that may be
> > + *                               exposed for peer use
> > + * @type:       resource type, see enum pci_epc_aux_resource_type
> > + * @phys_addr:  physical base address of the resource
> > + * @size:       size of the resource in bytes
> > + * @bar:        BAR number where this resource is already exposed to the RC
> > + *              (NO_BAR if not)
> > + * @bar_offset: offset within @bar where the resource starts (valid iff
> > + *              @bar != NO_BAR)
> > + * @u:          type-specific metadata
> > + */
> > +struct pci_epc_aux_resource {
> > +	enum pci_epc_aux_resource_type type;
> > +	phys_addr_t phys_addr;
> > +	resource_size_t size;
> > +	enum pci_barno bar;
> > +	resource_size_t bar_offset;
> > +
> > +	union {
> > +		/* PCI_EPC_AUX_DOORBELL_MMIO */
> > +		struct {
> > +			int irq; /* IRQ number for the doorbell handler */
> > +			u32 data; /* write value to ring the doorbell */
> > +		} db_mmio;
> > +	} u;
> > +};
> > +
> >  /**
> >   * struct pci_epc_ops - set of function pointers for performing EPC operations
> >   * @write_header: ops to populate configuration space header
> > @@ -84,6 +125,7 @@ struct pci_epc_map {
> >   * @start: ops to start the PCI link
> >   * @stop: ops to stop the PCI link
> >   * @get_features: ops to get the features supported by the EPC
> > + * @get_aux_resources: ops to retrieve controller-owned auxiliary resources
> >   * @owner: the module owner containing the ops
> >   */
> >  struct pci_epc_ops {
> > @@ -115,6 +157,9 @@ struct pci_epc_ops {
> >  	void	(*stop)(struct pci_epc *epc);
> >  	const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
> >  						       u8 func_no, u8 vfunc_no);
> > +	int	(*get_aux_resources)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > +				     struct pci_epc_aux_resource *resources,
> > +				     int num_resources);
> >  	struct module *owner;
> >  };
> >  
> > @@ -339,6 +384,9 @@ int pci_epc_start(struct pci_epc *epc);
> >  void pci_epc_stop(struct pci_epc *epc);
> >  const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
> >  						    u8 func_no, u8 vfunc_no);
> > +int pci_epc_get_aux_resources(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > +			      struct pci_epc_aux_resource *resources,
> > +			      int num_resources);
> >  enum pci_barno
> >  pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
> >  enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
> > -- 
> > 2.51.0
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்