Re: [PATCH 0/4 v3] cxl/core: Enable Region creation on x86 with Low Mem Hole

[Robert Richter <rrichter@amd.com>]

Dave,

thank you for your answer.

On 28.03.25 14:10:00, Dave Jiang wrote:
> 
> 
> On 3/28/25 2:02 AM, Robert Richter wrote:
> > On 25.03.25 17:13:50, Fabio M. De Francesco wrote:

> >> Interference? Do you mean that this series would make the driver fail on 
> >> other platforms? 
> > 
> > No, other platforms must deal with that specific code and constrains.
> > 
> >>
> >> 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?
> > 
> > Other platforms should not care about platform-specifics of others. So
> > again, use a platform check and only enable that code there necessary.
> > And this requires a well defined interface to common code.
> 
> Hi Robert,

> Can you please share more on the background information and/or your
> specific concerns on the possible memory holes in the other
> platforms that need to be considered and not covered by Fabio's
> code? Let's all get on the same page of what specifics we need to
> consider to make this work. Preferably I want to avoid arch and
> platform specific code in CXL if possible. Of course that may not
> always be possible. Would like see if we can avoid a bunch of #ifdef
> X86/ARM/INTEL/AMD and do it more cleanly. But fully understand the
> situation first would be helpful to determine that. Thank you!

We implement a "special" case in the main path. This adds unnecessary
complexity to the code, makes it hard to maintain, change or even to
understand in the future. It becomes more error-prone. Though it is
limited to x86 arch, the code runs for all platforms. A reuse for
other archs will enable it for all platforms of that archs too.

This general approach to add "special" cases does not scale. We see
this already with the "extended linear cache" and now the "low mem
hole". While I am fine with all those special cases (AMD address
translation is another), we need a proper way to enable and implement
those by reducing complexity and with a good isolation. This makes
future changes easier and reduces conflicts with other
implementations.

The change of this series does not much, just find a CFMWS region that
is unaligned to the EP decoder's range and then just shrink the used
SPA range of the EP to match that region. That can be implemented in a
very simple way if we introduce a spa_range paramater plus a custom
port setup. The generalized part of my address translation part alrady
implements this, it can be reused here. To implement LMH support only
the following is needed then:

 * add a setup function with a platform check to add a custom
   callback,

 * the callback checks for the LMH range and adjusts the spa_range.

The modified spa_range matches then with the region range (no changes
needed here). That's it.

I can help making this work.

I hope that makes sense?

Thanks,

-Robert