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:   Wed, 30 Nov 2022 01:30:12 -0800
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: [PATCH 07/11] cxl/mem: Trace Memory Module Event Record
Message-ID: <Y4cipHMHN3s7EDEd@iweiny-desk3>
References: <20221110185758.879472-1-ira.weiny@intel.com>
 <20221110185758.879472-8-ira.weiny@intel.com>
 <20221116153528.00005af7@Huawei.com>
 <Y3WNLtyZYLQNMZzj@iweiny-desk3>
 <20221117112235.00003243@Huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221117112235.00003243@Huawei.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?tvERXdelwdaY7sldpjY32/ED3J2uq28AGXIhzWlef4tF0pPJRS9AS+chWsxR?=
 =?us-ascii?Q?8d6pRIuqNzRa+qYbh5GP0jZN86dArL+9T/BKcjBlwSDdbInH1h3g0EBeu7tq?=
 =?us-ascii?Q?HNDlxChBAsWR2nibwUg6/vMkujnL+L2/l0FyhZxSy4lyasGJi5JPfhhdHijY?=
 =?us-ascii?Q?xf0+1LYj+p0Wf2uVmtViu2wX5myTJtcsNHUqKeGl/t2/iO6v1znVBE4iAYAn?=
 =?us-ascii?Q?VBR6Towmc9mb5UGYjjJeWmUIV40ttSNK/1P2NyQRCRQLsafmlRJp7t+apkJo?=
 =?us-ascii?Q?qZYO7CwXrhc5RVchED8AQpc7dc0/yp/3WqQz7yviUr0T0Z0a4LR1feM4Dfme?=
 =?us-ascii?Q?Hp/4fsl7Azug51Nt8/Z51DGchnvvSU/nLz6JL/X8c3dmHCj8M8Dow74otJpY?=
 =?us-ascii?Q?QoeqQqebvFH+fWVoXqCufc1b7vPJKXDhgIkd2wD/nT+y4SBj3w6mbbwsaSZE?=
 =?us-ascii?Q?za4CdEdO1b+ZH+aVrkZnY+YIX5uV5BZ4mqQARjHi5HjBAnmkk21XT4tO4uMa?=
 =?us-ascii?Q?nr1B8n0CxNNrIY05JtYs1YizCnIUy8nge9nVFkfg+pjIuYeBQoVGfRE5BORE?=
 =?us-ascii?Q?OQlS7tArUfsbu7vE1fZqAoI5NhV3Spg2sWG6V/XusMW5Hm3Qa3hhH4FevAoI?=
 =?us-ascii?Q?VfwnhFMrwoN3Sq4iTEtMiAV5zIm8PUeQpE+7NiX9QYnHuyazyODs+jf4mZOi?=
 =?us-ascii?Q?uHpgKrNHxVVwZDI/+UY9ylCjMLzEk3TY6owxKpIosL/jSSg3aK0tnY4jOImN?=
 =?us-ascii?Q?vWfpXLxHTrF7pgqryn1K5F+M4nQriO03RTeOItOI6tze3uT6WM7GqRd04+RQ?=
 =?us-ascii?Q?rioAUbM7yqvAZlRbsUcQ8kPrQhGYAbsL2u6g3f7x4wsbq1Wln8s1ak0KKm5m?=
 =?us-ascii?Q?rvKj+gW2+dR1rzTDnlCJv327strIiymTml/PXj+y5g9yT6+ArFs/Pk1zmLqC?=
 =?us-ascii?Q?n1YisR9YV1eEdiM6wJfENMU3m29hVBg3We3cjfSH0QVTswqfOVxFMFNbky3O?=
 =?us-ascii?Q?21khyAqwk63MSyA6e/p3O6QtTZw0y8An3V2heDVHGW1TdsHep/wbdI5bZJdX?=
 =?us-ascii?Q?iMce/fv+hxwN+iHnMZ8BVz5AdUfdJCGwR9YU4amR3igbZEWmiuSj3JQ12t93?=
 =?us-ascii?Q?s1fwYuyDWTXIXygHAPa+atKkd6rFVQiiTeGciQ5ERwzq/WrzwtX4oPK3029p?=
 =?us-ascii?Q?7utpwO5sTdli4OywFqu79QQgkTXCveKp9kbTGKp3fdjTdQ1hheJbeftZVVT9?=
 =?us-ascii?Q?AQo2Twg7UtXuKKsyNMMNBKidAcN+KcXh63wydjHDoiQzj/zcPY0QmfUbRJKh?=
 =?us-ascii?Q?GdRJ+uMc7Zzw1XnawcrbJW5Sq8/sr8FVHMBmSWaSDkQTmq2xS/DMIZ81K649?=
 =?us-ascii?Q?XFicoDNzA63XoVxlmSxwdrBj0R+l3Sc1lEeNRqmC2ztET8wkCKxSJnvYTiID?=
 =?us-ascii?Q?elATeFwn6opI+Zaq/NBXIImwgoICJyyQ+tEKZoHhhlTDQLEwKEp94QEKKKzh?=
 =?us-ascii?Q?c93ukkoqQKSM1Vx//TMCDIPvS2T8lZjPlPl9hF7vV0GDBPo0l3e+yGIPV0Hb?=
 =?us-ascii?Q?Tjx1LHEF6yv8FlSo6l0sTfVIAPgzNa8tCy0O550p?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c7926cdf-2d8b-4227-0c81-08dad2b57e0c
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2022 09:30:17.3228
 (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: RDg8ZPlG5IGn++EhQ2dTtpjsNPMbRkF7bkemWFq8NPiYDLyZxK65Q1GD6Cp8gjZO2ndgNisl2ORPtsa2H0EjEA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7333
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_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 Thu, Nov 17, 2022 at 11:22:35AM +0000, Jonathan Cameron wrote:
> On Wed, 16 Nov 2022 17:23:58 -0800
> Ira Weiny <ira.weiny@intel.com> wrote:
> 
> > On Wed, Nov 16, 2022 at 03:35:28PM +0000, Jonathan Cameron wrote:
> > > On Thu, 10 Nov 2022 10:57:54 -0800
> > > ira.weiny@intel.com wrote:
> > >   
> > > > From: Ira Weiny <ira.weiny@intel.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@intel.com>
> > > >   
> > > Noticed that we have a mixture of fully capitalized and not for flags.
> > > With that either explained or tidied up:
> > > 
> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > >   
> > > > +/*
> > > > + * Device Health Information - DHI
> > > > + *
> > > > + * CXL res 3.0 section 8.2.9.8.3.1; Table 8-100
> > > > + */
> > > > +#define CXL_DHI_HS_MAINTENANCE_NEEDED				BIT(0)
> > > > +#define CXL_DHI_HS_PERFORMANCE_DEGRADED				BIT(1)
> > > > +#define CXL_DHI_HS_HW_REPLACEMENT_NEEDED			BIT(2)
> > > > +#define show_health_status_flags(flags)	__print_flags(flags, "|",	   \
> > > > +	{ CXL_DHI_HS_MAINTENANCE_NEEDED,	"Maintenance Needed"	}, \
> > > > +	{ CXL_DHI_HS_PERFORMANCE_DEGRADED,	"Performance Degraded"	}, \
> > > > +	{ CXL_DHI_HS_HW_REPLACEMENT_NEEDED,	"Replacement Needed"	}  \  
> > > 
> > > Why are we sometime using capitals for flags (e.g patch 5) and not other times?  
> > 
> > Not sure what you mean.  Do you mean this from patch 5?
> Nope
> 
> +#define CXL_DPA_VOLATILE			BIT(0)
> +#define CXL_DPA_NOT_REPAIRABLE			BIT(1)
> +#define show_dpa_flags(flags)	__print_flags(flags, "|",		   \
> +	{ CXL_DPA_VOLATILE,			"VOLATILE"		}, \
> +	{ CXL_DPA_NOT_REPAIRABLE,		"NOT_REPAIRABLE"	}  \
> +)
> +
> 
> Where they are all capitals.  I thought that was maybe a flags vs other fields
> thing but it doesn't seem to be.

