mbox series

[NDCTL,v3,0/3] ndctl: Add support of qos_class for CXL CLI

Message ID 170612961495.2745924.4942817284170536877.stgit@djiang5-mobl3
Headers show
Series ndctl: Add support of qos_class for CXL CLI | expand

Message

Dave Jiang Jan. 24, 2024, 8:54 p.m. UTC
Hi Vishal,
With the QoS class series merged to the v6.8 kernel, can you please review and
apply this series to ndctl if acceptable?

v3:
- Rebase against latest ndctl/pending branch.

The series adds support for the kernel enabling of QoS class in the v6.8
kernel. The kernel exports a qos_class token for the root decoders (CFMWS) and as
well as for the CXL memory devices. The qos_class exported for a device is
calculated by the driver during device probe. Currently a qos_class is exported
for the volatile partition (ram) and another for the persistent partition (pmem).
In the future qos_class will be exported for DCD regions. Display of qos_class is
through the CXL CLI list command with -vvv for extra verbose.

A qos_class check as also been added for region creation. A warning is emitted
when the qos_class of a memory range of a CXL memory device being included in
the CXL region assembly does not match the qos_class of the root decoder. Options
are available to suppress the warning or to fail the region creation. This
enabling provides a guidance on flagging memory ranges being used is not
optimal for performance for the CXL region to be formed.

---

Dave Jiang (3):
      ndctl: cxl: Add QoS class retrieval for the root decoder
      ndctl: cxl: Add QoS class support for the memory device
      ndctl: cxl: add QoS class check for CXL region creation


 Documentation/cxl/cxl-create-region.txt |  9 ++++
 cxl/filter.h                            |  4 ++
 cxl/json.c                              | 46 ++++++++++++++++-
 cxl/lib/libcxl.c                        | 62 +++++++++++++++++++++++
 cxl/lib/libcxl.sym                      |  3 ++
 cxl/lib/private.h                       |  3 ++
 cxl/libcxl.h                            | 10 ++++
 cxl/list.c                              |  1 +
 cxl/region.c                            | 67 ++++++++++++++++++++++++-
 util/json.h                             |  1 +
 10 files changed, 204 insertions(+), 2 deletions(-)

--

Comments

Dan Williams Jan. 25, 2024, 9:16 p.m. UTC | #1
Dave Jiang wrote:
> Hi Vishal,
> With the QoS class series merged to the v6.8 kernel, can you please review and
> apply this series to ndctl if acceptable?
> 
> v3:
> - Rebase against latest ndctl/pending branch.
> 
> The series adds support for the kernel enabling of QoS class in the v6.8
> kernel. The kernel exports a qos_class token for the root decoders (CFMWS) and as
> well as for the CXL memory devices. The qos_class exported for a device is
> calculated by the driver during device probe. Currently a qos_class is exported
> for the volatile partition (ram) and another for the persistent partition (pmem).
> In the future qos_class will be exported for DCD regions. Display of qos_class is
> through the CXL CLI list command with -vvv for extra verbose.
> 
> A qos_class check as also been added for region creation. A warning is emitted
> when the qos_class of a memory range of a CXL memory device being included in
> the CXL region assembly does not match the qos_class of the root decoder. Options
> are available to suppress the warning or to fail the region creation. This
> enabling provides a guidance on flagging memory ranges being used is not
> optimal for performance for the CXL region to be formed.
> 
> ---
> 
> Dave Jiang (3):
>       ndctl: cxl: Add QoS class retrieval for the root decoder
>       ndctl: cxl: Add QoS class support for the memory device
>       ndctl: cxl: add QoS class check for CXL region creation
> 
> 
>  Documentation/cxl/cxl-create-region.txt |  9 ++++
>  cxl/filter.h                            |  4 ++
>  cxl/json.c                              | 46 ++++++++++++++++-
>  cxl/lib/libcxl.c                        | 62 +++++++++++++++++++++++
>  cxl/lib/libcxl.sym                      |  3 ++
>  cxl/lib/private.h                       |  3 ++
>  cxl/libcxl.h                            | 10 ++++
>  cxl/list.c                              |  1 +
>  cxl/region.c                            | 67 ++++++++++++++++++++++++-
>  util/json.h                             |  1 +
>  10 files changed, 204 insertions(+), 2 deletions(-)

