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:   Fri, 14 Oct 2022 16:45:39 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Jonathan Cameron <Jonathan.Cameron@huawei.com>
CC:     Dan Williams <dan.j.williams@intel.com>,
        Alison Schofield <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        "Ben Widawsky" <bwidawsk@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        <linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>
Subject: Re: [RFC V2 PATCH 06/11] cxl/mem: Trace DRAM Event Record
Message-ID: <Y0n0o0r85ldlLMqp@iweiny-desk3>
References: <20221010224131.1866246-1-ira.weiny@intel.com>
 <20221010224131.1866246-7-ira.weiny@intel.com>
 <20221011144712.00004a05@huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221011144712.00004a05@huawei.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?wjbgRUhOw9HIAcZPruEwELr5AvCGPkbcu4DcK5qVtJEnuwh+APv3sq8rawR1?=
 =?us-ascii?Q?49HAgqoanYxzh4octmAY4lcksIERwNlLCFyiuhHAYBl6jGF5Lt/+I0UEY6kH?=
 =?us-ascii?Q?pmwqmdDY6wIknfZYV4FAJKAU1UKUpw/FZ3jVf9SLfRGdM7XljQZyWqLQHcoP?=
 =?us-ascii?Q?eGd3Xe1Kr3bKrc/qT4psU2w3qL+jVP4f6j9GLdUBLOGnYCzvYYR2d6gmr5n2?=
 =?us-ascii?Q?45X2BqPuTPa33C/9OyVs8Bt9R7EJxEgC1amiHdTFTUVtVGayN3nnvYyaWKf0?=
 =?us-ascii?Q?3YhraKY79J0o2RhfYmA3+2sUQXVoYPKnEZdQnEv2fmHrjCwJhxRgfd6+iPa0?=
 =?us-ascii?Q?uIDXYmnBQeQFDIRKRFb8u9RYUZP7ZABbDw9Wcfywiqscgtp83RAgFyvIBZzW?=
 =?us-ascii?Q?Qns3O6NFuZRCkq6hmduI5Y52pEDLWvavNAFW3FFwbi6/oh/76SUNZHlzkwiD?=
 =?us-ascii?Q?SOXbVUyJvcoHQUfyAB0Kp6UiCfqC2YmWuEYjDEU334gS77iPnZNxLgcU2bKt?=
 =?us-ascii?Q?JZD/aF5yYwQLH6lcTgbX+hpQzpoZCM46SH6CFr7HgvChB6G0xx7H8W2UvJMS?=
 =?us-ascii?Q?piH8DQZZb1dcjMkqK3mV8taHpOnoQIOYwPffX1iLb0lxa7w0lILbRL0xr5rz?=
 =?us-ascii?Q?+1EYOnzcV/sCVUChqq/EFfyWB02+X7ffoS01Zk5BNlP5f8mL+gsWNf9mO+NS?=
 =?us-ascii?Q?AQpaqz6xHoZCzEZxLOQiEA9WepUillBF3088OgIOIq/dL4m9ekbD8AwUAYz+?=
 =?us-ascii?Q?+NM/xbUgkzACMdAo8EXMKFZPZG0Hvt/MGZH9nQTn3Dl00VGhDMe7n3IblaDd?=
 =?us-ascii?Q?MrQGD05VNUu3CiSogB+6fVQs/mzKgHXCrGlOhKVOKP4o9x5KCZ6iXPBUbnZX?=
 =?us-ascii?Q?TvYcp9H6T6d08KDJBAMwKqDuqGm4kKlaLl17uAaLwY3uMkrGpEsNcVnLHhjB?=
 =?us-ascii?Q?2GoHIBCYtMUyKkzYwcDADjdhmLZbzj1MJronr3T6whaMxX9GlzjnLbFrvGlf?=
 =?us-ascii?Q?O1TdOOhN9X1+c8fyIWIZ3uZoe4Oc516PoBngHm6BWEAh4oZmPmwCiWxuFvD+?=
 =?us-ascii?Q?l18jic/EVybJWTMakME3Omn0ytH+AsDhbDx5ALxAz8+ZxOE6XyF/jB43uaGZ?=
 =?us-ascii?Q?L/uuNimN84rCkKsozmo86eX/vUWN92DZfpXbMVEp8zAHzC6fL1jrmbnjLbzo?=
 =?us-ascii?Q?f723N+S0oIpBTSdcg/jjqoBxcNduNdb9Ds5GW3wvNr9DrszliT4HNuSuBgxB?=
 =?us-ascii?Q?JtQJlJd6EQeGbqB2rKUOtbOXAsv3ZOV1AmVQT213aN34k8S7/UcIT4NnYI9B?=
 =?us-ascii?Q?HSLlblgaHHVMo/AjjmTrDF7c2iTq6qGs5Rz0dYoHUFTUUOe0G2IwMtw9n2qk?=
 =?us-ascii?Q?J2YDS6r2Klcn1p1KzKCb0RxWH4LSw1Bsd53AtDsZGupGA+MEq2UEliTx5fCL?=
 =?us-ascii?Q?PdCIjvvFxrW/tYsKA3krhRxIJpATaBfIJpaa9lPoY2ovv9dE2qvoDz3osrIk?=
 =?us-ascii?Q?c/Tnn6/Is4e5i7yRyxubqjCulg+BcrhRfjx//SjrhHhSkuafEJCXfBRtm8rB?=
 =?us-ascii?Q?oIHji2k8NLO68Tu8AKTId79lTuoiTIW5SD9iKndx?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6037f7f1-34d6-4b5a-14ae-08daae3e35c6
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Oct 2022 23:45:44.1131
 (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: TjKqIyXA81OSGdjudHM5ppKjMH4VuYjlaNqIo4IBhHP/zg+Q2WkpjkWvpLiiorkGTwur1DngKRGp+SPsUDVgEA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6233
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_PASS,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

On Tue, Oct 11, 2022 at 02:47:12PM +0100, Jonathan Cameron wrote:
> On Mon, 10 Oct 2022 15:41:26 -0700
> ira.weiny@intel.com wrote:
> 
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > CXL rev 3.0 section 8.2.9.2.1.2 defines the DRAM Event Record.
> > 
> > Determine if the event read is a DRAM event record and if so trace the
> > record.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > 
> 
> Trivial comments inline
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> > diff --git a/include/trace/events/cxl.h b/include/trace/events/cxl.h
> > index 82a8d3b750a2..7a90cfea348b 100644
> > --- a/include/trace/events/cxl.h
> > +++ b/include/trace/events/cxl.h
> > @@ -230,6 +230,100 @@ TRACE_EVENT(general_media,
> >  		)
> >  );
> >  
> 
> > +
> > +TRACE_EVENT(dram,
> > +
> > +	TP_PROTO(const char *dev_name, enum cxl_event_log_type log,
> > +		 struct cxl_event_dram *rec),
> > +
> > +	TP_ARGS(dev_name, log, rec),
> > +
> > +	TP_STRUCT__entry(
> > +		CXL_EVT_TP_entry
> > +		/* DRAM */
> > +		__field(u64, phys_addr)
> > +		__field(u8, descriptor)
> > +		__field(u8, type)
> > +		__field(u8, transaction_type)
> > +		__field(u8, channel)
> > +		__field(u16, validity_flags)
> > +		__field(u16, column)	/* Out of order to pack trace record */
> > +		__field(u32, nibble_mask)
> > +		__field(u32, row)
> > +		__array(u8, cor_mask, CXL_EVENT_DER_CORRECTION_MASK_SIZE)
> > +		__array(u8, reserved, CXL_EVENT_DER_RES_SIZE)
> 
> If we are going to have this, why not put it at the end?  Will that affect the
> packing badly?

I removed it.

[snip]

> > +
> > +	CXL_EVT_TP_printk("phys_addr=%llx volatile=%s desc='%s' type='%s' " \
> > +		"trans_type='%s' channel=%u rank=%u nibble_mask=%x " \
> > +		"bank_group=%u bank=%u row=%u column=%u cor_mask=%s " \
> > +		"valid_flags='%s' reserved=%s",
> > +		__entry->phys_addr & CXL_GMER_PHYS_ADDR_MASK,
> > +		(__entry->phys_addr & CXL_GMER_PHYS_ADDR_VOLATILE) ? "TRUE" : "FALSE",
> > +		show_event_desc_flags(__entry->descriptor),
> > +		show_mem_event_type(__entry->type),
> > +		show_trans_type(__entry->transaction_type),
> > +		__entry->channel, __entry->rank, __entry->nibble_mask,
> > +		__entry->bank_group, __entry->bank,
> > +		__entry->row, __entry->column,
> > +		__print_hex(__entry->cor_mask, CXL_EVENT_DER_CORRECTION_MASK_SIZE),
> > +		show_dram_valid_flags(__entry->validity_flags),
> > +		__print_hex(__entry->reserved, CXL_EVENT_DER_RES_SIZE)
> > +		)
> Probably one less tab on that trailing )?

Done.

Thanks!
Ira