linux-kernel - Re: [PATCH v7 1/8] cxl/mem: Read, trace, and clear events on driver load

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <645861555f7a8_4513a29424@iweiny-mobl.notmuch>
Date:   Sun, 7 May 2023 19:41:25 -0700
From:   Ira Weiny <ira.weiny@...el.com>
To:     Huai-Cheng <hchkuo@...ry-design.com.tw>,
        Ira Weiny <ira.weiny@...el.com>
CC:     Dan Williams <dan.j.williams@...el.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Alison Schofield <alison.schofield@...el.com>,
        "Vishal Verma" <vishal.l.verma@...el.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Ben Widawsky <bwidawsk@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        <linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
        <linux-acpi@...r.kernel.org>, <linux-cxl@...r.kernel.org>
Subject: Re: [PATCH v7 1/8] cxl/mem: Read, trace, and clear events on driver
 load

Huai-Cheng wrote:
> On Wed, Jan 18, 2023 at 1:53 PM Ira Weiny <ira.weiny@...el.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