Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 8 Jun 2022 14:38:35 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Ben Widawsky <bwidawsk@kernel.org>
CC:     Dan Williams <dan.j.williams@intel.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        "Alison Schofield" <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        <linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
        <linux-pci@vger.kernel.org>
Subject: Re: [PATCH V10 5/9] cxl/port: Find a DOE mailbox which supports CDAT
Message-ID: <YqEW21rs+LkOHOWS@iweiny-desk3>
References: <20220605005049.2155874-1-ira.weiny@intel.com>
 <20220605005049.2155874-6-ira.weiny@intel.com>
 <20220606174819.hhnb4jh27nxxvrdf@bwidawsk-mobl5>
 <YqD7BZnbK4wuN6Zm@iweiny-desk3>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YqD7BZnbK4wuN6Zm@iweiny-desk3>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?wo325Oeb753BRRYW5+YvaAneETX3oLP/c7TNF5IKgQR1d0D2DQshIAC1jM6p?=
 =?us-ascii?Q?TC90APyLb+Socq58JNXAPWFHBd7RZZ3olHkFUz1CLA8obP7hbn73F+1xHEUR?=
 =?us-ascii?Q?GHAcKmluQVIUXJSuBkXnNsOrVxvOg5AQzhcMy9B2KNxG6DW3WSSEOsjcNAe9?=
 =?us-ascii?Q?hk4cn50r0pnggS9nlL6vQZXG8GiQ2dj+OBM8Ob9lu8QolmOIByFDR3tZp9hc?=
 =?us-ascii?Q?0Zd9yrJ5KN4opMsQVzZfIMVCKwi9wtSnjkrWKnTgbiprxS2dsPcBipBi/kqj?=
 =?us-ascii?Q?oxCIGvo83NE16Nt16Gqr2ra1LN9aCY3Ggj6cf8S293VLF6krlJqfcK60klsq?=
 =?us-ascii?Q?7RN3sWXqIwo7tTjwGdjVTQNxi0w3J0r1RZD4sm/gjdbNycntLZJwbJFsFYg4?=
 =?us-ascii?Q?LVinF3bc66s+y/yiL88TJrTCMX4HJQHykvO2I7jw35q6pDa/us3fJ9+Si3oL?=
 =?us-ascii?Q?4AYhK2uEErvr6EOxwGpgqareb3fvhsbLxMCtsYXMA/5PzkfcadtXalisZ1c/?=
 =?us-ascii?Q?7Rd4Litf5i/ahnKCs/fFMx3EILDkiK7KK6jql6+/qW4FpQ12+Z/J2Wqdt0A6?=
 =?us-ascii?Q?KiPDyjfS+9W37GHj/+tMMpXvMzvmM5knVfiJBCUPzsBPDE3X2zAI0EQnIdJh?=
 =?us-ascii?Q?kHILIOaBNsKlP5yzLskgACJfFWJnOc1ulBjrPJXmnpzP45jxlKJ8VYo+anT9?=
 =?us-ascii?Q?UXVPgvSmT/fKAyM35a71In6qGiO8tTREVB1lGIW6AbLBgZpsyzC9LvhDxbRK?=
 =?us-ascii?Q?ZAsFRut0loVgFSjYVjmIWvDcUjvDpYr/h4PggKN4fRftYhsK/Vro49b37rPe?=
 =?us-ascii?Q?pL2bPFeKyaTcQQ2NVZbfNNR1shSslmrrKYPt4IW4kwiY7QIgGzQT7NKa/GQt?=
 =?us-ascii?Q?ewGNTmJf1HO5bgcU8Mn9fqsn/T54D52Teq/ihPmpENdscqUOwLwRJFq2f3eV?=
 =?us-ascii?Q?YOZHhF2VxlTNubcwHByT/rliZWIX8TNmOwr+R/FUMPdfWqy9ALYw5lklGZ4c?=
 =?us-ascii?Q?Z5H7OqYJBKOkJCcB0wNmvyOy9RGJialKX1C7NZzJsCGnwoV9j055RIsKjO0Q?=
 =?us-ascii?Q?xu9P27e4BQpWCRVqFPiJbglpcMUcYwTkWQX2HH+zt/R55/jKelHxSGnyeIRj?=
 =?us-ascii?Q?TlN/fX8ZRL/WUhwj58AlcXyNDH42Eijk3hZUYgxC6PJIgPVwgB1F1FWRNOak?=
 =?us-ascii?Q?vvK7zzSh99zBX4oii3imcuZiJbFBR+I+8/7CuXyk2n9vJrX+tqp8cxOao+aN?=
 =?us-ascii?Q?vfiOtH/WcQXIM09ZM2t2LCs4pXwK3QkbuNuyIaVwqT5f8q36FgmV8y3ppimp?=
 =?us-ascii?Q?TyXLgmmV+VydSlcQGkqS6SrynSMFjYzk90JTJBbZbwnmiuPLae0wM/qyAN0e?=
 =?us-ascii?Q?/LRFka6OP0vLTOglCafvTkIE+zVTDFuWMwogoi/iMawL+1hNc5ixGLBSmUl8?=
 =?us-ascii?Q?7fsmFJzRmA/EUB5bhetM+fKfkf6Na+l5GBG5BBu+KlWbscpmq5miB8wx14JL?=
 =?us-ascii?Q?sbfqIHsuACnE+0VEmIma65cj7PdPMNok28tdyrITB0PfB871eR+Frp1obfpX?=
 =?us-ascii?Q?VpxgasBSklsqu21eb3Yop8Y0cr9NAxL26KMauJwMz0lQSpxMJHbQvOGzb27f?=
 =?us-ascii?Q?dpySsrEgyzqIgpSSwL0YkV6oxk/y7o8Y+GUBcx3PY4rgd8CL4lmPvf4fH45A?=
 =?us-ascii?Q?fqj2pW7BpAyiSjA/gBfAL9h9F1cE4WM8oBCvsULv/AQMVp1GTh3JXcsRiJN6?=
 =?us-ascii?Q?9tapKiW7vA=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5ce76b2f-0a0d-471b-f17e-08da49974093
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6311.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jun 2022 21:38:39.9923
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6xmOpGqnyAwoAQjErfMs+pd/acl7YJ+hzocW8p6oqTudm8RA769E3IYhFWJWdTbYKK9OEfZ2+0o6q2FEJmBf0A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB3449
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Jun 08, 2022 at 12:39:49PM -0700, Ira wrote:
> On Mon, Jun 06, 2022 at 10:48:19AM -0700, Ben Widawsky wrote:
> > On 22-06-04 17:50:45, ira.weiny@intel.com wrote:
> > > From: Ira Weiny <ira.weiny@intel.com>
> > > 

