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 80540A0A0A; Thu, 3 Jun 2021 09:30:21 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 65B3040689; Thu, 3 Jun 2021 09:30:21 +0200 (CEST) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by mails.dpdk.org (Postfix) with ESMTP id EF3364067A for ; Thu, 3 Jun 2021 09:30:20 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id 7E6DF5C0053; Thu, 3 Jun 2021 03:30:20 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Thu, 03 Jun 2021 03:30:20 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= E8Y3mmiDcd4YNX9AeHcuJAJQ+3CDJBgGdZsfDmDNVYY=; b=vEnBGc6RsnWkBgUx 4P3G28tXNMOwVRFXIWgFu44amE6hmsns1Yg3sqBmGGd8nxrw6FXvxMKdoA/JRInW i8VXEnB/St+peJx33kWgoXjnWm1NtAdqIAI0tfvb2bzF4B0a21zTh2QtkcSOjBYc xEDChZa9xZMspsQUSJ/AiCGDSYTWddAAwRpmAySmZqDH+00zXMHFf4VER2s7Iu+n uIsIrqximrRQLffaBK2gN8/7J7u0H+abwbLf38H47xSWxtyMndjVbuZUaUw08Uo0 yYZx2GuqGgyn0kCHzM5Rxi/J0Ko+bHKbyzy3ruZpEhNDsw1frKEl+v+5ahVGZxJA O0zZBQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=E8Y3mmiDcd4YNX9AeHcuJAJQ+3CDJBgGdZsfDmDNV YY=; b=lyaix4AdSihz1FibYRfIND/Jn99LWy2NDX5+l8oxZZH5+x0n0W3QR41ZL ArYQLw9MtK4L9qn9/H5QkrJAlmgPAOE8mUJ6Nwlb2Kwhf7cH3dnuoweYPVTyKJtw xlGPPeyFkqwDkz3AGllx3wkl/0zCcQrBT1YG8AA4nudC46/cmi1GD1uj5JxbdlWj b7TMy9X64gWQKBbiorXNhFzlxmzCtKXWMJ/ICfuqJOUC2F1nThZXcAEZQhNpXzCo dKF+cpy274SYsTXKTpikkVQPJGyO8On6g3nuCAANFwkiTwdSv/pZVLUQdQFQIG3n w/1siXhxAkLKchtsH7IFWY69M8lJQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvdelkedguddulecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdej ueeiiedvffegheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 3 Jun 2021 03:30:19 -0400 (EDT) From: Thomas Monjalon To: David Marchand Cc: dev , Elena Agostini Date: Thu, 03 Jun 2021 09:30:17 +0200 Message-ID: <2771373.JYJR53bpLu@thomas> In-Reply-To: References: <20210602203531.2288645-1-thomas@monjalon.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] gpudev: introduce memory API 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 Sender: "dev" 03/06/2021 09:18, David Marchand: > Quick pass: > > On Wed, Jun 2, 2021 at 10:36 PM Thomas Monjalon wrote: > > diff --git a/lib/gpudev/gpu_driver.h b/lib/gpudev/gpu_driver.h > > new file mode 100644 > > index 0000000000..5ff609e49d > > --- /dev/null > > +++ b/lib/gpudev/gpu_driver.h [...] > > +struct rte_gpu_dev; > > + > > +typedef int (*gpu_malloc_t)(struct rte_gpu_dev *dev, size_t size, void **ptr); > > +typedef int (*gpu_free_t)(struct rte_gpu_dev *dev, void *ptr); > > + > > Great to see this structure hidden in a driver-only header. > > > > +struct rte_gpu_dev { > > We could have a name[] field here, that will be later pointed at, in > rte_gpu_info. > Who is responsible for deciding of the device name? The driver is responsible for the name of the device. Yes I agree to store the name here. > > + /* Backing device. */ > > + struct rte_device *device; > > + /* GPU info structure. */ > > + struct rte_gpu_info info; > > + /* Counter of processes using the device. */ > > + uint16_t process_cnt; > > + /* If device is currently used or not. */ > > + enum rte_gpu_state state; > > + /* FUNCTION: Allocate memory on the GPU. */ > > + gpu_malloc_t gpu_malloc; > > + /* FUNCTION: Allocate memory on the CPU visible from the GPU. */ > > + gpu_malloc_t gpu_malloc_visible; > > + /* FUNCTION: Free allocated memory on the GPU. */ > > + gpu_free_t gpu_free; > > + /* Device interrupt handle. */ > > + struct rte_intr_handle *intr_handle; > > + /* Driver-specific private data. */ > > + void *dev_private; > > +} __rte_cache_aligned; > > + > > +struct rte_gpu_dev *rte_gpu_dev_allocate(const char *name); > > +struct rte_gpu_dev *rte_gpu_dev_get_by_name(const char *name); > > Those symbols will have to be marked internal (__rte_internal + > version.map) for drivers to see them. OK, good catch. [...] > > +/** Maximum number of GPU engines. */ > > +#define RTE_GPU_MAX_DEVS UINT16_C(32) > > Bleh. > Let's stop with max values. > The iterator _next should return a special value indicating there is > no more devs to list. I fully agree. I would like to define gpu_id as an int to simplify comparisons with error value. int or int16_t ? > > +/** Maximum length of device name. */ > > +#define RTE_GPU_NAME_MAX_LEN 128 Will be dropped as well. > > + > > +/** Flags indicate current state of GPU device. */ > > +enum rte_gpu_state { > > + RTE_GPU_STATE_UNUSED, /**< not initialized */ > > + RTE_GPU_STATE_INITIALIZED, /**< initialized */ > > +}; > > + > > +/** Store a list of info for a given GPU. */ > > +struct rte_gpu_info { > > + /** GPU device ID. */ > > + uint16_t gpu_id; > > + /** Unique identifier name. */ > > + char name[RTE_GPU_NAME_MAX_LEN]; > > const char *name; > > Then the gpu generic layer simply fills this with the > rte_gpu_dev->name field I proposed above. Yes. > > + /** Total memory available on device. */ > > + size_t total_memory; > > + /** Total processors available on device. */ > > + int processor_count; > > +};