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, 16 Nov 2022 11:24:48 -0800
From:   Dan Williams <dan.j.williams@intel.com>
To:     Robert Richter <rrichter@amd.com>,
        Alison Schofield <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        "Ira Weiny" <ira.weiny@intel.com>,
        Ben Widawsky <bwidawsk@kernel.org>,
        Dan Williams <dan.j.williams@intel.com>
CC:     <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Len Brown <lenb@kernel.org>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        "Davidlohr Bueso" <dave@stgolabs.net>,
        Dave Jiang <dave.jiang@intel.com>,
        Robert Richter <rrichter@amd.com>
Subject: RE: [PATCH v3 5/9] cxl/pci: Only register RCDs with device 0,
 function 0 as CXL memory device
Message-ID: <63753900717c6_12cdff29439@dwillia2-xfh.jf.intel.com.notmuch>
References: <20221109104059.766720-1-rrichter@amd.com>
 <20221109104059.766720-6-rrichter@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221109104059.766720-6-rrichter@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?tjBhuIjhZ77hOCEenVx7nfCgXkJVITR0yxRRTKM5qyEHPpab0mch3iV+VshA?=
 =?us-ascii?Q?uNkuns+e34g9Mb2w2ShlTESq+HrqBKLZO3tgLSxdNyX8LCz5zdDM259/8kZr?=
 =?us-ascii?Q?5531x66qg1J1SM1+KMZsGfe1CwxpynC9znGiFv3NTEWI+IGLYsxmMMn/WIqo?=
 =?us-ascii?Q?SH6M6od554bVdxRrEAPNsjg/nJ/Kavp4wR7R1n1TSTIjbFDjbeDSc2MwcPYf?=
 =?us-ascii?Q?BXVR8g17kMEbQKObQlZ1mbso46KhZdLK7qiRKoNjjhX8SpzibnGY3ykiyvnD?=
 =?us-ascii?Q?bbBDT9UVu0IIVskSPUKf474b3ZFMFKSOy7vjPwir6yt1qY50wSyXUElEifAe?=
 =?us-ascii?Q?U3mOudCbqI6j5+fqxIKpHuj/M/k64VODzTQqPiHNOTZiiDoJafSAQMDJgTBd?=
 =?us-ascii?Q?o1yqMZ5qvws8q5pnwRd+4/n3+iDmfvsWBxxtn9ES8GSO7/+Z+O7I9iLz5kh4?=
 =?us-ascii?Q?G0PneMeUlK7ySet6d21if2o+EVJZ1m/nP+BQRNfMug1wsHEbUQYYoS34tUY/?=
 =?us-ascii?Q?WuyMGwAU5fCWNrluQC3vKkFkCAPGKoal1bboDYFjVX5iEqXDZsbCH+cD30eY?=
 =?us-ascii?Q?Kxj13xk9gv2OnzIS4gKfoIIJeZA6WJ3H2xa0B0j8W6wT+hHzlyOsDKKn7rF3?=
 =?us-ascii?Q?GatP3Y2vG2MsXJS8WO2TavwDBqhfQREe72a029gdtK+l0x4eYdYOcP0DMS/w?=
 =?us-ascii?Q?GsgxA60UhZIqB7Ybp6U4domuHYYvLihYzIvnSY+9hqGAx2FJ3aI5MZNsow1X?=
 =?us-ascii?Q?jWRbTpKuURPhaQ/NP14RFf+9koznUXwp+WwoZXgnjxE+9GlIKugmbmcr1xzm?=
 =?us-ascii?Q?oIrwopA+vzExlHWQ7xiO/8SjTwGNYri+4zOWjteHJ/JSjt83JGvkfoEhsUF/?=
 =?us-ascii?Q?DB/4XzUC9smbPQnXrIbVsvjtUGSINj+13v/SuRIq95vnwyEdimh4ZAzbXmhA?=
 =?us-ascii?Q?O8IxmfKeOmgUm5xKeLLpWO/Hf3APCk/l1dzlYHZIYCCD0GIFWsdHz8GsyDQR?=
 =?us-ascii?Q?Kamv1htZHQ22Mh7NBJOnnM5d5R3nmfDpm4dI+0VP8pneawuY45P61nGcMRCH?=
 =?us-ascii?Q?C/IZBCQv8hDOn71YztsD043wHiTXNDJnbKxSqxrmvLUbkmYSxeVTZTFyL6q/?=
 =?us-ascii?Q?3oe7qY31ovsjAg1+VMA+JGQt224wAI6yUvtYo2UI7m1CpxX0VY5qKyOwFclB?=
 =?us-ascii?Q?90B17TuwuAajh5Gy+j8XmfJg9JG9oKIEdm8MjE85NOoeCb+Hrf86jboXCNXz?=
 =?us-ascii?Q?zgQWZIaWi7yqAPTPJVjVRKwfsHmvERsAB9M71LoZvIP0OaPtWFkuH6LE5hfP?=
 =?us-ascii?Q?h/N61SFTVtmSgZCzJX5KGadSfFVn5WtTvY3ZhaMR/JVcfiiWHTZKD9GNHoTa?=
 =?us-ascii?Q?meYiJIW1IcrvRNcBdlue9IsFEl3RTvEZ9h8zIBADFVcfvVrfTFizazOlD+6i?=
 =?us-ascii?Q?EmqWQeOi1/sxcNdvdw4T8sQfvQffDhpxns6VCFoKbwKHqV0B1pingYQ5kfLb?=
 =?us-ascii?Q?cH91M3gLK6PiMebU9P5En+WATobSD3SiUOGTD+qLJrJ9gkzOjra4ESBKUyXi?=
 =?us-ascii?Q?XnFX04R2sisV9sQOKIoL11n5gPhO7aAphYDZwXxrvMgPqssC3hCNd3pqbWx2?=
 =?us-ascii?Q?Xw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a1c30a01-4b52-46f8-d419-08dac8083bd6
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Nov 2022 19:24:52.7917
 (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: cl9TlIWhUHLjTvSB0nNgGCc9VrZ4XqCf2hanHhM/bfFEvvDPMQmACwEkydMLRDsW7JqT83qTsIyQQrjdtnnQUYf9ruDeJ0ozjCMF8qgjODw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7195
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 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_PASS,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

Robert Richter wrote:
> The Device 0, Function 0 DVSEC controls the CXL functionality of the
> entire device. Add a check to prevent registration of any other PCI
> device on the bus as a CXL memory device.

Can you reference the specification wording that indicates that the OS
needs to actively avoid these situations, or otherwise point to the real
world scenario where this filtering is needed?

> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/pci.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index faeb5d9d7a7a..cc4f206f24b3 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -428,11 +428,26 @@ static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
>  	}
>  }
>  
> +static int check_restricted_device(struct pci_dev *pdev, u16 pcie_dvsec)
> +{
> +	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_RC_END)
> +		return 0;		/* no RCD */
> +
> +	if (pdev->devfn == PCI_DEVFN(0, 0) && pcie_dvsec)
> +		return 0;		/* ok */
> +
> +	dev_warn(&pdev->dev, "Skipping RCD: devfn=0x%02x dvsec=%u\n",

s/0x%02x/%#02x/

> +		pdev->devfn, pcie_dvsec);

