I agree. Other adminq commands like gve_adminq_report_link_speed use gve_alloc_dma_mem and gve_free_dma_mem (which use rte_memzone_reserve_aligned under the hood) so let's stick to that.
On Fri, May 19, 2023 at 3:04 AM Ferruh Yigit <ferruh.yi...@amd.com> wrote: > > On 5/19/2023 8:41 AM, Rushil Gupta wrote: > > Hi Ferruh > > I have updated the latest patch here: > > http://patchwork.dpdk.org/project/dpdk/patch/20230519072600.1444309-1-rush...@google.com/ > > However, using calloc causes issue while executing verify-compatibility > > command. > > > > sudo dpdk-testpmd -a 00:04.0 -l 0-32 -- --forward-mode=rxonly --txq=16 > > --rxq=16 --nb-cores=16 --stats-period 5 > > ... > > EAL: Using IOMMU type 8 (No-IOMMU) > > EAL: Probe PCI driver: net_gve (1ae0:42) device: 0000:00:04.0 (socket -1) > > gve_adminq_parse_err(): AQ command failed with status -9 > > gve_init_priv(): Could not verify driver compatibility: err=-22 > > EAL: Releasing PCI mapped resource for 0000:00:04.0 > > ... > > EAL: Error - exiting with code: 1 > > > > > > This is probably because the adminq command is sharing memory to > > report driver-info to the gvnic device and that needs to be in dpdk > > memory. > > > > > > Hi Rushil, > > That is OK, I missed this requirement. > > Admin command passes pointers for device to process, what is the address > type requirement here? > Some other commands pass pysical address (iova address) via the command. > > Both 'calloc()' and 'rte_zmalloc()' returns (guest) virtual address, why > one doesn't work but other does? > > Initial version was passing 'rte_memzone' pointer, are you updating the > hyperviser side based on changes on dpdk side? > > > And perhaps better to switch to 'rte_memzone_reserve_aligned()' as done > initial version and pass (guest) pysical address via adminq, if that is > the requirement? > > > > > > On Fri, May 19, 2023 at 12:26 AM Rushil Gupta <rush...@google.com> wrote: > >> > >> Change gve_driver_info fields to report DPDK as OS type and DPDK RTE > >> version as OS version, reserving driver_version fields for GVE driver > >> version based on features. > >> > >> Signed-off-by: Rushil Gupta <rush...@google.com> > >> Signed-off-by: Joshua Washington <joshw...@google.com> > >> Signed-off-by: Junfeng Guo <junfeng....@intel.com> > >> Signed-off-by: Jeroen de Borst <jeroe...@google.com> > >> --- > >> drivers/net/gve/base/gve.h | 3 -- > >> drivers/net/gve/base/gve_adminq.c | 19 ++++++++++ > >> drivers/net/gve/base/gve_adminq.h | 46 ++++++++++++++++++++++- > >> drivers/net/gve/base/gve_osdep.h | 24 ++++++++++++ > >> drivers/net/gve/gve_ethdev.c | 61 +++++++++++++++++++++++++------ > >> drivers/net/gve/gve_ethdev.h | 2 +- > >> drivers/net/gve/gve_version.c | 13 +++++++ > >> drivers/net/gve/gve_version.h | 24 ++++++++++++ > >> drivers/net/gve/meson.build | 5 ++- > >> 9 files changed, 179 insertions(+), 18 deletions(-) > >> create mode 100644 drivers/net/gve/gve_version.c > >> create mode 100644 drivers/net/gve/gve_version.h > >> > >> diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h > >> index 2b7cf7d99b..f7b297e759 100644 > >> --- a/drivers/net/gve/base/gve.h > >> +++ b/drivers/net/gve/base/gve.h > >> @@ -9,9 +9,6 @@ > >> #include "gve_desc.h" > >> #include "gve_desc_dqo.h" > >> > >> -#define GVE_VERSION "1.3.0" > >> -#define GVE_VERSION_PREFIX "GVE-" > >> - > >> #ifndef GOOGLE_VENDOR_ID > >> #define GOOGLE_VENDOR_ID 0x1ae0 > >> #endif > >> diff --git a/drivers/net/gve/base/gve_adminq.c > >> b/drivers/net/gve/base/gve_adminq.c > >> index e963f910a0..41202725e6 100644 > >> --- a/drivers/net/gve/base/gve_adminq.c > >> +++ b/drivers/net/gve/base/gve_adminq.c > >> @@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv, > >> case GVE_ADMINQ_GET_PTYPE_MAP: > >> priv->adminq_get_ptype_map_cnt++; > >> break; > >> + case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY: > >> + priv->adminq_verify_driver_compatibility_cnt++; > >> + break; > >> default: > >> PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode); > >> } > >> @@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct > >> gve_priv *priv, > >> return gve_adminq_execute_cmd(priv, &cmd); > >> } > >> > >> +int gve_adminq_verify_driver_compatibility(struct gve_priv *priv, > >> + u64 driver_info_len, > >> + dma_addr_t driver_info_addr) > >> +{ > >> + union gve_adminq_command cmd; > >> + > >> + memset(&cmd, 0, sizeof(cmd)); > >> + cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY); > >> + cmd.verify_driver_compatibility = (struct > >> gve_adminq_verify_driver_compatibility) { > >> + .driver_info_len = cpu_to_be64(driver_info_len), > >> + .driver_info_addr = cpu_to_be64(driver_info_addr), > >> + }; > >> + > >> + return gve_adminq_execute_cmd(priv, &cmd); > >> +} > >> + > >> int gve_adminq_deconfigure_device_resources(struct gve_priv *priv) > >> { > >> union gve_adminq_command cmd; > >> diff --git a/drivers/net/gve/base/gve_adminq.h > >> b/drivers/net/gve/base/gve_adminq.h > >> index 05550119de..e30b184913 100644 > >> --- a/drivers/net/gve/base/gve_adminq.h > >> +++ b/drivers/net/gve/base/gve_adminq.h > >> @@ -1,6 +1,6 @@ > >> /* SPDX-License-Identifier: MIT > >> * Google Virtual Ethernet (gve) driver > >> - * Copyright (C) 2015-2022 Google, Inc. > >> + * Copyright (C) 2015-2023 Google, Inc. > >> */ > >> > >> #ifndef _GVE_ADMINQ_H > >> @@ -23,6 +23,7 @@ enum gve_adminq_opcodes { > >> GVE_ADMINQ_REPORT_STATS = 0xC, > >> GVE_ADMINQ_REPORT_LINK_SPEED = 0xD, > >> GVE_ADMINQ_GET_PTYPE_MAP = 0xE, > >> + GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY = 0xF, > >> }; > >> > >> /* Admin queue status codes */ > >> @@ -145,6 +146,44 @@ enum gve_sup_feature_mask { > >> }; > >> > >> #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0 > >> +enum gve_driver_capbility { > >> + gve_driver_capability_gqi_qpl = 0, > >> + gve_driver_capability_gqi_rda = 1, > >> + gve_driver_capability_dqo_qpl = 2, /* reserved for future use */ > >> + gve_driver_capability_dqo_rda = 3, > >> +}; > >> + > >> +#define GVE_CAP1(a) BIT((int)a) > >> + > >> +#define GVE_DRIVER_CAPABILITY_FLAGS1 \ > >> + (GVE_CAP1(gve_driver_capability_gqi_qpl) | \ > >> + GVE_CAP1(gve_driver_capability_gqi_rda) | \ > >> + GVE_CAP1(gve_driver_capability_dqo_rda)) > >> + > >> +#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0 > >> +#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0 > >> +#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0 > >> + > >> +struct gve_driver_info { > >> + u8 os_type; /* 0x05 = DPDK */ > >> + u8 driver_major; > >> + u8 driver_minor; > >> + u8 driver_sub; > >> + __be32 os_version_major; > >> + __be32 os_version_minor; > >> + __be32 os_version_sub; > >> + __be64 driver_capability_flags[4]; > >> + u8 os_version_str1[OS_VERSION_STRLEN]; > >> + u8 os_version_str2[OS_VERSION_STRLEN]; > >> +}; > >> + > >> +struct gve_adminq_verify_driver_compatibility { > >> + __be64 driver_info_len; > >> + __be64 driver_info_addr; > >> +}; > >> + > >> +GVE_CHECK_STRUCT_LEN(16, gve_adminq_verify_driver_compatibility); > >> + > >> > >> struct gve_adminq_configure_device_resources { > >> __be64 counter_array; > >> @@ -345,6 +384,8 @@ union gve_adminq_command { > >> struct gve_adminq_report_stats report_stats; > >> struct gve_adminq_report_link_speed > >> report_link_speed; > >> struct gve_adminq_get_ptype_map get_ptype_map; > >> + struct gve_adminq_verify_driver_compatibility > >> + verify_driver_compatibility; > >> }; > >> }; > >> u8 reserved[64]; > >> @@ -378,4 +419,7 @@ struct gve_ptype_lut; > >> int gve_adminq_get_ptype_map_dqo(struct gve_priv *priv, > >> struct gve_ptype_lut *ptype_lut); > >> > >> +int gve_adminq_verify_driver_compatibility(struct gve_priv *priv, > >> + u64 driver_info_len, > >> + dma_addr_t driver_info_addr); > >> #endif /* _GVE_ADMINQ_H */ > >> diff --git a/drivers/net/gve/base/gve_osdep.h > >> b/drivers/net/gve/base/gve_osdep.h > >> index abf3d379ae..5e8ae1eac6 100644 > >> --- a/drivers/net/gve/base/gve_osdep.h > >> +++ b/drivers/net/gve/base/gve_osdep.h > >> @@ -21,9 +21,14 @@ > >> #include <rte_malloc.h> > >> #include <rte_memcpy.h> > >> #include <rte_memzone.h> > >> +#include <rte_version.h> > >> > >> #include "../gve_logs.h" > >> > >> +#ifdef __linux__ > >> +#include <sys/utsname.h> > >> +#endif > >> + > >> typedef uint8_t u8; > >> typedef uint16_t u16; > >> typedef uint32_t u32; > >> @@ -73,6 +78,12 @@ typedef rte_iova_t dma_addr_t; > >> > >> #define msleep(ms) rte_delay_ms(ms) > >> > >> +#define OS_VERSION_STRLEN 128 > >> +struct os_version_string { > >> + char os_version_str1[OS_VERSION_STRLEN]; > >> + char os_version_str2[OS_VERSION_STRLEN]; > >> +}; > >> + > >> /* These macros are used to generate compilation errors if a struct/union > >> * is not exactly the correct length. It gives a divide by zero error if > >> * the struct/union is not of the correct size, otherwise it creates an > >> @@ -160,4 +171,17 @@ gve_free_dma_mem(struct gve_dma_mem *mem) > >> mem->pa = 0; > >> } > >> > >> +static inline void > >> +populate_driver_version_strings(char *str1, char *str2) > >> +{ > >> + struct utsname uts; > >> + if (uname(&uts) >= 0) { > >> + /* release */ > >> + rte_strscpy(str1, uts.release, > >> + OS_VERSION_STRLEN); > >> + /* version */ > >> + rte_strscpy(str2, uts.version, > >> + OS_VERSION_STRLEN); > >> + } > >> +} > >> #endif /* _GVE_OSDEP_H_ */ > >> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c > >> index 1dcb3b3a01..71d131fb41 100644 > >> --- a/drivers/net/gve/gve_ethdev.c > >> +++ b/drivers/net/gve/gve_ethdev.c > >> @@ -5,21 +5,13 @@ > >> #include "gve_ethdev.h" > >> #include "base/gve_adminq.h" > >> #include "base/gve_register.h" > >> - > >> -const char gve_version_str[] = GVE_VERSION; > >> -static const char gve_version_prefix[] = GVE_VERSION_PREFIX; > >> +#include "base/gve_osdep.h" > >> +#include "gve_version.h" > >> > >> static void > >> gve_write_version(uint8_t *driver_version_register) > >> { > >> - const char *c = gve_version_prefix; > >> - > >> - while (*c) { > >> - writeb(*c, driver_version_register); > >> - c++; > >> - } > >> - > >> - c = gve_version_str; > >> + const char *c = gve_version_string(); > >> while (*c) { > >> writeb(*c, driver_version_register); > >> c++; > >> @@ -245,6 +237,48 @@ gve_dev_close(struct rte_eth_dev *dev) > >> return err; > >> } > >> > >> +static int > >> +gve_verify_driver_compatibility(struct gve_priv *priv) > >> +{ > >> + struct gve_driver_info *driver_info; > >> + int err; > >> + > >> + driver_info = rte_zmalloc("driver info", > >> + sizeof(struct gve_driver_info), 0); > >> + if (driver_info == NULL) { > >> + PMD_DRV_LOG(ERR, > >> + "Could not alloc for verify driver > >> compatibility"); > >> + return -ENOMEM; > >> + } > >> + *driver_info = (struct gve_driver_info) { > >> + .os_type = 5, /* DPDK */ > >> + .driver_major = GVE_VERSION_MAJOR, > >> + .driver_minor = GVE_VERSION_MINOR, > >> + .driver_sub = GVE_VERSION_SUB, > >> + .os_version_major = cpu_to_be32(DPDK_VERSION_MAJOR), > >> + .os_version_minor = cpu_to_be32(DPDK_VERSION_MINOR), > >> + .os_version_sub = cpu_to_be32(DPDK_VERSION_SUB), > >> + .driver_capability_flags = { > >> + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS1), > >> + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS2), > >> + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS3), > >> + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS4), > >> + }, > >> + }; > >> + > >> + populate_driver_version_strings((char > >> *)driver_info->os_version_str1, > >> + (char *)driver_info->os_version_str2); > >> + > >> + err = gve_adminq_verify_driver_compatibility(priv, > >> + sizeof(struct gve_driver_info), (dma_addr_t)driver_info); > >> + /* It's ok if the device doesn't support this */ > >> + if (err == -EOPNOTSUPP) > >> + err = 0; > >> + > >> + rte_free(driver_info); > >> + return err; > >> +} > >> + > >> static int > >> gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info > >> *dev_info) > >> { > >> @@ -679,6 +713,11 @@ gve_init_priv(struct gve_priv *priv, bool > >> skip_describe_device) > >> PMD_DRV_LOG(ERR, "Failed to alloc admin queue: err=%d", > >> err); > >> return err; > >> } > >> + err = gve_verify_driver_compatibility(priv); > >> + if (err) { > >> + PMD_DRV_LOG(ERR, "Could not verify driver compatibility: > >> err=%d", err); > >> + goto free_adminq; > >> + } > >> > >> if (skip_describe_device) > >> goto setup_device; > >> diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h > >> index cd62debd22..c9bcfa553c 100644 > >> --- a/drivers/net/gve/gve_ethdev.h > >> +++ b/drivers/net/gve/gve_ethdev.h > >> @@ -268,7 +268,7 @@ struct gve_priv { > >> uint32_t adminq_report_stats_cnt; > >> uint32_t adminq_report_link_speed_cnt; > >> uint32_t adminq_get_ptype_map_cnt; > >> - > >> + uint32_t adminq_verify_driver_compatibility_cnt; > >> volatile uint32_t state_flags; > >> > >> /* Gvnic device link speed from hypervisor. */ > >> diff --git a/drivers/net/gve/gve_version.c b/drivers/net/gve/gve_version.c > >> new file mode 100644 > >> index 0000000000..5fe34dc179 > >> --- /dev/null > >> +++ b/drivers/net/gve/gve_version.c > >> @@ -0,0 +1,13 @@ > >> +/* SPDX-License-Identifier: BSD-3-Clause > >> + * Copyright (C) 2015-2023 Google, Inc. > >> + */ > >> +#include "gve_version.h" > >> + > >> +const char *gve_version_string(void) > >> +{ > >> + static char gve_version[20]; > >> + snprintf(gve_version, sizeof(gve_version), "%s%d.%d.%d", > >> + GVE_VERSION_PREFIX, GVE_VERSION_MAJOR, GVE_VERSION_MINOR, > >> + GVE_VERSION_SUB); > >> + return gve_version; > >> +} > >> diff --git a/drivers/net/gve/gve_version.h b/drivers/net/gve/gve_version.h > >> new file mode 100644 > >> index 0000000000..4dd998dca1 > >> --- /dev/null > >> +++ b/drivers/net/gve/gve_version.h > >> @@ -0,0 +1,24 @@ > >> +/* SPDX-License-Identifier: BSD-3-Clause > >> + * Copyright (C) 2015-2023 Google, Inc. > >> + */ > >> + > >> +#ifndef _GVE_VERSION_H_ > >> +#define _GVE_VERSION_H_ > >> + > >> +#include <rte_version.h> > >> + > >> +#define GVE_VERSION_PREFIX "DPDK-" > >> +#define GVE_VERSION_MAJOR 1 > >> +#define GVE_VERSION_MINOR 0 > >> +#define GVE_VERSION_SUB 0 > >> + > >> +#define DPDK_VERSION_MAJOR (100 * RTE_VER_YEAR + RTE_VER_MONTH) > >> +#define DPDK_VERSION_MINOR RTE_VER_MINOR > >> +#define DPDK_VERSION_SUB RTE_VER_RELEASE > >> + > >> + > >> +const char * > >> +gve_version_string(void); > >> + > >> + > >> +#endif /* GVE_VERSION_H */ > >> diff --git a/drivers/net/gve/meson.build b/drivers/net/gve/meson.build > >> index c9d87903f9..61d195009c 100644 > >> --- a/drivers/net/gve/meson.build > >> +++ b/drivers/net/gve/meson.build > >> @@ -1,9 +1,9 @@ > >> # SPDX-License-Identifier: BSD-3-Clause > >> # Copyright(C) 2022 Intel Corporation > >> > >> -if is_windows > >> +if not is_linux > >> build = false > >> - reason = 'not supported on Windows' > >> + reason = 'only supported on Linux' > >> subdir_done() > >> endif > >> > >> @@ -14,5 +14,6 @@ sources = files( > >> 'gve_rx_dqo.c', > >> 'gve_tx_dqo.c', > >> 'gve_ethdev.c', > >> + 'gve_version.c', > >> ) > >> includes += include_directories('base') > >> -- > >> 2.40.1.698.g37aff9b760-goog > >> >