This needs changes to test/cxl-topology.sh to validate that the
qos_class file pops in the right place per and has prepopulated values
per cxl_test expectation.
Dave Jiang Jan. 25, 2024, 9:40 p.m. UTC | #2
On 1/25/24 14:16, Dan Williams wrote:
> Dave Jiang wrote:
>> Hi Vishal,
>> With the QoS class series merged to the v6.8 kernel, can you please review and
>> apply this series to ndctl if acceptable?
>>
>> v3:
>> - Rebase against latest ndctl/pending branch.
>>
>> The series adds support for the kernel enabling of QoS class in the v6.8
>> kernel. The kernel exports a qos_class token for the root decoders (CFMWS) and as
>> well as for the CXL memory devices. The qos_class exported for a device is
>> calculated by the driver during device probe. Currently a qos_class is exported
>> for the volatile partition (ram) and another for the persistent partition (pmem).
>> In the future qos_class will be exported for DCD regions. Display of qos_class is
>> through the CXL CLI list command with -vvv for extra verbose.
>>
>> A qos_class check as also been added for region creation. A warning is emitted
>> when the qos_class of a memory range of a CXL memory device being included in
>> the CXL region assembly does not match the qos_class of the root decoder. Options
>> are available to suppress the warning or to fail the region creation. This
>> enabling provides a guidance on flagging memory ranges being used is not
>> optimal for performance for the CXL region to be formed.
>>
>> ---
>>
>> Dave Jiang (3):
>>       ndctl: cxl: Add QoS class retrieval for the root decoder
>>       ndctl: cxl: Add QoS class support for the memory device
>>       ndctl: cxl: add QoS class check for CXL region creation
>>
>>
>>  Documentation/cxl/cxl-create-region.txt |  9 ++++
>>  cxl/filter.h                            |  4 ++
>>  cxl/json.c                              | 46 ++++++++++++++++-
>>  cxl/lib/libcxl.c                        | 62 +++++++++++++++++++++++
>>  cxl/lib/libcxl.sym                      |  3 ++
>>  cxl/lib/private.h                       |  3 ++
>>  cxl/libcxl.h                            | 10 ++++
>>  cxl/list.c                              |  1 +
>>  cxl/region.c                            | 67 ++++++++++++++++++++++++-
>>  util/json.h                             |  1 +
>>  10 files changed, 204 insertions(+), 2 deletions(-)
> 
> This needs changes to test/cxl-topology.sh to validate that the
> qos_class file pops in the right place per and has prepopulated values
> per cxl_test expectation.

Do we need to plumb cxl_test to support qos_class with mock functions? Currently cxl_test does not support qos_class under memdev due it not support CDAT, HMAT/SRAT, and any of the PCIe bandwidth/latency attributes. It only has root decoder qos_class exposed.
Dan Williams Jan. 25, 2024, 10:07 p.m. UTC | #3
Dave Jiang wrote:
[..]
> > This needs changes to test/cxl-topology.sh to validate that the
> > qos_class file pops in the right place per and has prepopulated values
> > per cxl_test expectation.
> 
> Do we need to plumb cxl_test to support qos_class with mock functions?
> Currently cxl_test does not support qos_class under memdev due it not support
> CDAT, HMAT/SRAT, and any of the PCIe bandwidth/latency attributes. It only
> has root decoder qos_class exposed. 

Something is need to avoid basic failures like "attribute published at
wrong path", but it does not need to go through full CDAT emulation.

I would do something like the below where ops->cxl_endpoint_parse_cdat()
does not need to actually read any real HMAT or CDAT. All it needs to do
is fake a final cxl_dpa_perf result(s) into the right places to make the
sysfs attribute show up.

Then we can do driver shutdown and access testing with the attribute
code to get lockdep coverage and other basic checkout.

diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index caff3834671f..030b388800f0 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -13,6 +13,7 @@ ldflags-y += --wrap=cxl_hdm_decode_init
 ldflags-y += --wrap=cxl_dvsec_rr_decode
 ldflags-y += --wrap=devm_cxl_add_rch_dport
 ldflags-y += --wrap=cxl_rcd_component_reg_phys
+ldflags-y += --wrap=cxl_endpoint_parse_cdat
 
 DRIVERS := ../../../drivers
 CXL_SRC := $(DRIVERS)/cxl
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index 1a61e68e3095..9133cae1f5c1 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -285,6 +285,19 @@ resource_size_t __wrap_cxl_rcd_component_reg_phys(struct device *dev,
 }
 EXPORT_SYMBOL_NS_GPL(__wrap_cxl_rcd_component_reg_phys, CXL);
 
+void __wrap_cxl_endpoint_parse_cdat(struct cxl_port *port)
+{
+       int index;
+       struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+       struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+
+       if (ops && ops->is_mock_dev(cxlmd->dev.parent))
+               ops->cxl_endpoint_parse_cdat(port);
+       else
+               cxl_endpoint_parse_cdat(port);
+       put_cxl_mock_ops(index);
+}
+
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS(ACPI);
 MODULE_IMPORT_NS(CXL);