On 3/4/2025 11:52 AM, Jason Gunthorpe wrote:
On Fri, Feb 28, 2025 at 05:35:53PM -0800, Shannon Nelson wrote:
+ /* reject unsupported and/or out of scope commands */
+ op_entry = (struct pds_fwctl_query_data_operation *)ep_info->operations->entries;
+ for (i = 0; i < ep_info->operations->num_entries; i++) {
+ if (PDS_FWCTL_RPC_OPCODE_CMP(rpc->in.op, op_entry[i].id)) {
+ if (scope < op_entry[i].scope)
+ return -EPERM;
+ return 0;
Oh I see, you are doing the scope like CXL with a "command intent's
report". That is a good idea.
+ if (rpc->in.len > 0) {
+ in_payload = kzalloc(rpc->in.len, GFP_KERNEL);
+ if (!in_payload) {
+ dev_err(dev, "Failed to allocate in_payload\n");
+ out = ERR_PTR(-ENOMEM);
+ goto done;
+ }
+
+ if (copy_from_user(in_payload, u64_to_user_ptr(rpc->in.payload),
+ rpc->in.len)) {
+ dev_err(dev, "Failed to copy in_payload from user\n");
+ out = ERR_PTR(-EFAULT);
+ goto done;
+ }
This wasn't really the intention to have a payload pointer inside the
existing payload pointer, is it the same issue as CXL where you wanted
a header?
I see your FW API also can't take a scatterlist, so it makes sense it
needs to be linearized like this.
I don't think it makes a difference to have the indirection or not, it
would be a bunch of stuff to keep the header seperate from the payload
and then still also end up with memcpy in the driver, so leave it.
+#define PDS_FWCTL_RPC_OPCODE_CMD_SHIFT 0
+#define PDS_FWCTL_RPC_OPCODE_CMD_MASK GENMASK(15, PDS_FWCTL_RPC_OPCODE_CMD_SHIFT)
+#define PDS_FWCTL_RPC_OPCODE_VER_SHIFT 16
+#define PDS_FWCTL_RPC_OPCODE_VER_MASK GENMASK(23, PDS_FWCTL_RPC_OPCODE_VER_SHIFT)
+
+#define PDS_FWCTL_RPC_OPCODE_GET_CMD(op) \
+ (((op) & PDS_FWCTL_RPC_OPCODE_CMD_MASK) >> PDS_FWCTL_RPC_OPCODE_CMD_SHIFT)
Isn't that FIELD_GET?
Hmm... I think so. I'll poke at that.
+struct fwctl_rpc_pds {
+ struct {
+ __u32 op;
+ __u32 ep;
+ __u32 rsvd;
+ __u32 len;
+ __u64 payload;
+ } in;
+ struct {
+ __u32 retval;
+ __u32 rsvd[2];
+ __u32 len;
+ __u64 payload;
+ } out;
+};
Use __aligned_u64 in the uapi headers
Sure.
Thanks,
sln
Jason
