From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Dan Williams <email@example.com>
Linux PCI <firstname.lastname@example.org>,
"Ben Widawsky" <email@example.com>,
Bjorn Helgaas <firstname.lastname@example.org>,
"Chris Browy" <email@example.com>,
Linux ACPI <firstname.lastname@example.org>,
"Schofield, Alison" <email@example.com>,
Vishal L Verma <firstname.lastname@example.org>,
"Weiny, Ira" <email@example.com>,
"Lorenzo Pieralisi" <firstname.lastname@example.org>,
Linuxarm <email@example.com>, Fangjian <firstname.lastname@example.org>
Subject: Re: [RFC PATCH 2/2] cxl/mem: Add CDAT table reading from DOE
Date: Tue, 16 Mar 2021 10:36:47 +0000 [thread overview]
Message-ID: <20210316103647.00002f4b@Huawei.com> (raw)
On Mon, 15 Mar 2021 15:00:08 -0700
Dan Williams <email@example.com> wrote:
> On Wed, Mar 10, 2021 at 10:15 AM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> > On Thu, 11 Mar 2021 02:03:06 +0800
> > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> > > This patch simply provides some debug print outs of the entries
> > > at probe time + a sysfs binary attribute to allow dumping of the
> > > whole table.
> > >
> > > Binary dumping is modelled on /sys/firmware/ACPI/tables/
> > >
> > > The ability to dump this table will be very useful for emulation of
> > > real devices once they become available as QEMU CXL type 3 device
> > > emulation will be able to load this file in.
> > >
> > > Open questions:
> > > * No support here for table updates. Worth including these from the
> > > start, or leave that complexity for later?
> > > * Worth logging the reported info for debug, or is the binary attribute
> > > sufficient? Larger open question of whether to expose this info to
> > > userspace or not left for another day!
> > > * Where to put the CDAT file? Is it worth a subdirectory?
> > > * What is maximum size of the SSLBIS entry - I haven't quite managed
> > > to figure that out and this is the record with largest size.
> > > We could support dynamic allocation of the record size, but it
> > > would add complexity that seems unnecessary.
> > > It would not be compliant with the specification for a type 3 memory
> > > device to report this record anyway so I'm not that worried about this
> > > for now. It will become relevant once we have support for reading
> > > CDAT from CXL switches.
> > > * cdat.h is formatted in a similar style to pci_regs.h on basis that
> > > it may well be helpful to share this header with userspace tools.
> > > * Move the generic parts of this out to driver/cxl/cdat.c or leave that
> > > until we have other CXL drivers wishing to use this?
> > Naturally I remembered another open question within 10 seconds of sending :(
> > * Do we want to add any sort of header to the RAW dump of CDAT to aid
> > tooling? Whilst it looks a little like an ACPI table it doesn't have
> > a signature.
> > My gut feeling is no, because the CDAT specification doesn't define one but
> > I can see that it might be very convenient to have something that identified
> > the data once it was put in a file.
> I'm not yet convinced raw dumping is worth it for the same reason that
> command payload logging was eliminated from the v5.12-rc1 submission.
> There's not much userspace can do with the information besides debug
> the kernel behavior. If the kernel assigns a numa node to target a
> given CXL memory range with NUMA apis then HMEM_REPORTING should
> enumerate the properties. In other words, don't expand the userspace
> ABI problem, funnel users to the canonical source for such data.
As someone who finds raw dumping of ACPI tables extremely helpful in every
day use for debugging of some of our 'interesting' hardware, I know I'm going
to end up carrying that element locally anyway. I don't have a particular
problem doing so if we decide to not to upstream it.
Much like the ACPI table dumping, it's not an interface you expect userspace
to ever use and I fully agree that we should expose things properly as you
Short term my interest here is to get the DOE code upstream as step 1 of
moving to a full solution. The printing and dumping is really just PoC element
to prove out the interface. Any issue with putting the prints under _dbg()?
next prev parent reply other threads:[~2021-03-16 10:40 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-10 18:03 [RFC PATCH 0/2] PCI Data Object Exchange support + CXL CDAT Jonathan Cameron
2021-03-10 18:03 ` [RFC PATCH 1/2] PCI/doe: Initial support PCI Data Object Exchange Jonathan Cameron
2021-03-15 19:45 ` Dan Williams
2021-03-16 16:29 ` Jonathan Cameron
2021-03-16 16:57 ` Jonathan Cameron
2021-03-16 18:14 ` Dan Williams
2021-03-16 23:26 ` Chris Browy
2021-03-18 1:30 ` Dan Williams
2021-03-18 14:25 ` Jonathan Cameron
2021-06-17 17:12 ` Jonathan Cameron
2021-06-17 19:48 ` Chris Browy
2021-03-23 18:22 ` Jonathan Cameron
2021-03-23 18:57 ` Dan Williams
2021-03-10 18:03 ` [RFC PATCH 2/2] cxl/mem: Add CDAT table reading from DOE Jonathan Cameron
2021-03-10 18:14 ` Jonathan Cameron
2021-03-10 22:59 ` Chris Browy
2021-03-15 22:00 ` Dan Williams
2021-03-16 10:36 ` Jonathan Cameron [this message]
2021-03-17 1:42 ` Dan Williams
2021-03-17 1:55 ` Dan Williams
[not found] <CAAJ9+9fq1=EcOaSoo3oD_5QjYNAv6PPDjKS+gC9o7XDp2p1XpQ@mail.gmail.com>
2022-01-12 22:16 ` Zayd Qumsieh
2022-01-13 9:25 ` Jonathan Cameron
2022-01-15 0:15 ` Zayd Qumsieh
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).