Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 3 Apr 2023 20:55:35 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Liang Kan <kan.liang@linux.intel.com>,
        <linux-cxl@vger.kernel.org>, <peterz@infradead.org>
CC:     <mingo@redhat.com>, <acme@kernel.org>, <mark.rutland@arm.com>,
        <will@kernel.org>, <dan.j.williams@intel.com>,
        <linuxarm@huawei.com>, <linux-perf-users@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Dave Jiang <dave.jiang@intel.com>
Subject: RE: [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support
Message-ID: <642b9fb7d6986_29cc2945e@dwillia2-xfh.jf.intel.com.notmuch>
References: <20230330164556.31533-1-Jonathan.Cameron@huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230330164556.31533-1-Jonathan.Cameron@huawei.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?01Y4LHe7E5IauOTfKwr7snJvGvc4b5iSlX3DOXXNwBY7AOSmC3fFrbhmPU3r?=
 =?us-ascii?Q?R1WECNcDJ6+orQnusteUqryO/q9wYEf/H0QIJpWsF7YrhHzvXN72v4mSPVbW?=
 =?us-ascii?Q?z5hleBIL3cuM6nRm3sjHLeNTlBW2UOcx8cbXKD5kWBnK3QAl2wuis9g0h+Iz?=
 =?us-ascii?Q?7fN6LoHLu7W6VAwBhQFLm0xHnJNB9J33lA1CNR/cMP+dcaVHN0X1sX4MJTan?=
 =?us-ascii?Q?BYeJ3siWAn+dwltq/jM17hLBS3yoc/qpqjOe0F9LXbMqeZHbGiFiY6Cj+Zv7?=
 =?us-ascii?Q?3n2ayBQ2kR1TbqIMhtnyOn8qBtOW4fCLGsOZ8mjH5A9odDZ6exxBkAXOl+kV?=
 =?us-ascii?Q?62lOJIqa9Xhn50jPPKmen0w9eGfwzi/bb+UcEZN0Lnyylr90fKzwa8Sjiwry?=
 =?us-ascii?Q?gV6CrSYGuCnX7zBtwpj9A57TLPpAt53PR+Q4eAMyHx41jT1zcdw+TUklLK+H?=
 =?us-ascii?Q?EyImDxw8AziAfM+sjEN4HT1yOLT8bpbjW2JywkbOiRe7HaNgYknUvFdEx1WI?=
 =?us-ascii?Q?X2OD9k5KTT7kfNPVWFVJ3Gt03F6VzeY4nmIAPTv0SLKg3qFx349x5ggCYbkm?=
 =?us-ascii?Q?v9QZ3dltiYxLp1aIUTaekerTc99piT05zGxzrLHGPS2DcLZVrY9T6JhPE08Y?=
 =?us-ascii?Q?M2bbpGmkfDamoiXL7JzY9RotucuTBSRJt7AkRAaOaXWrnNI37lMmEi4Btvl6?=
 =?us-ascii?Q?3bCXPNXhuer38lOeWgtmLF7arXnhAepOaF3CZnlap/MhC39OKxnQ1KGICpym?=
 =?us-ascii?Q?NVD1dpVU7RW6mkP1JZUNJpUNRwdvFQC14R3zNmvq13CH82gEcmCc29VWqZyP?=
 =?us-ascii?Q?kFWftGcUzG+Z7E8QrsYRY0BiOHrtukasCRtsjXpaQ5YwnDdE4Heeah58qjVa?=
 =?us-ascii?Q?S0sE6ZDu5tpr7zTfsO9ENxeJzG+n2uld7SSkX2IIqGE/K9j7gNoMaJzkdRxF?=
 =?us-ascii?Q?TivxNnb/fgUJyiy7XSXwImeK6KSZN+I8pJJXdobnNjjlSqKxKiV4w8H0+ZSU?=
 =?us-ascii?Q?EZlTZUzaX+nDF+1gQgUr+EF6LMUSP8qnRBGt1BU1yUfAxH+2dmAWvtQMXxJm?=
 =?us-ascii?Q?6FTn0dRpnuEcSpHTMO6iCHN4mSlnV8B27Js3clDK9Ge9GGZAEkFVMk70NVaB?=
 =?us-ascii?Q?6Lmzh8hrgcU+pvAIlwMr3/7z7XMUteJ9FB/xcjVr2kEOo3z+UhWwr5A5zAXw?=
 =?us-ascii?Q?P0x44paqrvTDRfbBnWQWLTcv1utXGDUapzFCaMyVul+MRaJk2AtfpWNBVpeO?=
 =?us-ascii?Q?luOw7c1YD0alTceB1FxynhzPQavJYLJjXX2rvlzOd6Y8uxncvfqCvgHJGAED?=
 =?us-ascii?Q?42tZ1/uvERstqUlUTsfF1orxFg58yXU3yCz9isrWbK+CP76AcLMH83t+3ydd?=
 =?us-ascii?Q?OQVxNvrhORxDo/QLYlo0u7VaWDfcz6tBLu3ekhly4IGJd+eqKQnNUXA2XCr1?=
 =?us-ascii?Q?EfJZy2JetKqpuhyGXfdox+F1X9QaGUDgro/uEmmP9scBo+2+YuddqrmlI/3K?=
 =?us-ascii?Q?oUYedEKmti141fkgyzCESjyW5tVN1l9507XdLhFfxBnule7jzY1BT7XtG5VK?=
 =?us-ascii?Q?KlB2jsJpYmne8an4dxb4DKXI1R56VYSsH32/jZLO0vFFGVjkSWmP2ljBhtua?=
 =?us-ascii?Q?LQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1904b57f-77b6-4e4d-9275-08db34c073e5
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2023 03:55:38.7628
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: epFiaNWwxa6tMS3k5dQhNS9kr/Rzi59hNQbvt/g2hYrkBAY1yvXHwlgoKsnpXS9+I5SQRhf/D7LVoU1iZasiw8XiqgUc6Pp7DeB11xhOz+0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5439
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,
        SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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
>