I've not made all flags capital on this and other patches.

Ira

> 
> 
> > 
> > ...
> >         { CXL_GMER_EVT_DESC_UNCORECTABLE_EVENT,         "Uncorrectable Event"   }, \
> >         { CXL_GMER_EVT_DESC_THRESHOLD_EVENT,            "Threshold event"       }, \
> >         { CXL_GMER_EVT_DESC_POISON_LIST_OVERFLOW,       "Poison List Overflow"  }  \
> > ...
> > 
> > Threshold event was a mistake.  This is the capitalization the spec uses.
> > 
> > Bit[0]: Uncorrectable Event: When set, indicates the reported event is
> >         ^^^^^^^^^^^^^^^^^^^
> > uncorrectable by the device. When cleared, indicates the reported
> > event was corrected by the device.
> > 
> > Bit[1]: Threshold Event: When set, the event is the result of a
> >         ^^^^^^^^^^^^^^^
> > threshold on the device having been reached. When cleared, the event
> > is not the result of a threshold limit.
> > 
> > Bit[2]: Poison List Overflow Event: When set, the Poison List has
> >         ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > overflowed, and this event is not in the Poison List. When cleared, the
> > Poison List has not overflowed.
> > 
> > 
> > I'll update this 'Event' in patch 5.  Probably need to add 'Event' to the
> > Poison List...
> > 
> > Ira
>