On 21-01-14 18:32:17, Jonathan Cameron wrote: > On Thu, 14 Jan 2021 10:13:40 -0800 > Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote: > > > On 21-01-14 18:02:11, Jonathan Cameron wrote: > > > On Mon, 11 Jan 2021 14:51:19 -0800 > > > Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote: > > > > > > > The Command Effects Log (CEL) is specified in the CXL 2.0 specification. > > > > The CEL is one of two types of logs, the other being vendor specific. > > > > They are distinguished in hardware/spec via UUID. The CEL is immediately > > > > useful for 2 things: > > > > 1. Determine which optional commands are supported by the CXL device. > > > > 2. Enumerate any vendor specific commands > > > > > > > > The CEL can be used by the driver to determine which commands are > > > > available in the hardware (though it isn't, yet). That set of commands > > > > might itself be a subset of commands which are available to be used via > > > > CXL_MEM_SEND_COMMAND IOCTL. > > > > > > > > Prior to this, all commands that the driver exposed were explicitly > > > > enabled. After this, only those commands that are found in the CEL are > > > > enabled. > > > > > > > > Signed-off-by: Ben Widawsky <ben.widawsky@xxxxxxxxx> > > > > > > This patch made me wonder if the model for the command in quite right. > > > I think it would end up simpler with a pair of payload pointers for send > > > and receive (that can be equal when it makes sense). > > > > > > A few other things inline. > > > > > > Jonathan > > > > I'll address the others separately, but could you elaborate on this? I'm not > > sure I follow your meaning. > > Further down in the review.. > " > The fact that you end up bypassing the payload transfer stuff in mbox_cmd > rather suggests it's not a particularly good model. + it keeps confusing > me. > > While the hardware uses a single region for the payload, there is nothing > saying the code has to work that way. Why not have separate payload_in and > payload_out pointers? Occasionally you might set them to the same buffer, but > elsewhere you could avoid the direct memcpy()s you are doing around the > send_cmd(). > > " > > Jonathan > > Ah I was confused if that was a separate statement. Can you specify the function prototype you're hoping for (or modification to the structure)? I really like the lowest level function to simply model the hardware. I get to write the 8 steps out and clearly implement them. I personally don't think it's so awkward, but again, give me something more specific and I'll consider it.
