From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 90C17432BB;
	Mon,  6 Nov 2023 19:30:46 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 13814402B1;
	Mon,  6 Nov 2023 19:30:46 +0100 (CET)
Received: from us-smtp-delivery-124.mimecast.com
 (us-smtp-delivery-124.mimecast.com [170.10.133.124])
 by mails.dpdk.org (Postfix) with ESMTP id 8BD3B4025D
 for <dev@dpdk.org>; Mon,  6 Nov 2023 19:30:44 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1699295443;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
 content-transfer-encoding:content-transfer-encoding:
 in-reply-to:in-reply-to:references:references;
 bh=H17LdVg+kfr+qWY3FLdfNhuTOFZQY4oYXIJitbDgsyM=;
 b=XwkGZV7q1XB3F5Rtt3F/Q8QNRYbUq0/fkAQSv6AVogIS+NNlfh7eyfxhG4XoycXCXl7k+h
 vJA8Dre6OQPw+aI5QzXe7NOB35k+c9pR3levsfA2QTVxMGYD9r7U8qxjzfQVuCHxRwNLix
 RvriAvPa0wX4sgUkvyQ6aK4+zhwIyY8=
Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com
 [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS
 (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id
 us-mta-576-UA6Aymq_OBmocWCkjgLBNg-1; Mon, 06 Nov 2023 13:30:33 -0500
X-MC-Unique: UA6Aymq_OBmocWCkjgLBNg-1
Received: by mail-lj1-f197.google.com with SMTP id
 38308e7fff4ca-2c6ed315cc3so49712991fa.2
 for <dev@dpdk.org>; Mon, 06 Nov 2023 10:30:32 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1699295431; x=1699900231;
 h=content-transfer-encoding:cc:to:subject:message-id:date:from
 :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc
 :subject:date:message-id:reply-to;
 bh=H17LdVg+kfr+qWY3FLdfNhuTOFZQY4oYXIJitbDgsyM=;
 b=QHEmzjAlenaI0LwiDfmPWNfiFiipGWKeLbF131tjxZqVTm+8efWyGiDJ+Qk/SM68Ip
 o3t4Ky24/KAbJuxREk69wOtQCz0fpu9K63BENwaNkICAD5CbRW73ful4JVdsK9QuMT8I
 Od1s5QesPkeid6pE1mPHe209IvfTR76x41ebQt29SCJdqdOBD7LSk/1oQJ7gGPxRLGwp
 9CKjVio1SB4vWGMON7y3vSjasVwKqcYfjfNLfYl+M/mzkirn6ovd76WEY9a80zjzqm7m
 HZz82LqI5STlmogDoLZJ09zkhA5Mj/2zdP32q7PZJfIJfPTCcIJVHZLrajCc0H5uXzu/
 cjmg==
X-Gm-Message-State: AOJu0YyijCb+YnhLZ/OY9r0Z9oC21vpAj20VWL7oDjZ7KG+xblNlkUf7
 6NPNudKRSmXBA/bzc/PoJvO0ATnpM/aH+EEd8LHFfHSYbP/M3Xh3mczmPYaoafTC9claudnPUI/
 qfpfxIdChdpADnfU4jyA=
X-Received: by 2002:a2e:a7ca:0:b0:2c7:fa6:7183 with SMTP id
 x10-20020a2ea7ca000000b002c70fa67183mr7681998ljp.47.1699295431708; 
 Mon, 06 Nov 2023 10:30:31 -0800 (PST)
X-Google-Smtp-Source: AGHT+IHbg9ixha+PP6tRwr7IomvrTMPd9pjAlbJgdtXqXBxmMkAbXBymn52yMyNTXaaQXW/nBRYh4mNAajDyflD8UvQ=
X-Received: by 2002:a2e:a7ca:0:b0:2c7:fa6:7183 with SMTP id
 x10-20020a2ea7ca000000b002c70fa67183mr7681979ljp.47.1699295431402; Mon, 06
 Nov 2023 10:30:31 -0800 (PST)
MIME-Version: 1.0
References: <20231106170521.3064038-1-abdullah.sevincer@intel.com>
 <20231106170521.3064038-2-abdullah.sevincer@intel.com>
In-Reply-To: <20231106170521.3064038-2-abdullah.sevincer@intel.com>
From: David Marchand <david.marchand@redhat.com>
Date: Mon, 6 Nov 2023 19:30:19 +0100
Message-ID: <CAJFAV8w_RjgMr1tyAGxQJeeDzairkaa5C_vBH7X=dMtM-bBKVA@mail.gmail.com>
Subject: Re: [PATCH v7 1/2] bus/pci: support PASID control
To: Abdullah Sevincer <abdullah.sevincer@intel.com>, thomas@monjalon.net
Cc: dev@dpdk.org, jerinj@marvell.com, mike.ximing.chen@intel.com, 
 bruce.richardson@intel.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Mon, Nov 6, 2023 at 6:05=E2=80=AFPM Abdullah Sevincer
<abdullah.sevincer@intel.com> wrote:
>
> Add an internal API to control PASID for a given PCIe device.
>
> For kernels when PASID enabled by default it breaks DLB functionality,
> hence disabling PASID is required for DLB to function properly.
>
> PASID capability is not exposed to users hence offset can not be
> retrieved by rte_pci_find_ext_capability() api. Therefore, api
> implemented in this commit accepts an offset for PASID with an enable
> flag which is used to enable/disable PASID.
>
> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> ---
>  drivers/bus/pci/pci_common.c  |  7 +++++++
>  drivers/bus/pci/rte_bus_pci.h | 13 +++++++++++++
>  drivers/bus/pci/version.map   |  1 +
>  lib/pci/rte_pci.h             |  4 ++++
>  4 files changed, 25 insertions(+)
>
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index 921d957bf6..ecf080c5d7 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -938,6 +938,13 @@ rte_pci_set_bus_master(const struct rte_pci_device *=
dev, bool enable)
>         return 0;
>  }
>
> +int
> +rte_pci_pasid_set_state(const struct rte_pci_device *dev, off_t offset, =
bool enable)
> +{
> +       uint16_t pasid =3D enable;
> +       return rte_pci_write_config(dev, &pasid, sizeof(pasid), offset) <=
 0 ? -1 : 0;
> +}

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 =3D enable;
-       return rte_pci_write_config(dev, &pasid, sizeof(pasid), offset) < 0
-               ? -1
-               : 0;
+       uint16_t state =3D enable;
+       off_t pasid_offset;
+       int ret =3D -1;
+
+       pasid_offset =3D rte_pci_find_ext_capability(dev,
RTE_PCI_EXT_CAP_ID_PASID);
+       if (pasid_offset >=3D 0 && rte_pci_write_config(dev, &state,
sizeof(state),
+                       pasid_offset + RTE_PCI_PASID_CTRL) =3D=3D sizeof(st=
ate))
+               ret =3D 0;
+
+       return ret;
 }

 struct rte_pci_bus rte_pci_bus =3D {
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 offse=
t */

 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 =3D 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;


--=20
David Marchand