| Message ID | e228edeeaa2f6527af89f10089e4bb6939de2532.1458583367.git.jerry.hoemann@hpe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > Add ioctl command ND_CMD_CALL_DSM to acpi_nfit_ctl and __nd_ioctl which > allow kernel to call a nvdimm's _DSM as a passthru without using the > marshaling code of the nd_cmd_desc. > > Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com> > --- > drivers/acpi/nfit.c | 141 ++++++++++++++++++++++++++++++++++++++++++++------- > drivers/nvdimm/bus.c | 43 +++++++++++++++- > 2 files changed, 166 insertions(+), 18 deletions(-) > > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c > index d0f35e6..9511ab4 100644 > --- a/drivers/acpi/nfit.c > +++ b/drivers/acpi/nfit.c > @@ -56,6 +56,21 @@ struct nfit_table_prev { > struct list_head flushes; > }; > > +struct cmd_family_tbl { > + enum nfit_uuids key_uuid; /* Internal handle */ > + int key_type; /* Exported handle */ > + int rev; /* _DSM rev */ > + u64 mask; /* 0 bit excludes underlying func.*/ > +}; > + > +struct cmd_family_tbl nfit_cmd_family_tbl[] = { > + { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL}, Per the comment on patch2 lets kill this for now. > + { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL}, > + { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL}, > + { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL}, Why does the mask default to all supported? I assume this should be the known-valid mask for each type, i.e. 0x3fe for ND_TYPE_DIMM_INTEL1. > + { -1, -1, -1, 0}, > +}; > + > static u8 nfit_uuid[NFIT_UUID_MAX][16]; > > const u8 *to_nfit_uuid(enum nfit_uuids id) > @@ -64,6 +79,19 @@ const u8 *to_nfit_uuid(enum nfit_uuids id) > } > EXPORT_SYMBOL(to_nfit_uuid); > > +static const u8 * > +family_to_uuid(int type) > +{ > + struct cmd_family_tbl *tbl = nfit_cmd_family_tbl; > + int i; > + > + for (i = 0; tbl[i].key_type >= 0 ; i++) { > + if (tbl[i].key_type == type) > + return to_nfit_uuid(tbl[i].key_uuid); > + } > + return 0; > +} > + > static struct acpi_nfit_desc *to_acpi_nfit_desc( > struct nvdimm_bus_descriptor *nd_desc) > { > @@ -171,8 +199,9 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, > unsigned int buf_len, int *cmd_rc) > { > struct acpi_nfit_desc *acpi_desc = to_acpi_nfit_desc(nd_desc); > - const struct nd_cmd_desc *desc = NULL; > union acpi_object in_obj, in_buf, *out_obj; > + struct nd_cmd_pkg *call_dsm = NULL; > + const struct nd_cmd_desc *desc = NULL; > struct device *dev = acpi_desc->dev; > const char *cmd_name, *dimm_name; > unsigned long dsm_mask; > @@ -180,6 +209,12 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, > const u8 *uuid; > u32 offset; > int rc, i; > + __u64 rev = 1, func = cmd; > + > + if (cmd == ND_CMD_CALL) { > + call_dsm = buf; > + func = call_dsm->nd_command; > + } > > if (nvdimm) { > struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm); > @@ -207,7 +242,7 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, > if (!desc || (cmd && (desc->out_num + desc->in_num == 0))) > return -ENOTTY; > > - if (!test_bit(cmd, &dsm_mask)) > + if (!test_bit(func, &dsm_mask)) > return -ENOTTY; > > in_obj.type = ACPI_TYPE_PACKAGE; > @@ -218,25 +253,49 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, > in_buf.buffer.length = 0; > > /* libnvdimm has already validated the input envelope */ > - for (i = 0; i < desc->in_num; i++) > + for (i = 0; i < desc->in_num; i++) { > in_buf.buffer.length += nd_cmd_in_size(nvdimm, cmd, desc, > i, buf); > + } > + > + if (call_dsm) { > + /* must skip over package wrapper */ > + in_buf.buffer.pointer = (void *) &call_dsm->nd_payload; > + in_buf.buffer.length = call_dsm->nd_size_in; > + uuid = family_to_uuid(call_dsm->nd_family); > + if (!uuid) { > + dev_dbg(dev, "%s:%s unsupported uuid\n", dimm_name, > + cmd_name); > + return -EINVAL; > + } > + } > > if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) { > - dev_dbg(dev, "%s:%s cmd: %s input length: %d\n", __func__, > - dimm_name, cmd_name, in_buf.buffer.length); > - print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4, > - 4, in_buf.buffer.pointer, min_t(u32, 128, > - in_buf.buffer.length), true); > + dev_dbg(dev, "%s:%s cmd: %d: %llu input length: %d\n", __func__, > + dimm_name, cmd, func, in_buf.buffer.length); > + print_hex_dump_debug("nvdimm in ", DUMP_PREFIX_OFFSET, 4, 4, > + in_buf.buffer.pointer, > + min_t(u32, 256, in_buf.buffer.length), true); > + > } > > - out_obj = acpi_evaluate_dsm(handle, uuid, 1, cmd, &in_obj); > + out_obj = acpi_evaluate_dsm(handle, uuid, rev, func, &in_obj); > if (!out_obj) { > - dev_dbg(dev, "%s:%s _DSM failed cmd: %s\n", __func__, dimm_name, > - cmd_name); > + dev_dbg(dev, "%s:%s unexpected output object type cmd: %s %llu, type: %d\n", > + __func__, dimm_name, cmd_name, func, out_obj->type); > return -EINVAL; > } > > + if (call_dsm) { > + call_dsm->nd_fw_size = out_obj->buffer.length; > + memcpy(call_dsm->nd_payload + call_dsm->nd_size_in, > + out_obj->buffer.pointer, > + min(call_dsm->nd_fw_size, call_dsm->nd_size_out)); > + > + ACPI_FREE(out_obj); > + return 0; > + } > + > if (out_obj->package.type != ACPI_TYPE_BUFFER) { > dev_dbg(dev, "%s:%s unexpected output object type cmd: %s type: %d\n", > __func__, dimm_name, cmd_name, out_obj->type); > @@ -918,13 +977,60 @@ static struct nvdimm *acpi_nfit_dimm_by_handle(struct acpi_nfit_desc *acpi_desc, > return NULL; > } > > + > +/* > + * determine if the _DSM specified by UUID is supported and return > + * mask of supported functions in nd_cmd_mask. > + */ > + > +static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid, > + int rev, unsigned long *nd_cmd_mask) > +{ > + int i; > + u64 mask = 0; > + union acpi_object *obj; > + > + obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL); > + if (!obj) > + return 0; > + /* For compatibility, old BIOSes may return an integer */ > + if (obj->type == ACPI_TYPE_INTEGER) > + mask = obj->integer.value; > + else if (obj->type == ACPI_TYPE_BUFFER) > + for (i = 0; i < obj->buffer.length && i < 8; i++) > + mask |= (((u8)obj->buffer.pointer[i]) << (i * 8)); > + ACPI_FREE(obj); > + > + *nd_cmd_mask = mask; > + > + return !!mask; > +} > + > + > +static inline void > +to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl, > + u8 const **cmd_uuid, unsigned long *cmd_mask) > +{ > + unsigned long mask = 0; > + int i; > + > + for (i = 0; tbl[i].key_uuid >= 0 ; i++) { > + const u8 *uuid = to_nfit_uuid(tbl[i].key_uuid); > + int rev = tbl[i].rev; > + > + if (acpi_nfit_sup_func(handle, uuid, rev, &mask)) { > + *cmd_mask = mask & tbl[i].mask; > + *cmd_uuid = uuid; > + break; > + } > + } > +} > + > static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, > struct nfit_mem *nfit_mem, u32 device_handle) > { > struct acpi_device *adev, *adev_dimm; > struct device *dev = acpi_desc->dev; > - const u8 *uuid = to_nfit_uuid(NFIT_DEV_DIMM); > - int i; > > nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en; > adev = to_acpi_dev(acpi_desc); > @@ -939,9 +1045,8 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, > return force_enable_dimms ? 0 : -ENODEV; > } > > - for (i = ND_CMD_SMART; i <= ND_CMD_VENDOR; i++) > - if (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i)) > - set_bit(i, &nfit_mem->dsm_mask); > + to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl, > + &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask); Why is this replacing acpi_check_dsm() with an open-coded implementation?
On Mon, Apr 11, 2016 at 12:15:14PM -0700, Dan Williams wrote: > On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > > Add ioctl command ND_CMD_CALL_DSM to acpi_nfit_ctl and __nd_ioctl which > > allow kernel to call a nvdimm's _DSM as a passthru without using the > > marshaling code of the nd_cmd_desc. > > > > Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com> > > --- > > drivers/acpi/nfit.c | 141 ++++++++++++++++++++++++++++++++++++++++++++------- > > drivers/nvdimm/bus.c | 43 +++++++++++++++- > > 2 files changed, 166 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c > > index d0f35e6..9511ab4 100644 > > --- a/drivers/acpi/nfit.c > > +++ b/drivers/acpi/nfit.c > > @@ -56,6 +56,21 @@ struct nfit_table_prev { > > struct list_head flushes; > > }; > > > > +struct cmd_family_tbl { > > + enum nfit_uuids key_uuid; /* Internal handle */ > > + int key_type; /* Exported handle */ > > + int rev; /* _DSM rev */ > > + u64 mask; /* 0 bit excludes underlying func.*/ > > +}; > > + > > +struct cmd_family_tbl nfit_cmd_family_tbl[] = { > > + { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL}, > > Per the comment on patch2 lets kill this for now. > > > + { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL}, > > + { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL}, > > + { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL}, > > Why does the mask default to all supported? I assume this should be > the known-valid mask for each type, i.e. 0x3fe for > ND_TYPE_DIMM_INTEL1. innocent until proven guilty. This mask is bit and'd with mask returned by firmware, so mask returned by firmware limits callers to those function firmware supports. My having this extra mask, it allows kernel a way to prevent user space application from calling a certain function totally. e.g. if we found that a function is dangerous or something that couldn't be coordinated with the kernel's use of the function. By not restricting it to currently defined functions, it allow testing firmware that might be implemented a new function w/o having to give firmware team a special kernel. > > > > + > > +/* > > + * determine if the _DSM specified by UUID is supported and return > > + * mask of supported functions in nd_cmd_mask. > > + */ > > + > > +static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid, > > + int rev, unsigned long *nd_cmd_mask) > > +{ > > + int i; > > + u64 mask = 0; > > + union acpi_object *obj; > > + > > + obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL); > > + if (!obj) > > + return 0; > > + /* For compatibility, old BIOSes may return an integer */ > > + if (obj->type == ACPI_TYPE_INTEGER) > > + mask = obj->integer.value; > > + else if (obj->type == ACPI_TYPE_BUFFER) > > + for (i = 0; i < obj->buffer.length && i < 8; i++) > > + mask |= (((u8)obj->buffer.pointer[i]) << (i * 8)); > > + ACPI_FREE(obj); > > + > > + *nd_cmd_mask = mask; > > + > > + return !!mask; > > +} > > + > > + > > +static inline void > > +to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl, > > + u8 const **cmd_uuid, unsigned long *cmd_mask) > > +{ > > + unsigned long mask = 0; > > + int i; > > + > > + for (i = 0; tbl[i].key_uuid >= 0 ; i++) { > > + const u8 *uuid = to_nfit_uuid(tbl[i].key_uuid); > > + int rev = tbl[i].rev; > > + > > + if (acpi_nfit_sup_func(handle, uuid, rev, &mask)) { > > + *cmd_mask = mask & tbl[i].mask; > > + *cmd_uuid = uuid; > > + break; > > + } > > + } > > +} > > + > > static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, > > struct nfit_mem *nfit_mem, u32 device_handle) > > { > > struct acpi_device *adev, *adev_dimm; > > struct device *dev = acpi_desc->dev; > > - const u8 *uuid = to_nfit_uuid(NFIT_DEV_DIMM); > > - int i; > > > > nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en; > > adev = to_acpi_dev(acpi_desc); > > @@ -939,9 +1045,8 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, > > return force_enable_dimms ? 0 : -ENODEV; > > } > > > > - for (i = ND_CMD_SMART; i <= ND_CMD_VENDOR; i++) > > - if (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i)) > > - set_bit(i, &nfit_mem->dsm_mask); > > + to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl, > > + &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask); > > Why is this replacing acpi_check_dsm() with an open-coded implementation? Not sure of the question. Is it why acpi_nfit_sup_func? or why to_nfit_uuid_msk? if why to_nfit_uuid_msk? We need to determine both mask and uuid (previously uuid was assumed.) if why acpi_nfit_sup_func? A few reasons. Want to allow user space to get mask. This is a "safe" way to know that calls are being made to/returned from firmware. Didn't want to hard code upper limit on function (at least not less than having function 0 .. 63 which the acpi layer is doing.) Finally didn't want to call acpi_check_dsm 64 times per nvdimm. :) But, we can go back to iterating over 0..63, both work. Question, if non-root nodes are changed from DSM to LSM, won't we need to add a function for that?
On Mon, Apr 11, 2016 at 4:43 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > On Mon, Apr 11, 2016 at 12:15:14PM -0700, Dan Williams wrote: >> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: >> > Add ioctl command ND_CMD_CALL_DSM to acpi_nfit_ctl and __nd_ioctl which >> > allow kernel to call a nvdimm's _DSM as a passthru without using the >> > marshaling code of the nd_cmd_desc. >> > >> > Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com> >> > --- >> > drivers/acpi/nfit.c | 141 ++++++++++++++++++++++++++++++++++++++++++++------- >> > drivers/nvdimm/bus.c | 43 +++++++++++++++- >> > 2 files changed, 166 insertions(+), 18 deletions(-) >> > >> > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c >> > index d0f35e6..9511ab4 100644 >> > --- a/drivers/acpi/nfit.c >> > +++ b/drivers/acpi/nfit.c >> > @@ -56,6 +56,21 @@ struct nfit_table_prev { >> > struct list_head flushes; >> > }; >> > >> > +struct cmd_family_tbl { >> > + enum nfit_uuids key_uuid; /* Internal handle */ >> > + int key_type; /* Exported handle */ >> > + int rev; /* _DSM rev */ >> > + u64 mask; /* 0 bit excludes underlying func.*/ >> > +}; >> > + >> > +struct cmd_family_tbl nfit_cmd_family_tbl[] = { >> > + { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL}, >> >> Per the comment on patch2 lets kill this for now. >> >> > + { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL}, >> > + { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL}, >> > + { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL}, >> >> Why does the mask default to all supported? I assume this should be >> the known-valid mask for each type, i.e. 0x3fe for >> ND_TYPE_DIMM_INTEL1. > > > innocent until proven guilty. > > This mask is bit and'd with mask returned by firmware, so mask > returned by firmware limits callers to those function firmware > supports. > > My having this extra mask, it allows kernel a way to prevent user space > application from calling a certain function totally. e.g. if we > found that a function is dangerous or something that couldn't be > coordinated with the kernel's use of the function. We should wait to cross that bridge when we come to it, not pre-create infrastructure without a user. However, we can already re-purpose this for limiting the supported functions to the ones listed in the specification. I.e. the kernel rejects undocumented commands by default. > By not restricting it to currently defined functions, it allow testing > firmware that might be implemented a new function w/o having to give > firmware team a special kernel. No, the kernel is not going to carry support for undefined function codes on the chance that maybe firmware wants to test with it at some point down the road. Just provide the firmware team a local kernel build. >> > + >> > +/* >> > + * determine if the _DSM specified by UUID is supported and return >> > + * mask of supported functions in nd_cmd_mask. >> > + */ >> > + >> > +static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid, >> > + int rev, unsigned long *nd_cmd_mask) >> > +{ >> > + int i; >> > + u64 mask = 0; >> > + union acpi_object *obj; >> > + >> > + obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL); >> > + if (!obj) >> > + return 0; >> > + /* For compatibility, old BIOSes may return an integer */ >> > + if (obj->type == ACPI_TYPE_INTEGER) >> > + mask = obj->integer.value; >> > + else if (obj->type == ACPI_TYPE_BUFFER) >> > + for (i = 0; i < obj->buffer.length && i < 8; i++) >> > + mask |= (((u8)obj->buffer.pointer[i]) << (i * 8)); >> > + ACPI_FREE(obj); >> > + >> > + *nd_cmd_mask = mask; >> > + >> > + return !!mask; >> > +} >> > + >> > + >> > +static inline void >> > +to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl, >> > + u8 const **cmd_uuid, unsigned long *cmd_mask) >> > +{ >> > + unsigned long mask = 0; >> > + int i; >> > + >> > + for (i = 0; tbl[i].key_uuid >= 0 ; i++) { >> > + const u8 *uuid = to_nfit_uuid(tbl[i].key_uuid); >> > + int rev = tbl[i].rev; >> > + >> > + if (acpi_nfit_sup_func(handle, uuid, rev, &mask)) { >> > + *cmd_mask = mask & tbl[i].mask; >> > + *cmd_uuid = uuid; >> > + break; >> > + } >> > + } >> > +} >> > + >> > static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, >> > struct nfit_mem *nfit_mem, u32 device_handle) >> > { >> > struct acpi_device *adev, *adev_dimm; >> > struct device *dev = acpi_desc->dev; >> > - const u8 *uuid = to_nfit_uuid(NFIT_DEV_DIMM); >> > - int i; >> > >> > nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en; >> > adev = to_acpi_dev(acpi_desc); >> > @@ -939,9 +1045,8 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, >> > return force_enable_dimms ? 0 : -ENODEV; >> > } >> > >> > - for (i = ND_CMD_SMART; i <= ND_CMD_VENDOR; i++) >> > - if (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i)) >> > - set_bit(i, &nfit_mem->dsm_mask); >> > + to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl, >> > + &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask); >> >> Why is this replacing acpi_check_dsm() with an open-coded implementation? > > > Not sure of the question. > > Is it why acpi_nfit_sup_func? or why to_nfit_uuid_msk? > > if why to_nfit_uuid_msk? We need to determine both mask and uuid > (previously uuid was assumed.) For readability I'd prefer two operations in acpi_nfit_add_dimm(): find_family() followed by probing the supported functions in that family rather than hiding that detail in a combined to_nfit_uuid_msk() routine. My preference is that a to_*() routine just do one conversion and return its value directly, not pass-by-reference multiple conversions. > if why acpi_nfit_sup_func? A few reasons. > > Want to allow user space to get mask. This is a "safe" way to know > that calls are being made to/returned from firmware. > > Didn't want to hard code upper limit on function (at least not less > than having function 0 .. 63 which the acpi layer is doing.) > > Finally didn't want to call acpi_check_dsm 64 times per nvdimm. :) > But, we can go back to iterating over 0..63, both work. If we limit the search to the known / specified function codes as mentioned above then we won't be searching the entire 64-bit space per-dimm. > Question, if non-root nodes are changed from DSM to LSM, won't > we need to add a function for that? To me, LSM just seems like yet another 'family'.
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c index d0f35e6..9511ab4 100644 --- a/drivers/acpi/nfit.c +++ b/drivers/acpi/nfit.c @@ -56,6 +56,21 @@ struct nfit_table_prev { struct list_head flushes; }; +struct cmd_family_tbl { + enum nfit_uuids key_uuid; /* Internal handle */ + int key_type; /* Exported handle */ + int rev; /* _DSM rev */ + u64 mask; /* 0 bit excludes underlying func.*/ +}; + +struct cmd_family_tbl nfit_cmd_family_tbl[] = { + { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL}, + { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL}, + { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL}, + { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL}, + { -1, -1, -1, 0}, +}; + static u8 nfit_uuid[NFIT_UUID_MAX][16]; const u8 *to_nfit_uuid(enum nfit_uuids id) @@ -64,6 +79,19 @@ const u8 *to_nfit_uuid(enum nfit_uuids id) } EXPORT_SYMBOL(to_nfit_uuid); +static const u8 * +family_to_uuid(int type) +{ + struct cmd_family_tbl *tbl = nfit_cmd_family_tbl; + int i; + + for (i = 0; tbl[i].key_type >= 0 ; i++) { + if (tbl[i].key_type == type) + return to_nfit_uuid(tbl[i].key_uuid); + } + return 0; +} + static struct acpi_nfit_desc *to_acpi_nfit_desc( struct nvdimm_bus_descriptor *nd_desc) { @@ -171,8 +199,9 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, unsigned int buf_len, int *cmd_rc) { struct acpi_nfit_desc *acpi_desc = to_acpi_nfit_desc(nd_desc); - const struct nd_cmd_desc *desc = NULL; union acpi_object in_obj, in_buf, *out_obj; + struct nd_cmd_pkg *call_dsm = NULL; + const struct nd_cmd_desc *desc = NULL; struct device *dev = acpi_desc->dev; const char *cmd_name, *dimm_name; unsigned long dsm_mask; @@ -180,6 +209,12 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, const u8 *uuid; u32 offset; int rc, i; + __u64 rev = 1, func = cmd; + + if (cmd == ND_CMD_CALL) { + call_dsm = buf; + func = call_dsm->nd_command; + } if (nvdimm) { struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm); @@ -207,7 +242,7 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, if (!desc || (cmd && (desc->out_num + desc->in_num == 0))) return -ENOTTY; - if (!test_bit(cmd, &dsm_mask)) + if (!test_bit(func, &dsm_mask)) return -ENOTTY; in_obj.type = ACPI_TYPE_PACKAGE; @@ -218,25 +253,49 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, in_buf.buffer.length = 0; /* libnvdimm has already validated the input envelope */ - for (i = 0; i < desc->in_num; i++) + for (i = 0; i < desc->in_num; i++) { in_buf.buffer.length += nd_cmd_in_size(nvdimm, cmd, desc, i, buf); + } + + if (call_dsm) { + /* must skip over package wrapper */ + in_buf.buffer.pointer = (void *) &call_dsm->nd_payload; + in_buf.buffer.length = call_dsm->nd_size_in; + uuid = family_to_uuid(call_dsm->nd_family); + if (!uuid) { + dev_dbg(dev, "%s:%s unsupported uuid\n", dimm_name, + cmd_name); + return -EINVAL; + } + } if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) { - dev_dbg(dev, "%s:%s cmd: %s input length: %d\n", __func__, - dimm_name, cmd_name, in_buf.buffer.length); - print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4, - 4, in_buf.buffer.pointer, min_t(u32, 128, - in_buf.buffer.length), true); + dev_dbg(dev, "%s:%s cmd: %d: %llu input length: %d\n", __func__, + dimm_name, cmd, func, in_buf.buffer.length); + print_hex_dump_debug("nvdimm in ", DUMP_PREFIX_OFFSET, 4, 4, + in_buf.buffer.pointer, + min_t(u32, 256, in_buf.buffer.length), true); + } - out_obj = acpi_evaluate_dsm(handle, uuid, 1, cmd, &in_obj); + out_obj = acpi_evaluate_dsm(handle, uuid, rev, func, &in_obj); if (!out_obj) { - dev_dbg(dev, "%s:%s _DSM failed cmd: %s\n", __func__, dimm_name, - cmd_name); + dev_dbg(dev, "%s:%s unexpected output object type cmd: %s %llu, type: %d\n", + __func__, dimm_name, cmd_name, func, out_obj->type); return -EINVAL; } + if (call_dsm) { + call_dsm->nd_fw_size = out_obj->buffer.length; + memcpy(call_dsm->nd_payload + call_dsm->nd_size_in, + out_obj->buffer.pointer, + min(call_dsm->nd_fw_size, call_dsm->nd_size_out)); + + ACPI_FREE(out_obj); + return 0; + } + if (out_obj->package.type != ACPI_TYPE_BUFFER) { dev_dbg(dev, "%s:%s unexpected output object type cmd: %s type: %d\n", __func__, dimm_name, cmd_name, out_obj->type); @@ -918,13 +977,60 @@ static struct nvdimm *acpi_nfit_dimm_by_handle(struct acpi_nfit_desc *acpi_desc, return NULL; } + +/* + * determine if the _DSM specified by UUID is supported and return + * mask of supported functions in nd_cmd_mask. + */ + +static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid, + int rev, unsigned long *nd_cmd_mask) +{ + int i; + u64 mask = 0; + union acpi_object *obj; + + obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL); + if (!obj) + return 0; + /* For compatibility, old BIOSes may return an integer */ + if (obj->type == ACPI_TYPE_INTEGER) + mask = obj->integer.value; + else if (obj->type == ACPI_TYPE_BUFFER) + for (i = 0; i < obj->buffer.length && i < 8; i++) + mask |= (((u8)obj->buffer.pointer[i]) << (i * 8)); + ACPI_FREE(obj); + + *nd_cmd_mask = mask; + + return !!mask; +} + + +static inline void +to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl, + u8 const **cmd_uuid, unsigned long *cmd_mask) +{ + unsigned long mask = 0; + int i; + + for (i = 0; tbl[i].key_uuid >= 0 ; i++) { + const u8 *uuid = to_nfit_uuid(tbl[i].key_uuid); + int rev = tbl[i].rev; + + if (acpi_nfit_sup_func(handle, uuid, rev, &mask)) { + *cmd_mask = mask & tbl[i].mask; + *cmd_uuid = uuid; + break; + } + } +} + static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, struct nfit_mem *nfit_mem, u32 device_handle) { struct acpi_device *adev, *adev_dimm; struct device *dev = acpi_desc->dev; - const u8 *uuid = to_nfit_uuid(NFIT_DEV_DIMM); - int i; nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en; adev = to_acpi_dev(acpi_desc); @@ -939,9 +1045,8 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, return force_enable_dimms ? 0 : -ENODEV; } - for (i = ND_CMD_SMART; i <= ND_CMD_VENDOR; i++) - if (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i)) - set_bit(i, &nfit_mem->dsm_mask); + to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl, + &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask); return 0; } @@ -1012,7 +1117,7 @@ static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc) if (!adev) return; - for (i = ND_CMD_ARS_CAP; i <= ND_CMD_CLEAR_ERROR; i++) + for (i = 0; i <= ND_CMD_CLEAR_ERROR; i++) if (acpi_check_dsm(adev->handle, uuid, 1, 1ULL << i)) set_bit(i, &nd_desc->dsm_mask); } @@ -2463,6 +2568,8 @@ static __init int nfit_init(void) acpi_str_to_uuid(UUID_PERSISTENT_VIRTUAL_CD, nfit_uuid[NFIT_SPA_PCD]); acpi_str_to_uuid(UUID_NFIT_BUS, nfit_uuid[NFIT_DEV_BUS]); acpi_str_to_uuid(UUID_NFIT_DIMM, nfit_uuid[NFIT_DEV_DIMM]); + acpi_str_to_uuid(UUID_NFIT_DIMM_N_HPE1, nfit_uuid[NFIT_DEV_DIMM_N_HPE1]); + acpi_str_to_uuid(UUID_NFIT_DIMM_N_HPE2, nfit_uuid[NFIT_DEV_DIMM_N_HPE2]); nfit_wq = create_singlethread_workqueue("nfit"); if (!nfit_wq) diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c index 3355748..f205d10 100644 --- a/drivers/nvdimm/bus.c +++ b/drivers/nvdimm/bus.c @@ -439,6 +439,12 @@ static const struct nd_cmd_desc __nd_cmd_dimm_descs[] = { .out_num = 3, .out_sizes = { 4, 4, UINT_MAX, }, }, + [ND_CMD_CALL] = { + .in_num = 2, + .in_sizes = {sizeof(struct nd_cmd_pkg), UINT_MAX, }, + .out_num = 1, + .out_sizes = { UINT_MAX, }, + }, }; const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd) @@ -473,6 +479,12 @@ static const struct nd_cmd_desc __nd_cmd_bus_descs[] = { .out_num = 3, .out_sizes = { 4, 4, 8, }, }, + [ND_CMD_CALL] = { + .in_num = 2, + .in_sizes = {sizeof(struct nd_cmd_pkg), UINT_MAX, }, + .out_num = 1, + .out_sizes = { UINT_MAX, }, + }, }; const struct nd_cmd_desc *nd_cmd_bus_desc(int cmd) @@ -500,6 +512,10 @@ u32 nd_cmd_in_size(struct nvdimm *nvdimm, int cmd, struct nd_cmd_vendor_hdr *hdr = buf; return hdr->in_length; + } else if (cmd == ND_CMD_CALL) { + struct nd_cmd_pkg *pkg = buf; + + return pkg->nd_size_in; } return UINT_MAX; @@ -522,6 +538,12 @@ u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd, return out_field[1]; else if (!nvdimm && cmd == ND_CMD_ARS_STATUS && idx == 2) return out_field[1] - 8; + else if (cmd == ND_CMD_CALL) { + struct nd_cmd_pkg *pkg = + (struct nd_cmd_pkg *) in_field; + return pkg->nd_size_out; + } + return UINT_MAX; } @@ -588,7 +610,9 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, unsigned int cmd = _IOC_NR(ioctl_cmd); void __user *p = (void __user *) arg; struct device *dev = &nvdimm_bus->dev; + struct nd_cmd_pkg call_dsm; const char *cmd_name, *dimm_name; + unsigned int func = cmd; unsigned long dsm_mask; void *buf; int rc, i; @@ -605,8 +629,14 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, dimm_name = "bus"; } + if (cmd == ND_CMD_CALL) { + if (copy_from_user(&call_dsm, p, sizeof(call_dsm))) + return -EFAULT; + func = call_dsm.nd_command; + } + if (!desc || (desc->out_num + desc->in_num == 0) || - !test_bit(cmd, &dsm_mask)) + !test_bit(func, &dsm_mask)) return -ENOTTY; /* fail write commands (when read-only) */ @@ -616,6 +646,7 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, case ND_CMD_SET_CONFIG_DATA: case ND_CMD_ARS_START: case ND_CMD_CLEAR_ERROR: + case ND_CMD_CALL: dev_dbg(&nvdimm_bus->dev, "'%s' command while read-only.\n", nvdimm ? nvdimm_cmd_name(cmd) : nvdimm_bus_cmd_name(cmd)); @@ -643,6 +674,16 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, in_len += in_size; } + if (cmd == ND_CMD_CALL) { + dev_dbg(dev, "%s:%s, idx: %llu, in: %zu, out: %zu, len %zu\n", + __func__, dimm_name, call_dsm.nd_command, + in_len, out_len, buf_len); + + for (i = 0; i < ARRAY_SIZE(call_dsm.nd_reserved2); i++) + if (call_dsm.nd_reserved2[i]) + return -EINVAL; + } + /* process an output envelope */ for (i = 0; i < desc->out_num; i++) { u32 out_size = nd_cmd_out_size(nvdimm, cmd, desc, i,
Add ioctl command ND_CMD_CALL_DSM to acpi_nfit_ctl and __nd_ioctl which allow kernel to call a nvdimm's _DSM as a passthru without using the marshaling code of the nd_cmd_desc. Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com> --- drivers/acpi/nfit.c | 141 ++++++++++++++++++++++++++++++++++++++++++++------- drivers/nvdimm/bus.c | 43 +++++++++++++++- 2 files changed, 166 insertions(+), 18 deletions(-)