DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Ronak Doshi <doshir@vmware.com>,
	Jochen Behrens <jbehrens@vmware.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Cc: dev@dpdk.org
Subject: Re: [PATCH next 2/7] vmxnet3: add support for capability registers
Date: Wed, 26 Apr 2023 17:55:03 +0100	[thread overview]
Message-ID: <bcbc0c77-2a74-f11a-6771-6eb642e02025@amd.com> (raw)
In-Reply-To: <20230412162636.30843-3-doshir@vmware.com>

On 4/12/2023 5:26 PM, Ronak Doshi wrote:
> This patch enhances vmxnet3 to suuport capability registers which

s/suuport/support/

> allows it to enable features selectively. The DCR register tracks
> the capabilities vmxnet3 device supports. The PTCR register states
> the capabilities that the passthrough device supports.
> 
> With the help of these registers, vmxnet3 can enable only those
> features which the passthrough device supoprts. This allows

s/supoprts/supports/

> smooth trasition to Uniform-Passthrough (UPT) mode if the virtual

s/trasition/transition/

> nic requests it. If PTCR register returns nothing or error it means

s/nic/NIC/

> UPT is not being requested and vnic will continue in emulation mode.
> 
> Signed-off-by: Ronak Doshi <doshir@vmware.com>
> Acked-by: Jochen Behrens <jbehrens@vmware.com>

<...>

> +static int
> +eth_vmxnet3_setup_capabilities(struct vmxnet3_hw *hw,
> +			       struct rte_eth_dev *eth_dev)
> +{
> +	uint32_t dcr, ptcr, value;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
> +
> +	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
> +			       VMXNET3_CMD_GET_MAX_CAPABILITIES);
> +	value = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_CMD);
> +	hw->max_capabilities[0] = value;
> +	dcr = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_DCR);
> +	hw->DCR_capabilities[0] = dcr;
> +	hw->used_DCR_capabilities[0] = 0;
> +	ptcr = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_PTCR);
> +	hw->PTCR_capabilities[0] = ptcr;
> +	hw->used_PTCR_capabilities[0] = 0;
> +
> +	if (hw->uptv2_enabled && !(ptcr & (1 << VMXNET3_DCR_ERROR))) {
> +		PMD_DRV_LOG(NOTICE, "UPTv2 enabled");
> +		hw->used_PTCR_capabilities[0] = ptcr;
> +	} else {
> +		/* Use all DCR capabilities, but disable large bar */
> +		hw->used_DCR_capabilities[0] = dcr &
> +					(~(1UL << VMXNET3_CAP_LARGE_BAR));
> +		PMD_DRV_LOG(NOTICE, "UPTv2 disabled");
> +	}
> +	if (hw->DCR_capabilities[0] & (1UL << VMXNET3_CAP_OOORX_COMP) &&
> +	    hw->PTCR_capabilities[0] & (1UL << VMXNET3_CAP_OOORX_COMP)) {
> +		if (hw->uptv2_enabled) {
> +			hw->used_PTCR_capabilities[0] |=
> +				(1UL << VMXNET3_CAP_OOORX_COMP);
> +		}
> +	}
> +	if (hw->used_PTCR_capabilities[0]) {
> +		VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DCR,
> +				       hw->used_PTCR_capabilities[0]);
> +	} else if (hw->used_DCR_capabilities[0]) {
> +		VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DCR,
> +				       hw->used_DCR_capabilities[0]);
> +	}
> +	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_DCR0_REG);
> +	dcr = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_CMD);
> +	hw->used_DCR_capabilities[0] = dcr;
> +	PMD_DRV_LOG(NOTICE, "Dev "PCI_PRI_FMT", vmxnet3 v%d, UPT enabled: %s, "

Checkpatch complains about syntax:

HECK:CONCATENATED_STRING: Concatenated strings should use spaces between
elements
#200: FILE: drivers/net/vmxnet3/vmxnet3_ethdev.c:346:
+	PMD_DRV_LOG(NOTICE, "Dev "PCI_PRI_FMT", vmxnet3 v%d, UPT enabled: %s, "

> +		    "DCR0=0x%08x, used DCR=0x%08x, "
> +		    "PTCR=0x%08x, used PTCR=0x%08x",
> +		    pci_dev->addr.domain, pci_dev->addr.bus,
> +		    pci_dev->addr.devid, pci_dev->addr.function, hw->version,
> +		    hw->uptv2_enabled ? "true" : "false",
> +		    hw->DCR_capabilities[0], hw->used_DCR_capabilities[0],
> +		    hw->PTCR_capabilities[0], hw->used_PTCR_capabilities[0]);


This log level is NOTICE, making it visible by default. And this
function always called when vesion >= 7.
Are you sure to make log this verbose? Or can it be DEBUG level?

<...>

> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1606,6 +1606,8 @@ struct rte_eth_conf {
>  #define RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP         RTE_BIT64(3)
>  /** Device supports keeping shared flow objects across restart. */
>  #define RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP RTE_BIT64(4)
> +/**< (vmxnet3) Device supports smart NIC pass through queues */
> +#define RTE_ETH_DEV_CAPA_PASS_THRU RTE_BIT64(5)

'dev_info->dev_capa' is for generic device capabilities, we can't have
device specific capability here.

Can you please describe why it is required to expose this capability to
user, what user should do with this information?


  reply	other threads:[~2023-04-26 16:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-12 16:26 [PATCH next 0/7] vmxnet3: upgrade to version 7 Ronak Doshi
2023-04-12 16:26 ` [PATCH next 1/7] vmxnet3: prepare for version 7 changes Ronak Doshi
2023-04-26 16:58   ` Ferruh Yigit
2023-04-12 16:26 ` [PATCH next 2/7] vmxnet3: add support for capability registers Ronak Doshi
2023-04-26 16:55   ` Ferruh Yigit [this message]
2023-04-12 16:26 ` [PATCH next 3/7] vmxnet3: add support for large passthrough BAR register Ronak Doshi
2023-04-12 16:26 ` [PATCH next 4/7] vmxnet3: add command to set ring buffer sizes Ronak Doshi
2023-04-26 16:58   ` Ferruh Yigit
2023-04-26 17:27     ` Ronak Doshi
2023-04-27  8:50       ` Ferruh Yigit
2023-04-27 15:59         ` Ronak Doshi
2023-05-03 10:03           ` Ferruh Yigit
2023-04-12 16:26 ` [PATCH next 5/7] vmxnet3: limit number of TXDs used for TSO packet Ronak Doshi
2023-04-12 16:26 ` [PATCH next 6/7] vmxnet3: avoid updating rxprod register frequently Ronak Doshi
2023-04-12 16:26 ` [PATCH next 7/7] vmxnet3: update to version 7 Ronak Doshi
2023-04-26 16:54 ` [PATCH next 0/7] vmxnet3: upgrade " Ferruh Yigit
2023-04-26 18:15   ` Ferruh Yigit
2023-04-26 18:33     ` Ronak Doshi
2023-04-27  9:15       ` Ferruh Yigit
2023-04-28  7:05         ` Ronak Doshi
2023-05-03 10:05           ` Ferruh Yigit

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=bcbc0c77-2a74-f11a-6771-6eb642e02025@amd.com \
    --to=ferruh.yigit@amd.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=doshir@vmware.com \
    --cc=jbehrens@vmware.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).