RE: [PATCH v2] cxl: core/region - ignore interleave granularity when ways=1

[Dan Williams]

Yasunori Gotou (Fujitsu) wrote:
> Hello, Gregory-san, and everyone
> 
> > Hi Gregory and CXL community
> > Cc Goto-san
> > 
> > Wow, our platform has encountered a similar issue, 
> 
> Yes, I actually encountered this issue. The endpoint decoders show the granularity value as 1024 bytes,
> but other decoders show 256 bytes, which is the default value, even when the interleave setting is one.
> 
> As a result, the error message that this patch avoids appeared, and initialization failed, as described in the following email:
> https://lore.kernel.org/linux-cxl/OSYPR01MB53525FD64A9AFBAEEC1E19A390112@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/#m811bdd93bca3887b4c14a2a20b8f21a77dcf2eae
> 
> So, Gregory's patch is welcome for me.
> 
> > and I am intending to consult the community regarding this matter.
> > I drafted similar patch locally, but I wonder if we should "ignore" the IG or
> > "program" the IG to the decoder.
> 
> I'll try to summarize Li-san's question.
> Does anyone know what will happen if the CXL driver does not program the IG value to the decoder?
> Will it work correctly without any problems?

The IG is always valid, either it is the default 0 which is 256, or a
stale value from a previous configuration.

> My initial approach to avoid this problem was to "program" the decoder, as shown below.
> This patch is a very early trial version to avoid the error we encountered.
> However, Li-san's concern is that this patch writes the IG value to the decoders.
> Is this "programming," as shown below, unnecessary?

If the decoder already has iw=1 and ig=1024 when the driver first
enumerates the decoder then that is a platform BIOS compatibility
concern where Linux should try to accommodate without touching the
decoder.

The concern about writing to the HDM control register is that there is a
lag awaiting the "committed" bit being set. See cxld_await_commit(). The
spec is silent on what what happens to inflight transactions between
setting "commit" and awaiting "committed". I interpret that as undefined
behavior and is why the driver is careful to make sure HDM is unmapped
before changing decoders.