linux-stable.git/drivers/cxl, branch v6.0

cxl/hdm: Fix skip allocations vs multiple pmem allocations

2022-08-05T23:11:38+00:00

Vishal notes that when attempting to define a second pmem region on a
device the DPA allocation fails with a message of the form:

    decoder11.1: failed to reserve skipped space

Recall that the skip setting is used when there is a pmem allocation in
the presence of free ram DPA space. The first pmem allocation skips over
the free ram and subsequent pmem allocations do not require a skip. The
bug is that a skip is still attempted and the DPA reservation code
flags the double skip allocation conflict.

Fixes: cf880423b6a0 ("cxl/hdm: Add support for allocating DPA to an endpoint decoder")
Reported-by: Vishal Verma 
Tested-by: Vishal Verma 
Reviewed-by: Ira Weiny 
Link: https://lore.kernel.org/r/165973754730.1558392.15466392461645857658.stgit@dwillia2-xfh.jf.intel.com
Signed-off-by: Dan Williams

cxl/region: Disallow region granularity != window granularity

2022-08-05T23:10:04+00:00

The endpoint decode granularity must be <= the window granularity
otherwise capacity in the endpoints is lost in the decode. Consider an
attempt to have a region granularity of 512 with 4 devices within a
window that maps 2 host bridges at a granularity of 256 bytes:

HPA	DPA Offset	HB	Port	EP
0x0	0x0		0	0	0
0x100	0x0		1	0	2
0x200	0x100		0	0	0
0x300	0x100		1	0	2
0x400	0x200		0	1	1
0x500	0x200		1	1	3
0x600	0x300		0	1	1
0x700	0x300		1	1	3
0x800	0x400		0	0	0
0x900	0x400		1	0	2
0xA00	0x500		0	0	0
0xB00	0x500		1	0	2

Notice how endpoint0 maps HPA 0x0 and 0x200 correctly, but then at HPA
0x800 it results in DPA 0x200-0x400 on being skipped.

Fix this by restricing the region granularity to be equal to the window
granularity resulting in the following for a x4 region under a x2 window
at a granularity of 256.

HPA	DPA Offset	HB	Port	EP
0x0	0x0		0	0	0
0x100	0x0		1	0	2
0x200	0x0		0	1	1
0x300	0x0		1	1	3
0x400	0x100		0	0	0
0x500	0x100		1	0	2
0x600	0x100		0	1	1
0x700	0x100		1	1	3

Not that it ever made practical sense to support region granularity >
window granularity. The window rotates host bridges causing endpoints to
never see a consecutive stream of requests at the desired granularity
without breaks to issue cycles to the other host bridge.

Fixes: 80d10a6cee05 ("cxl/region: Add interleave geometry attributes")
Cc: Jonathan Cameron 
Reviewed-by: Vishal Verma 
Reviewed-by: Ira Weiny 
Link: https://lore.kernel.org/r/165973127171.1526540.9923273539049172976.stgit@dwillia2-xfh.jf.intel.com
Signed-off-by: Dan Williams

cxl/region: Fix x1 interleave to greater than x1 interleave routing

2022-08-05T23:10:03+00:00

In cases where the decode fans out as it traverses downstream, the
interleave granularity needs to increment to identify the port selector
bits out of the remaining address bits. For example, recall that with an
x2 parent port intereleave (IW == 1), the downstream decode for children
of those ports will either see address bit IG+8 always set, or address
bit IG+8 always clear. So if the child port needs to select a downstream
port it can only use address bits starting at IG+9 (where IG and IW are
the CXL encoded values for interleave granularity (ilog2(ig) - 8) and
ways (ilog2(iw))).

When the parent port interleave is x1 no such masking occurs and the
child port can maintain the granularity that was routed to the parent
port.

Reported-by: Jonathan Cameron 
Reviewed-by: Vishal Verma 
Reviewed-by: Ira Weiny 
Link: https://lore.kernel.org/r/165973126583.1526540.657948655360009242.stgit@dwillia2-xfh.jf.intel.com
Signed-off-by: Dan Williams

cxl/region: Move HPA setup to cxl_region_attach()

2022-08-05T23:10:03+00:00

A recent bug fix added the setup of the endpoint decoder interleave
geometry settings to cxl_region_attach(). Move the HPA setup there as
well to keep all endpoint decoder parameter setting in a central
location.

For symmetry, move endpoint HPA teardown to cxl_region_detach(), and for
switches move HPA setup / teardown to cxl_port_{setup,reset}_targets().

Cc: Jonathan Cameron 
Signed-off-by: Vishal Verma 
Reviewed-by: Ira Weiny 
Link: https://lore.kernel.org/r/165973126020.1526540.14701949254436069807.stgit@dwillia2-xfh.jf.intel.com
Signed-off-by: Dan Williams

cxl/region: Fix decoder interleave programming

2022-08-05T15:41:19+00:00

Jonathan notes:

"Curiously interleave ways = 1 for the EPs which is obviously wrong"

...while testing the latest CXL development branch on QEMU.

