linux-kernel - Re: [RFC V2 PATCH 07/11] cxl/mem: Trace Memory Module Event Record

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221011145457.00001e20@huawei.com>
Date:   Tue, 11 Oct 2022 14:54:57 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     <ira.weiny@...el.com>
CC:     Dan Williams <dan.j.williams@...el.com>,
        Alison Schofield <alison.schofield@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        "Ben Widawsky" <bwidawsk@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        <linux-kernel@...r.kernel.org>, <linux-cxl@...r.kernel.org>
Subject: Re: [RFC V2 PATCH 07/11] cxl/mem: Trace Memory Module Event Record

On Mon, 10 Oct 2022 15:41:27 -0700
ira.weiny@...el.com wrote:

> From: Ira Weiny <ira.weiny@...el.com>
> 
> CXL rev 3.0 section 8.2.9.2.1.3 defines the Memory Module Event Record.
> 
> Determine if the event read is memory module record and if so trace the
> record.
> 
> Signed-off-by: Ira Weiny <ira.weiny@...el.com>
> 
A few trivial comments inline. I'm happy either way
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>


> +#define show_add_status(as) __print_symbolic(as,	   \
> +	{ CXL_DHI_AS_NORMAL,		"Normal"	}, \
> +	{ CXL_DHI_AS_WARNING,		"Warning"	}, \
> +	{ CXL_DHI_AS_CRITICAL,		"Critical"	}  \
> +)
> +
> +#define CXL_DHI_AS_LIFE_USED(as)			(as & 0x3)
> +#define CXL_DHI_AS_DEV_TEMP(as)				((as & 0xC) >> 2)
> +#define CXL_DHI_AS_COR_VOL_ERR_CNT(as)			((as & 0x10) >> 4)
> +#define CXL_DHI_AS_COR_PER_ERR_CNT(as)			((as & 0x20) >> 5)
>

> +	),
> +
> +	CXL_EVT_TP_printk("evt_type='%s' health_status='%s' media_status='%s' " \
> +		"as_life_used=%s as_dev_temp=%s as_cor_vol_err_cnt=%s " \
> +		"as_cor_per_err_cnt=%s life_used=%u dev_temp=%d " \
> +		"dirty_shutdown_cnt=%u cor_vol_err_cnt=%u cor_per_err_cnt=%u " \
> +		"reserved=%s",
> +		show_dev_evt_type(__entry->event_type),
> +		show_health_status_flags(__entry->health_status),
> +		show_media_status(__entry->media_status),
> +		show_add_status(CXL_DHI_AS_LIFE_USED(__entry->add_status)),
> +		show_add_status(CXL_DHI_AS_DEV_TEMP(__entry->add_status)),
> +		show_add_status(CXL_DHI_AS_COR_VOL_ERR_CNT(__entry->add_status)),
> +		show_add_status(CXL_DHI_AS_COR_PER_ERR_CNT(__entry->add_status)),
A little nasty to use same show_add_status() for the 2 bit and 1 bit versions.
Obviously it works, but maybe it's worth two macros for the 1 and 2 bit version?

> +		__entry->life_used, __entry->device_temp,
> +		__entry->dirty_shutdown_cnt, __entry->cor_vol_err_cnt,
> +		__entry->cor_per_err_cnt,
> +		__print_hex(__entry->reserved, CXL_EVENT_MEM_MOD_RES_SIZE)
> +		)
Aligned one tab too far?