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 4B720A09E4 for ; Fri, 29 Jan 2021 09:59:14 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3CF4324012D; Fri, 29 Jan 2021 09:59:14 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id AA2E140694; Fri, 29 Jan 2021 09:59:10 +0100 (CET) IronPort-SDR: mg/zaY8SgkV3y8HK+8qGrLaSoy/20e+kBvlW7Va5d96UH39TvH4pdqfwxy+IOf509l6bAVwqSG +uDxDDhKQMpg== X-IronPort-AV: E=McAfee;i="6000,8403,9878"; a="176884325" X-IronPort-AV: E=Sophos;i="5.79,385,1602572400"; d="scan'208";a="176884325" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Jan 2021 00:59:09 -0800 IronPort-SDR: 7rrUT5gqapZH2WFFMqGSHazVnDxcRPBlnG5ak4QxVbKSM41BG3ZCpyezA1wKReSgXIwXsQRN7j Xpr9KpStgWtQ== X-IronPort-AV: E=Sophos;i="5.79,385,1602572400"; d="scan'208";a="389222876" Received: from mpkenny-mobl.ger.corp.intel.com (HELO [10.213.197.204]) ([10.213.197.204]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Jan 2021 00:59:07 -0800 To: "Huang, Wei" , "dev@dpdk.org" , "Xu, Rosen" , "Zhang, Qi Z" Cc: "stable@dpdk.org" , "Zhang, Tianfei" , Ray Kinsella , Hemant Agrawal References: <1611643528-18311-1-git-send-email-wei.huang@intel.com> <1611643528-18311-2-git-send-email-wei.huang@intel.com> <9d4e0dba-4630-15fe-0552-574a960ed598@intel.com> From: Ferruh Yigit Message-ID: <3f9644ee-1c83-299b-d00a-04ee7b2783cf@intel.com> Date: Fri, 29 Jan 2021 08:59:04 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v12 1/4] raw/ifpga: add fpga rsu function X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 1/29/2021 7:38 AM, Huang, Wei wrote: > > > -----Original Message----- > From: Ferruh Yigit > Sent: Thursday, January 28, 2021 21:25 > To: Huang, Wei ; dev@dpdk.org; Xu, Rosen ; Zhang, Qi Z > Cc: stable@dpdk.org; Zhang, Tianfei ; Ray Kinsella ; Hemant Agrawal > 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 >> Acked-by: Tianfei Zhang >> Acked-by: Rosen Xu >> --- >> 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. Btw, in the .map, it needs a special block for the experimental APIs, and since all your APIs will be experimental at this stage, they will all go in that block, it is like: EXPERIMENTAL { global: # added in }; > 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 ? Sample: https://git.dpdk.org/dpdk/tree/lib/librte_ethdev/rte_ethdev.h?h=v20.11#n2105 It is simply adding a note/warning that is saying API is experimental. > 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. >