Messages in this thread | | | Subject | Re: [PATCH v3 24/27] powerpc/powernv/pmem: Expose SMART data via ndctl | From | Andrew Donnellan <> | Date | Thu, 5 Mar 2020 14:36:13 +1100 |
| |
On 21/2/20 2:27 pm, Alastair D'Silva wrote: > +static int ndctl_smart(struct ocxlpmem *ocxlpmem, struct nd_cmd_pkg *pkg) > +{ > + u32 length, i; > + struct nd_ocxl_smart *out; > + int rc; > + > + mutex_lock(&ocxlpmem->admin_command.lock); > + > + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_SMART); > + if (rc) > + goto out; > + > + rc = admin_command_execute(ocxlpmem); > + if (rc) > + goto out; > + > + rc = admin_command_complete_timeout(ocxlpmem, ADMIN_COMMAND_SMART); > + if (rc < 0) { > + dev_err(&ocxlpmem->dev, "SMART timeout\n"); > + goto out; > + } > + > + rc = admin_response(ocxlpmem); > + if (rc < 0) > + goto out; > + if (rc != STATUS_SUCCESS) { > + warn_status(ocxlpmem, "Unexpected status from SMART", rc); > + goto out; > + } > + > + rc = smart_header_parse(ocxlpmem, &length); > + if (rc) > + goto out; > + > + pkg->nd_fw_size = length; > + > + length = min(length, pkg->nd_size_out); // bytes > + out = (struct nd_ocxl_smart *)pkg->nd_payload; > + // Each SMART attribute is 2 * 64 bits > + out->count = length / (2 * sizeof(u64)); // attributes
From what I can tell - 8 bytes of nd_ocxl_smart are taken up for the count + reserved bytes, so this is going to potentially overrun the user buffer.
> + > + for (i = 0; i < length; i += sizeof(u64)) {
It might be neater to make i count up by 1 and then multiply by sizeof(u64) later.
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, > + ocxlpmem->admin_command.data_offset + sizeof(u64) + i,
+ 0x08 rather than + sizeof(u64) for consistency.
> + OCXL_LITTLE_ENDIAN, > + &out->attribs[i/sizeof(u64)]); > + if (rc) > + goto out; > + } > + > + rc = admin_response_handled(ocxlpmem); > + if (rc) > + goto out; > + > + rc = 0; > + goto out; > + > +out: > + mutex_unlock(&ocxlpmem->admin_command.lock); > + return rc; > +} > + > +static int ndctl_call(struct ocxlpmem *ocxlpmem, void *buf, unsigned int buf_len) > +{ > + struct nd_cmd_pkg *pkg = buf; > + > + if (buf_len < sizeof(struct nd_cmd_pkg)) { > + dev_err(&ocxlpmem->dev, "Invalid ND_CALL size=%u\n", buf_len); > + return -EINVAL; > + } > + > + if (pkg->nd_family != NVDIMM_FAMILY_OCXL) { > + dev_err(&ocxlpmem->dev, "Invalid ND_CALL family=0x%llx\n", pkg->nd_family); > + return -EINVAL; > + } > + > + switch (pkg->nd_command) { > + case ND_CMD_OCXL_SMART: > + ndctl_smart(ocxlpmem, pkg);
Did you intend to dispose of the return code here?
> + break; > + > + default: > + dev_err(&ocxlpmem->dev, "Invalid ND_CALL command=0x%llx\n", pkg->nd_command); > + return -EINVAL; > + } > + > + > + return 0; > +} > + > static int ndctl(struct nvdimm_bus_descriptor *nd_desc, > struct nvdimm *nvdimm, > unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc) > @@ -88,6 +211,10 @@ static int ndctl(struct nvdimm_bus_descriptor *nd_desc, > struct ocxlpmem *ocxlpmem = container_of(nd_desc, struct ocxlpmem, bus_desc); > > switch (cmd) { > + case ND_CMD_CALL: > + *cmd_rc = ndctl_call(ocxlpmem, buf, buf_len); > + return 0; > + > case ND_CMD_GET_CONFIG_SIZE: > *cmd_rc = ndctl_config_size(buf); > return 0; > @@ -171,6 +298,7 @@ static int register_lpc_mem(struct ocxlpmem *ocxlpmem) > set_bit(ND_CMD_GET_CONFIG_SIZE, &nvdimm_cmd_mask); > set_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm_cmd_mask); > set_bit(ND_CMD_SET_CONFIG_DATA, &nvdimm_cmd_mask); > + set_bit(ND_CMD_CALL, &nvdimm_cmd_mask); > > set_bit(NDD_ALIASING, &nvdimm_flags); > > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h > index 927690f4888f..0eb7a35d24ae 100644 > --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h > @@ -7,6 +7,7 @@ > #include <linux/libnvdimm.h> > #include <uapi/nvdimm/ocxl-pmem.h> > #include <linux/mm.h> > +#include <linux/ndctl.h> > > #define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT) > #define DEFAULT_TIMEOUT 100 > @@ -98,6 +99,23 @@ struct ocxlpmem_function0 { > struct ocxl_fn *ocxl_fn; > }; > > +struct nd_ocxl_smart { > + __u8 count; > + __u8 reserved[7]; > + __u64 attribs[0]; > +} __packed; > + > +struct nd_pkg_ocxl { > + struct nd_cmd_pkg gen; > + union { > + struct nd_ocxl_smart smart; > + }; > +}; > + > +enum nd_cmd_ocxl { > + ND_CMD_OCXL_SMART = 1, > +}; > + > struct ocxlpmem { > struct device dev; > struct pci_dev *pdev; > diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h > index de5d90212409..2885052e7f40 100644 > --- a/include/uapi/linux/ndctl.h > +++ b/include/uapi/linux/ndctl.h > @@ -244,6 +244,7 @@ struct nd_cmd_pkg { > #define NVDIMM_FAMILY_HPE2 2 > #define NVDIMM_FAMILY_MSFT 3 > #define NVDIMM_FAMILY_HYPERV 4 > +#define NVDIMM_FAMILY_OCXL 6 > > #define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\ > struct nd_cmd_pkg) >
-- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited
| |