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 1184C43249;
	Sun,  5 Nov 2023 06:43:35 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id A9041402A3;
	Sun,  5 Nov 2023 06:43:34 +0100 (CET)
Received: from mail-qt1-f180.google.com (mail-qt1-f180.google.com
 [209.85.160.180])
 by mails.dpdk.org (Postfix) with ESMTP id B685F40278
 for <dev@dpdk.org>; Sun,  5 Nov 2023 06:43:33 +0100 (CET)
Received: by mail-qt1-f180.google.com with SMTP id
 d75a77b69052e-41cd7a3e8f8so23390381cf.0
 for <dev@dpdk.org>; Sat, 04 Nov 2023 22:43:33 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=gmail.com; s=20230601; t=1699163013; x=1699767813; darn=dpdk.org;
 h=content-transfer-encoding:cc:to:subject:message-id:date:from
 :in-reply-to:references:mime-version:from:to:cc:subject:date
 :message-id:reply-to;
 bh=yrwyFYfTXVi7i9yMC8aAveafUGKKc7hY90BL7vzAMpg=;
 b=LGvml6HqFXcRr1kxrTj1VBCW7xKMQQ5Ent5bsQEXf27rFMdrX9dgrbwISwz9zaQkMk
 pdEFEA7zR1XLi2mCvmCgXWyQwe7/1p24L9bSa4SSjXD2qn27Qd1P48Cac25h1zBRPq2R
 zQ3mCCqGmlTjpLBAFB03rAzWfe2yQQshWR4aXhkGVaMi28aNk7mmeVQwk0zBY/wdMsTc
 GVx++U+HL8Mkci13kEZWXrR2kSMzm5cuNFnvJkSv5qTwT7VKzM4+3em9GHdGe+yI5LH2
 Vl85hk9nkQwM44noqm9ZcN0iBwGXF3Kg5oKM/Uhe/7OzWGFK4iVp7kco9+myBDn3mpx7
 4vxQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1699163013; x=1699767813;
 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=yrwyFYfTXVi7i9yMC8aAveafUGKKc7hY90BL7vzAMpg=;
 b=OjoLK2+zYieDx5+TdGC5S4h33yph6gd7FLs66TVLBS/cgB5nh41hUaPrbmXn46YVLc
 Dqn8mTxKnNfIxvAnWn8Q00UH2y5DqT+eOoaGqf4DxAcUNQYRPwnZniaTKHW8KzclowKt
 g9F5n956Ojxaun3ewu/F0R+Rwz8pMNPBq6eEwRO+ESFWeD6F+aAXFiE3r9orurE8rGUz
 jOnmCZE+9BVmZmcN7ddvtEeEMD5+C9gsNLaa0z3IkXfO8583xB8bmryGcPx2lT1abQxG
 lzRtct/sVZp3CXeTIJd+/ZijVnsXvg+V+an5DaDLHMOsyYj3lpqNJFofFySs5Xl8B/tT
 RgMg==
X-Gm-Message-State: AOJu0YwJRq+wIS9NDU4tVOxzKGWjCTvozEy5a/mRUCvfwlDdxE0M3FFG
 mh5G1QYmTWPIozbCScVMuev1TsxvMS2cZgdITQY=
X-Google-Smtp-Source: AGHT+IGEf3w24TTF6H3+9Jy0uPQaA0ASr87E0FewWPSTr5GmaADOpPxFOhPG7+J5nSXczicgzKfeoqpCQtqmx/qMVgQ=
X-Received: by 2002:ac8:7f83:0:b0:41c:da69:e917 with SMTP id
 z3-20020ac87f83000000b0041cda69e917mr34919708qtj.53.1699163012943; Sat, 04
 Nov 2023 22:43:32 -0700 (PDT)
MIME-Version: 1.0
References: <20231103182933.2831662-1-abdullah.sevincer@intel.com>
 <20231103182933.2831662-2-abdullah.sevincer@intel.com>
 <ZUZOqB71+tBs463z@bricha3-MOBL.ger.corp.intel.com>
In-Reply-To: <ZUZOqB71+tBs463z@bricha3-MOBL.ger.corp.intel.com>
From: Jerin Jacob <jerinjacobk@gmail.com>
Date: Sun, 5 Nov 2023 11:13:06 +0530
Message-ID: <CALBAE1OddSNTwgC2tVTTgm4_7zZjzd0TP+WWnxG5u9Lh_SbigQ@mail.gmail.com>
Subject: Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: Abdullah Sevincer <abdullah.sevincer@intel.com>, dev@dpdk.org,
 jerinj@marvell.com, mike.ximing.chen@intel.com, thomas@monjalon.net
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 Sat, Nov 4, 2023 at 7:31=E2=80=AFPM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Fri, Nov 03, 2023 at 01:29:32PM -0500, Abdullah Sevincer wrote:
> > This commit implements an internal api to enable and disable PASID for
> > a device e.g. device driver event/dlb2.
> >
> > 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..5aac2406f1 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_ena_dis(const struct rte_pci_device *dev, off_t offset, =
bool enable)
>
> While I realise we are now at v6 of this patchset, and the name was
> suggested on v4, seeing it implemented I'm afraid I think
> rte_pci_pasid_ena_dis is not a great name! I also agree that the pasid_se=
t
> name was a bit misleading too, leaving us with a naming problem.
> I have two suggestions:
>
> * if we want to keep one function - "rte_pci_pasid_set_state", which make=
s
>   it clear we are not setting the pasid, but the pasid state.
> * separate this explicitly into rte_pci_pasid_enable() and
>   rte_pci_pasid_disable() functions. This is the cleanest solution but it
>   doesn't align with some of the other functions in pci lib which set the
>   state.
>
> Jerin, any further thoughts? and sorry for late feedback.

Yes. Above two functions are better than ena_dis().

Other option could be, rte_pci_pasid_ctrl() which is aligned with PCI
register name.

No strong opinion for the name. Feel free to pick one from above three opti=
ons.


>
> /Bruce