From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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 Date: Mon, 6 Nov 2023 19:30:19 +0100 Message-ID: Subject: Re: [PATCH v7 1/2] bus/pci: support PASID control To: Abdullah Sevincer , 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Mon, Nov 6, 2023 at 6:05=E2=80=AFPM Abdullah Sevincer 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 > --- > 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