public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Fabio M. De Francesco" <fabio.m.de.francesco@linux.intel.com>
To: Robert Richter <rrichter@amd.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	ming.li@zohomail.com, linux-kernel@vger.kernel.org,
	linux-cxl@vger.kernel.org
Subject: Re: [PATCH 0/4 v3] cxl/core: Enable Region creation on x86 with Low Mem Hole
Date: Tue, 25 Mar 2025 17:13:50 +0100	[thread overview]
Message-ID: <3301434.hkbZ0PkbqX@fdefranc-mobl3> (raw)
In-Reply-To: <Z91Au5en7r6D7IsW@rric.localdomain>

On Friday, March 21, 2025 11:34:35 AM Central European Standard Time Robert Richter wrote:
> Fabio,
> 
> On 14.03.25 12:36:29, Fabio M. De Francesco wrote:
> > The CXL Fixed Memory Window Structure (CFMWS) describes zero or more Host
> > Physical Address (HPA) windows that are associated with each CXL Host
> > Bridge. Each window represents a contiguous HPA that may be interleaved
> > with one or more targets (CXL v3.1 - 9.18.1.3).
> > 
> > The Low Memory Hole (LMH) of x86 is a range of addresses of physical low
> > memory to which systems cannot send transactions. On those systems, BIOS
> > publishes CFMWS which communicate the active System Physical Address (SPA)
> > ranges that map to a subset of the Host Physical Address (HPA) ranges. The
> > SPA range trims out the hole, and capacity in the endpoint is lost with no
> > SPA to map to CXL HPA in that hole.
> > 
> > In the early stages of CXL Regions construction and attach on platforms
> > with Low Memory Holes, the driver fails and returns an error because it
> > expects that the CXL Endpoint Decoder range is a subset of the Root
> > Decoder's (SPA >= HPA). On x86 with LMH's, it happens that SPA < HPA.
> > 
> > Therefore, detect x86 Low Memory Holes, match CXL Root and Endpoint
> > Decoders or already made CXL Regions and Decoders to allow the
> > construction of new CXL Regions and the attachment of Endpoint Decoders,
> > even if SPA < HPA. If needed because of LMH's, adjust the Endpoint Decoder
> > range end to match Root Decoder's.
> > 
> > - Patch 1/4 changes the calling conventions of three match_*_by_range()
> >   helpers in preparation of 3/4.
> > - Patch 2/4 Introduces helpers to detect LMH's and also one to adjust
> >   the HPA range end for CXL Regions construction.
> > - Patch 3/4 enables CXL Regions construction and Endpoint Decoders
> >   attachment by matching Root Decoders or Regions with Endpoint
> >   Decoders, adjusting Endpoint Decoders HPA range end, and relaxing
> >   constraints while Endpoints decoders' attachment.
> > - Patch 4/4 simulates a LMH for the CXL tests on patched CXL driver.
> > 
> > Many thanks to Alison, Dan, and Ira for their help and for their reviews
> > of my RFC on Intel's internal ML.
> > 
> > Commenting on v1, Alison wrote a couple of observations on what users
> > will see. I suggest anyone interested to see how this series affect
> > users to take a look at her observations.[0] Thank you!
> > 
> > Changes for v3:
> > 
> >   Re-base the series on cxl/next.
> > 
> >   1/4 - 2/4:
> > 	Constify local variables.
> >   3/4:
> > 	Call arch_match_region() from region_res_match_cxl_range().
> >   4/4:
> > 	arch_match_region() - Check that region end is under start + 4G;
> > 	arch_match_spa() - Check that SPA range start is cfmws_range_start.
> 
> I have sent comments for version 1 and suggested a simpler approach
> for this to implement.
>
I replied to your comments for version 1. Please find my message at 
https://lore.kernel.org/linux-cxl/9930375.eNJFYEL58v@fdefranc-mobl3/.
>
> My comments haven't been addressed yet,
>
I think it's not possible to detect an LMH while still in cxl_port_add().
Therefore, I think that this is the best way to go. It works to prevent
the driver to fail to create Regions and attach Endpoint Decoders on x86
with Low Memory Holes, provided that the lower SPA range starts at 0x0.

On platforms with other kind of holes, the driver will continue to fail
as it currently does. 
>
> but we
> need better isolation to reduce interference with other platforms and
> archs. Please take a look again.
>
Interference? Do you mean that this series would make the driver fail on 
other platforms? 

Of course I don't want anything like that. I'm not clear about it...
Would you please describe how would this series interfere and what
would happen on other platforms?

Thanks,

Fabio
>
> Many thanks,
> 
> -Robert
> 





  reply	other threads:[~2025-03-25 16:13 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-14 11:36 [PATCH 0/4 v3] cxl/core: Enable Region creation on x86 with Low Mem Hole Fabio M. De Francesco
2025-03-14 11:36 ` [PATCH 1/4 v3] cxl/core: Change match_*_by_range() calling convention Fabio M. De Francesco
2025-03-21 15:43   ` Dave Jiang
2025-03-14 11:36 ` [PATCH 2/4 v3] cxl/core: Add helpers to detect Low memory Holes on x86 Fabio M. De Francesco
2025-03-18 15:15   ` Ira Weiny
2025-03-21 10:21   ` Robert Richter
2025-03-26 16:47     ` Fabio M. De Francesco
2025-03-28 10:26       ` Robert Richter
2025-03-28 23:40   ` Dan Williams
2025-03-29 10:05     ` Fabio M. De Francesco
2025-03-14 11:36 ` [PATCH 3/4 v3] cxl/core: Enable Region creation on x86 with Low Memory Hole Fabio M. De Francesco
2025-03-18 20:35   ` Ira Weiny
2025-03-21 10:29   ` Robert Richter
2025-03-14 11:36 ` [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests Fabio M. De Francesco
2025-03-18 21:16   ` Ira Weiny
2025-03-21 10:42   ` Robert Richter
2025-03-26 16:58     ` Fabio M. De Francesco
2025-03-28 10:52       ` Robert Richter
2025-03-28 23:40   ` Dan Williams
2025-03-29 10:16     ` Fabio M. De Francesco
2025-03-29 22:01       ` Fabio M. De Francesco
2025-04-03  4:00     ` Dan Williams
2025-03-20  1:46 ` [PATCH 0/4 v3] cxl/core: Enable Region creation on x86 with Low Mem Hole Alison Schofield
2025-03-26 16:23   ` Fabio M. De Francesco
2025-03-20 18:10 ` Alison Schofield
2025-03-26 16:24   ` Fabio M. De Francesco
2025-03-21 10:34 ` Robert Richter
2025-03-25 16:13   ` Fabio M. De Francesco [this message]
2025-03-28  9:02     ` Robert Richter
2025-03-28 21:10       ` Dave Jiang
2025-04-02 11:51         ` Robert Richter
2025-04-02 15:31           ` Dave Jiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3301434.hkbZ0PkbqX@fdefranc-mobl3 \
    --to=fabio.m.de.francesco@linux.intel.com \
    --cc=alison.schofield@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=ira.weiny@intel.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.li@zohomail.com \
    --cc=rrichter@amd.com \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox