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:   Sun, 16 Oct 2022 15:10:54 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Davidlohr Bueso <dave@stgolabs.net>
CC:     <dan.j.williams@intel.com>, <Jonathan.Cameron@huawei.com>,
        <dave.jiang@intel.com>, <alison.schofield@intel.com>,
        <vishal.l.verma@intel.com>, <bwidawsk@kernel.org>,
        <a.manzanares@samsung.com>, <linux-kernel@vger.kernel.org>,
        <linux-cxl@vger.kernel.org>
Subject: Re: [PATCH 1/2] cxl/pci: Add generic MSI/MSI-X interrupt support
Message-ID: <Y0yBbjJgqyBS+viw@iweiny-desk3>
References: <20221014194930.2630416-1-dave@stgolabs.net>
 <20221014194930.2630416-2-dave@stgolabs.net>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221014194930.2630416-2-dave@stgolabs.net>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?t1U3i+HoAKIiXhviwsq/1yEw9lrc4QoGzo/Gt5YMX4M8NbM69oS2PJ+gLYMW?=
 =?us-ascii?Q?a4+nFImrBN5W7KtqG+LdmuJosnigr/oT3Wgn4FrGFlnUdlIhyOfMV1o5IqOR?=
 =?us-ascii?Q?PmNTAM5sOXPJ0/yO6gHW13s6Vusp0j8ybvGwHOpErpbnuRTK5ZFZqB3ygQSr?=
 =?us-ascii?Q?+t4lcOtB4PrsGC0tWPpRi/TmB5jygfd9RfO1NqI+AOo0WhwOwuIFG5zREOL7?=
 =?us-ascii?Q?UijQ71FRC7X/DOsu3uFUYv4Kvd9CEdq1tmEyZGKOWu3e2+JJ7RJlrSLGBVCp?=
 =?us-ascii?Q?6nEuqfN19UIjvZrQumD9YzPkNWnlaHPA6imkk3se5w9/zRdfLh+CuyJL5yGa?=
 =?us-ascii?Q?ZW9UM0uKSDkNIJErUjBc4pXfgLOMhY7EyZ2b/26sodQ2KTromHfZanRufJZT?=
 =?us-ascii?Q?m8WgujT7iPW8gM5c3pch//+A5w9xpqnFYZCyDcbtr/Vbe/6B814KBPykCSrG?=
 =?us-ascii?Q?u9T4XO20YaD2Nf66uyO/5P6QS/y2BKKO7hBv2MPQiGiqR7ZlGvEvlwDDJgRk?=
 =?us-ascii?Q?hMlH2YXlsAAhuMpDSrWJkqpFAez6OrNpOuHshGTJv65wu+BCTDzDk6sOA+mF?=
 =?us-ascii?Q?R7o00bfpJ/1RxzYHlqPmY6+I4kItGOTMpslGvZG86C69O0DWGw0RiQ1yXjYP?=
 =?us-ascii?Q?Us8+hNAu712yYkFlgAZ1Yl0YjY7495rUMk21G2gL6WJqExnvgIWgtNZ/Z4HN?=
 =?us-ascii?Q?zASU/KOpDt4NGL5aQCJIMpIac4t/WevVCbokZClZFN3Qfa544IrOp2FmHjTw?=
 =?us-ascii?Q?8/QGqqA6yJF3vmSe78x/fX2/O7/d3G8UgK5rHNZudIJNxDagjRNspWiulQcI?=
 =?us-ascii?Q?iGcvAwGMoZcLd6agfjF5JuAHGqNl2X+SN8JAAR2EAFwiQJpOIhBCiH3ZHoE5?=
 =?us-ascii?Q?YRPwCOeDd8KVvVBVYGSuxV4cAovGm0E/N3AjZNeyGAtgzYR3jeafRjChRVBn?=
 =?us-ascii?Q?h2tjCtloJofY1uRCHnrQr5DC3bCvVuZ2wxkh8hQf6WfEWXA9AszwoF81kxBV?=
 =?us-ascii?Q?E8EtPZWIDc9AnouJzAcDw/vAx1JT93BQLN9EpXYXyyCjFUVI4OCb1ZlaawaC?=
 =?us-ascii?Q?1NQwHkCZ+86DrbEGfdvX+C0fgG9L8Q3xYzBoPjybgmbQ0Jnmoe2o6xwxKFuh?=
 =?us-ascii?Q?/QxRcdJx45aOzQtR8E6D7gllzr6vjf9FYfMbZD4PSNAOTUVlDrA5WQ1fn2Yi?=
 =?us-ascii?Q?Llkcn7p0txKkYTZd+c2eIEpvcnf/GiJNsrs/TRHxWXnlngGvVLjQ6wl9a2sE?=
 =?us-ascii?Q?rSv2Vi9AJBGwqTajz4jfvzU2HhziOQn7nv2PXGwpmaGFxIa1En+gpmcNh1qg?=
 =?us-ascii?Q?lOesUX2wmJWLbSzfmymOZRN7Cz80ANSd9SjNRN56U4suMKtYHEfxmbGK7ODK?=
 =?us-ascii?Q?e+Q8skY5gfXMQrd+F9xzx7VSE8FiVRQX7w2LkBswqg5BDG1JihbwQkEBOuDl?=
 =?us-ascii?Q?1dEZD4p7PJxPtZmtn1Pp92eWgIONZ3HvShQUDT3ror0tbkg5QYc/bD150t20?=
 =?us-ascii?Q?NdJg7PN9DenQgHOw020fqpvU9YntDikl0Nq2q9uHvwLPGsvXTUbR/UsXIL46?=
 =?us-ascii?Q?Hh/35PTvOeEGr6aCwCay0tUAuqe4mhf7Bteh4LB7?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1e1305de-40d3-4d6c-4a46-08daafc34d82
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Oct 2022 22:10:58.2400
 (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: h+PlMNYafQ4/4CzzDEc6NQhs7jgOGbQZKS2d6m+J3DpHIFci2ZO4CmjhApwDpsf/vzCoRUHXtn+h4owjW4KnUA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6906
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.7 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 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 Fri, Oct 14, 2022 at 12:49:29PM -0700, Davidlohr Bueso wrote:
> Introduce a generic irq table for CXL components that can have
> standard irq support - DOE requires dynamic vector sizing and is
> as such is not considered here.
> 
> Create an infrastructure to query the max vectors required for the CXL
> device. Users can check the irq_type in the device state to figure
> if they want to attempt to register a handler for it's specific irq
> and deal with it accordingly.
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  drivers/cxl/cxl.h    |  5 ++++
>  drivers/cxl/cxlmem.h |  2 ++
>  drivers/cxl/pci.c    | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f680450f0b16..879661702054 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -119,6 +119,11 @@ static inline int ways_to_cxl(unsigned int ways, u8 *iw)
>  	return 0;
>  }
>  
> +enum {
> +	CXL_IRQ_NONE,
> +	CXL_IRQ_MSI,
> +};

