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, 16 Sep 2022 11:04:01 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Robert Richter <rrichter@amd.com>,
        Dan Williams <dan.j.williams@intel.com>
CC:     Alison Schofield <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Ira Weiny <ira.weiny@intel.com>,
        Ben Widawsky <bwidawsk@kernel.org>,
        <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>
Subject: Re: [PATCH 02/15] cxl/core: Check physical address before mapping it
 in devm_cxl_iomap_block()
Message-ID: <6324ba91a05cb_2a6ded29457@dwillia2-xfh.jf.intel.com.notmuch>
References: <20220831081603.3415-1-rrichter@amd.com>
 <20220831081603.3415-3-rrichter@amd.com>
 <6319824968fa7_580162946f@dwillia2-xfh.jf.intel.com.notmuch>
 <YxsvPwzxoYBMnN2y@rric.localdomain>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YxsvPwzxoYBMnN2y@rric.localdomain>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?0ai8uvC7D9ActblvMOMXvYiSQd9DxR0UnLxfftobiD8GCGeRzHVbajjSnsz5?=
 =?us-ascii?Q?FVtqPC74uc9kbWR0ZRO3u8rcQqgQ4bTs686Dz5BCkqgiqh1vqpeBHwD5sQJK?=
 =?us-ascii?Q?RrMrZ6u4Sy4PaQnzv5tRZYE5V9b5trZ7t827631vU/Zgv4PiIMhVGnZIhXXk?=
 =?us-ascii?Q?peypcfmIS/d2C4RlnLi4e0C65xq8WNKxf2ViZeKXcYjMplVBnRUFZ7vZ4Miz?=
 =?us-ascii?Q?MQvc0iiSbWTgK4fFOqWGx323KDNp3ChIWM+Z6Ip+nQdZApW1QWuChxo6Kgf9?=
 =?us-ascii?Q?P9V9sgy3y1PKq1cvJj+XRvLeKkMU+0SEXUYOpxtX9ZV2Dt7tj528ZXnpymbi?=
 =?us-ascii?Q?F+R7XoYBxLX3++5kOAEHqac5ME81ehV5vw5fRMeamB6HlxOHx+7zTR3DXq3e?=
 =?us-ascii?Q?3Q3KACf+2N2dexGKmKLFcWrksyGJWCncxxu2N3zB6WH8C7d7TDV5t0u0T5oN?=
 =?us-ascii?Q?liVcGfuNnqUoH2nr3jdW5nNJPsAx+kLLwty7blF8rYy3skQ2OVxzsQ2mERl7?=
 =?us-ascii?Q?2PlAFfchpSgz2MKT9vg2AYywCDt+Wn2Dy0Kh5nFBXcXh6jliLLlSAEhJvrox?=
 =?us-ascii?Q?pOE+48HWACkZwKEhYNpn1dJlhWuBFkwa3lcOOKzYhRVhPj5x/CqqeipH2cs7?=
 =?us-ascii?Q?Ic55t91pCOKsRdZ2w/pwJqWruVbk7FxkP2m+ojb0mnHXjh80DJY7JNqEB3yk?=
 =?us-ascii?Q?7T7iWYywNkVuUN8cvmqhibgcSeApu7R15xA/f3sAcOFhEUzWFRMHGCK1Jzz5?=
 =?us-ascii?Q?cMqlXK8DnMk0dLI8iqrfY/rnAEDVfGdKzJRJ+374Cha5tjLcjYvvTl+8XN2R?=
 =?us-ascii?Q?Es0ihtwelxyHlZz3LEPLyGOOR40kc2FBMhayqAU52ZZciEmSrMwCidYNfGju?=
 =?us-ascii?Q?G/lHrJHAY90PLakVwMyFRM7cFJYf8NJ2LCcIHCIkZFRDmGPGqgXYfeJIS7ef?=
 =?us-ascii?Q?qe8/h6puFP3Ra3DbcwwXoZmYWROGK36OBDMfj9sMkAtTJhmFOeyGfCeWZxWY?=
 =?us-ascii?Q?nvywl8AZGZSqzjB2VwDTw6gyFLZ/PU0Yd5hNjhM5aGvQW6Igf6vVyUK7ikkc?=
 =?us-ascii?Q?vZTGsVXeXGvEYKryowEDN762F09zoLT40AKsGh8qJfV2W/mnLsH1o/EkAIE/?=
 =?us-ascii?Q?u9aXvZ5XYtvla27V0lsL7JKwCUpHuI0cB6YNBiWjWNZHQupEVlYgOoiBi5Hd?=
 =?us-ascii?Q?Fm9LcjjA+oxKLGnXqL0thSd14mitavkdQybi4x5dhZTfmYxF7B85I7qrkjnq?=
 =?us-ascii?Q?ae3lCLxQfRlc4KN6vaGZ0B+evm3DEBm62gCCQkRO4Om+bSg19JVXnCXoGTaI?=
 =?us-ascii?Q?W7Fe8aEoztAVAh7zaU2oC20Ot/26J1ibUWy/xzItJGR+ZSrlOYVZgdpfzc5q?=
 =?us-ascii?Q?S0+elbm18x7mQtA0VS7OUn0ZBXgYuZlRQktV1QaJfmCEYK+m2ANQ75mXLXOz?=
 =?us-ascii?Q?kcCr80TDaZEJGfH12yxt07xM5pVW70QDod+6iD38UiYjPw/vj0+rKQOcgBiA?=
 =?us-ascii?Q?YBwIqWYBTQAGo3453YH6FXfx08FPZwnkTzVqEIoU48rHkUxPIXTzjxs9fRXc?=
 =?us-ascii?Q?kAuv6UTbQx5SKBe4q+qkIUD06VRxk9dmC+OStEt5dh5jNOAVKi5gyOz0GQlv?=
 =?us-ascii?Q?kQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2fd79824-e755-45e5-56e2-08da980dd734
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Sep 2022 18:04:04.1843
 (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: LietZGQoqsovSo1F40jg3FaTPiUTx7gAHKZaw2/kF8bizleyFPq7XuyK/Skaf3nS+F9sQMiswCRuyiXAf9X3BVG3Wxd319KCNrM13hTSDOY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR11MB6228
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 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:
> On 07.09.22 22:48:57, Dan Williams wrote:
> > Robert Richter wrote:
> > > The physical base address of a CXL range can be invalid and is then
> > > set to CXL_RESOURCE_NONE. Early check this case before mapping a
> > > memory block in devm_cxl_iomap_block().
> > > 
> > > Signed-off-by: Robert Richter <rrichter@amd.com>
> > > ---
> > >  drivers/cxl/core/regs.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> > > index 39a129c57d40..f216c017a474 100644
> > > --- a/drivers/cxl/core/regs.c
> > > +++ b/drivers/cxl/core/regs.c
> > > @@ -165,6 +165,9 @@ void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
> > >  	void __iomem *ret_val;
> > >  	struct resource *res;
> > >  
> > > +	if (addr == CXL_RESOURCE_NONE)
> > > +		return NULL;
> > > +
> > >  	res = devm_request_mem_region(dev, addr, length, dev_name(dev));
> > >  	if (!res) {
> > >  		resource_size_t end = addr + length - 1;
> > > -- 
> > > 2.30.2
> > > 
> > 
> > devm_request_mem_region() succeeds for you when this happens? More
> > details about the failure scenario please.
> 
> No, CXL_RESOURCE_NONE (all FFs) is used as address. A broken range is
> calculated that even overflows. I only vaguely remember the exact
> error message.
> 
> This may happen e.g. if the Component Register Block is missing in the
> DVSEC. cxl_find_regblock() may fail then and returns
> CXL_RESOURCE_NONE. There are a couple of code paths there
> component_reg_phys is set to CXL_RESOURCE_NONE without exiting
> immediately.
> 
> I saw it during code development, when I pre-inititalized a port with
> component_reg_phys set to CXL_RESOURCE_NONE. Since that case can
> generally happen, I think it must be checked.

I think Jonathan had it right when we posited that the code should
probably have failed before getting to this point. For example, the
scenarios where the driver looks for a component register block via the
register locator DVSEC are not valid for RCDs in the first instance.