On Mon, Oct 30, 2017 at 01:47:08PM -0700, Dan Williams wrote:
> Per v1.6 of the NVDIMM_FAMILY_INTEL command set [1] some of the new
> commands require rev-id 2. In addition to enabling ND_CMD_CALL for these
> new function numbers, add a lookup table for revision-ids by family
> and function number.
>
> [1]: http://pmem.io/documents/NVDIMM_DSM_Interface-V1.6.pdf
>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> drivers/acpi/nfit/core.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
> drivers/acpi/nfit/nfit.h | 26 +++++++++++++++++++++++++-
> 2 files changed, 65 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 444832b372ec..1b2c613a9d8b 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -370,6 +370,32 @@ static union acpi_object *acpi_label_info(acpi_handle
> handle)
> return pkg_to_buf(buf.pointer);
> }
>
> +static u8 nfit_dsm_revid(unsigned family, unsigned func)
> +{
> + static const u8 revid_table[NVDIMM_FAMILY_MAX][32] = {
> + [NVDIMM_FAMILY_INTEL] = {
> + [NVDIMM_INTEL_GET_MODES] = 2,
> + [NVDIMM_INTEL_GET_FWINFO] = 2,
> + [NVDIMM_INTEL_START_FWUPDATE] = 2,
> + [NVDIMM_INTEL_SEND_FWUPDATE] = 2,
> + [NVDIMM_INTEL_FINISH_FWUPDATE] = 2,
> + [NVDIMM_INTEL_QUERY_FWUPDATE] = 2,
> + [NVDIMM_INTEL_SET_THRESHOLD] = 2,
> + [NVDIMM_INTEL_INJECT_ERROR] = 2,
> + },
> + };
> + u8 id;
> +
> + if (family > NVDIMM_FAMILY_MAX)
> + return 0;
> + if (func > 31)
> + return 0;
> + id = revid_table[family][func];
> + if (id == 0)
> + return 1; /* default */
> + return id;
> +}
> +
Elsewhere we find NVDIMM_FAMILY_MAX == NVDIMM_FAMILY_MSFT.
Code is off by 1 in the dimension the array revid_table.
This approach only ever calls revision ID 1 of a function that
was initially defined in revision ID 1 of a spec.
Is it required that FW returns same data for a given function if
called with different revision IDs? I.e. could firmware implement
a function N such that it returns different information if called
with revision ID 2 versus being called with revision ID 1?
E.g. a field that is reserved in revision 1 could have a definition
in revision 2. My reading of the ACPI spec is this is acceptable.
Not sure if this makes a difference w/ the Intel FW or not, but
some of the structures returns were modified from the earlier
spec. I have concerns that this approach can be correctly
extended to other vendors.
> int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm
> *nvdimm,
> unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
> {
> @@ -468,8 +494,15 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
> struct nvdimm *nvdimm,
>
> out_obj = acpi_label_write(handle, p->in_offset, p->in_length,
> p->in_buf);
> - } else
> - out_obj = acpi_evaluate_dsm(handle, guid, 1, func, &in_obj);
> + } else {
> + u8 revid;
> +
> + if (nfit_mem)
> + revid = nfit_dsm_revid(nfit_mem->family, func);
> + else
> + revid = 1;
> + out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj);
> + }
>
> if (!out_obj) {
> dev_dbg(dev, "%s:%s _DSM failed cmd: %s\n", __func__, dimm_name,
> @@ -1640,7 +1673,7 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc
> *acpi_desc,
> * different command sets. Note, that checking for function0 (bit0)
> * tells us if any commands are reachable through this GUID.
> */
> - for (i = NVDIMM_FAMILY_INTEL; i <= NVDIMM_FAMILY_MSFT; i++)
> + for (i = 0; i <= NVDIMM_FAMILY_MAX; i++)
> if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 1))
> if (family < 0 || i == default_dsm_family)
> family = i;
> @@ -1650,7 +1683,7 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc
> *acpi_desc,
> if (override_dsm_mask && !disable_vendor_specific)
> dsm_mask = override_dsm_mask;
> else if (nfit_mem->family == NVDIMM_FAMILY_INTEL) {
> - dsm_mask = 0x3fe;
> + dsm_mask = NVDIMM_INTEL_CMDMASK;
> if (disable_vendor_specific)
> dsm_mask &= ~(1 << ND_CMD_VENDOR);
> } else if (nfit_mem->family == NVDIMM_FAMILY_HPE1) {
> @@ -1670,7 +1703,9 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc
> *acpi_desc,
>
> guid = to_nfit_uuid(nfit_mem->family);
> for_each_set_bit(i, &dsm_mask, BITS_PER_LONG)
> - if (acpi_check_dsm(adev_dimm->handle, guid, 1, 1ULL << i))
> + if (acpi_check_dsm(adev_dimm->handle, guid,
> + nfit_dsm_revid(nfit_mem->family, i),
> + 1ULL << i))
> set_bit(i, &nfit_mem->dsm_mask);
>
> obj = acpi_label_info(adev_dimm->handle);
> diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
> index b987196bf132..341be9511d0e 100644
> --- a/drivers/acpi/nfit/nfit.h
> +++ b/drivers/acpi/nfit/nfit.h
> @@ -24,7 +24,7 @@
> /* ACPI 6.1 */
> #define UUID_NFIT_BUS "2f10e7a4-9e91-11e4-89d3-123b93f75cba"
>
> -/* http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf */
> +/* http://pmem.io/documents/NVDIMM_DSM_Interface-V1.6.pdf */
> #define UUID_NFIT_DIMM "4309ac30-0d11-11e4-9191-0800200c9a66"
>
> /* https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/ */
> @@ -38,12 +38,36 @@
> | ACPI_NFIT_MEM_RESTORE_FAILED | ACPI_NFIT_MEM_FLUSH_FAILED \
> | ACPI_NFIT_MEM_NOT_ARMED | ACPI_NFIT_MEM_MAP_FAILED)
>
> +#define NVDIMM_FAMILY_MAX NVDIMM_FAMILY_MSFT
Suggest this be defined immediately after where NVDIMM_FAMILY_MSFT
is defined. That way if a new family is ever defined, its more obvious
that NVDIMM_FAMILY_MAX needs to be redefined as well.
> +
> #define NVDIMM_STANDARD_CMDMASK \
> (1 << ND_CMD_SMART | 1 << ND_CMD_SMART_THRESHOLD | 1 << ND_CMD_DIMM_FLAGS \
> | 1 << ND_CMD_GET_CONFIG_SIZE | 1 << ND_CMD_GET_CONFIG_DATA \
> | 1 << ND_CMD_SET_CONFIG_DATA | 1 << ND_CMD_VENDOR_EFFECT_LOG_SIZE \
> | 1 << ND_CMD_VENDOR_EFFECT_LOG | 1 << ND_CMD_VENDOR)
>
> +/*
> + * Command numbers that the kernel needs to know about to handle
> + * non-default DSM revision ids
> + */
> +enum nvdimm_family_cmds {
> + NVDIMM_INTEL_GET_MODES = 11,
> + NVDIMM_INTEL_GET_FWINFO = 12,
> + NVDIMM_INTEL_START_FWUPDATE = 13,
> + NVDIMM_INTEL_SEND_FWUPDATE = 14,
> + NVDIMM_INTEL_FINISH_FWUPDATE = 15,
> + NVDIMM_INTEL_QUERY_FWUPDATE = 16,
> + NVDIMM_INTEL_SET_THRESHOLD = 17,
> + NVDIMM_INTEL_INJECT_ERROR = 18,
> +};
> +
> +#define NVDIMM_INTEL_CMDMASK \
> +(NVDIMM_STANDARD_CMDMASK | 1 << NVDIMM_INTEL_GET_MODES \
> + | 1 << NVDIMM_INTEL_GET_FWINFO | 1 << NVDIMM_INTEL_START_FWUPDATE \
> + | 1 << NVDIMM_INTEL_SEND_FWUPDATE | 1 << NVDIMM_INTEL_FINISH_FWUPDATE \
> + | 1 << NVDIMM_INTEL_QUERY_FWUPDATE | 1 << NVDIMM_INTEL_SET_THRESHOLD \
> + | 1 << NVDIMM_INTEL_INJECT_ERROR)
> +
> enum nfit_uuids {
> /* for simplicity alias the uuid index with the family id */
> NFIT_DEV_DIMM = NVDIMM_FAMILY_INTEL,
>
> _______________________________________________
> Linux-nvdimm mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/linux-nvdimm
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
