Messages in this thread | | | Date | Fri, 13 Jan 2023 14:28:47 +0100 | From | Robert Richter <> | Subject | Re: [PATCH] cxl/mbox: Fix Payload Length check for Get Log command |
| |
Dan,
On 04.01.23 12:31:59, Robert Richter wrote: > On 03.01.23 14:11:33, Dan Williams wrote: > > Robert Richter wrote: > > > Commit 2aeaf663b85e introduced strict checking for variable length > > > payload size validation. The payload length of received data must > > > match the size of the requested data by the caller except for the case > > > where the min_out value is set. > > > > > > The Get Log command does not have a header with a length field set. > > > The Log size is determined by the Get Supported Logs command (CXL 3.0, > > > 8.2.9.5.1). However, the actual size can be smaller and the number of > > > valid bytes in the payload output must be determined reading the > > > Payload Length field (CXL 3.0, Table 8-36, Note 2). > > > > > > Two issues arise: The command can successfully complete with a payload > > > length of zero. And, the valid payload length must then also be > > > consumed by the caller. > > > > Perhaps this is confusion about what the "Log Size" field of Get > > Supported Logs means? My reading is that the "Log Size" field indicates > > the data "currently available" in the log. Correct me if I am wrong, but > > it seems your reading is that it is the "possibly available" data and > > software can not assume anything is available until it actually goes to > > read the log. > > > Are you sure that this is not a device-side implementation issue where > > it needs to make sure that Get Supported Logs indicates what Get Log can > > expect? > > The spec is not really clear here and I have seen a CXL device > firmware implementation that interprets it like that. We could demand > a firmware fix for that, but the kernel driver would be more robust if > we lower the strictness here. > > Reading the spec again I just found that "the maximum size of each > Log" is mentioned in the description: > > """ > 8.2.9.5.1 Get Supported Logs (Opcode 0400h) > > Retrieve the list of device specific logs (identified by UUID) and > the maximum size of each Log. > """ > > With that and the note in Table 8-36 stating that the exact payload of > a variable length command should be determined using the Payload > Length field, I think the commands can return different payload > lengths.
any opinion here? Looks like the device may send a smaller payload in the 401h command than the size given in 400h.
I have sent an updated patch last week:
[PATCH v2] cxl/mbox: Fix Payload Length check for Get Log command https://patchwork.kernel.org/project/cxl/patch/20230104202954.1163366-1-rrichter@amd.com/
Please take a look.
Thanks,
-Robert
| |