| Subject | Re: [PATCH v3 18/27] powerpc/powernv/pmem: Add controller dump IOCTLs | From | Andrew Donnellan <> | Date | Wed, 4 Mar 2020 17:53:09 +1100 |
| |
On 21/2/20 2:27 pm, Alastair D'Silva wrote: > +static int ioctl_controller_dump_data(struct ocxlpmem *ocxlpmem, > + struct ioctl_ocxl_pmem_controller_dump_data __user *uarg) > +{ > + struct ioctl_ocxl_pmem_controller_dump_data args; > + u16 i; > + u64 val; > + int rc; > + > + if (copy_from_user(&args, uarg, sizeof(args))) > + return -EFAULT; > + > + if (args.buf_size % 8) > + return -EINVAL; > + > + if (args.buf_size > ocxlpmem->admin_command.data_size) > + return -EINVAL; > + > + mutex_lock(&ocxlpmem->admin_command.lock); > + > + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_CONTROLLER_DUMP); > + if (rc) > + goto out; > + > + val = ((u64)args.offset) << 32; > + val |= args.buf_size; > + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, > + ocxlpmem->admin_command.request_offset + 0x08, > + OCXL_LITTLE_ENDIAN, val); > + if (rc) > + goto out; > + > + rc = admin_command_execute(ocxlpmem); > + if (rc) > + goto out; > + > + rc = admin_command_complete_timeout(ocxlpmem, > + ADMIN_COMMAND_CONTROLLER_DUMP); > + if (rc < 0) { > + dev_warn(&ocxlpmem->dev, "Controller dump timed out\n"); > + goto out; > + } > + > + rc = admin_response(ocxlpmem); > + if (rc < 0) > + goto out; > + if (rc != STATUS_SUCCESS) { > + warn_status(ocxlpmem, > + "Unexpected status from retrieve error log",
Controller dump
> + rc); > + goto out; > + } > + > + for (i = 0; i < args.buf_size; i += 8) { > + u64 val; > + > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, > + ocxlpmem->admin_command.data_offset + i, > + OCXL_HOST_ENDIAN, &val);
Is a controller dump something where we want to do endian swapping?
Any reason we're not doing the usual check of the data identifier, additional data length etc?
> + if (rc) > + goto out; > + > + if (copy_to_user(&args.buf[i], &val, sizeof(u64))) { > + rc = -EFAULT; > + goto out; > + } > + } > + > + if (copy_to_user(uarg, &args, sizeof(args))) { > + rc = -EFAULT; > + goto out; > + } > + > + rc = admin_response_handled(ocxlpmem); > + if (rc) > + goto out; > + > +out: > + mutex_unlock(&ocxlpmem->admin_command.lock); > + return rc; > +} > + > +int request_controller_dump(struct ocxlpmem *ocxlpmem) > +{ > + int rc; > + u64 busy = 1; > + > + rc = ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIC, > + OCXL_LITTLE_ENDIAN, > + GLOBAL_MMIO_CHI_CDA);
This return code is ignored
> + > + > + rc = ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCI, > + OCXL_LITTLE_ENDIAN, > + GLOBAL_MMIO_HCI_CONTROLLER_DUMP); > + if (rc) > + return rc; > + > + while (busy) { > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, > + GLOBAL_MMIO_HCI, > + OCXL_LITTLE_ENDIAN, &busy); > + if (rc) > + return rc; > + > + busy &= GLOBAL_MMIO_HCI_CONTROLLER_DUMP; > + cond_resched(); > + } > + > + return 0; > +}
-- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited
|