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:   Sun, 4 Dec 2022 14:42:10 -0800
From:   Dan Williams <dan.j.williams@intel.com>
To:     <alison.schofield@intel.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Ira Weiny <ira.weiny@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        Ben Widawsky <bwidawsk@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>
CC:     Alison Schofield <alison.schofield@intel.com>,
        <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3 1/6] trace, cxl: Introduce a TRACE_EVENT for CXL
 poison records
Message-ID: <638d224226ebf_c957294fa@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <cover.1668115235.git.alison.schofield@intel.com>
 <5746274c905f57f117987c8268c9f6dc9bd33337.1668115235.git.alison.schofield@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <5746274c905f57f117987c8268c9f6dc9bd33337.1668115235.git.alison.schofield@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?aZfvwuqLcuGeujnWKPqUMdbDSTIcydqfS71NTn05xnRX7eXbkVYwxRcDgNYZ?=
 =?us-ascii?Q?SEe2iSARqPUEzuRgc/NcyaHxSQjXX1sPJFqDl8GPOIQdx0QgzPIlwFmR+3qk?=
 =?us-ascii?Q?5o7j8bildrPM5NGDq0nMvePIxY/3qSlAiLct9bphfqPyWYX+I4QUlnHnVe+o?=
 =?us-ascii?Q?n9Hkx2x1d+aVn4AGnDmIILw8szEXnPliTYt4VzqkZFJNRPt6pxBEHlJFDJit?=
 =?us-ascii?Q?nuaj66IWhT3RsMxjALjVZegRDtASdz6tDcrdbW9LcVdDpHYHhlra/3cCEaMF?=
 =?us-ascii?Q?C1PeP8SXirMJl84ENL5EZhYeUYHo/1QfYSSviq3v1c7duj8JtSMPKiZSZW4r?=
 =?us-ascii?Q?qe22GE9pOIcgTDN0Ik5bPFmxZRY1Hcyej/DfCFgHhhiz6gr4zYAPo9N+detn?=
 =?us-ascii?Q?gatXboDk2oPj5OVlEmUiC6UTqR7vc8xadvisodk5M9bQDknNvBfm+GbTeUaa?=
 =?us-ascii?Q?CKVJ1IR++kPy28x8NfATiT/dU1hXZxqjdMQbDsQeQKNN79OJ8renzno9Wrip?=
 =?us-ascii?Q?OLYGrmiFIA37Hp43S1qkqkWposcd7p5HC9/xRerMqxWebsLEBPM3hBqtagVh?=
 =?us-ascii?Q?f2HWkNwdjkCx1mFcGqx3uf13Ar2wVvchxmKekR4fJ1m6qhbmQ78PL5Gxxs43?=
 =?us-ascii?Q?mJOiu7bmZ0lcNp8iJ1Tn0Aqgji4AzMiYhBZ8gnNp7R3TshjJ7t3pJQ5vzBTF?=
 =?us-ascii?Q?Bcbsw6pKifyPRFhGtc76R9IOHOzsrFAJZjIFqi7/nOckiuC6b3/FrudcWrJa?=
 =?us-ascii?Q?2BGW14mzvCiLeF2hnTmAG+eelIWN28WTddtOA9RNiq5umKCQljyofguVKM+/?=
 =?us-ascii?Q?+gVP5kiIwO0xJjR4+xbCCjVo02LRiwuwKCjzY57UokvdTa5LLnTtqIwdpEEZ?=
 =?us-ascii?Q?gQe+nh67hrEfFDFkYTFRmn8JWqtBgrEpJDhgKmLzwNx3WItGr6YJxY9OGwMl?=
 =?us-ascii?Q?vsQ5GJ4UHVwV2YVOmLACCZDKNB6LQUv0cpo/2AYrlvOl8E2fw6HUmajw/EmP?=
 =?us-ascii?Q?N+gDbzcC/z35Oh6IkEo86PiDPIA3TC7D7DiijwjZl4gF7JZjSDImuQHetfCN?=
 =?us-ascii?Q?Rb215WslpL94BVKtf0HzubYUP78/1IaHSiVtE4D8nsQm0PTeOvZTlvLqjcB4?=
 =?us-ascii?Q?+iIcBe3kfrgC4rVUMMHrA0A6KnCHqIBl6nbvWIrNLDa7fcdne0oZp3muB7Hw?=
 =?us-ascii?Q?WD4sDr02FsC0FhxA0LUoPnjl/vD5RWrIWS3fvHu/3K2ob6rlfKYwsMXngzXL?=
 =?us-ascii?Q?eod846CK6j/4xzHuSD2WuRalmQXYgITVWmgKAUZ7uQmWLpFWHf2n+E5DhPbv?=
 =?us-ascii?Q?4Ie21FrfZuoyOUELZKDkBpBzFkhT4QxvtWZPr1fbEAnTcH1W/XfnrIyiBblg?=
 =?us-ascii?Q?VzQNcFEgZ1p0TcxxyIZ2MPdwEZQ8gFJttAp+Bb01AG+6sQotx6US3zslkETo?=
 =?us-ascii?Q?dlmik+SFw2qUfAFPGPbUPmAKj5xh6G2Xr4Q8BmvaiUg2LGGJYXX0g102hHMM?=
 =?us-ascii?Q?98aJKoUnRZPpnh22J2rJWnEHwsO3/4pmForpj0w42Kj+Zs6Z7ikMoBmqZGSi?=
 =?us-ascii?Q?qHdgsdEbf8FtgGL6Ip68eCa656EEOn31kwFScMWwDgwTeVdpHGHfaU46LLBU?=
 =?us-ascii?Q?BQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: d79584b9-ecdd-4fdb-33db-08dad648ce41
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Dec 2022 22:42:21.4746
 (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: 8jBEZ9dHEEnXXXs7eV9dYAgcXVPUvjQesQp6YUpUqjqYm1lJEefqB/2U2L/SMEvccmlp42la4MPiU+DE28OXqckps2A8VWX4ZY5+AvxiQyA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5565
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        SPF_HELO_NONE,SPF_NONE autolearn=ham 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

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> CXL devices may support the retrieval of a device poison list.
> Introduce a trace event that the CXL subsystem can use to log
> the poison error records.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  drivers/cxl/cxlmem.h       | 14 +++++++
>  include/trace/events/cxl.h | 80 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
>  create mode 100644 include/trace/events/cxl.h
> 
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 88e3a8e54b6a..669868cc1553 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -347,6 +347,20 @@ struct cxl_mbox_set_partition_info {
>  
>  #define  CXL_SET_PARTITION_IMMEDIATE_FLAG	BIT(0)
>  
> +/* Get Poison List  CXL 3.0 Spec 8.2.9.8.4.1 */
> +
> +/* Get Poison List: Payload out flags */
> +#define CXL_POISON_FLAG_MORE            BIT(0)
> +#define CXL_POISON_FLAG_OVERFLOW        BIT(1)
> +#define CXL_POISON_FLAG_SCANNING        BIT(2)
> +
> +/* Get Poison List: Poison Source */
> +#define CXL_POISON_SOURCE_UNKNOWN	0
> +#define CXL_POISON_SOURCE_EXTERNAL	1
> +#define CXL_POISON_SOURCE_INTERNAL	2
> +#define CXL_POISON_SOURCE_INJECTED	3
> +#define CXL_POISON_SOURCE_VENDOR	7
> +
>  /**
>   * struct cxl_mem_command - Driver representation of a memory device command
>   * @info: Command information as it exists for the UAPI
> diff --git a/include/trace/events/cxl.h b/include/trace/events/cxl.h
> new file mode 100644
> index 000000000000..03428125573f
> --- /dev/null
> +++ b/include/trace/events/cxl.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM cxl
> +
> +#if !defined(_CXL_TRACE_H) ||  defined(TRACE_HEADER_MULTI_READ)
> +#define _CXL_TRACE_H
> +
> +#include <linux/tracepoint.h>
> +#include <cxlmem.h>
> +
> +#define __show_poison_source(source)                          \
> +	__print_symbolic(source,                              \
> +		{ CXL_POISON_SOURCE_UNKNOWN,   "Unknown"  },  \
> +		{ CXL_POISON_SOURCE_EXTERNAL,  "External" },  \
> +		{ CXL_POISON_SOURCE_INTERNAL,  "Internal" },  \
> +		{ CXL_POISON_SOURCE_INJECTED,  "Injected" },  \
> +		{ CXL_POISON_SOURCE_VENDOR,    "Vendor"   })
> +
> +#define show_poison_source(source)			     \
> +	(((source > CXL_POISON_SOURCE_INJECTED) &&	     \
> +	 (source != CXL_POISON_SOURCE_VENDOR)) ? "Reserved"  \
> +	 : __show_poison_source(source))
> +
> +#define show_poison_flags(flags)                             \
> +	__print_flags(flags, "|",                            \
> +		{ CXL_POISON_FLAG_MORE,      "More"     },   \
> +		{ CXL_POISON_FLAG_OVERFLOW,  "Overflow"  },  \
> +		{ CXL_POISON_FLAG_SCANNING,  "Scanning"  })
> +
> +TRACE_EVENT(cxl_poison,
> +
> +	    TP_PROTO(const char *memdev, const char *pcidev, const char *region,

Same feedback as on the Events patch, make these arguments a 'struct
device *', 'struct pci_dev *', and 'struct cxl_region *' to maximize
type-safety. Otherwise you could do something like pass "region0",
"mem1", and "0000:64:09.0" in the wrong order and the compiler cannot
help prevent that.

> +		     const uuid_t *uuid, u64 dpa, u32 length, u8 source,
> +		     u8 flags, u64 overflow_t),
> +
> +	    TP_ARGS(memdev, pcidev, region, uuid, dpa, length, source,
> +		    flags, overflow_t),
> +
> +	    TP_STRUCT__entry(
> +		__string(memdev, memdev)
> +		__string(pcidev, pcidev)
> +		__string(region, region ? region : "")
> +		__array(char, uuid, 16)
> +		__field(u64, dpa)
> +		__field(u32, length)
> +		__field(u8, source)
> +		__field(u8, flags)
> +		__field(u64, overflow_t)
> +	    ),
> +
> +	    TP_fast_assign(
> +		__assign_str(memdev, memdev);
> +		__assign_str(pcidev, pcidev);
> +		__assign_str(region, region ? region : "");
> +		if (uuid)
> +			memcpy(__entry->uuid, uuid, 16);
> +		__entry->dpa = dpa;
> +		__entry->length = length;
> +		__entry->source = source;
> +		__entry->flags = flags;
> +		__entry->overflow_t = overflow_t;
> +	    ),
> +
> +	    TP_printk("memdev=%s pcidev=%s region=%s region_uuid=%pU dpa=0x%llx length=0x%x source=%s flags=%s overflow_time=%llu",
> +		__get_str(memdev),
> +		__get_str(pcidev),
> +		__get_str(region),
> +		__entry->uuid,
> +		__entry->dpa,
> +		__entry->length,
> +		show_poison_source(__entry->source),
> +		show_poison_flags(__entry->flags),
> +		__entry->overflow_t)
> +);
> +#endif /* _CXL_TRACE_H */
> +
> +/* This part must be outside protection */
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE cxl
> +#include <trace/define_trace.h>
> -- 
> 2.37.3
>