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:   Fri, 9 Jun 2023 20:09:52 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Terry Bowman <terry.bowman@amd.com>, <alison.schofield@intel.com>,
        <vishal.l.verma@intel.com>, <ira.weiny@intel.com>,
        <bwidawsk@kernel.org>, <dan.j.williams@intel.com>,
        <dave.jiang@intel.com>, <Jonathan.Cameron@huawei.com>,
        <linux-cxl@vger.kernel.org>
CC:     <terry.bowman@amd.com>, <rrichter@amd.com>,
        <linux-kernel@vger.kernel.org>, <bhelgaas@google.com>
Subject: RE: [PATCH v5 19/26] cxl/pci: Add RCH downstream port AER register
 discovery
Message-ID: <6483e9801c0ae_e067a29432@dwillia2-xfh.jf.intel.com.notmuch>
References: <20230607221651.2454764-1-terry.bowman@amd.com>
 <20230607221651.2454764-20-terry.bowman@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230607221651.2454764-20-terry.bowman@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MA8p5fQUHWzMp8PThSaTCv5dv3oTz9ui1vUxkaussSJpyN1cJKON7zdWnmh0?=
 =?us-ascii?Q?lMCTmsNCz/OXT2jUKuvch5q8vANCYb2f6XhvIzALn9/E9rDURt3cDhqICB/c?=
 =?us-ascii?Q?6OdP3CPQ9hu7mvmUaQmpf7931NjLHe9JzeHIN9qaDqpO8TxXQnY4BQvhWE1n?=
 =?us-ascii?Q?mWeQnZMVFVF+rvtXunJbVglkrBaDp65eDhxKkg2QeVG+neLDYqozfRX1GSlZ?=
 =?us-ascii?Q?NTyBb3xp57Vh/HDOJiBEkId5trrvQohEtUSP/hpDxqyzZA94F7PGs1IdxTmV?=
 =?us-ascii?Q?zraOS++F6fANwrLgpdEYRKorniuJ9AvPcuFJOc835RZcKctq1wyJBa8xDIk6?=
 =?us-ascii?Q?FErD6NkXuJLEvsfLupc6yc+IalgIMKTZZZxcxBEVUxz4NfRREnIkaZUWbVm/?=
 =?us-ascii?Q?xC/Pf8LbruaxKYYnRRrV3jCASHDhNhTejkBaD24eZz+IJs5Hsz5LmQQPpPMy?=
 =?us-ascii?Q?2yBQOB82cGL1fD81CwNizaE7JftiaTJQINo3i3lIGcxmwWUrxIWyrwHfoSFB?=
 =?us-ascii?Q?Uj50FLC03ioDSCbLzS1TncCl8ny+LanfZbbRzAqq9QzLV72Ln3Z5ocdvBL/a?=
 =?us-ascii?Q?ZpEkt4z0BKSWGonD1XekXZw5ihwgXXHoftnXaWIuANVKTjqhJPuBJvM8pN/z?=
 =?us-ascii?Q?gC3HeBRxMcF0LJlYYEeFuc4GEBqKFl29A/V30LVSS4092I/u5QbFk9B0INhr?=
 =?us-ascii?Q?qs0GJzgvuhzXWodB2a1blrKQ6OKDD9kbzLJkp2neh0j6QwFUdnYvPPgKjccS?=
 =?us-ascii?Q?Og/Jr7NY6Zw8vBbmP3p8CsO2nfeTbX0kpCuvytW8EWNaet/Sa1DE0pmbGabZ?=
 =?us-ascii?Q?gnfj4gr0wGzFW8HotdqXa/Y6i+uQBXjOi+9iuW+alu9cuzf5JJJ11qRuRtZE?=
 =?us-ascii?Q?04kx/pjVJ6V7MyCdery1CeFTIG0Nf+WcYHMA4Vsn3eu7mBFxSZuD7dpwotaB?=
 =?us-ascii?Q?g5/Qto+JuNwH9lehzmBi7kTtlCZL2gduHVNyJEZhPQ3GnCgGAOgYZfjbR0WI?=
 =?us-ascii?Q?eFxr/C+XUgNqcFI4sD+sVnxrOPRIit0ueikNfVsKtTEIShnc5v+HQPxxpHNE?=
 =?us-ascii?Q?fbGI9ZDViynxLDAJEPMbVdy4gK6APjBVbf3BY09k3UuJ2EkgBboloT4BwK9F?=
 =?us-ascii?Q?Lg/f+SrtdddwplriwVYbORfcyJN8PLWp0yW5HiVF8lLz2jp/IfvWVoJF/FkI?=
 =?us-ascii?Q?CNKMztkZ473O1TxUYi6o3cNmt/1j8lblxdZnrTurCZ2BmSaqm9jTKpbYP20o?=
 =?us-ascii?Q?EalWi0gh+bIU8huHvoWgx8nH+LtkLDngcoPcYNuWm5X+f4QtiZdzJCxnCuI3?=
 =?us-ascii?Q?7nKM7JB5A4MbU23kvhgr5RGriguSQeZlhlrRELvFzN+DOWbBlauHJtqpRgcG?=
 =?us-ascii?Q?IVfuoSG/QEVsx9pFRdxhBjDMQldPE6EIdbQ3T7Uh+dlleE5eUpvG+/FXFvcU?=
 =?us-ascii?Q?ABAHbArYFDQYxz8LPlE7c3zqxuf8dmMD5xATv0JfEgPpDVebR5QYt+tOvYRU?=
 =?us-ascii?Q?xSxCPJ69JLDpieLqJ+akPBhsrzbf2KVTpU01gyE9XHihe4uN0GYg7Srw7V1Z?=
 =?us-ascii?Q?ep2LJGQt6PS4rkcYPdK670WDS8adDc89mk371NQvNOt/6JAi6sbSAJYIcbMX?=
 =?us-ascii?Q?ug=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 79698fba-2ef3-4d7c-85ec-08db69602b00
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jun 2023 03:09:56.6034
 (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: HLDCa7KjRD2jqfbobo5V4szLtj6qb6vpfEvSYQ04wnlehcevIpg88Z3IL3VcZDTHKTMXBQi76+Da0VQS/y79dxq8oVJbTpB6vhc3Om+QaKA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8363
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_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        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

Terry Bowman wrote:
> Restricted CXL host (RCH) downstream port AER information is not currently
> logged while in the error state. One problem preventing the error logging
> is the AER and RAS registers are not accessible. The CXL driver requires
> changes to find RCH downstream port AER and RAS registers for purpose of
> error logging.
> 
> RCH downstream ports are not enumerated during a PCI bus scan and are
> instead discovered using system firmware, ACPI in this case.[1] The
> downstream port is implemented as a Root Complex Register Block (RCRB).
> The RCRB is a 4k memory block containing PCIe registers based on the PCIe
> root port.[2] The RCRB includes AER extended capability registers used for
> reporting errors. Note, the RCH's AER Capability is located in the RCRB
> memory space instead of PCI configuration space, thus its register access
> is different. Existing kernel PCIe AER functions can not be used to manage
> the downstream port AER capabilities and RAS registers because the port was
> not enumerated during PCI scan and the registers are not PCI config
> accessible.
> 
> Discover RCH downstream port AER extended capability registers. Use MMIO
> accesses to search for extended AER capability in RCRB register space.
> 
> [1] CXL 3.0 Spec, 9.11.2 - System Firmware View of CXL 1.1 Hierarchy
> [2] CXL 3.0 Spec, 8.2.1.1 - RCH Downstream Port RCRB
> 
> Co-developed-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/cxl/core/regs.c | 51 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index ba2b1763042c..dd6c3c898cff 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -408,6 +408,54 @@ int cxl_setup_regs(struct cxl_register_map *map)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_setup_regs, CXL);
>  
> +static void __iomem *cxl_map_reg(struct device *dev, resource_size_t addr,
> +				resource_size_t length)
> +{
> +	struct resource *res;
> +
> +	if (WARN_ON_ONCE(addr == CXL_RESOURCE_NONE))
> +		return NULL;
> +
> +	res = request_mem_region(addr, length, dev_name(dev));
> +	if (!res)
> +		return NULL;
> +
> +	return ioremap(addr, length);
> +}
> +
> +static void cxl_unmap_reg(void __iomem *base, resource_size_t addr,
> +			 resource_size_t length)
> +{
> +	iounmap(base);
> +	release_mem_region(addr, length);
> +}

Why redo the {request,release}_mem_region() and ioremap() vs handling
this inside of the existing mapping of the RCRB in this function?