It turns out the region creation process failed to program the endpoint
decoders. This was missed because the default settings of x1 at 4K
intereleave still results in the region appearing to function. Jonathan
caught the bug by reverse mapping the translations that need to happen
for the QEMU support.

Link: https://lore.kernel.org/r/62e95fdf9f6e2_30440294e4@dwillia2-xfh.jf.intel.com.notmuch
Fixes: 384e624bb211 ("cxl/region: Attach endpoint decoders")
Reported-by: Jonathan Cameron 
Link: https://lore.kernel.org/r/165951146336.967013.11160153960900111443.stgit@dwillia2-xfh.jf.intel.com
Acked-by: Jonathan Cameron 
Signed-off-by: Dan Williams

cxl/region: describe targets and nr_targets members of cxl_region_params

2022-08-05T15:41:02+00:00

Sphinx reported undescribed parameters in cxl_region_params struct:

./drivers/cxl/cxl.h:376: warning: Function parameter or member 'targets' not described in 'cxl_region_params'
./drivers/cxl/cxl.h:376: warning: Function parameter or member 'nr_targets' not described in 'cxl_region_params'

Describe these members.

Fixes: b9686e8c8e39 ("cxl/region: Enable the assignment of endpoint decoders to regions")
Signed-off-by: Bagas Sanjaya 
Reviewed-by: Jonathan Cameron 
Link: https://lore.kernel.org/r/20220804075448.98241-3-bagasdotme@gmail.com
Signed-off-by: Dan Williams

cxl/regions: add padding for cxl_rr_ep_add nested lists

2022-08-05T15:41:02+00:00

Sphinx reported indentation warnings:

Documentation/driver-api/cxl/memory-devices:457: ./drivers/cxl/core/region.c:732: WARNING: Unexpected indentation.
Documentation/driver-api/cxl/memory-devices:457: ./drivers/cxl/core/region.c:733: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/driver-api/cxl/memory-devices:457: ./drivers/cxl/core/region.c:735: WARNING: Unexpected indentation.

These warnings above are due to missing blank line padding in the nested list
in kernel-doc comment for cxl_rr_ep_add().

Add the paddings to fix the warnings.

Fixes: 384e624bb211b4 ("cxl/region: Attach endpoint decoders")
Signed-off-by: Bagas Sanjaya 
Reviewed-by: Jonathan Cameron 
Link: https://lore.kernel.org/r/20220804075448.98241-2-bagasdotme@gmail.com
Signed-off-by: Dan Williams

cxl/region: Fix IS_ERR() vs NULL check

2022-08-05T15:41:02+00:00

The nvdimm_pmem_region_create() function returns NULL on error.  It does
not return error pointers.

Fixes: 04ad63f086d1 ("cxl/region: Introduce cxl_pmem_region objects")
Signed-off-by: Dan Carpenter 
Reviewed-by: Jonathan Cameron 
Link: https://lore.kernel.org/r/Yuo65lq2WtfdGJ0X@kili
Signed-off-by: Dan Williams

cxl/region: Fix region reference target accounting

2022-08-05T15:41:02+00:00

Dan reports:

    The error handling in cxl_port_attach_region() looks like it might
    have a similar bug.  The cxl_rr->nr_targets++; might want a --.

    That function is more complicated.

Indeed cxl_rr->nr_targets leaks when cxl_rr_ep_add() fails, but that
flow is not clear. Fix the bug and the clarity by separating the 'new'
region-reference case from the 'extend' region-reference case. This also
moves the host-physical-address (HPA) validation, that the HPA of a new
region being accounted to the port is greater than the HPA of all other
regions associated with the port, to alloc_region_ref().

Introduce @nr_targets_inc to track when the error exit path needs to
clean up cxl_rr->nr_targets.

Fixes: 384e624bb211 ("cxl/region: Attach endpoint decoders")
Reported-by: Dan Carpenter 
Reviewed-by: Ira Weiny 
Link: http://lore.kernel.org/r/165939482134.252363.1915691883146696327.stgit@dwillia2-xfh.jf.intel.com
Signed-off-by: Dan Williams

cxl/region: Fix region commit uninitialized variable warning

2022-08-05T15:41:02+00:00

0day robot reports:

drivers/cxl/core/region.c:196 cxl_region_decode_commit() error: uninitialized symbol 'rc'.

The re-checking of loop termination conditions to determine "success"
makes it hard to see that @rc is initialized in all cases. Remove those
to make it explicit that @rc reflects a commit error and that the rest
of logic is concerned with unwinding committed decoders.

This change potentially results in cxl_region_decode_reset() being
called with @count == 0 where it was not called before, but
cxl_region_decode_reset() treats that as a nop.

Fixes: 176baefb2eb5 ("cxl/hdm: Commit decoder state to hardware")
Reported-by: kernel test robot 
Reviewed-by: Ira Weiny 
Reviewed-by: Jonathan Cameron 
Link: http://lore.kernel.org/r/165951148105.967013.14191992449932268431.stgit@dwillia2-xfh.jf.intel.com
Signed-off-by: Dan Williams