Re: [PATCH] cxl/hdm: Extend DVSEC range register emulation for region enumeration

[Gregory Price <gregory.price@memverge.com>]

On Wed, Mar 29, 2023 at 01:20:03PM -0400, Gregory Price wrote:
> 
> Given this system is single-socket, and has only a single CXL memory
> expander connected to it, and that a CFMWS for 0-1fff.. seems
> irrational, I'm going to go with the Range Register is wrong.
> 
> > > confirming this is the issue, i *forced* the dvsec range register to read
> > > out base+0x1050000000
> > 
> > Nice!
> > 
> > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> > > index 7328a2552411..6fc6df0f7b5a 100644
> > > --- a/drivers/cxl/core/pci.c
> > > +++ b/drivers/cxl/core/pci.c
> > > @@ -340,12 +344,16 @@ int cxl_dvsec_rr_decode(struct device *dev, int d,
> > >                 if (rc)
> > >                         return rc;
> > > 
> > >                 base |= temp & CXL_DVSEC_MEM_BASE_LOW_MASK;
> > > 
> > >                 info->dvsec_range[i] = (struct range) {
> > > -                       .start = base,
> > > -                       .end = base + size - 1
> > > +                       .start = 0x1050000000 + base,
> > > +                       .end = 0x1050000000 + base + size - 1
> > >                 };
> > > 
> > > 
> > > And this resulted in everything working "as one would expect"
> > > 
> > > [user@host0 ~]# ls /sys/bus/cxl/devices/
> > > dax_region0  decoder0.0  decoder1.0  endpoint1  mem0  region0  root0
> > > 
> > > [user@host0 ~]# numactl --hardware
> > > available: 2 nodes (0-1)
> > > node 1 cpus:
> > > node 1 size: 0 MB
> > > node 1 free: 0 MB
> > > node distances:
> > > node   0   1
> > >   0:  10  50
> > >   1:  255  10
> > > 
> > > [user@host0 ~]# echo online_movable > /sys/bus/node/devices/node1/memory33/state
> > > [user@host0 ~]# numactl --hardware
> > > available: 2 nodes (0-1)
> > > node 1 size: 2048 MB
> > > node 1 free: 2048 MB
> > > node distances:
> > > node   0   1
> > >   0:  10  50
> > >   1:  255  10
> > > 
> > > 
> > > Basically the question is:
> > > 
> > > Is the DVSEC Range Register expected to be programmed by bios, and are
> > > not being programmed correctly?
> > 
> > This debug experiment makes me think perhaps the *device* is at fault,
> > not the BIOS. Perhaps the device accepts writes to CXL_DVSEC_RANGE_BASE
> > to set up the decode as expected, but reads return 0? That's the only
> > way that I can see that forcing that offset results in successfully
> > talking to memory.
> > 
> 

Wanted to follow this up with some additional information.

The reason this patch / debug test worked is because the platform

1) Does explicitly programs the range register base with 0x0.

and

2) Does address translation on the root complex and puts 0-indexed
   addresses on the wire to the target devices.

So under this platform, one would expect all of the RCD's to have their
range register filled in with a 0x0 base.

This is frustrating and this won't work with the existing driver code,
because the driver code expects the device's to have the correct range
register value set.

The only thing I can think of is adding code here to check whether this
situation is occurring, and then attempting to read information from the
RCH topology to fill in the info->dvsec_range[*] with the correct value.

I'm not really sure what that would look like, nor whether that's a
defensible patch given that the spec wants RR encoded with the HPA, not
a 0-indexed address.

Just food for thought, my frustration is palpable at the moment with
this problem.

~Gregory