From: Bruce Richardson <bruce.richardson@intel.com>
To: Abdullah Sevincer <abdullah.sevincer@intel.com>
Cc: <dev@dpdk.org>, <jerinj@marvell.com>,
<mike.ximing.chen@intel.com>, <thomas@monjalon.net>
Subject: Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
Date: Sat, 4 Nov 2023 14:01:12 +0000 [thread overview]
Message-ID: <ZUZOqB71+tBs463z@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20231103182933.2831662-2-abdullah.sevincer@intel.com>
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_set
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 makes
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.
/Bruce
next prev parent reply other threads:[~2023-11-04 14:01 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-03 18:29 [PATCH v6 0/2] *** Disable PASID for DLB Device *** Abdullah Sevincer
2023-11-03 18:29 ` [PATCH v6 1/2] bus/pci: add function to enable/disable PASID Abdullah Sevincer
2023-11-04 7:32 ` Jerin Jacob
2023-11-04 9:19 ` Thomas Monjalon
2023-11-05 5:48 ` Jerin Jacob
2023-11-04 14:01 ` Bruce Richardson [this message]
2023-11-05 5:43 ` Jerin Jacob
2023-11-06 1:16 ` Chenbo Xia
2023-11-06 15:44 ` Sevincer, Abdullah
2023-11-07 8:17 ` Ruifeng Wang
2023-11-03 18:29 ` [PATCH v6 2/2] event/dlb2: fix disable PASID Abdullah Sevincer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZUZOqB71+tBs463z@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=abdullah.sevincer@intel.com \
--cc=dev@dpdk.org \
--cc=jerinj@marvell.com \
--cc=mike.ximing.chen@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).