[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20220822180852.000002f9@huawei.com>
Date: Mon, 22 Aug 2022 18:09:12 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: <linuxarm@...wei.com>, <linux-cxl@...r.kernel.org>,
Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>
CC: <john.garry@...wei.com>, Peter Zijlstra <peterz@...radead.org>,
"Ingo Molnar" <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
<linux-perf-users@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
"Alison Schofield" <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
Ira Weiny <ira.weiny@...el.com>,
Ben Widawsky <bwidawsk@...nel.org>,
Dan Williams <dan.j.williams@...el.com>
Subject: Re: [RFC PATCH 2/4] cxl/pci: Find and register CXL PMU devices
On Fri, 12 Aug 2022 16:12:12 +0100
Jonathan Cameron <Jonathan.Cameron@...wei.com> wrote:
> CXL PMU devices can be found from entries in the Register
> Locator DVSEC.
>
> In order to register the minimum number of IRQ vectors necessary
> to support all CPMUs found, separate the registration into two
> steps. First find the devices, and query the IRQs used and then
> register the devices. Between these two steps, request the
> IRQ vectors necessary and enable bus master support.
>
> Future IRQ users for CXL type 3 devices (e.g. DOEs) will need to
> follow a similar pattern the number of vectors necessary is known
> before any parts of the driver stack rely on their availability.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
A few bugs in here that the kernel test robot found.
I'll send a v2 shortly with those tidied up.
One observation is that cxl/core/core.h dereferences
a struct cxl_memdev element which means we have a header ordering
dependency between that and cxl/cxlmem.h which we should find a clean
way to fix.
>
> ---
>
> Open questions
> 1) Is this IRQ vector 'count' / request then use approach flexible
> enough for other interrupt users on these devices.
> 2) Does hanging the CPMU off the PCI device make sense. These can
> occur in Switch upstream ports as well.
> 3) Naming. It would be nice if the device naming indicated which
> EP these were associated with, but that wouldn't be inline
> with the rest of the CXL bus device naming. What is best
> option here?
> ---
> drivers/cxl/core/Makefile | 1 +
> drivers/cxl/core/core.h | 3 ++
> drivers/cxl/core/cpmu.c | 67 ++++++++++++++++++++++++++++++++++
> drivers/cxl/core/port.c | 2 ++
> drivers/cxl/core/regs.c | 29 +++++++++++++++
> drivers/cxl/cpmu.h | 54 ++++++++++++++++++++++++++++
> drivers/cxl/cxl.h | 15 ++++++++
> drivers/cxl/cxlpci.h | 1 +
> drivers/cxl/pci.c | 76 ++++++++++++++++++++++++++++++++++++++-
> 9 files changed, 247 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 79c7257f4107..1318e8a6830f 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -10,4 +10,5 @@ cxl_core-y += memdev.o
> cxl_core-y += mbox.o
> cxl_core-y += pci.o
> cxl_core-y += hdm.o
> +cxl_core-y += cpmu.o
> cxl_core-$(CONFIG_CXL_REGION) += region.o
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 1d8f87be283f..d2b12cdfd61f 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -14,12 +14,14 @@ extern struct device_attribute dev_attr_create_pmem_region;
> extern struct device_attribute dev_attr_delete_region;
> extern struct device_attribute dev_attr_region;
> extern const struct device_type cxl_pmem_region_type;
> +extern const struct device_type cxl_cpmu_type;
> extern const struct device_type cxl_region_type;
> void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled);
> #define CXL_REGION_ATTR(x) (&dev_attr_##x.attr)
> #define CXL_REGION_TYPE(x) (&cxl_region_type)
> #define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
> #define CXL_PMEM_REGION_TYPE(x) (&cxl_pmem_region_type)
> +#define CXL_CPMU_TYPE(x) (&cxl_cpmu_region_type)
> int cxl_region_init(void);
> void cxl_region_exit(void);
> #else
> @@ -37,6 +39,7 @@ static inline void cxl_region_exit(void)
> #define CXL_REGION_TYPE(x) NULL
> #define SET_CXL_REGION_ATTR(x)
> #define CXL_PMEM_REGION_TYPE(x) NULL
> +#define CXL_CPMU_TYPE(x) NULL
> #endif
>
Side note, the presence of
cxl_ep_load() in here means anything including this header needs to include
cxlmem.h which seems a bit of a violation of layering between the core
headers and more driver specific ones.
> struct cxl_send_command;
> diff --git a/drivers/cxl/core/cpmu.c b/drivers/cxl/core/cpmu.c
> new file mode 100644
> index 000000000000..2d17d4083c8f
> --- /dev/null
> +++ b/drivers/cxl/core/cpmu.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2022 Huawei. All rights reserved. */
> +
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/idr.h>
> +#include <cpmu.h>
> +#include <cxl.h>
> +
> +static DEFINE_IDA(cpmu_ida);
> +
> +static void cxl_cpmu_release(struct device *dev)
> +{
> + struct cxl_cpmu *cpmu = container_of(dev, struct cxl_cpmu, dev);
> +
> + ida_free(&cpmu_ida, cpmu->id);
> + kfree(cpmu);
> +}
> +
> +const struct device_type cxl_cpmu_type = {
Should include core/core.h to avoid a should this be static warning.
> + .name = "cxl_cpmu",
> + .release = cxl_cpmu_release,
> +};
> +
> +static void remove_dev(void *dev)
> +{
> + device_del(dev);
> +}
> +
> diff --git a/drivers/cxl/cpmu.h b/drivers/cxl/cpmu.h
> new file mode 100644
> index 000000000000..880077bf0b9f
> --- /dev/null
> +++ b/drivers/cxl/cpmu.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright(c) 2022 Huawei
> + * CXL Specification rev 3.0 Setion 8.2.7 (CPMU Register Interface)
> + */
> +#ifndef CXL_CPMU_H
> +#define CXL_CPMU_H
> +
> +#define CPMU_CAP_REG 0x0
> +#define CPMU_CAP_NUM_COUNTERS_MSK GENMASK_ULL(4, 0)
> +#define CPMU_CAP_COUNTER_WIDTH_MSK GENMASK_ULL(15, 8)
> +#define CPMU_CAP_NUM_EVN_CAP_REG_SUP_MSK GENMASK_ULL(24, 20)
> +#define CPMU_CAP_FILTERS_SUP_MSK GENMASK_ULL(39, 32)
> +#define CPMU_CAP_MSI_N_MSK GENMASK_ULL(47, 44)
> +#define CPMU_CAP_WRITEABLE_WHEN_FROZEN BIT(48)
> +#define CPMU_CAP_FREEZE BIT(49)
> +#define CPMU_CAP_INT BIT(50)
BIT_ULL() needed for these.
> +#define CPMU_CAP_VERSION_MSK GENMASK_ULL(63, 60)
> +
> +#define CPMU_OVERFLOW_REG 0x10
> +#define CPMU_FREEZE_REG 0x18
> +#define CPMU_EVENT_CAP_REG(n) (0x100 + 8 * (n))
> +#define CPMU_EVENT_CAP_SUPPORTED_EVENTS_MSK GENMASK_ULL(31, 0)
> +#define CPMU_EVENT_CAP_GROUP_ID_MSK GENMASK_ULL(47, 32)
> +#define CPMU_EVENT_CAP_VENDOR_ID_MSK GENMASK_ULL(63, 48)
> +
> +#define CPMU_COUNTER_CFG_REG(n) (0x200 + 8 * (n))
> +#define CPMU_COUNTER_CFG_TYPE_MSK GENMASK_ULL(1, 0)
> +#define CPMU_COUNTER_CFG_TYPE_FREE_RUN 0
> +#define CPMU_COUNTER_CFG_TYPE_FIXED_FUN 1
> +#define CPMU_COUNTER_CFG_TYPE_CONFIGURABLE 2
> +#define CPMU_COUNTER_CFG_ENABLE BIT(8)
> +#define CPMU_COUNTER_CFG_INT_ON_OVRFLW BIT(9)
> +#define CPMU_COUNTER_CFG_FREEZE_ON_OVRFLW BIT(10)
> +#define CPMU_COUNTER_CFG_EDGE BIT(11)
> +#define CPMU_COUNTER_CFG_INVERT BIT(12)
> +#define CPMU_COUNTER_CFG_THRESHOLD_MSK GENMASK_ULL(23, 16)
> +#define CPMU_COUNTER_CFG_EVENTS_MSK GENMASK_ULL(55, 24)
> +#define CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK GENMASK_ULL(63, 59)
> +
> +#define CPMU_FILTER_CFG_REG(n, f) (0x400 + 4 * ((f) + (n) * 8))
> +#define CPMU_FILTER_CFG_VALUE_MSK GENMASK(15, 0)
> +
> +#define CPMU_COUNTER_REG(n) (0xc00 + 8 * (n))
> +
> +#define CPMU_REGMAP_SIZE 0xe00 /* Table 8-32 CXL 3.0 specification */
> +struct cxl_cpmu {
> + struct device dev;
> + void __iomem *base;
> + int id;
> +};
> +
> +#define to_cxl_cpmu(dev) container_of(dev, struct cxl_cpmu, dev)
> +#endif
Powered by blists - more mailing lists