>+I don't see much point in providing a wrapper that does nothing more than 
>call rte_pci_write_config() and let the driver pass the right offsets.

>+If anything, can't this wrapper find out about the pasid offset itself?
>+There is a extended capability for this, so I would expect it can be used.

>+Something like (only compile tested):

>+diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c 
>index ba5e280d33..2ca28bd4d4 100644
>+--- a/drivers/bus/pci/pci_common.c
>++++ b/drivers/bus/pci/pci_common.c
>+@@ -939,13 +939,18 @@ rte_pci_set_bus_master(const struct rte_pci_device 
>*dev, bool enable)  }

>+ int
>+-rte_pci_pasid_set_state(const struct rte_pci_device *dev,
>+-               off_t offset, bool enable)
>++rte_pci_pasid_set_state(const struct rte_pci_device *dev, bool enable)
>+ {
>+-       uint16_t pasid = enable;
>+-       return rte_pci_write_config(dev, &pasid, sizeof(pasid), offset) < 0
>+-               ? -1
>+-               : 0;
>++       uint16_t state = enable;
>++       off_t pasid_offset;
>++       int ret = -1;
>++
>++       pasid_offset = rte_pci_find_ext_capability(dev,
>+RTE_PCI_EXT_CAP_ID_PASID);
>++       if (pasid_offset >= 0 && rte_pci_write_config(dev, &state,
>+sizeof(state),
>++                       pasid_offset + RTE_PCI_PASID_CTRL) == sizeof(state))
>++               ret = 0;
>++
>++       return ret;
 >+}

 >+struct rte_pci_bus rte_pci_bus = {
>+diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h 
>index f07bf9b588..6d5dbc1d50 100644
>+--- a/drivers/bus/pci/rte_bus_pci.h
>++++ b/drivers/bus/pci/rte_bus_pci.h
>+@@ -160,14 +160,14 @@ int rte_pci_set_bus_master(const struct rte_pci_device 
>*dev, bool enable);
 >+ *
  >+* @param dev
  >+*   A pointer to a rte_pci_device structure.
>+- * @param offset
>+- *   Offset of the PASID external capability.
>+  * @param enable
>+  *   Flag to enable or disable PASID.
>++ *
>++ *  @return
>++ *  0 on success, -1 on error in PCI config space read/write.
>+  */
>+ __rte_internal
>+-int rte_pci_pasid_set_state(const struct rte_pci_device *dev,
>+-               off_t offset, bool enable);
>++int rte_pci_pasid_set_state(const struct rte_pci_device *dev, bool 
>++enable);

 >+/**
  >+* Read PCI config space.
>+diff --git a/drivers/event/dlb2/pf/dlb2_main.c
>+b/drivers/event/dlb2/pf/dlb2_main.c
>+index 61a7b39eef..bd1ee4af27 100644
>+--- a/drivers/event/dlb2/pf/dlb2_main.c
>++++ b/drivers/event/dlb2/pf/dlb2_main.c
>+@@ -26,7 +26,6 @@
>+ #define PF_ID_ZERO 0   /* PF ONLY! */
 >+#define NO_OWNER_VF 0  /* PF ONLY! */
>+ #define NOT_VF_REQ false /* PF ONLY! */
>+-#define DLB2_PCI_PASID_CAP_OFFSET        0x148   /* PASID capability offset 
>*/

 >+static int
 >+dlb2_pf_init_driver_state(struct dlb2_dev *dlb2_dev) @@ -518,8 +517,7 @@ 
 >dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
  >+      /* Disable PASID if it is enabled by default, which
   >+      * breaks the DLB if enabled.
  >+       */
>+-       off = DLB2_PCI_PASID_CAP_OFFSET + RTE_PCI_PASID_CTRL;
>+-       if (rte_pci_pasid_set_state(pdev, off, false)) {
>++       if (rte_pci_pasid_set_state(pdev, false)) {
 >+               DLB2_LOG_ERR("[%s()] failed to write the pcie config space at 
 >offset %d\n",
 >+                               __func__, (int)off);
 >+               return -1;

Hi David,
>++       pasid_offset = rte_pci_find_ext_capability(dev,
>+RTE_PCI_EXT_CAP_ID_PASID);

That  rte_pci_find_ext_capability() api does not work for PASID since PASID is 
not exposed to user from kernel.
So, we can not retrieve offset. Instead we came up with a solution that passes 
an offset to an internal function to disable PASID and make the function 
internal so we can change it later.
When the linux limitation is lifted we can re-write the functions and use 
rte_pci_find_ext_capability api to retrieve offset and your 
solution above can be done.

Reply via email to