Received-SPF: pass (google.com: domain of linux-kernel+bounces-16074-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Wed, 3 Jan 2024 14:30:02 -0800
From: Ira Weiny <ira.weiny@intel.com>
To: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>, Ira Weiny
	<ira.weiny@intel.com>, <linux-efi@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>
CC: Ard Biesheuvel <ardb@kernel.org>, Alison Schofield
	<alison.schofield@intel.com>, Vishal Verma <vishal.l.verma@intel.com>, "Dan
 Williams" <dan.j.williams@intel.com>, Jonathan Cameron
	<Jonathan.Cameron@huawei.com>, Yazen Ghannam <yazen.ghannam@amd.com>
Subject: Re: [PATCH 2/4] efi/cper, cxl: Make definitions and structures global
Message-ID: <6595dfead80fd_3ec8294ec@iweiny-mobl.notmuch>
References: <20240102150933.161009-1-Smita.KoralahalliChannabasappa@amd.com>
 <20240102150933.161009-3-Smita.KoralahalliChannabasappa@amd.com>
 <65943a1da3368_151dc1294c2@iweiny-mobl.notmuch>
 <e75e6c3a-c84e-a64c-d438-cae15d2e1f62@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <e75e6c3a-c84e-a64c-d438-cae15d2e1f62@amd.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?5tG4rdXBxcC1jQ3IBvhn09QdpbCPc4BheYsEdSj8V9N81XueI+msgJx/UCDh?=
 =?us-ascii?Q?r3SPzXUpI4EIImHV53BmGybakHDtFa/0cMk5IFr0clKV0V3YwtBHIIXgU5CD?=
 =?us-ascii?Q?Sq/qTF065HSmmNr4HGJAkNhC7mZPZwzZStOh1uKosy9Bq8r26YIDDBeoT2HN?=
 =?us-ascii?Q?clRZo8eIb5uURnErJAKBeao3SCYSKlLcVWxMj9EXfd6JAkobpVWJp6ucV0hm?=
 =?us-ascii?Q?2UB1lrEAS0hnRQINF9zLXVjmvcX3CBDH+WNHj8uCTErrrE1py+O2g5NLk9F+?=
 =?us-ascii?Q?8p4Hwcvk99jOfO3MB3sfZMnH8Kz2IWBuzQ3dAaYExIQRa1YYDou28rWKqqSN?=
 =?us-ascii?Q?V067C9mvDSciOmWpthYISiE5EuTx12JBStRiCyvFfQoXmVfjYBnckPB1My0z?=
 =?us-ascii?Q?hToUJFkST+a29/jWGtT+unS0mC3ht9B+pDtfLoFMpWOXsoIUaPggIYG06FqS?=
 =?us-ascii?Q?Bcmm3m77tnk7d0yTZykjV/FXmua6yF2w7WN4q67Lap/77vuhyND/aZrSf3vs?=
 =?us-ascii?Q?uog0PAxe8dKhRC7oZxHXunTZK7Vywc1msrs6sRMF5xvKt02NEwMoCoxJXVwg?=
 =?us-ascii?Q?ufxIZsRuL3zh0dH0k+bGFJAJpuqwIRiLqvHoN/ytyAxzM5w879Icx1wtZX7L?=
 =?us-ascii?Q?69Vm/u1Qykc10zb0C+h1YPFGMmeLSTNPHjH2E+sF7i+xsghLyi9/tJk01n/K?=
 =?us-ascii?Q?nywmNLSrLy6qHAtmnZtavnzTh5kAhl9J+6eYW+RNXa4Ii2eQ4mkM9KLLHYU8?=
 =?us-ascii?Q?/EqV5dBJOuLVviRVujqSYeytYmqg02PeJMNieIYXMwsg57LWLll4kOIzekbg?=
 =?us-ascii?Q?vAYBg/TgFmT8EMVup8P6tSU1sRKHDd2RSYOofVEg5QzRvAu/ksE02Uyt2rHz?=
 =?us-ascii?Q?fmI91cfQqCpbqrx+fax8TnJvd++6U9cD6kIj2IQv+9OofclG/Hu8zJY6Q4+p?=
 =?us-ascii?Q?/TRF6Jxnh8rNThbxJHBCaFDEcWA34eFqNbctNWTpdnTeeJg9pjvQAl+EjjJM?=
 =?us-ascii?Q?3rQ8V++RH7C/fzDaUPtCa1ZDx5iTnS+4MP4L55HKiPBRh/OCyvrz8xFyKe9h?=
 =?us-ascii?Q?ZTxPYgKeFXoIGAzETMzUJPrsy/uVUjQQGsTK4uIzTqsmdHf97wbatZ4nA/V1?=
 =?us-ascii?Q?CKiL+lV99HU52fiKWtW+XaZqD4Iu7UMr0/QzZxplr+5xyzx4omiGAXNP2T09?=
 =?us-ascii?Q?BZYBBhY1pYgr6ViHbbCg7UxOMJisdrfmb1Q4d/H5xsfin+Wpx0o79vLFaXFV?=
 =?us-ascii?Q?1ly7OC5spAaE6CzXtaEuTCV2NcAOS+DTGi6roVZvbxzv77UkA7ULVbFgWGzR?=
 =?us-ascii?Q?2gYqOa9WMXuYKx+IwdsdEv789sUNNNUr+vazVheguZhbav0bjDWOBMMtNVAj?=
 =?us-ascii?Q?Ck96f8AJX3Xnk/3dNaOA08mQ7/QgIaHnnc4MZZYf1rrCggb4jGmtibWcWeqP?=
 =?us-ascii?Q?xK38apGok4MDDGrfwQjfv9+sEq4Emqu3/NkG5p4tIiAwukTWrUlKu85273gx?=
 =?us-ascii?Q?lzPaNM+PpJSO9ZQSXr2j/hQ98yyyIVuSH41pwymPjGYq/bamFLgXDcOnoX+m?=
 =?us-ascii?Q?WbEsV9XztBDxFuikmtR0UyL4F9kULFLMqx/t3wUE?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bb3e0cef-dd91-49b3-dbac-08dc0cab8946
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jan 2024 22:30:06.3555
 (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: CShRjLYuQjmvjJboM47tYc6PAPRdAftnD7ofd01CJhP+r3sJnfk9fRM+2I3cjMJEPkhiGjEIDUEPTtYyZoysoQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4587
X-OriginatorOrg: intel.com

Smita Koralahalli wrote:
> On 1/2/2024 8:30 AM, Ira Weiny wrote:
> > Smita Koralahalli wrote:
> >> In preparation to add tracepoint support, move protocol error UUID
> >> definition to a common location and make CXL RAS capability struct
> >> global for use across different modules.
> >>
> >> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> > 
> > [snip]
> > 
> >> diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h
> >> index 86bfcf7909ec..6f8c00495708 100644
> >> --- a/drivers/firmware/efi/cper_cxl.h
> >> +++ b/drivers/firmware/efi/cper_cxl.h
> >> @@ -7,14 +7,11 @@
> >>    * Author: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> >>    */
> >>   
> >> +#include <linux/cxl-event.h>
> >> +
> >>   #ifndef LINUX_CPER_CXL_H
> >>   #define LINUX_CPER_CXL_H
> >>   
> >> -/* CXL Protocol Error Section */
> >> -#define CPER_SEC_CXL_PROT_ERR						\
> >> -	GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78,	\
> >> -		  0x4B, 0x77, 0x10, 0x48)
> >> -
> >>   #pragma pack(1)
> >>   
> >>   /* Compute Express Link Protocol Error Section, UEFI v2.10 sec N.2.13 */
> >> diff --git a/include/linux/cper.h b/include/linux/cper.h
> >> index c1a7dc325121..2cbf0a93785a 100644
> >> --- a/include/linux/cper.h
> >> +++ b/include/linux/cper.h
> >> @@ -89,6 +89,10 @@ enum {
> >>   #define CPER_NOTIFY_DMAR						\
> >>   	GUID_INIT(0x667DD791, 0xC6B3, 0x4c27, 0x8A, 0x6B, 0x0F, 0x8E,	\
> >>   		  0x72, 0x2D, 0xEB, 0x41)
> >> +/* CXL Protocol Error Section */
> >> +#define CPER_SEC_CXL_PROT_ERR						\
> >> +	GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78,	\
> >> +		  0x4B, 0x77, 0x10, 0x48)
> > 
> > Is this shared with code outside of GHES?  I did not need my GUID defines
> > outside of ghes.c and further becuase the events are defined as UUID's I
> > chose to keep the GUID definition as local as possible to ghes.c.
> > 
> > Can you do the same with this define?
> 
> Actually, it is shared with efi/cper.
> https://elixir.bootlin.com/linux/v6.7-rc8/source/drivers/firmware/efi/cper.c#L602

Ah ok.

> 
> But this would be something to look into. Should we continue to support 
> logging from efi/cper or just confine it to ghes..

I missed that you were not removing the efi/cper print.  I kind of thought
that was part of the series.

> 
> If we just log it from ghes similar to component events, we might loose 
> error records from RCH Downstream Port and other agent_types which do 
> not log device_ids.

That is a good reason to keep the efi/cper print AFAICS.

> Also, I'm not sure how useful are other fields in 
> protocol error CPER, the ones like Capability struct and DVSEC len etc 
> as the tracepoints doesn't log all of them.

I'm not sure about their importance but if they are important I would say
they should be added to the tracepoint.

Ira