I don't recall this being in v1?

Right now do we have any users who will register irq's without having MSI
support?

> +
>  /* CXL 2.0 8.2.8.1 Device Capabilities Array Register */
>  #define CXLDEV_CAP_ARRAY_OFFSET 0x0
>  #define   CXLDEV_CAP_ARRAY_CAP_ID 0
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 88e3a8e54b6a..ca020767f7fc 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -247,6 +247,8 @@ struct cxl_dev_state {
>  
>  	struct xarray doe_mbs;
>  
> +	int irq_type;
> +
>  	int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
>  };
>  
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index faeb5d9d7a7a..942c4449d30f 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -428,6 +428,67 @@ static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
>  	}
>  }
>  
> +/**
> + * struct cxl_irq_cap - CXL feature that is capable of receiving MSI/MSI-X irqs.
> + *
> + * @name: Name of the device generating this interrupt.
> + * @get_max_msgnum: Get the feature's largest interrupt message number.  If the
> + *		    feature does not have the Interrupt Supported bit set, then
> + *		    return -1.
> + */
> +struct cxl_irq_cap {
> +	const char *name;
> +	int (*get_max_msgnum)(struct cxl_dev_state *cxlds);
> +};
> +
> +static const struct cxl_irq_cap cxl_irq_cap_table[] = { NULL };
> +
> +static void cxl_pci_free_irq_vectors(void *data)
> +{
> +	pci_free_irq_vectors(data);
> +}
> +
> +/*
> + * Attempt to allocate the largest amount of necessary vectors.
> + *
> + * Returns 0 upon a successful allocation of *all* vectors, or a
> + * negative value otherwise.
> + */
> +static int cxl_pci_alloc_irq_vectors(struct cxl_dev_state *cxlds)
> +{
> +	struct device *dev = cxlds->dev;
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	int rc, i, vectors = -1;
> +
> +	for (i = 0; i < ARRAY_SIZE(cxl_irq_cap_table); i++) {
> +		int irq;
> +
> +		if (!cxl_irq_cap_table[i].get_max_msgnum)
> +			continue;
> +
> +		irq = cxl_irq_cap_table[i].get_max_msgnum(cxlds);
> +		vectors = max_t(int, irq, vectors);
> +	}
> +
> +	if (vectors == -1)
> +		return -1;
> +
> +	vectors++;
> +	rc = pci_alloc_irq_vectors(pdev, vectors, vectors,
> +				   PCI_IRQ_MSIX | PCI_IRQ_MSI);

Yea without PCI_IRQ_LEGACY I don't think we need any communication about which
type of vectors were allocated.

Basically if cxl_pci_alloc_irq_vectors() is successful all users can assume
that at least MSI is available...

For the mailboxes they could key off of the message number being set in cxlds.

> +	if (rc < 0)
> +		return rc;
> +
> +	if (rc != vectors) {
> +		dev_err(dev, "Not enough interrupts; use polling instead.\n");
> +		/* some got allocated, clean them up */
> +		cxl_pci_free_irq_vectors(pdev);
> +		return -ENOSPC;
> +	}
> +
> +	return devm_add_action_or_reset(dev, cxl_pci_free_irq_vectors, pdev);
> +}
> +
>  static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>  	struct cxl_register_map map;
> @@ -478,6 +539,11 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	devm_cxl_pci_create_doe(cxlds);
>  
> +	if (!cxl_pci_alloc_irq_vectors(cxlds))

This can't be here for the event stuff because I need the mailboxes set up to
find out the message numbers for those events.  I had a hell of a time by
accident putting it here.  :-(

> +		cxlds->irq_type = CXL_IRQ_MSI;
> +	else
> +		cxlds->irq_type = CXL_IRQ_NONE;
> +
>  	rc = cxl_pci_setup_mailbox(cxlds);
>  	if (rc)
>  		return rc;

Can't the mailbox irq's be set up after this call?  Mailbox access during set
up is probably fine using polling, right?

Ira