Messages in this thread | | | Date | Mon, 3 Apr 2023 20:55:35 -0700 | From | Dan Williams <> | Subject | RE: [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support |
| |
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 >
| |