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:   Thu, 20 Oct 2022 22:11:57 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
CC:     Dan Williams <dan.j.williams@intel.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Alison Schofield <alison.schofield@intel.com>,
        "Vishal Verma" <vishal.l.verma@intel.com>,
        Ben Widawsky <bwidawsk@kernel.org>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Davidlohr Bueso <dave@stgolabs.net>,
        <linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
        <yazen.ghannam@amd.com>
Subject: Re: [RFC V2 PATCH 02/11] cxl/mem: Implement Get Event Records command
Message-ID: <Y1IqHTlyD3L7CS9a@iweiny-desk3>
References: <20221010224131.1866246-1-ira.weiny@intel.com>
 <20221010224131.1866246-3-ira.weiny@intel.com>
 <e29a0dfc-b8b0-ace7-0db9-d63ea6e635c3@amd.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <e29a0dfc-b8b0-ace7-0db9-d63ea6e635c3@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?UR9uXeZqjK588wQd2F5ssStIKVRic0FmuFZd/Yfiwq73rWyHjzfxAriILa?=
 =?iso-8859-1?Q?VyXIqvq5eSSVydCyYapyj1iWlsL5mfl6aXbLAMpsrW/6+EAOTd9jV0rDM8?=
 =?iso-8859-1?Q?skyOiVLjrBXBo5eP8/+U3mCPUgamVTQuIhK6H4C5n4YpHp4QX7FIbMR7qC?=
 =?iso-8859-1?Q?pTU+RRDQDzW49cKgcnlgXyhXgZiRepuBDTr8yHWZ2hxufYTaKfVPy1aOXd?=
 =?iso-8859-1?Q?PuFY7ig87thZu+rHwOYeLortk0NYNbjbqP6MSgSg3gkksBKxU8admXjx82?=
 =?iso-8859-1?Q?bckK//oq3BHYvovBqzsGO2ryijxetLGHsMofoB9vhCmvcXgFFm6B7Rop1F?=
 =?iso-8859-1?Q?i1j3jm0CAzNowsRjwTeRxL+TFMlss1g6aHrQxqImNZQxnSMGb3tbuDGLTy?=
 =?iso-8859-1?Q?YTFnQtgaRwH+K/nzXwd0UTmjUE6pJnzynxwynrYkF5yDNRuB2VNlnuVfia?=
 =?iso-8859-1?Q?WGwfnEav5rxVO5wkxIo6DLHJiI/sB83mAnZ1xc2geqALQIMgfpdd0UwBds?=
 =?iso-8859-1?Q?PDxo/xNl9N09sNpcHGInpS9N2jcOu9IUPj8C+6jgaFcFPt4hVzefA0a14r?=
 =?iso-8859-1?Q?W6bAAeWS2roRRl7Kz1bB1Ko6ROQr5Z/REGiSK7ChMOTtZHSuXz8A1LC+7m?=
 =?iso-8859-1?Q?dBtp+PPQ2BOvFcO8VgKTb3LrkReAEVy7nono+2TrRafwAiUCNBZksmKabh?=
 =?iso-8859-1?Q?t6zOP8F0GDmTBFDj5VDo5KRiWFNTqe4WO/BZ3N3COyg2ytvHoMy346EhQC?=
 =?iso-8859-1?Q?pwJC+z3+h4LCMOGMD0aOo+87CRe6hgcVKch3OOnwqScSIScbzrWl4dk8/c?=
 =?iso-8859-1?Q?ht+Orzlbz8QcNqgKmokdGknX8cqjTw7ESc1OPvngPZ+N/hF14zhS2Eu4Wc?=
 =?iso-8859-1?Q?eImJB5am0KMPnrngo8u/2OyFJTIt7s2laT2u4boX4HBhxk7vWNrerIoqQp?=
 =?iso-8859-1?Q?heIj2eYZG7KoqtrylVbxQUauDdqW0i6lsftarvwgBt6x/ovvhk2TRBjDS+?=
 =?iso-8859-1?Q?ow7locRDBMS5qAzbgH9fNGQ44DjyxvPsRoahLFopuOueorinGkQK2g9nAj?=
 =?iso-8859-1?Q?oBwp4XV3goyqNklIPUkGhpc4h1V0augAgRgk839WTNLKeo1ytJolq9DLwJ?=
 =?iso-8859-1?Q?pwnNM5tXJ7sm+ALHUvl8AtdbGNC02oH3LOR/6GkB8d2syj0jpXvSZOBil4?=
 =?iso-8859-1?Q?zlm9DlCaZuitXojTkYhsajWetwXIih97+yACkjWdqJdZDd7bTFzqgUA/W8?=
 =?iso-8859-1?Q?rzZi2F1lh0J1pFc7ZskGPL1U2O3eTx7xmg58sChEq6eV1jL0p1xJmLtTfE?=
 =?iso-8859-1?Q?kAUwe//m5lnK4yr/x8Rujed8m9aBgKN7MmSbIMLdHxsTiQ8fK1IgsdVC0A?=
 =?iso-8859-1?Q?/CIl50VS6/mTAbFGrb0WQEkqZ3zVVNkgc5OYgGHPIaTvs4s+PeaeXaorA/?=
 =?iso-8859-1?Q?814m8ISly0DGhNIRbIWcRqJABDOEG5qilvhAuA0XwL86uGCsVBtnm8FWi9?=
 =?iso-8859-1?Q?fxPFySqo14T3A+2cC//+l8+ikdXM7KnIISsXiNtfCulNhMXMNeSESfv/m3?=
 =?iso-8859-1?Q?XaGPjp0WeVV9/144Y20pnx8qke1Ry3apxtHwbw0f3gp0CMpqlfwx4UocxI?=
 =?iso-8859-1?Q?eCN5FZ/Y/uoj4bOWTp3aCLL8zWdRJeDrxX?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2a84bda5-be19-4189-05bc-08dab322ca30
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Oct 2022 05:12:03.0313
 (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: UaJFw/rmh34R+ekDHCw9to7nSrZvhF9awTJXouGjZP0boe9mw8jUrVapp1jL4pKZ8NiNk/LFp9CMFNMkdNvWlg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4593
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED 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, Oct 20, 2022 at 04:50:30PM -0500, Smita Koralahalli wrote:
> Hi Ira,
> 
> On 10/10/22 5:41 PM, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > 

[snip]

> > +
> > +/**
> > + * cxl_mem_get_event_records - Get Event Records from the device
> > + * @cxlds: The device data for the operation
> > + *
> > + * Retrieve all event records available on the device and report them as trace
> > + * events.
> > + *
> > + * See CXL rev 3.0 @8.2.9.2.2 Get Event Records
> > + */
> > +void cxl_mem_get_event_records(struct cxl_dev_state *cxlds)
> > +{
> > +	enum cxl_event_log_type log_type;
> > +
> > +	dev_dbg(cxlds->dev, "Reading event logs\n");
> > +
> > +	for (log_type = CXL_EVENT_TYPE_INFO;
> > +	     log_type < CXL_EVENT_TYPE_MAX; log_type++)
> 
> Why should we loop through each event log here?

The idea was to clear all the event logs.  I think this made more sense before
the addition of the optional dynamic capacity log.

>
> What if the event
> record doesn't exist in the event log?

An empty log does not cause any issue.  The query will simply return 0 records
which is valid.

> 
> I got some Mailbox error messages like this while bootup..
> [? 346.387010] cxl_pci 0000:7f:00.0: Sending command
> [? 346.387181] cxl_pci 0000:7f:00.0: Doorbell wait took 0ms
> [? 346.387197] cxl_pci 0000:7f:00.0: Mailbox operation had an error: cmd
> input was invalid
> [? 346.387205] cxl_pci 0000:7f:00.0: Event log 'Warning': Failed to query
> event records : -6
> ..
> 
> Can we just read the "Event Status" field from Event Status Register
> (Device Status Registers Capability Offset + 00h) 8.2.8.3.1 in CXL Spec,
> determine if the records exist and just query those event logs?

Likely the hardware does not have the dynamic capacity log and so the code is
asking for something invalid.  I did not think of that when I added that new
log.  Checking status register looks to be the proper solution.

I'll throw in some testing in QEMU for this.  I'll also have to implement the
status register in QEMU to fully test.

Thanks for the testing!  :-D

Ira

> 
> Thanks,
> Smita
> 
> > +		cxl_mem_get_records_log(cxlds, log_type);
> > +}
> > +EXPORT_SYMBOL_NS_GPL(cxl_mem_get_event_records, CXL);
> > +
> >   /**
> >    * cxl_mem_get_partition_info - Get partition info
> >    * @cxlds: The device data for the operation