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, 11 Dec 2022 08:21:26 -0800
From:   Ira Weiny <ira.weiny@intel.com>
To:     Dan Williams <dan.j.williams@intel.com>
CC:     Bjorn Helgaas <bhelgaas@google.com>,
        Alison Schofield <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Dave Jiang <dave.jiang@intel.com>,
        <linux-kernel@vger.kernel.org>, <linux-pci@vger.kernel.org>,
        <linux-acpi@vger.kernel.org>, <linux-cxl@vger.kernel.org>
Subject: Re: [PATCH V3 4/8] cxl/mem: Trace DRAM Event Record
Message-ID: <Y5YDhiDt3yeNrnGS@iweiny-mobl>
References: <20221208052115.800170-1-ira.weiny@intel.com>
 <20221208052115.800170-5-ira.weiny@intel.com>
 <6393b35111f0_579c12947f@dwillia2-xfh.jf.intel.com.notmuch>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <6393b35111f0_579c12947f@dwillia2-xfh.jf.intel.com.notmuch>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?o9Mz3WAZNBlGvvWZSF0bmdt5pSsAgxHs3ts6F3+ub3u07MhfHJ58QEw/qy+e?=
 =?us-ascii?Q?oyYPbDPAru4hR8NtPrNrHGpcNWjHTCk+LtsvRk61iAMXkgJ+DgmPM+Eyw3hO?=
 =?us-ascii?Q?tVJM/zigP0+3v4GHFr87TkDxyHagxsNOyrRmHbfPIBeAbpbf4OnzrIMDDNkj?=
 =?us-ascii?Q?qCJy6ndb8bNSewJHpZ+ZA/F//AHRCRN2eCYDtQwT2MAwanU1+2pWH59X5gXC?=
 =?us-ascii?Q?j3icMGFx0Fp4BcSjO5bXt49ePx2o2ItH5KLbT11qxZ88ck5eJ6i5Z8yuknzr?=
 =?us-ascii?Q?yStrhOwfVq15gAuAymQDv7X6TLJAuiBcIMzH8xoUeJivL+U9c71n0XaYq7ec?=
 =?us-ascii?Q?sI+FjcOoyzMZxwKkgUpWwNOu0a+fJTp9TEKCKXixiQVstdck0t7rdfIyC1rI?=
 =?us-ascii?Q?cG9ozhUdNTwD0rPSAly0OfAekMLXkrFKlBeFaTo4bFjt02UTJrBaFqvvgV53?=
 =?us-ascii?Q?dgVmU0hW/Dqx2OVVcdwQNjJMOWEf8sTI8gN1EVCPLIqCY0rjmc8lALDA86Lx?=
 =?us-ascii?Q?9XMAPVBXtSYhpFA9qm1rjrCDBh4r+btsAW2VCt1YW/oBuPBFE3jecjPeQYEW?=
 =?us-ascii?Q?z9l8mpfQQu2V9ta2eGSC0RUgHUriy0k27wNsex+BcE9yVN4U4wLDM6J9IvaY?=
 =?us-ascii?Q?cuPebe7xbz1+8ebcHq8W76xmdXFcpB7IEbm74VErCzb6gYkGePwFpkORMZpD?=
 =?us-ascii?Q?okgORZ1ftsExpHcgPANoXN79etlPnnFIIfcRtCl/FYLX69YA2b/DpPWcLvCt?=
 =?us-ascii?Q?aCyti34zKH6hPPhrKljmyTbAltO1fh4peXK/rSP7S6ExNe9PvjvXcrwYWroX?=
 =?us-ascii?Q?ucZWtAqet0uf1XDXmSaiYGy9i6yMHevE11YsSy+gnQqcxqTIM7RY0wmBB347?=
 =?us-ascii?Q?A2S59IgMwtVGcvuEcvjjQL3GmEHuf/WMHUpfP6T8XzcienZC2KBC7Omggxwj?=
 =?us-ascii?Q?PECER8N7XaGAwHavoreR3bdE/tz0s6tb2k5rbcJERN56LhT/tXbTtEJ9ZL4s?=
 =?us-ascii?Q?2ZROnNijIekziZ0IsrWrL4IRmED5yOJrm+BbarxcprTioQ5UCgyF/flaK/P+?=
 =?us-ascii?Q?yZmdIG+zVX7QUk2//Z45POiH7iG3hu2kZe6s6ewVAQYgNqiDQcGOgzOf3psT?=
 =?us-ascii?Q?FDU+UDNhYarjpDdLluun8xz3B+oTu5sRwCLS39W4MZOtIOTRZ/qDzeuo/IWP?=
 =?us-ascii?Q?9BAo8qm02sOhZJrfsdE/tuZGppthWBbXBPqSW2VB0Hiy3LE6mohWAJPyhZlZ?=
 =?us-ascii?Q?LJfUxPTOVNXJ6xjktbSoFR8qj/gRLSw6pee6MULPOvmmO1U9F3MrTgwJMLA9?=
 =?us-ascii?Q?/sfJPYN+ewOin9Cmi4h4geVWv1Cg56rC2SVooHBVxYhNgarHxSDRwA3Sa49i?=
 =?us-ascii?Q?3bteOz6s8ptclS8J7i7Wn3hpaaPKsFArI3XVl0hR3G4EtKbUPLXBHZtVhXGo?=
 =?us-ascii?Q?vqkZOBL9SfPKQU9CFbAfd8QHxWTsriTFIEhZSZysf3E645KnP9VOBNRE6Ves?=
 =?us-ascii?Q?GsA1ZJYVre9OwZkOTYabjq6OBhap4aRgegqFsBgmW2qIRYPeNuOlSUU8k92v?=
 =?us-ascii?Q?2VFN5cX4q2huDCyqI/6wKJ3z0WhKVQxq+CT1xWyW?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 466c2017-6f70-4906-7be1-08dadb93c35f
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Dec 2022 16:21:31.1930
 (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: rrCBAeCSNAJR+htbqvTzEEriZr4MVZQwOi/SyFbmcP9CoZe63zs84+CCuOH7mAnRU8uDCkwJWlP3n2tU1mEl4w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5321
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,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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

On Fri, Dec 09, 2022 at 02:14:41PM -0800, Dan Williams wrote:
> ira.weiny@ 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>
> > 
> > ---
> > Changes from v2:
> > 	Dan
> > 		Move tracing to cxl core
> > 		Remove trace_*_enabled() calls
> > 		Pass struct device to trace points
> > 
> > Changes from RFC v2:
> > 	Output DPA flags as a separate field.
> > 	Ensure field names match TP_print output
> > 	Steven
> > 		prefix TRACE_EVENT with 'cxl_'
> > 	Jonathan
> > 		Formatting fix
> > 		Remove reserved field
> > 
> > Changes from RFC:
> > 	Add reserved byte data
> > 	Use new CXL header macros
> > 	Jonathan
> > 		Use get_unaligned_le{24,16}() for unaligned fields
> > 		Use 'else if'
> > 	Dave Jiang
> > 		s/cxl_dram_event/dram
> > 		s/cxl_evt_dram_rec/cxl_event_dram
> > 	Adjust for new phys addr mask
> > ---
> >  drivers/cxl/core/mbox.c  | 13 ++++++
> >  drivers/cxl/core/trace.h | 92 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/cxl/cxlmem.h     | 23 ++++++++++
> >  3 files changed, 128 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > index 0d8c66f1cdc5..2fa4645f0ed9 100644
> > --- a/drivers/cxl/core/mbox.c
> > +++ b/drivers/cxl/core/mbox.c
> > @@ -726,6 +726,14 @@ static const uuid_t gen_media_event_uuid =
> >  	UUID_INIT(0xfbcd0a77, 0xc260, 0x417f,
> >  		  0x85, 0xa9, 0x08, 0x8b, 0x16, 0x21, 0xeb, 0xa6);
> >  
> > +/*
> > + * DRAM Event Record
> > + * CXL rev 3.0 section 8.2.9.2.1.2; Table 8-44
> > + */
> > +static const uuid_t dram_event_uuid =
> > +	UUID_INIT(0x601dcbb3, 0x9c06, 0x4eab,
> > +		  0xb8, 0xaf, 0x4e, 0x9b, 0xfb, 0x5c, 0x96, 0x24);
> > +
> >  static void cxl_trace_event_record(const struct device *dev,
> >  				   enum cxl_event_log_type type,
> >  				   struct cxl_event_record_raw *record)
> > @@ -738,6 +746,11 @@ static void cxl_trace_event_record(const struct device *dev,
> >  
> >  		trace_cxl_general_media(dev, type, rec);
> >  		return;
> > +	} else if (uuid_equal(id, &dram_event_uuid)) {
> > +		struct cxl_event_dram *rec = (struct cxl_event_dram *)record;
> > +
> > +		trace_cxl_dram(dev, type, rec);
> > +		return;
> 
> I think I mentioned this before,

Sorry, I don't remember seeing that.

>
> but rather than a "return" in every
> branch just make the 'unknown' case the final else in this if block.

Sounds good.  However the previous patch started this pattern so I fixed it
there and continued through these.

> 
> With that feel free to add:
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Thanks!
Ira