[snip]

> > > +
> > > +/**
> > > + * cxl_cache_cdat_mb() -- cache the DOE mailbox which suports the CDAT protocol
> > > + *
> > > + * @port: Port to containing DOE Mailboxes
> > > + *
> > > + * Cache a pointer to the doe mailbox which supports CDAT.
> > > + */
> > > +void cxl_cache_cdat_mb(struct cxl_port *port)
> > > +{
> > > +	struct device *dev = port->uport;
> > > +	struct cxl_memdev *cxlmd;
> > > +	struct cxl_dev_state *cxlds;
> > > +	int i;
> > > +
> > > +	if (!is_cxl_memdev(dev))
> > > +		return;
> > > +
> > > +	cxlmd = to_cxl_memdev(dev);
> > > +	cxlds = cxlmd->cxlds;
> > > +
> > > +	for (i = 0; i < cxlds->num_mbs; i++) {
> > > +		struct pci_doe_mb *cur = cxlds->doe_mbs[i];
> > > +
> > > +		if (pci_doe_supports_prot(cur, PCI_DVSEC_VENDOR_ID_CXL,
> > > +					  CXL_DOE_PROTOCOL_TABLE_ACCESS)) {
> > > +			port->cdat_mb = cur;
> > 
> > What happens if cxl_pci is unloaded after this? Would it be better to copy out
> > the CDAT info? Otherwise, I think you need to hold a ref on the PCI device
> > (though I only took a quick look).
> 
> <sigh> I thought that could not happen but I see I was wrong.
> 
> A reference will need to be taken for at least the duration of the query.
> Originally I had this set up to try and make it easier to query later.  But I
> think that is a waste ATM.
> 
> I'm going to rework this ownership.

BTW this means this patch is going away.  I think just searching for the
mailbox with CDAT support on each query is the way to go at this time.

Ira

> 
> Thanks for the review,
> Ira
>