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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D80D5C433F5 for ; Thu, 4 Nov 2021 21:25:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B13C16124D for ; Thu, 4 Nov 2021 21:25:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230162AbhKDV1t (ORCPT ); Thu, 4 Nov 2021 17:27:49 -0400 Received: from mga06.intel.com ([134.134.136.31]:16842 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229924AbhKDV1t (ORCPT ); Thu, 4 Nov 2021 17:27:49 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10158"; a="292632542" X-IronPort-AV: E=Sophos;i="5.87,209,1631602800"; d="scan'208";a="292632542" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Nov 2021 14:25:10 -0700 X-IronPort-AV: E=Sophos;i="5.87,209,1631602800"; d="scan'208";a="450583248" Received: from wangli6-mobl.amr.corp.intel.com (HELO intel.com) ([10.252.138.130]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Nov 2021 14:25:10 -0700 Date: Thu, 4 Nov 2021 14:25:09 -0700 From: Ben Widawsky To: Dan Williams Cc: linux-cxl@vger.kernel.org, Chet Douglas , Alison Schofield , Ira Weiny , Jonathan Cameron , Vishal Verma Subject: Re: [RFC PATCH v2 08/28] cxl/port: Introduce a port driver Message-ID: <20211104212509.gqefhq6jbwq2a4bn@intel.com> References: <20211022183709.1199701-1-ben.widawsky@intel.com> <20211022183709.1199701-9-ben.widawsky@intel.com> <20211101175314.lrq3ccqkts725bjt@intel.com> <20211102165801.6mxobbz3ow55nkzl@intel.com> <20211104194906.6du3jtcpkxeumhsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-cxl@vger.kernel.org On 21-11-04 13:04:13, Dan Williams wrote: > On Thu, Nov 4, 2021 at 12:49 PM Ben Widawsky wrote: > > > > On 21-11-04 12:10:09, Dan Williams wrote: > > > On Tue, Nov 2, 2021 at 9:58 AM Ben Widawsky wrote: > > > > > > > > On 21-11-01 20:31:03, Dan Williams wrote: > > > > > On Mon, Nov 1, 2021 at 10:53 AM Ben Widawsky wrote: > > > > > > > > > > > > On 21-10-29 18:37:36, Dan Williams wrote: > > > > > > > On Fri, Oct 22, 2021 at 11:37 AM Ben Widawsky wrote: > > > > > > > > > > > > > > > > > > > > [snip] > > > > > > > > > > > > > > diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile > > > > > > > > index cf07ae6cea17..40b386aaedf7 100644 > > > > > > > > --- a/drivers/cxl/Makefile > > > > > > > > +++ b/drivers/cxl/Makefile > > > > > > > > @@ -1,5 +1,6 @@ > > > > > > > > # SPDX-License-Identifier: GPL-2.0 > > > > > > > > obj-$(CONFIG_CXL_BUS) += core/ > > > > > > > > +obj-$(CONFIG_CXL_MEM) += cxl_port.o > > > > > > > > > > > > > > It feel odd that CONFIG_CXL_MEM builds cxl_port, why not have a > > > > > > > CONFIG_CXL_PORT that is simply selected by CONFIG_CXL_MEM, or a > > > > > > > CONFIG_CXL_PORT that defaults to the value of CONFIG_CXL_BUS? > > > > > > > > > > > > > > > > > > > Can you help me understand when CONFIG_CXL_MEM is useful when > > > > > > #CONFIG_CXL_PORT=n? I was unable to figure out such a case and so I tied the two > > > > > > together. > > > > > > > > > > With a 'select' dependency it's impossible to have the > > > > > CONFIG_CXL_PORT=n and CONFIG_CXL_MEM=m combination. The extra config > > > > > symbol is for idiomatic (one config-symbol per module .ko) reasons to > > > > > reflect the module dependency in the Kconfig. > > > > > > > > > > > > > Should CXL_MEM also select CXL_PCI? I don't see a way CXL_MEM can work without > > > > CXL_PCI. > > > > > > The "select" dependency should be an "I use services from module X" > > > relationship. CXL_MEM uses port services and can't operate unless that > > > module is present. CXL_MEM does not strictly need CXL_PCI especially > > > since drivers/cxl/core/memdev.c abandoned all PCI refernces. > > > > The problem I see is cxl_mem devices have a soft dependency on cxl_pci. For > > example what we discussed regard DVSEC range registers. My plan was if the > > endpoint driver can't determine range registers aren't in use, it won't bind. It > > can't make this determination without cxl_pci. DOE has a similar implication > > though not fatal. > > I think this gets back to my original preference of just making the > port driver a part of the core rather than its own independent module > for no real reason. In fact it makes the validation job harder because > it expands the testing surface for different combinations of module > loading and device binding. > > For the specific issue of CXL_MEM dependencies on CXL_PCI, isn't > CXL_PCI passing in cacheed data for CXL_MEM? CXL_MEM has no idea that > CXL_PCI exists, but it certainly knows that CXL_PORT exists, Thinking about it further, the current bar for entry in cxl_mem is there must as least be component registers. If CXL_PCI doesn't do that, nothing will proceed, so I think this is fine. Since you brought it up, further down the road, I'm not opposed toward moving the port driver into core, but for now I like it as the separate module. Some of the topology host games we're playing now might have been avoidable, although I think they're not a bad idea anyway.