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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241022175827.000033ef@Huawei.com>
Date: Tue, 22 Oct 2024 17:58:27 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: <shiju.jose@...wei.com>
CC: <dave.jiang@...el.com>, <dan.j.williams@...el.com>,
	<alison.schofield@...el.com>, <vishal.l.verma@...el.com>,
	<ira.weiny@...el.com>, <dave@...olabs.net>, <linux-cxl@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
	<tanxiaofei@...wei.com>, <prime.zeng@...ilicon.com>
Subject: Re: [PATCH v2 2/6] cxl/events: Add Component Identifier formatting
 for CXL spec rev 3.1

On Tue, 22 Oct 2024 11:58:45 +0100
<shiju.jose@...wei.com> wrote:

> From: Shiju Jose <shiju.jose@...wei.com>
> 
> Add Component Identifier formatting for CXL spec rev 3.1, Section
> 8.2.9.2.1, Table 8-44.
> 
> Signed-off-by: Shiju Jose <shiju.jose@...wei.com>
> ---
>  drivers/cxl/core/trace.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 7305974e2301..81908072c6f3 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -268,6 +268,26 @@ TRACE_EVENT(cxl_generic_event,
>  	{ CXL_DPA_NOT_REPAIRABLE,		"NOT_REPAIRABLE"	}  \
>  )
>  
> +/*
> + * Component ID Format
> + * CXL 3.1 section 8.2.9.2.1; Table 8-44
> + */
> +#define CXL_PLDM_COMPONENT_ID_ENTITY_VALID	BIT(0)
> +#define CXL_PLDM_COMPONENT_ID_RES_VALID		BIT(1)
> +
> +#define comp_id_pldm_flags(flags)  __print_flags(flags, " | ",			\
> +	{ CXL_PLDM_COMPONENT_ID_ENTITY_VALID,   "PLDM_Entity_ID[5:0] " },	\
> +	{ CXL_PLDM_COMPONENT_ID_RES_VALID,      "Resource_ID[9:6] " }		\
> +)
> +
> +#define cxl_print_component_id(flags, valid_comp_id, valid_id_format, comp_id)			\
> +	(flags & valid_comp_id && flags & valid_id_format) ?					\
> +	(comp_id[0] & (CXL_PLDM_COMPONENT_ID_ENTITY_VALID | CXL_PLDM_COMPONENT_ID_RES_VALID)) ?	\
> +	__print_hex(&comp_id[1], 10) : (comp_id[0] & CXL_PLDM_COMPONENT_ID_ENTITY_VALID) ?	\
> +	__print_hex(&comp_id[1], 6) : (comp_id[0] & CXL_PLDM_COMPONENT_ID_RES_VALID) ?		\
> +	__print_hex(&comp_id[7], 4) : __print_hex(comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE) :	\

> +	__print_hex(comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
> +
Hi Shiju,

This is hard to read and I've lost track of what the aim is.
Side note that it is probably good to state that in the patch description
+ give some examples of what this print looks like in the various cases.
If we are going to print the whole thing in the case where we
have no valid bits set in byte[1], maybe we just print the whole thing
in all cases and just add the info on whether it is formatted and what
those valid bits are in byte[1]?

I was never keen on printing the invalid fields in the first place
but this code adopted the convention of validity bits and print what
is there where valid or not. So maybe the things we should print are:
Taking GMER as an example.

Add the component id valid format to show_valid_flags() 
Then always print the flags for comp_id_pldm_flags() above whether
or not they are valid. Finally print the two broken out fields
Enity ID and Resource ID.

Whether we also just print the raw versions is another question we
need to figure out - in the formatted case it is duplication, but
if not it includes more bytes.

Jonathan



>  /*
>   * General Media Event Record - GMER
>   * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