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 8049EA034C; Tue, 30 Aug 2022 09:09:14 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5800240F18; Tue, 30 Aug 2022 09:09:14 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 002E440F17 for ; Tue, 30 Aug 2022 09:09:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1661843351; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5npX1fnQK2oR2RE9B6n09yLck1PZ0evz9yaA7h0Cz3A=; b=SpHy5kSybf725FG3+C0SFHgqoxFf6EcMZLm26xWbsFkTSHXGrrui1qIGMDwfz67NPc+Aol AsyPxUSPwY9Q6w/jzf7V+waxms0dfhBpu7u2SoWYrzDQvXtJ41PzFw7JwRMFOU1mUn/2ac BmtzMM1lwJuCDwKsZ7AgXU3a4xK3L+U= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-616-ZoKnKvJvPmit90ATvRIcHQ-1; Tue, 30 Aug 2022 03:09:04 -0400 X-MC-Unique: ZoKnKvJvPmit90ATvRIcHQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 60C413C0D19A; Tue, 30 Aug 2022 07:09:03 +0000 (UTC) Received: from [10.39.208.27] (unknown [10.39.208.27]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 434241121314; Tue, 30 Aug 2022 07:09:01 +0000 (UTC) Message-ID: Date: Tue, 30 Aug 2022 09:08:59 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH v5 2/7] bbdev: add device status info To: "Chautru, Nicolas" , "dev@dpdk.org" , "thomas@monjalon.net" , "gakhil@marvell.com" , "hemant.agrawal@nxp.com" Cc: "trix@redhat.com" , "mdr@ashroe.eu" , "Richardson, Bruce" , "david.marchand@redhat.com" , "stephen@networkplumber.org" References: <1655491040-183649-6-git-send-email-nicolas.chautru@intel.com> <1657150110-69957-1-git-send-email-nicolas.chautru@intel.com> <1657150110-69957-3-git-send-email-nicolas.chautru@intel.com> <57ad0b38-4d48-e174-5b4a-a42425098901@redhat.com> <768c7c88-c697-8cd1-b256-dc43ce88ac90@redhat.com> From: Maxime Coquelin In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 8/29/22 18:10, Chautru, Nicolas wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin >> Sent: Friday, August 26, 2022 3:13 AM >> To: Chautru, Nicolas ; dev@dpdk.org; >> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com >> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce >> ; david.marchand@redhat.com; >> stephen@networkplumber.org >> Subject: Re: [PATCH v5 2/7] bbdev: add device status info >> >> Hi, >> >> On 8/25/22 20:30, Chautru, Nicolas wrote: >>> Thanks Maxime, >>> >>>> -----Original Message----- >>>> From: Maxime Coquelin >>>> Sent: Thursday, August 25, 2022 7:19 AM >>>> To: Chautru, Nicolas ; dev@dpdk.org; >>>> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com >>>> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce >>>> ; david.marchand@redhat.com; >>>> stephen@networkplumber.org >>>> Subject: Re: [PATCH v5 2/7] bbdev: add device status info >>>> >>>> >>>> >>>> On 7/7/22 01:28, Nicolas Chautru wrote: >>>>> Added device status information, so that the PMD can expose >>>>> information related to the underlying accelerator device status. >>>>> Minor order change in structure to fit into padding hole. >>>>> >>>>> Signed-off-by: Nicolas Chautru >>>>> --- >>>>> drivers/baseband/acc100/rte_acc100_pmd.c | 1 + >>>>> drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1 + >>>>> drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 1 + >>>>> drivers/baseband/la12xx/bbdev_la12xx.c | 1 + >>>>> drivers/baseband/null/bbdev_null.c | 1 + >>>>> drivers/baseband/turbo_sw/bbdev_turbo_software.c | 1 + >>>>> lib/bbdev/rte_bbdev.c | 22 ++++++++++++++ >>>>> lib/bbdev/rte_bbdev.h | 35 ++++++++++++++++++++-- >>>>> lib/bbdev/version.map | 6 ++++ >>>>> 9 files changed, 67 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c >>>>> b/drivers/baseband/acc100/rte_acc100_pmd.c >>>>> index de7e4bc..17ba798 100644 >>>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c >>>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c >>>>> @@ -1060,6 +1060,7 @@ >>>>> >>>>> /* Read and save the populated config from ACC100 registers */ >>>>> fetch_acc100_config(dev); >>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED; >>>>> >>>>> /* This isn't ideal because it reports the maximum number of >>>>> queues >>>> but >>>>> * does not provide info on how many can be uplink/downlink or >>>>> different diff --git >>>>> a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c >>>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c >>>>> index 82ae6ba..57b12af 100644 >>>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c >>>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c >>>>> @@ -369,6 +369,7 @@ >>>>> dev_info->capabilities = bbdev_capabilities; >>>>> dev_info->cpu_flag_reqs = NULL; >>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN; >>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED; >>>>> >>>>> /* Calculates number of queues assigned to device */ >>>>> dev_info->max_num_queues = 0; >>>>> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c >>>>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c >>>>> index 21d3529..2a330c4 100644 >>>>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c >>>>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c >>>>> @@ -645,6 +645,7 @@ struct __rte_cache_aligned fpga_queue { >>>>> dev_info->capabilities = bbdev_capabilities; >>>>> dev_info->cpu_flag_reqs = NULL; >>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN; >>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED; >>>>> >>>>> /* Calculates number of queues assigned to device */ >>>>> dev_info->max_num_queues = 0; >>>>> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c >>>>> b/drivers/baseband/la12xx/bbdev_la12xx.c >>>>> index 4d1bd16..c1f88c6 100644 >>>>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c >>>>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c >>>>> @@ -100,6 +100,7 @@ struct bbdev_la12xx_params { >>>>> dev_info->capabilities = bbdev_capabilities; >>>>> dev_info->cpu_flag_reqs = NULL; >>>>> dev_info->min_alignment = 64; >>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED; >>>>> >>>>> rte_bbdev_log_debug("got device info from %u", dev->data- >>>>> dev_id); >>>>> } >>>>> diff --git a/drivers/baseband/null/bbdev_null.c >>>>> b/drivers/baseband/null/bbdev_null.c >>>>> index 248e129..94a1976 100644 >>>>> --- a/drivers/baseband/null/bbdev_null.c >>>>> +++ b/drivers/baseband/null/bbdev_null.c >>>>> @@ -82,6 +82,7 @@ struct bbdev_queue { >>>>> * here for code completeness. >>>>> */ >>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN; >>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED; >>>>> >>>>> rte_bbdev_log_debug("got device info from %u", dev->data- >>>>> dev_id); >>>>> } >>>>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c >>>>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c >>>>> index af7bc41..dbc5524 100644 >>>>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c >>>>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c >>>>> @@ -254,6 +254,7 @@ struct turbo_sw_queue { >>>>> dev_info->min_alignment = 64; >>>>> dev_info->harq_buffer_size = 0; >>>>> dev_info->data_endianness = RTE_LITTLE_ENDIAN; >>>>> + dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED; >>>>> >>>>> rte_bbdev_log_debug("got device info from %u\n", dev->data- >>>>> dev_id); >>>>> } >>>>> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index >>>>> 4da8047..38630a2 100644 >>>>> --- a/lib/bbdev/rte_bbdev.c >>>>> +++ b/lib/bbdev/rte_bbdev.c >>>>> @@ -1133,3 +1133,25 @@ struct rte_mempool * >>>>> rte_bbdev_log(ERR, "Invalid operation type"); >>>>> return NULL; >>>>> } >>>>> + >>>>> +const char * >>>>> +rte_bbdev_device_status_str(enum rte_bbdev_device_status status) { >>>>> + static const char * const dev_sta_string[] = { >>>>> + "RTE_BBDEV_DEV_NOSTATUS", >>>>> + "RTE_BBDEV_DEV_NOT_SUPPORTED", >>>>> + "RTE_BBDEV_DEV_RESET", >>>>> + "RTE_BBDEV_DEV_CONFIGURED", >>>>> + "RTE_BBDEV_DEV_ACTIVE", >>>>> + "RTE_BBDEV_DEV_FATAL_ERR", >>>>> + "RTE_BBDEV_DEV_RESTART_REQ", >>>>> + "RTE_BBDEV_DEV_RECONFIG_REQ", >>>>> + "RTE_BBDEV_DEV_CORRECT_ERR", >>>>> + }; >>>>> + >>>>> + if (status < sizeof(dev_sta_string) / sizeof(char *)) >>>>> + return dev_sta_string[status]; >>>>> + >>>>> + rte_bbdev_log(ERR, "Invalid device status"); >>>>> + return NULL; >>>>> +} >>>>> diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index >>>>> b88c881..9b1ffa4 100644 >>>>> --- a/lib/bbdev/rte_bbdev.h >>>>> +++ b/lib/bbdev/rte_bbdev.h >>>>> @@ -223,6 +223,21 @@ struct rte_bbdev_queue_conf { >>>>> int >>>>> rte_bbdev_queue_stop(uint16_t dev_id, uint16_t queue_id); >>>>> >>>>> +/** >>>>> + * Flags indicate the status of the device */ enum >>>>> +rte_bbdev_device_status { >>>>> + RTE_BBDEV_DEV_NOSTATUS, /**< Nothing being reported */ >>>>> + RTE_BBDEV_DEV_NOT_SUPPORTED, /**< Device status is not >>>> supported on the PMD */ >>>>> + RTE_BBDEV_DEV_RESET, /**< Device in reset and un- >>>> configured state */ >>>>> + RTE_BBDEV_DEV_CONFIGURED, /**< Device is configured and >>>> ready to use */ >>>>> + RTE_BBDEV_DEV_ACTIVE, /**< Device is configured and VF is >>>> being used */ >>>>> + RTE_BBDEV_DEV_FATAL_ERR, /**< Device has hit a fatal >>>> uncorrectable error */ >>>>> + RTE_BBDEV_DEV_RESTART_REQ, /**< Device requires application >>>> to restart */ >>>>> + RTE_BBDEV_DEV_RECONFIG_REQ, /**< Device requires >>>> application to reconfigure queues */ >>>>> + RTE_BBDEV_DEV_CORRECT_ERR, /**< Warning of a correctable >>>> error event happened */ >>>>> +}; >>>> >>>> I don't have a strong opinion on this, but I think NOT_SUPPORTED >>>> should be a special value. If you want to keep 0 value for NOSTATUS, >>>> maybe you could >>>> do: >>>> >>>> enum rte_bbdev_device_status { >>>> RTE_BBDEV_DEV_NOT_SUPPORTED = -1, /**< Device status is not >>>> supported >>>> on the PMD */ >>>> RTE_BBDEV_DEV_NOSTATUS = 0, /**< Nothing being reported >>>> */ >>>> RTE_BBDEV_DEV_RESET, /**< Device in reset and un- >>>> configured >>>> state */ >>>> ... >>> >>> Thanks Maxime. My concern is that I am upstreaming in parallel in >> pf_bb_config in parallel hence would like to keep it unchanged if possible. >>> Given you don’t have a strong opinion is that okay to keep as is? Or I can >> force special value 1 for NOT_SUPPORTED so that this is explicitly defined. But >> really enum should always be used. >> >> I don't understand. It should not have any impact on pf_bb_config, given >> pf_bb_config does not use DPDK. >> >> Maxime > > That device status is being shared from pf_bb_config to the bbdev PMD through PF2VF communications, hence they share that same enum. > Ok, but generic DPDK ABI should not be dependent on a vendor internal implementation IMHO. Maxime