From: Dan Williams <email@example.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: firstname.lastname@example.org, Linux PCI <email@example.com>,
Ben Widawsky <firstname.lastname@example.org>,
Bjorn Helgaas <email@example.com>,
Chris Browy <firstname.lastname@example.org>,
Linux ACPI <email@example.com>,
"Schofield, Alison" <firstname.lastname@example.org>,
Vishal L Verma <email@example.com>,
"Weiny, Ira" <firstname.lastname@example.org>,
Lorenzo Pieralisi <email@example.com>,
Linuxarm <firstname.lastname@example.org>, Fangjian <email@example.com>
Subject: Re: [RFC PATCH 1/2] PCI/doe: Initial support PCI Data Object Exchange
Date: Tue, 23 Mar 2021 11:57:01 -0700 [thread overview]
Message-ID: <CAPcyv4j2gSKr2dPnk3Z9PeHDsJJjLVcWRkYbNpXy-R-6Az5KzA@mail.gmail.com> (raw)
On Tue, Mar 23, 2021 at 11:25 AM Jonathan Cameron
> > I was thinking something like 'struct pcie_doe_object' pointers rather
> > than u32 arrays.
> Possibly... I'm not convinced that it doesn't end up being
> struct pcie_doe_object doe_obj;
> DOE_OBJ_INIT(&doe_obj, vid, type, request_pl, request_pl_sz,
> response_pl, response_pl_sz)
> ret = pcie_doe_exchange(doe, &doe_obj);
> If that's the pattern we see, why split it?
> We might well have a struct pcie_doe_object internally but it doesn't
> seem like a sensible external interface to me simply because we'd
> just be filling it in and immediately passing it to a 'send' function.
I don't think there are going to be so many DOE users that would
justify DOE_OBJ_INIT(). I am thinking of a model where the payload
construction is open coded and typesafe similar to how
intel_bus_fwa_activate()  builds its mailbox payload.
> > > The disadvantage is that at least some of the specs just have the
> > > header as their first few DW. So there isn't a clear distinction
> > > between header and payload. May lead to people getting offsets wrong
> > > in a way they wouldn't do if driver was responsible for building the
> > > whole message.
> > Aren't they more likely to get offsets wrong with u32 arrays rather
> > than data structures?
> I'm not sure what level you mean this at. The CDAT patch review you
> followed this with suggested just breaking out vid and type which is
> fine because those are always packed the same and we can do appropriate
> special handling.
> If you meant the whole object as packed structure, then it is a whole
> different matter.
> Easy one to point is that u64 values are going to end up with their
> top and bottom halves swapped. Things get even messier if we break
> up below the u32 level.
> We can do this at a higher level by having wrappers that deal with
> each protocol and do a serialize / deserialize for the protocol.
> I'm not sure if that will make sense or not yet though.
Again, I think the protocols to support are limited (CDAT and IDE/SPDM
are the only ones on the horizon), so not much value to having a rich
set of wrappers and macros to obfuscate payload generation.
> One thing I don't understand is why you proposed a delayed work queue
> above? I'm not seeing that model in the libsas SMP for example. As far
> as I can tell that just processes work items asap.
> Can you point to a more specific example if you thinks that we should
> use one?
For polling on a timeout a delayed workqueue can poll at an interval
without need for any explicit sleep calls. There are several examples
of queue_delayed_work() in drivers/ being used to advance a state
machine after a protocol relative timeout.
next prev parent reply other threads:[~2021-03-23 18:58 UTC|newest]
Thread overview: 20+ 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 [this message]
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
2021-03-17 1:42 ` Dan Williams
2021-03-17 1:55 ` Dan Williams
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).