RE: [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support

[Dan Williams]

Jonathan Cameron wrote:
> Changes since v3: Kan Liang gave feedback on v2 incorporated here.
> - All updates in patch 4, see details there.
> 
> Updated cover letter.
> 
> The CXL rev 3.0 specification introduces a CXL Performance Monitoring
> Unit definition. CXL components may have any number of these blocks. The
> definition is highly flexible, but that does bring complexity in the
> driver.
> 
> Notes.
> 
> 1) The QEMU model against which this was developed needs tidying up.
>    Latest tree at https://gitlab.com/jic23/qemu branch cxl-2023-02-28
>    It's backed up behind other series that I plan to upstream first.
> 2) There are quite a lot of corner cases that will need working through
>    with variants of the model, or I'll have to design a pathological
>    set of CPMUs to hit all the corner cases in one go. So whilst I believe
>    the driver should be fine for what it supports we may find issues
>    as those corners of what is allowed are explored.
> 3) I'm not sure it makes sense to hang this of the cxl/pci driver but
>    couldn't really figure out where else in the current structure we could
>    make it fit cleanly.
> 4) Driver location. In past perf maintainers have requested perf drivers
>    for PCI etc be under drivers/perf. That would require moving some
>    CXL headers to be more generally visible, but is certainly possible
>    if there is agreement between CXL and perf maintainers on the correct
>    location.

I am ok the bulk of the logic living under drivers/perf/

Are drivers/perf/ folks ok with a:

CFLAGS_cxl.o := -I$(srctree)/drivers/cxl/

...or similar in their Makefile, because I don't think this by itself is
a reason to push CXL headers to include/.

I say this without having looked at the code yet and whether this driver
will need new exports from the cxl/core etc.

> 5) Patch 1 led to a discussion of how to handle the self describing
>    and extensible nature of the CPMU counters.  That is likely to involve
>    a description in the "caps" sysfs directory and perf tool code that is
>    aware of the different event combinations that make sense and can
>    establish which sets are available on a given device.
>    That is likely to take a while to converge on, so as the driver is useful
>    in the current state, I'm looking to upstream this first and deal with
>    the more complex handling later.

What's "this" in this last sentence, a canned set of base counters?

> 6) There is a lot of other functionality that can be added in future
>    include allowing for simpler hardware implementations that may not
>    support the minimum level of features currently required by the driver
>    (freeze, overflow interrupts etc).

Curious if the the minimal set determined by the specification, like the
minimal features a CXL Memory Expander device must implement, or by what
you decided was fit to be emulated in QEMU?

> 
> CXL rev 3.0 specification available from https://www.computeexpresslink.org
> 
> 
> Jonathan Cameron (5):
>   cxl: Add function to count regblocks of a given type
>   perf: Allow a PMU to have a parent
>   cxl/pci: Find and register CXL PMU devices
>   cxl: CXL Performance Monitoring Unit driver
>   docs: perf: Minimal introduction the the CXL PMU device and driver
> 
>  Documentation/admin-guide/perf/cxl.rst   |  65 ++
>  Documentation/admin-guide/perf/index.rst |   1 +
>  drivers/cxl/Kconfig                      |  13 +
>  drivers/cxl/Makefile                     |   1 +
>  drivers/cxl/core/Makefile                |   1 +
>  drivers/cxl/core/core.h                  |   1 +
>  drivers/cxl/core/cpmu.c                  |  72 ++
>  drivers/cxl/core/pci.c                   |   2 +-
>  drivers/cxl/core/port.c                  |   4 +-
>  drivers/cxl/core/regs.c                  |  50 +-
>  drivers/cxl/cpmu.c                       | 940 +++++++++++++++++++++++
>  drivers/cxl/cpmu.h                       |  56 ++
>  drivers/cxl/cxl.h                        |  17 +-
>  drivers/cxl/cxlpci.h                     |   1 +
>  drivers/cxl/pci.c                        |  27 +-
>  include/linux/perf_event.h               |   1 +
>  kernel/events/core.c                     |   1 +
>  17 files changed, 1245 insertions(+), 8 deletions(-)
>  create mode 100644 Documentation/admin-guide/perf/cxl.rst
>  create mode 100644 drivers/cxl/core/cpmu.c
>  create mode 100644 drivers/cxl/cpmu.c
>  create mode 100644 drivers/cxl/cpmu.h
> 
> -- 
> 2.37.2
>