DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Huang, Wei" <wei.huang@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>, "Xu, Rosen" <rosen.xu@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>
Cc: "stable@dpdk.org" <stable@dpdk.org>,
	"Zhang, Tianfei" <tianfei.zhang@intel.com>,
	Ray Kinsella <mdr@ashroe.eu>,
	Hemant Agrawal <hemant.agrawal@nxp.com>
Subject: Re: [dpdk-dev] [PATCH v12 1/4] raw/ifpga: add fpga rsu function
Date: Fri, 29 Jan 2021 07:38:12 +0000
Message-ID: <DM6PR11MB35309255C28714BD1D795D4DEFB99@DM6PR11MB3530.namprd11.prod.outlook.com> (raw)
In-Reply-To: <9d4e0dba-4630-15fe-0552-574a960ed598@intel.com>



-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Thursday, January 28, 2021 21:25
To: Huang, Wei <wei.huang@intel.com>; dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Hemant Agrawal <hemant.agrawal@nxp.com>
Subject: Re: [dpdk-dev] [PATCH v12 1/4] raw/ifpga: add fpga rsu function

On 1/26/2021 6:45 AM, Wei Huang wrote:
> RSU (Remote System Update) depends on secure manager which may be 
> different on various implementations, so a new secure manager device 
> is implemented for adapting such difference.
> There are three major functions added:
> 1. ifpga_rawdev_update_flash() updates flash with specific image file.
> 2. ifpga_rawdev_stop_flash_update() aborts flash update process.
> 3. ifpga_rawdev_reload() reloads FPGA from updated flash.
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
> Acked-by: Rosen Xu <rosen.xu@intel.com>
> ---
> v2: fix coding style issue in ifpga_fme_rsu.c and ifpga_sec_mgr.c
> ---
> v3: fix compilation issues in ifpga_fme_rsu.c
> ---
> v4: fix compilation issues in opae_intel_max10.c
> ---

<...>

> @@ -43,7 +43,7 @@ enum ifpga_rawdev_device_state {
>   static inline struct opae_adapter *
>   ifpga_rawdev_get_priv(const struct rte_rawdev *rawdev)
>   {
> -	return rawdev->dev_private;
> +	return (struct opae_adapter *)rawdev->dev_private;
>   }
>   
>   #define IFPGA_RAWDEV_MSIX_IRQ_NUM 7
> @@ -76,4 +76,9 @@ int
>   ifpga_unregister_msix_irq(enum ifpga_irq_type type,
>   		int vec_start, rte_intr_callback_fn handler, void *arg);
>   
> +int ifpga_rawdev_update_flash(struct rte_rawdev *dev, const char *image,
> +	uint64_t *status);
> +int ifpga_rawdev_stop_flash_update(struct rte_rawdev *dev, int 
> +force); int ifpga_rawdev_reload(struct rte_rawdev *dev, int type, int 
> +page);
> +
>   #endif /* _IFPGA_RAWDEV_H_ */
> 

Hi Wei, Rosen,

This patch introduces three new PMD specific APIs, adding new API has some requirements.

1)
There must be a header file for user application to include, that has the definitions of the APIs.

This header file should be installed in meson via "headers = ..." syntax.

Indeed for rawdev a header always should be installed, because of the rawdev design, the user application should know about the some driver structures, to share those structures PMD should provide a header. This header seems missing.

You can start for providing the missing header, even before this patch.

Header file should be named as 'rte_pmd_.....', and since it is a public header now it should be fully documented via doxygen comments.

This header file should be added to 'doc/api/doxy-api-index.md' for API documentathion.
OK, I will add rte_pmd_ifpga.h.
2)
All APIs should start with 'rte_pmd_' prefix.
OK, I will use 'rte_pmd_' prefix before driver APIs.
3)
All APIs should be in the .map file
OK, I will add them into .map file.
4)
Since these are new APIs, they should be marked as experimental. This is done both documenting this in the doxygen comments and marking the function decleration via '__rte_experimental' tag
How to document it in doxygen comments ? Could you give an example ?
5)
Please don't use "struct rte_rawdev *dev" as a argument in the APIs, since that structure is rawdev internal structures, applications (that will call your API) should not know or access to this struct.
Instead you can you 'dev_id' (ethdev 'port_id' equivalent) in the API, as done in the rawdev APIs. Driver can easily get the rawdevice from 'dev_id'.
OK, I will use dev_id in driver APIs' argument.
cc'ed Ray & Hemant in case I missed something related to rawdev and API/ABIs.


And for the ifpga implementation, it is hard for me to review it, I trust Rosen for it.

  reply	other threads:[~2021-01-29  7:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-26  6:45 [dpdk-dev] [PATCH v12 0/4] raw/ifpga: add extra OPAE APIs Wei Huang
2021-01-26  6:45 ` [dpdk-dev] [PATCH v12 1/4] raw/ifpga: add fpga rsu function Wei Huang
2021-01-28 13:24   ` Ferruh Yigit
2021-01-29  7:38     ` Huang, Wei [this message]
2021-01-29  8:59       ` Ferruh Yigit
2021-01-26  6:45 ` [dpdk-dev] [PATCH v12 2/4] raw/ifpga: add fpga property get function Wei Huang
2021-01-26  6:45 ` [dpdk-dev] [PATCH v12 3/4] raw/ifpga: add fpga helper function Wei Huang
2021-01-28 13:30   ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2021-01-29  7:42     ` Huang, Wei
2021-01-26  6:45 ` [dpdk-dev] [PATCH v12 4/4] examples/ifpga: add example for ifpga opae API Wei Huang
2021-01-28 13:34   ` Ferruh Yigit
2021-01-29  7:44     ` Huang, Wei

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=DM6PR11MB35309255C28714BD1D795D4DEFB99@DM6PR11MB3530.namprd11.prod.outlook.com \
    --to=wei.huang@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=mdr@ashroe.eu \
    --cc=qi.z.zhang@intel.com \
    --cc=rosen.xu@intel.com \
    --cc=stable@dpdk.org \
    --cc=tianfei.zhang@intel.com \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git