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 2DF4E424F4; Tue, 5 Sep 2023 17:45:36 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1DCE040E40; Tue, 5 Sep 2023 17:45:36 +0200 (CEST) Received: from NAM12-MW2-obe.outbound.protection.outlook.com (mail-mw2nam12on2082.outbound.protection.outlook.com [40.107.244.82]) by mails.dpdk.org (Postfix) with ESMTP id 32E3840279 for ; Tue, 5 Sep 2023 17:45:34 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BcBqdcP79xhnXmdowRM6zYxnzCEL7FwjICNlQhUC/3kPKcmUOy06h42+T6U9APdc5pTqcWP+p3jgxHOR/Y3JMRWu7yB9svo/ePjD1uFGD2GSa3PTT/2hInVw7Vvu9EGGqEqHhINj+Bh9cbQilm6NOG+yyG7LHhMV/NXN+eKBzDu32bnvJz+u9BsRcWF8EfFOBejV+Xng+zfTX27oZkM2YPZElwEII6Pg0AjJtk+4k+BiSLAn5Y+sNw4XolKjiRQyARegrt5FghPFje7/wWwZRsQwFoIZyz0L/S31WKHnOOraY/t1+WVqoYOnQLE1yvqpabvasbfwIoPxcB7Tm757KQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=fwbAR/Dau8s7tKKJeT9Gm9qLsKCMe48wyBpkhICtAis=; b=NEVARIQYEVe1SCbkmYLQ8GlB/8P9nwqRhPt0s3zh7SxPT2afk/vz9s8hsDqXjytP3UsLUqbIQjLzA5YF2+Su6ofiUTrIM8dDQoNrw+dS+/cYjuO2BNfX89KEyhsCN4agKBTIWC0gfuxgp+c5jX9+Yw6+O2uhrwgAtVbCFG6lgFEe8TaoXPXu4t46EhmosmZodX/QvuaWiB+oxd1XZzOGXOaB4xLpogS9pR13sjDqXoat2zBXv4w6C/7rYYxBr8e69OiqbwBfsRoBiNTFI8f5JOpqlhVPdo4Pef8I/3SOBNgvaFn5ztXiZk5iJJDb3YubsRe8EBeohIvMm2N+gaY7sQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=fwbAR/Dau8s7tKKJeT9Gm9qLsKCMe48wyBpkhICtAis=; b=2PO8lheiyx5HetEeur3dhdWM/hfWDG1mdyWof8lCeuv6VZ4ZizUyYfojDQEukQfSK8wzmP3PNJHw+V1ibErNii3xgyDbYXXgQ09cwmZRWSYeP8+LKLnWaxJw0PgDyRLtRb+0cVuPcbI26aXjNXZQK4zOk4kktNlKt5Vba1xAr5s= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by DM4PR12MB5166.namprd12.prod.outlook.com (2603:10b6:5:395::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6745.33; Tue, 5 Sep 2023 15:45:31 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::49e9:2bf6:7f06:bbbd]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::49e9:2bf6:7f06:bbbd%3]) with mapi id 15.20.6745.030; Tue, 5 Sep 2023 15:45:31 +0000 Message-ID: <4ab252f9-56d1-b9c1-1dd2-50ad1d97f4b7@amd.com> Date: Tue, 5 Sep 2023 16:45:26 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 Content-Language: en-US To: Wenbo Cao Cc: dev@dpdk.org, thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru, yaojun@mucse.com, Stephen Hemminger References: <20230901023050.40893-1-caowenbo@mucse.com> <20230901023050.40893-5-caowenbo@mucse.com> From: Ferruh Yigit Subject: Re: [PATCH v6 4/8] net/rnp: add mbx basic api feature In-Reply-To: <20230901023050.40893-5-caowenbo@mucse.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO0P265CA0014.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:355::16) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|DM4PR12MB5166:EE_ X-MS-Office365-Filtering-Correlation-Id: ef5dc103-10ae-406a-c96c-08dbae2722e3 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Z4jwhq1rlNfOb6D7wPvKKhzylvdlM5Of7Rdx4dCOvuZAuSIPrJk5Lj7XEsuOgBTeTlWUtmCUBk61y3O9sECkcu8sNDVer5VXORCy0BGTO/4gSWWg5dQObdQQARZuh0RHL708Phqt+324ZjoZLgmOYYZIRgqXOni0EV0mhKjO/6+9F32SYzkDRemUXnsenc7kLrZjeT+1GjfxoVIMQd5ZuW8HjqErKh3Xo39TxVggfhLqHD6MX7hKMZHTJBnrveIWXAI68QuZ0AixSSm5PRVV5MugzjfIG+ErwalIgw2rrVSQTW07vFqcCehS9hMNgECpdYgr9pLjoDxExQBlqdoadmxHcMhIiwaDAT1Ay5uoboerAwpy+qbfwz7HV9MCqnVKz+z/FzSYY9yCGDD861cH6az0ey04pbIf/k1Ss0S/Dowk0mPWpjMfqk4+T3s5TVxxqgyteG1EtW9v+2MoKBNoCD1h9hBHSi01FIL/bLD85UoYiiuj2R6yg6XX0+KtZLrPhy1hd7njFNTf0taI9b0159aApeHEnaBtdfJuq8EjaXPPTzPiDdkgzQnRZJOM05mvfmeCVNbRT9Tn20N+k1wSdeypGtqAf/8ppilFcfKE4tgjj7MYfkVLSKvEGfefeySGiB6OSaJCx6FXFM9maejUpA== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230031)(39860400002)(136003)(396003)(346002)(376002)(366004)(451199024)(1800799009)(186009)(36756003)(6666004)(53546011)(6486002)(6506007)(26005)(2616005)(6512007)(6916009)(316002)(66556008)(41300700001)(66946007)(38100700002)(478600001)(66476007)(31686004)(83380400001)(30864003)(2906002)(31696002)(86362001)(5660300002)(8676002)(4326008)(8936002)(44832011)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Qk80b0RuWUhKQ0I5YXNyWEFuNjJPQ3JobFZwOW9nclFEdEp3Nm9LU2N0aElI?= =?utf-8?B?bmJyU2pkNTVHZlVtUURSMUtkRDJkWExNc2hGY3RGT1U1MWtDMFVnUkt0Znhn?= =?utf-8?B?YUdzVktrcU5KdzdIa3U1Z213NUZkbnMyL1ZFL0xucFpXbUJEazRha0hrK2U3?= =?utf-8?B?WTVoWWg2TGNaTnZXeDBWUHFIL3ZzUDJRdUVRSnIzSi9JWjh0eEgzdjlBamFP?= =?utf-8?B?bXhVSlg2MWFuYzU4Wi9rMWQ4ekNvOUxxUzlyNDlQNU02b3RnaGw3UFlhQXdh?= =?utf-8?B?REpwVHFDY2FFdVpNcTlkN1lIUVBhMGc4NXNaQ3VtejM4VjFzT2pkYXpGVlRm?= =?utf-8?B?T3plVXZkM2I4Nk43K0xiMTFDMkxXTmsrOXo4Rk5lTTNrYnlPaktoY1BEY1Bn?= =?utf-8?B?a09Zd3gyRG9OQVkrT29JNXRaakJJZ3NNeVVWMjRHNXUveWhkWEFvUUtoRUgx?= =?utf-8?B?SUNtZHFOSXlHUjZVU2JVWXNBRlN4YTZIa3ZvMm9sNnd0UUNlOVZCM2hBZXdj?= =?utf-8?B?UVpFSTBJRmNrOEp3WmxkbUFmUHZvRXBQb2MrWm45bVNxdE0vcWx3aVVqMHc0?= =?utf-8?B?MW9TWlY0amxRSC90d2hUVUN6aTh2Q0h4WlhJUHhKTExNOWNwSGtwSkV3OGxz?= =?utf-8?B?a09lbU9KOEh3UXlIV05yWEFESmQ4ZHE4M0lLeG1aNWdab0JRTU9hUkNZMEZX?= =?utf-8?B?b0JYVldQa2hQNVF1QTE0Z2ZLOTl0dDdOcGo1K1VEekFZNlhlVTYvS2d6UmFS?= =?utf-8?B?T1NmWEhIeFc2QWZVTzNwN1hvcThUYXZlOWV2U29tWnZyQ1JETUNVWFVUd3hC?= =?utf-8?B?RU5mUUlIM2E2K3U0NGVMWTkxa2xqWFBUUGNaWnduVkRGc2tTd1dpN2JvaWc3?= =?utf-8?B?NGRCUTVqNTdWSXZBczE3ekpmN0d3Zjd3ZDRsUzRZc3VrYlUvRnFUdTBHY084?= =?utf-8?B?dXRWRVBUdnJzZlpPdnV4RnFWZ215Y2ozczFtd0ppMVFnRHRJVjJCb3Vqd1hy?= =?utf-8?B?ZmdGYTZoaTBHRk1tZDFaSVVpUkhuODFIRXBnczduakVjeGpUVjFmaU9abzk1?= =?utf-8?B?dEY5UlYvSnZ5R2c2VGxTS3MyUVZRZnhydFRjOTdQRjZscmIrelkzL1JPbCtF?= =?utf-8?B?MTFlSU15cm9iclhLSHBKNGFxd29vS3M3TG94c3VWdUs2cEo0eEdGeUhkS0pm?= =?utf-8?B?b25PNWphZlV4bTVJT3RNcnJCekNPQ004MVo2QUNzUGhoRzJEV25TUURHQWVz?= =?utf-8?B?M3RBTjdhRjEzMEV6WithalBxU21LdzVNMlFvZkRXaVVWam1IcGYyMnRLYU9y?= =?utf-8?B?Z3JhQ3MxdERBNzMzSDNxaStmTitsMWJPRmhzc29sSnNvWGNDdW5oV2d0eWlG?= =?utf-8?B?Y0Z4QzY0Qlc4OEZSU2t0eVdjdjlJdzFUR1hQelZrdTdTTkxXZTRWR3ZieHYr?= =?utf-8?B?UXFkcmxocXZmT0s2Ylp0MzZPWXBncUJUVk1iLzdWa1hnZ1lHc1FXV01qS2Zp?= =?utf-8?B?TTBDV1NxbmhaZ3Q0MGIrTGZTT0JtM3BObFZJVzJSZGFEcE1KY1Y4Mm9tMzE4?= =?utf-8?B?dTdEV2Y5aStHSGxTUm1hVmRVaXZ2c3VWWlpoMklvTG5aeEpUdEoyOXJGd0sr?= =?utf-8?B?d0pmOUtDQS84Q3J3UVZKTkhOYi9YZThwVEk0eUhrWjVyVjJkaXBDTE5ja1I2?= =?utf-8?B?VGpQVjYvb2xFbm1WeDQxbXl3bGxmNUQrejR2eWdSS0RsdWs5dU1DdFJqQjdC?= =?utf-8?B?ZG10cTFZWENXRkJKWmQ3WFRHaG51dTFYb2NOZWFIcXRveThvN3AvM2Nkd2xr?= =?utf-8?B?QzcwQ3lkblM5OE42SEYxVHJXcGJkZkYxV0F1R3EvM0IyZGd0Y0FWSVBHL0Vp?= =?utf-8?B?aTBjL3BHMkNhVnhZQjZVdkZ0cFgrc085ajAwTEZHanRKUGpJR1VQSGFDa2RV?= =?utf-8?B?NUFPVVZQd3g1YlNJMDIycGtsckNJOVRGMWpGTGFQS2NKc2tWRXcwZTZ6TGNW?= =?utf-8?B?QmgzNjIwRjJhUjkrdElrcGJwcmgyUTkrdUxSamE5MTYrR0lTRWxURTJGaUpj?= =?utf-8?B?bjJNOVhBY1AzVG0vSCtRK0FXSjVCMkF0alVsSUpROERWdXFkVU1sV0tMc3JD?= =?utf-8?Q?zbU1lVsagF4CwmQqV1siAkuFH?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: ef5dc103-10ae-406a-c96c-08dbae2722e3 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Sep 2023 15:45:31.8110 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: V4WBpzzpeZvZvG9/RqOvtlz1yDjfmqUjLspIdF2MKeFpfM7EXoq1QlyT6Lqtltdw X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5166 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 9/1/2023 3:30 AM, Wenbo Cao wrote: > mbx base code is for communicate with the firmware > I assume mbx is mailbox, can you please use full name in the patch title and commit log. How the parties get notified about new messages, is the interrupt support enabled or polling, can you please elabore the support in the commit log? > Signed-off-by: Wenbo Cao > Suggested-by: Stephen Hemminger > --- > drivers/net/rnp/base/rnp_api.c | 23 ++ > drivers/net/rnp/base/rnp_api.h | 7 + > drivers/net/rnp/base/rnp_cfg.h | 7 + > drivers/net/rnp/base/rnp_dma_regs.h | 73 ++++ > drivers/net/rnp/base/rnp_eth_regs.h | 124 +++++++ > drivers/net/rnp/base/rnp_hw.h | 112 +++++- Why there is no meson file in base folder? > drivers/net/rnp/meson.build | 1 + > drivers/net/rnp/rnp.h | 35 ++ > drivers/net/rnp/rnp_ethdev.c | 70 +++- > drivers/net/rnp/rnp_logs.h | 9 + > drivers/net/rnp/rnp_mbx.c | 522 ++++++++++++++++++++++++++++ > drivers/net/rnp/rnp_mbx.h | 139 ++++++++ > drivers/net/rnp/rnp_mbx_fw.c | 271 +++++++++++++++ > drivers/net/rnp/rnp_mbx_fw.h | 22 ++ Should 'rnp_mbx*' files also go under base folder? I can see they use some 'u8' like typedef, making me think they are shared between other platforms, if so base folder suits better. > 14 files changed, 1412 insertions(+), 3 deletions(-) > create mode 100644 drivers/net/rnp/base/rnp_api.c > create mode 100644 drivers/net/rnp/base/rnp_api.h > create mode 100644 drivers/net/rnp/base/rnp_cfg.h > create mode 100644 drivers/net/rnp/base/rnp_dma_regs.h > create mode 100644 drivers/net/rnp/base/rnp_eth_regs.h > create mode 100644 drivers/net/rnp/rnp_mbx.c > create mode 100644 drivers/net/rnp/rnp_mbx.h > create mode 100644 drivers/net/rnp/rnp_mbx_fw.c > create mode 100644 drivers/net/rnp/rnp_mbx_fw.h > > diff --git a/drivers/net/rnp/base/rnp_api.c b/drivers/net/rnp/base/rnp_api.c > new file mode 100644 > index 0000000000..550da6217d > --- /dev/null > +++ b/drivers/net/rnp/base/rnp_api.c > @@ -0,0 +1,23 @@ > +#include "rnp.h" > +#include "rnp_api.h" > + > +int > +rnp_init_hw(struct rte_eth_dev *dev) > +{ > Base code is mostly for works as HAL and you don't need to pass "struct rte_eth_dev" in this level, but mostly adapte (struct rnp_eth_adapter) or hw (struct rnp_hw) should be sufficient. I assume you are passing "struct rte_eth_dev" because "struct rnp_mac_api" is stored in 'process_private', this part I am not clear. Why need 'mac_ops' pointers for secondary process? Primary process should be doing all hw initialization, otherwise both primary and secondar(y|ies) trying to configure the MAC can cause unexpected results. Seconday process is only for datapath, which can access to the queues and do packet processing. Needing "mac_ops" in the secondary can be bad design desicion, can you please double check it? <...> > diff --git a/drivers/net/rnp/rnp.h b/drivers/net/rnp/rnp.h > index cab1b8e85d..6e12885877 100644 > --- a/drivers/net/rnp/rnp.h > +++ b/drivers/net/rnp/rnp.h > @@ -3,6 +3,7 @@ > */ > #ifndef __RNP_H__ > #define __RNP_H__ > +#include > > #include "base/rnp_hw.h" > > @@ -14,14 +15,17 @@ > > struct rnp_eth_port { > struct rnp_eth_adapter *adapt; > + struct rnp_hw *hw; adapter already has the 'hw', is there a specific reason not to access it via 'port->adapt->hw' > struct rte_eth_dev *eth_dev; > } __rte_cache_aligned; > > struct rnp_share_ops { > + const struct rnp_mbx_api *mbx_api; > } __rte_cache_aligned; > > struct rnp_eth_adapter { > struct rnp_hw hw; > + uint16_t max_vfs; > struct rte_pci_device *pdev; > struct rte_eth_dev *eth_dev; /* primary eth_dev */ > struct rnp_eth_port *ports[RNP_MAX_PORT_OF_PF]; > @@ -34,5 +38,36 @@ struct rnp_eth_adapter { > (((struct rnp_eth_port *)((eth_dev)->data->dev_private))) > #define RNP_DEV_TO_ADAPTER(eth_dev) \ > ((struct rnp_eth_adapter *)(RNP_DEV_TO_PORT(eth_dev)->adapt)) > +#define RNP_DEV_TO_HW(eth_dev) \ > + (&((struct rnp_eth_adapter *)(RNP_DEV_TO_PORT((eth_dev))->adapt))->hw) > +#define RNP_DEV_PP_PRIV_TO_MBX_OPS(dev) \ > + (((struct rnp_share_ops *)(dev)->process_private)->mbx_api) > +#define RNP_DEV_TO_MBX_OPS(dev) RNP_DEV_PP_PRIV_TO_MBX_OPS(dev) > > +static inline void rnp_reg_offset_init(struct rnp_hw *hw) > +{ > + uint16_t i; > + > + if (hw->device_id == RNP_DEV_ID_N10G && hw->mbx.pf_num) { > + hw->iobar4 = (void *)((uint8_t *)hw->iobar4 + 0x100000); > + hw->msix_base = (void *)((uint8_t *)hw->iobar4 + 0xa4000); > + hw->msix_base = (void *)((uint8_t *)hw->msix_base + 0x200); > + } else { > + hw->msix_base = (void *)((uint8_t *)hw->iobar4 + 0xa4000); > + } > + /* === dma status/config====== */ > + hw->link_sync = (void *)((uint8_t *)hw->iobar4 + 0x000c); > + hw->dma_axi_en = (void *)((uint8_t *)hw->iobar4 + 0x0010); > + hw->dma_axi_st = (void *)((uint8_t *)hw->iobar4 + 0x0014); > + > + if (hw->mbx.pf_num) > + hw->msix_base = (void *)((uint8_t *)0x200); > + /* === queue registers === */ > + hw->dma_base = (void *)((uint8_t *)hw->iobar4 + 0x08000); > + hw->veb_base = (void *)((uint8_t *)hw->iobar4 + 0x0); > + hw->eth_base = (void *)((uint8_t *)hw->iobar4 + 0x10000); There are lots of hardcoded values in this function, can you make them macros? > + /* mac */ > + for (i = 0; i < RNP_MAX_HW_PORT_PERR_PF; i++) > + hw->mac_base[i] = (void *)((uint8_t *)hw->iobar4 + 0x60000 + 0x10000 * i); > +} > #endif /* __RNP_H__ */ > diff --git a/drivers/net/rnp/rnp_ethdev.c b/drivers/net/rnp/rnp_ethdev.c > index ae737643a7..a2dc27548a 100644 > --- a/drivers/net/rnp/rnp_ethdev.c > +++ b/drivers/net/rnp/rnp_ethdev.c > @@ -8,6 +8,7 @@ > #include > > #include "rnp.h" > +#include "rnp_mbx.h" > #include "rnp_logs.h" > > static int > @@ -89,6 +90,58 @@ rnp_alloc_eth_port(struct rte_pci_device *primary_pci, char *name) > return NULL; > } > > +static void rnp_get_nic_attr(struct rnp_eth_adapter *adapter) > +{ > + RTE_SET_USED(adapter); > +} > + > +static int > +rnp_process_resource_init(struct rte_eth_dev *eth_dev) > +{ > + struct rnp_share_ops *share_priv; > + > + /* allocate process_private memory this must can't > + * belone to the dpdk mem resource manager > + * such as from rte_malloc or rte_dma_zone.. > + */ > + /* use the process_prive point to resolve secondary process > + * use point-func. This point is per process will be safe to cover. > + * This will cause secondary process core-dump because of IPC > + * Secondary will call primary process point func virt-address > + * secondary process don't alloc user/pmd to alloc or free > + * the memory of dpdk-mem resource it will cause hugepage > + * mem exception > + * be careful for secondary Process to use the share-mem of > + * point correlation > + */ As commented above, I am not sure why you need this, it should be sufficient to have mac_ops in primary proccess. > + share_priv = calloc(1, sizeof(*share_priv)); > + if (!share_priv) { > + PMD_DRV_LOG(ERR, "calloc share_priv failed"); > + return -ENOMEM; > + } > + memset(share_priv, 0, sizeof(*share_priv)); > + eth_dev->process_private = share_priv; > + > + return 0; > +} > + > +static void > +rnp_common_ops_init(struct rnp_eth_adapter *adapter) > +{ > + struct rnp_share_ops *share_priv; > + > + share_priv = adapter->share_priv; > + share_priv->mbx_api = &rnp_mbx_pf_ops; > +} > + > +static int > +rnp_special_ops_init(struct rte_eth_dev *eth_dev) > +{ > + RTE_SET_USED(eth_dev); > + > + return 0; > +} > + > static int > rnp_eth_dev_init(struct rte_eth_dev *dev) > { > @@ -124,6 +177,20 @@ rnp_eth_dev_init(struct rte_eth_dev *dev) > hw->device_id = pci_dev->id.device_id; > hw->vendor_id = pci_dev->id.vendor_id; > hw->device_id = pci_dev->id.device_id; > + adapter->max_vfs = pci_dev->max_vfs; > + ret = rnp_process_resource_init(dev); > + if (ret) { > + PMD_DRV_LOG(ERR, "share prive resource init failed"); > + return ret; > + } > + adapter->share_priv = dev->process_private; Isn't 'adapter' shared betwen primary and secondary ('port' is dev_private and 'port->adapt' is 'adapter'), so updating "adapter->share_priv" defeats the purpose of process_private, no? > + rnp_common_ops_init(adapter); > + rnp_get_nic_attr(adapter); > + /* We need Use Device Id To Change The Resource Mode */ > + rnp_special_ops_init(dev); > You can add this when it is needed, so in same patch we can see where it is called and how it is implemented, right now it is not possible that what 'special' init does. > + port->adapt = adapter; > + port->hw = hw; At this stage port is NULL, probably above needs to go to other patch that sets 'port'. > + rnp_init_mbx_ops_pf(hw); > for (p_id = 0; p_id < adapter->num_ports; p_id++) { > /* port 0 resource has been allocated When Probe */ > if (!p_id) { > @@ -158,11 +225,10 @@ rnp_eth_dev_init(struct rte_eth_dev *dev) > continue; > if (port->eth_dev) { > rnp_dev_close(port->eth_dev); > - rte_eth_dev_release_port(port->eth_dev); > if (port->eth_dev->process_private) > free(port->eth_dev->process_private); > + rte_eth_dev_release_port(port->eth_dev); > } > - rte_free(port); > } > rte_free(adapter); > > diff --git a/drivers/net/rnp/rnp_logs.h b/drivers/net/rnp/rnp_logs.h > index 1b3ee33745..f1648aabb5 100644 > --- a/drivers/net/rnp/rnp_logs.h > +++ b/drivers/net/rnp/rnp_logs.h > @@ -13,6 +13,15 @@ extern int rnp_drv_logtype; > #define RNP_PMD_DRV_LOG(level, fmt, args...) \ > rte_log(RTE_LOG_##level, rnp_drv_logtype, \ > "%s() " fmt, __func__, ##args) > +#define PMD_DRV_LOG_RAW(level, fmt, args...) \ > + rte_log(RTE_LOG_ ## level, rnp_drv_logtype, "%s(): " fmt, \ > + __func__, ## args) > +#define PMD_DRV_LOG(level, fmt, args...) \ > + PMD_DRV_LOG_RAW(level, fmt "\n", ## args) > + > +#define RNP_PMD_LOG(level, fmt, args...) \ > + rte_log(RTE_LOG_##level, rnp_drv_logtype, \ > + "rnp_net: (%d) " fmt, __LINE__, ##args) With these there are three different macros used to log in 'rnp_drv_logtype' type: RNP_PMD_LOG PMD_DRV_LOG RNP_PMD_DRV_LOG 'RNP_PMD_DRV_LOG' and 'PMD_DRV_LOG' looks exact same, do you need all these macros, why not stick to one? If more than one required, can you please comment what is used for what, and perhaps rename macros in a way that it is clear which one to use where (as I assume one is specific to base files). <...> > +/** > + * rnp_write_mbx_pf - Places a message in the mailbox > + * @hw: pointer to the HW structure > + * @msg: The message buffer > + * @size: Length of buffer > + * @mbx_id: the VF index > + * > + * returns SUCCESS if it successfully copied message into the buffer > + **/ > +static int32_t rnp_write_mbx_pf(struct rnp_hw *hw, u32 *msg, > + u16 size, enum MBX_ID mbx_id) > +{ > + u32 DATA_REG = (mbx_id == MBX_CM3CPU) ? > + CPU_PF_SHM_DATA : PF_VF_SHM_DATA(mbx_id); > + u32 CTRL_REG = (mbx_id == MBX_CM3CPU) ? > + PF2CPU_MBOX_CTRL : PF2VF_MBOX_CTRL(mbx_id); > + int32_t ret_val = 0; > + u32 stat __rte_unused; > + u16 i; > + > + if (size > RNP_VFMAILBOX_SIZE) { > + RNP_PMD_LOG(ERR, "%s: size:%d should <%d\n", __func__, > + size, RNP_VFMAILBOX_SIZE); > + return -EINVAL; > + } > + > + /* lock the mailbox to prevent pf/vf/cpu race condition */ > + ret_val = rnp_obtain_mbx_lock_pf(hw, mbx_id); > + if (ret_val) { > + RNP_PMD_LOG(WARNING, "PF[%d] Can't Get Mbx-Lock Try Again\n", > + hw->function); > + return ret_val; > + } > + > + /* copy the caller specified message to the mailbox memory buffer */ > + for (i = 0; i < size; i++) { > +#ifdef MBX_WR_DEBUG > + mbx_pwr32(hw, DATA_REG + i * 4, msg[i]); > +#else > + mbx_wr32(hw, DATA_REG + i * 4, msg[i]); > +#endif Who sets this define, if not used please remove it.