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 > > [snip] > >
