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:   Sun, 7 May 2023 19:41:25 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Huai-Cheng <hchkuo@avery-design.com.tw>,
        Ira Weiny <ira.weiny@intel.com>
CC:     Dan Williams <dan.j.williams@intel.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Alison Schofield <alison.schofield@intel.com>,
        "Vishal Verma" <vishal.l.verma@intel.com>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Dave Jiang <dave.jiang@intel.com>,
        Ben Widawsky <bwidawsk@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        <linux-kernel@vger.kernel.org>, <linux-pci@vger.kernel.org>,
        <linux-acpi@vger.kernel.org>, <linux-cxl@vger.kernel.org>
Subject: Re: [PATCH v7 1/8] cxl/mem: Read, trace, and clear events on driver
 load
Message-ID: <645861555f7a8_4513a29424@iweiny-mobl.notmuch>
References: <20221216-cxl-ev-log-v7-0-2316a5c8f7d8@intel.com>
 <20221216-cxl-ev-log-v7-1-2316a5c8f7d8@intel.com>
 <CA+2kVq_L0_OGLR5VXL1sHDeGa+RPTt-Kj9Ajnc8j82jsi2XM7Q@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CA+2kVq_L0_OGLR5VXL1sHDeGa+RPTt-Kj9Ajnc8j82jsi2XM7Q@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aU9TN29xaU9DQWtWSldwTjZYNW9iUDdPYVJEcDdOUkhyQmE4ZWo4VGdZeEo5?=
 =?utf-8?B?aGU2OU0za2hUU3R2ek5kWTdXcmNXcnk1OTRjM3Y1R3lBSGF4UmlsaVVSOHlC?=
 =?utf-8?B?dTUxdk81bEsybnh6NkZXbFZwa2Nrc1lOK0h0aVlINnZKR2VvOEdRcDVwUk84?=
 =?utf-8?B?WnZiYnZuZkpEUkxUTTRadkliOFBsUWoxOS9UaHpuaWtoM2VXbm9LbEM1aGRs?=
 =?utf-8?B?ekx6RHFRRXpQNjhMYVVYREcrc29VRzQvZ1UwMEhrYjMydXdOQlg0bFdhQ3Va?=
 =?utf-8?B?bC9FUVQ5WGVxQkpPUTZEbXVtdXVDT0UxcVorSXJVMmdBZnB0SW9jTTkyeW55?=
 =?utf-8?B?V2NmZlM1Zkwxb2FzVmVPSUFhWWVZMTI5MEkza1B0WWM2UFFNbVJwc1hHejV6?=
 =?utf-8?B?MHBRZW1uUHhZelBUa0ptZ0w3NlNscTBzS3crQmZPMWtLRHRYVklXTXEyZlBR?=
 =?utf-8?B?cE44TVhrbFB3RlhLd0J1ZjZLZ0pjWnBMeTV5K3ZYejdWSGdnZCtBa1g2akxM?=
 =?utf-8?B?eStwTnRXeEZFbWZna01acVVWOTlFZ2EyNFB0VVB4RXBtaDZIanlRK1JDNHVG?=
 =?utf-8?B?ejE4c2Y1b3BLV3NhYldKVXFOem10ZGxIZk5TblZPSk5JTS93bUFHTExhVkNY?=
 =?utf-8?B?UkNxSUJ4S0ZldklFdWVYdWV6K0g0TjdFejdXVFBnQkkyTm5hTytHcVJVQVhS?=
 =?utf-8?B?VVllZGhzenZOS0x5UDVGb1YrbUZCWFliVWhocm1TVVB6WXl6WUFCTFFTR1lU?=
 =?utf-8?B?V2x0R3FtYTZPNmYySWVyZXZTZE8ycjJGSlM3TjFyK3lPdVhiZ1UxRjRMOW9l?=
 =?utf-8?B?SmdiU1NhYUlFVk1wUlNOSTFEVnkrMTdWczFZU25rQnhkUEZGL1NSV3pvWjdl?=
 =?utf-8?B?MGYyc0ZvSkFyZkxHZzVDYW5VMFdGUVhTc1NEb2VCRUl1WllMMyszZG8xcTBs?=
 =?utf-8?B?SDU3bVpHbytXZ01kaDhMSVFKajFyNkNtUmVPZFVoOHcwSmZpQmNhSUxtcEJp?=
 =?utf-8?B?STdtM0pDZ05nTEU2cTlTTTZlSmNxZStHMEYvS2xhTTVxZDRoSExkbVQrL1or?=
 =?utf-8?B?V295eVI0R21YWjlNaHliMkZ0MTFZaysvNXJtdEJOcGZGbzEwTitUaXdsYnZI?=
 =?utf-8?B?MlJoSTI4dEJzdi9EOUpkbm9PVW85WEEyd2xqd0J4ZlJRM3M1ZnRISFZTOXRI?=
 =?utf-8?B?aHdaeWh2VkxwdkdrUnY3U3lJVEZCVERGM3g1M3cxc1VMSEFDUDZmOVhwd3lv?=
 =?utf-8?B?Q1pvMjNUdit4NXdkRmxDYVNNWUlmYk56L0tUU3pDZ1NLZThQZ051cTlSZzZW?=
 =?utf-8?B?RHhQTUF1QnVHQ1RDbzlPQXNaMTJIdGo1OWEvRXhQMG5yK1JFRWk3VDJiaHo0?=
 =?utf-8?B?TU15aTlySVVrUGdvOVBPRnNqTWJmSHl3QkxoTS9lYS8rakxHRlRyU3VWNWRR?=
 =?utf-8?B?ZnBZOS9WZGdIVWkrb0FVTTZJRW5ScWQyRVhKYytQK3Q3Z09IVGFMOHR5eG83?=
 =?utf-8?B?VVRneEJyNEZNTGs2MThGODh0cFBkOWtWd3JhNUZ5ayt6NmpUTmV2ZGQweU5T?=
 =?utf-8?B?dWZubVNOQmpVWXBmQUFQdXBwYzFpcUpub2NJY3luUXYrK292S3Y3WFVuRDlG?=
 =?utf-8?B?NmxDdHJYNUNUTnpoUi9LS2U1K1B5OWlsNFJZY3lxM2RRaEEwYnNTNjdaNEgv?=
 =?utf-8?B?YnhMajhLRG96TTlKd2hoR2FsQkZzZ3VxaVlzZXNPWlcxdFdqNjc3a0M3UE5V?=
 =?utf-8?B?L1crcFpjdmYzZ0lxL0JWdWptWDNVZ0Y5Tnc2dkEvWFV3eU9FTGYxRkd6a2Mz?=
 =?utf-8?B?ZlV4MHlzaGZ1Smh5VmszQkNlbEllaXd0Qk1uWmxKTzMwSHRUcTM2TmhEaHBS?=
 =?utf-8?B?M05ZbFAvNXMrOVdudS9Tb201cFlPVjdkSExiNHE4MVdJT0NCandXT3VPQmlL?=
 =?utf-8?B?OS8ycGxwOUw4V1MxUDluSkJETEFPdFhIMW11eVVQWTk1akQ5WGNYMEdtUzVU?=
 =?utf-8?B?bStXOGJieTU3TGlrZHg1aFYzMitVLzZLT0VldEJuSGhLTDNzRlI4dndUYzlX?=
 =?utf-8?B?T2hkR1lzNXlUS0JONytXbFRGVmhobkw2eUpEOEJleHpINWJVcTcxSjJqcVh6?=
 =?utf-8?B?WmR0SjJRWTdiWmFXSkhKTVkzVDVNeVl2SEdQeDkyTHorc0RjWldWYjArUVFr?=
 =?utf-8?Q?L+zIS0U/1/quosEeuSAvuQua2RQn7i8Q3dHdHvuXwrcy?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2edb2d6a-6b6b-484e-9651-08db4f6dbbef
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 May 2023 02:41:33.1018
 (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: RDTX86lPkdjjbjM82h6lqjEV4vSJTS+GJNaJkSxvmXwweFrgBzG6Iqdri8tx2iYDzJGpKxiBYgBOJFAMdsmEzw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6113
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,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        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

Huai-Cheng wrote:
> On Wed, Jan 18, 2023 at 1:53 PM Ira Weiny <ira.weiny@intel.com> wrote:

[snip]

> > +static void cxl_mem_get_records_log(struct cxl_dev_state *cxlds,
> > +                                   enum cxl_event_log_type type)
> > +{
> > +       struct cxl_get_event_payload *payload;
> > +       struct cxl_mbox_cmd mbox_cmd;
> > +       u8 log_type = type;
> > +       u16 nr_rec;
> > +
> > +       mutex_lock(&cxlds->event.log_lock);
> > +       payload = cxlds->event.buf;
> > +
> > +       mbox_cmd = (struct cxl_mbox_cmd) {
> > +               .opcode = CXL_MBOX_OP_GET_EVENT_RECORD,
> > +               .payload_in = &log_type,
> > +               .size_in = sizeof(log_type),
> > +               .payload_out = payload,
> > +               .size_out = cxlds->payload_size,
> > +               .min_out = struct_size(payload, records, 0),
> > +       };
> > +
> > +       do {
> > +               int rc, i;
> > +
> > +               rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
> > +               if (rc) {
> > +                       dev_err_ratelimited(cxlds->dev,
> > +                               "Event log '%d': Failed to query event records : %d",
> > +                               type, rc);
> > +                       break;
> > +               }
> > +
> > +               nr_rec = le16_to_cpu(payload->record_count);
> > +               if (!nr_rec)
> > +                       break;
> > +
> > +               for (i = 0; i < nr_rec; i++)
> > +                       trace_cxl_generic_event(cxlds->dev, type,
> > +                                               &payload->records[i]);
> > +
> > +               if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> > +                       trace_cxl_overflow(cxlds->dev, type, payload);
> > +
> > +               rc = cxl_clear_event_record(cxlds, type, payload);
> > +               if (rc) {
> > +                       dev_err_ratelimited(cxlds->dev,
> > +                               "Event log '%d': Failed to clear events : %d",
> > +                               type, rc);
> > +                       break;
> > +               }
> > +       } while (nr_rec);
> Should the (payload->flags & CXL_GET_EVENT_FLAG_MORE_RECORDS) be used
> instead of (nr_rec) in this while condition? According to the spec,
> this bit is used to see if there
> are more event records.

It is not an error to query for events with no events being present.
Because we want to read all the events we keep reading if there 'may' be
more and don't care about the overhead of an extra read if there is not.

Ira