This looks like a dev_dbg() to me. Otherwise a warning will always fire
on a benign condition.

> +
> +	return -ENODEV;
> +}
> +
>  static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>  	struct cxl_register_map map;
>  	struct cxl_memdev *cxlmd;
>  	struct cxl_dev_state *cxlds;
> +	u16 pcie_dvsec;
>  	int rc;
>  
>  	/*
> @@ -442,6 +457,13 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	BUILD_BUG_ON(offsetof(struct cxl_regs, memdev) !=
>  		     offsetof(struct cxl_regs, device_regs.memdev));
>  
> +	pcie_dvsec = pci_find_dvsec_capability(
> +		pdev, PCI_DVSEC_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
> +
> +	rc = check_restricted_device(pdev, pcie_dvsec);
> +	if (rc)
> +		return rc;
> +
>  	rc = pcim_enable_device(pdev);
>  	if (rc)
>  		return rc;
> @@ -451,8 +473,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		return PTR_ERR(cxlds);
>  
>  	cxlds->serial = pci_get_dsn(pdev);
> -	cxlds->cxl_dvsec = pci_find_dvsec_capability(
> -		pdev, PCI_DVSEC_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
> +	cxlds->cxl_dvsec = pcie_dvsec;
>  	if (!cxlds->cxl_dvsec)
>  		dev_warn(&pdev->dev,
>  			 "Device DVSEC not present, skip CXL.mem init\n");
> -- 
> 2.30.2
>