From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 092E9C433EF for ; Thu, 9 Sep 2021 21:05:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E9644611B0 for ; Thu, 9 Sep 2021 21:05:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347686AbhIIVHE (ORCPT ); Thu, 9 Sep 2021 17:07:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51042 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347899AbhIIVGp (ORCPT ); Thu, 9 Sep 2021 17:06:45 -0400 Received: from mail-pg1-x529.google.com (mail-pg1-x529.google.com [IPv6:2607:f8b0:4864:20::529]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EE73BC061574 for ; Thu, 9 Sep 2021 14:05:35 -0700 (PDT) Received: by mail-pg1-x529.google.com with SMTP id t1so3134161pgv.3 for ; Thu, 09 Sep 2021 14:05:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=r9nTFihajPT2Iqk8ZopYtdl8lBrpPnI8ljufyAKr/Pg=; b=WJP/qryO/ykQpdkF715lk6SUgRWDnPXqdE5cUrfLrpAiOTFlGFe6o7ivyT5hFmgFqr jj+cwVygT00VOkUeLdWCbOxfWgz5YybNumnmU6QD1/bMF5UQJgvFiQ2RCfqU2ssunav5 drD6mxR3/cVRmAeq1PPY9ivp9Yym/KJGyzDeLk+ZJi5x0aZ+8xDOBiZwcqWpyE2T9DUU GhmNfXuZNsbBSzXVH7HSA7mfbHQnD65db9yGDMv3plNfEzi2YtHFZKmhkY2l28dRkVJW q3JlI+pF6M1FjvHyfuDHcryYlotp0+3bU1cWxp2QdvMKOoV8VXz+Ae34+VKbb9+S9TY8 JtKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=r9nTFihajPT2Iqk8ZopYtdl8lBrpPnI8ljufyAKr/Pg=; b=a2Qu4xoSvh16Ek//qumFUA8StJAI9aW2ZmhCY2unm4LSH8hcct0M62+PctRnfi6Xhp KI08rmSzOZJm+zB/exRudE/hyMgw/1q5AibkUIPcxunv7fZqAtGUmaLHpv8fm27Lik41 meDKP8BdFdOJgd/NGsOfgjfT0O6KpvHAtNjE00rUaq8w249aWjGcVY2neN5kCDX0bqeg ZMuoeC3y6tsa/+NhYxst0i+qdv2Tu/4ChjUChzUtH53dEXxeA5s/bks5We/eIFMVmmlK gw1DDTAQ2ACBo0hLjrOfMCjK7XaZkRiYbYZk75qcvrVv3v9vSojIe97Bz8q8VMj6UKGU lKnQ== X-Gm-Message-State: AOAM533Q4wH9952btsPrqx2OqSzcpXUDcCMKcBkz1j4UznLcEfLcFpLg vgZ1oPsaLqWaUeARNxpXVYqQcPiUNlLck6WZHHDzHA== X-Google-Smtp-Source: ABdhPJy5t00AiPBsUbAmSCDIvNoWFWeQLItHqldUuEFfYjgZjINCpyLTrOzu9xzyoVljKWz6lbDFpTMxpZh770ctrdQ= X-Received: by 2002:a62:1b92:0:b0:3eb:3f92:724 with SMTP id b140-20020a621b92000000b003eb3f920724mr4778204pfb.3.1631221535444; Thu, 09 Sep 2021 14:05:35 -0700 (PDT) MIME-Version: 1.0 References: <163116429183.2460985.5040982981112374615.stgit@dwillia2-desk3.amr.corp.intel.com> <163116435233.2460985.16197340449713287180.stgit@dwillia2-desk3.amr.corp.intel.com> <20210909164125.ttvptq6eiiirvnnp@intel.com> <20210909203528.frq547zd26efumpz@intel.com> In-Reply-To: <20210909203528.frq547zd26efumpz@intel.com> From: Dan Williams Date: Thu, 9 Sep 2021 14:05:24 -0700 Message-ID: Subject: Re: [PATCH v4 11/21] cxl/mbox: Move mailbox and other non-PCI specific infrastructure to the core To: Ben Widawsky Cc: linux-cxl@vger.kernel.org, kernel test robot , Vishal L Verma , Linux NVDIMM , "Schofield, Alison" , "Weiny, Ira" , Jonathan Cameron Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-cxl@vger.kernel.org On Thu, Sep 9, 2021 at 1:35 PM Ben Widawsky wrote: > > On 21-09-09 11:50:01, Dan Williams wrote: > > On Thu, Sep 9, 2021 at 9:41 AM Ben Widawsky wrote: > > > > > > On 21-09-08 22:12:32, Dan Williams wrote: > > > > Now that the internals of mailbox operations are abstracted from the PCI > > > > specifics a bulk of infrastructure can move to the core. > > > > > > > > The CXL_PMEM driver intends to proxy LIBNVDIMM UAPI and driver requests > > > > to the equivalent functionality provided by the CXL hardware mailbox > > > > interface. In support of that intent move the mailbox implementation to > > > > a shared location for the CXL_PCI driver native IOCTL path and CXL_PMEM > > > > nvdimm command proxy path to share. > > > > > > > > A unit test framework seeks to implement a unit test backend transport > > > > for mailbox commands to communicate mocked up payloads. It can reuse all > > > > of the mailbox infrastructure minus the PCI specifics, so that also gets > > > > moved to the core. > > > > > > > > Finally with the mailbox infrastructure and ioctl handling being > > > > transport generic there is no longer any need to pass file > > > > file_operations to devm_cxl_add_memdev(). That allows all the ioctl > > > > boilerplate to move into the core for unit test reuse. > > > > > > > > No functional change intended, just code movement. > > > > > > At some point, I think some of the comments and kernel docs need updating since > > > the target is no longer exclusively memory devices. Perhaps you do this in later > > > patches.... > > > > I would wait to rework comments when/if it becomes clear that a > > non-memory-device driver wants to reuse the mailbox core. I do not see > > any indications that the comments are currently broken, do you? > > I didn't see anything which is incorrect, no. But to would be non-memory-driver > writers, they could be scared off by such comments... I don't mean that it > should hold this patch up btw. Ok, we can cross that bridge when it comes to it. I don't expect someone to reinvent mailbox command infrastructure especially when the the changelog for the commit creating drivers/cxl/core/mbox.c specifically mentions disconnecting the mailbox core from the cxl_pci driver. > > > > > [..] > > > > diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h > > > > index 036a3c8106b4..c85b7fbad02d 100644 > > > > --- a/drivers/cxl/core/core.h > > > > +++ b/drivers/cxl/core/core.h > > > > @@ -14,7 +14,15 @@ static inline void unregister_cxl_dev(void *dev) > > > > device_unregister(dev); > > > > } > > > > > > > > +struct cxl_send_command; > > > > +struct cxl_mem_query_commands; > > > > +int cxl_query_cmd(struct cxl_memdev *cxlmd, > > > > + struct cxl_mem_query_commands __user *q); > > > > +int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s); > > > > + > > > > int cxl_memdev_init(void); > > > > void cxl_memdev_exit(void); > > > > +void cxl_mbox_init(void); > > > > +void cxl_mbox_exit(void); > > > > > > cxl_mbox_fini()? > > > > The idiomatic kernel module shutdown function is suffixed _exit(). > > > > [..] > > Got it, I argue that these aren't kernel module init/exit functions though. I > will leave it at that. I would expect a _fini() function to destruct a single object, not a void _exit() that is tearing down global static infrastructure related to a compilation unit.