From: Dan Williams <dan.j.williams@intel.com> To: Jerry Hoemann <Jerry.Hoemann@hpe.com> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org> Subject: Re: [PATCH v4 3/6] libnvdimm, acpi, nfit: Add bus level dsm mask for pass thru. Date: Sat, 1 Jul 2017 13:08:50 -0700 [thread overview] Message-ID: <CAPcyv4hAMHzi7OK-z-B9LpT83Q1evH2-B-NXUMB2ebtVbjOdsg@mail.gmail.com> (raw) In-Reply-To: <20170701195805.GA13259@anatevka.americas.hpqcorp.net> On Sat, Jul 1, 2017 at 12:58 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > On Fri, Jun 30, 2017 at 08:55:22PM -0700, Dan Williams wrote: > > ... > >> On Fri, Jun 30, 2017 at 9:09 AM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: >> > + if (cmd == ND_CMD_CALL) >> > + dsm_mask = nd_desc->bus_dsm_mask; >> > desc = nd_cmd_bus_desc(cmd); >> > uuid = to_nfit_uuid(NFIT_DEV_BUS); >> > handle = adev->handle; >> > @@ -1613,6 +1615,7 @@ static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc) >> > struct nvdimm_bus_descriptor *nd_desc = &acpi_desc->nd_desc; >> > const u8 *uuid = to_nfit_uuid(NFIT_DEV_BUS); >> > struct acpi_device *adev; >> > + unsigned long dsm_mask; >> > int i; >> > >> > nd_desc->cmd_mask = acpi_desc->bus_cmd_force_en; >> > @@ -1624,6 +1627,11 @@ static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc) >> > if (acpi_check_dsm(adev->handle, uuid, 1, 1ULL << i)) >> > set_bit(i, &nd_desc->cmd_mask); >> > set_bit(ND_CMD_CALL, &nd_desc->cmd_mask); >> > + >> > + dsm_mask = 0x3bf; >> >> I went ahead and fixed this up to use dsm_mask defined like this: >> >> + dsm_mask = >> + (1 << ND_CMD_ARS_CAP) | >> + (1 << ND_CMD_ARS_START) | >> + (1 << ND_CMD_ARS_STATUS) | >> + (1 << ND_CMD_CLEAR_ERROR) | >> + (1 << NFIT_CMD_TRANSLATE_SPA) | >> + (1 << NFIT_CMD_ARS_INJECT_SET) | >> + (1 << NFIT_CMD_ARS_INJECT_CLEAR) | >> + (1 << NFIT_CMD_ARS_INJECT_GET); >> >> This drops function number 0 which userspace has no need to call. > > Actually I like to call function 0. Its an excellent test when > modifying the code path as its a no side effects function whose output > is known in advance and instantly recognizable. I also use it when > testing new firmware. > > What is the downside to allowing it? What bad things happen? It allows implementations to bypass the standardization process and ship new root DSMs. It's always possible to patch the kernel locally for development, so I see no reason to ship this capability globally. > Also, I do have to ask why you allow function zero for NVDIMM_FAMILY_MSFT? Yeah, that's an oversight / mistake, but it's also benign since it can't be used to add support for new function numbers to the family since all 32 numbers are already taken. We also allow override for leaf devices since there's quite a bit more per vendor differentiation that might take a while to standardize. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com> To: Jerry Hoemann <Jerry.Hoemann@hpe.com> Cc: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v4 3/6] libnvdimm, acpi, nfit: Add bus level dsm mask for pass thru. Date: Sat, 1 Jul 2017 13:08:50 -0700 [thread overview] Message-ID: <CAPcyv4hAMHzi7OK-z-B9LpT83Q1evH2-B-NXUMB2ebtVbjOdsg@mail.gmail.com> (raw) In-Reply-To: <20170701195805.GA13259@anatevka.americas.hpqcorp.net> On Sat, Jul 1, 2017 at 12:58 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > On Fri, Jun 30, 2017 at 08:55:22PM -0700, Dan Williams wrote: > > ... > >> On Fri, Jun 30, 2017 at 9:09 AM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: >> > + if (cmd == ND_CMD_CALL) >> > + dsm_mask = nd_desc->bus_dsm_mask; >> > desc = nd_cmd_bus_desc(cmd); >> > uuid = to_nfit_uuid(NFIT_DEV_BUS); >> > handle = adev->handle; >> > @@ -1613,6 +1615,7 @@ static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc) >> > struct nvdimm_bus_descriptor *nd_desc = &acpi_desc->nd_desc; >> > const u8 *uuid = to_nfit_uuid(NFIT_DEV_BUS); >> > struct acpi_device *adev; >> > + unsigned long dsm_mask; >> > int i; >> > >> > nd_desc->cmd_mask = acpi_desc->bus_cmd_force_en; >> > @@ -1624,6 +1627,11 @@ static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc) >> > if (acpi_check_dsm(adev->handle, uuid, 1, 1ULL << i)) >> > set_bit(i, &nd_desc->cmd_mask); >> > set_bit(ND_CMD_CALL, &nd_desc->cmd_mask); >> > + >> > + dsm_mask = 0x3bf; >> >> I went ahead and fixed this up to use dsm_mask defined like this: >> >> + dsm_mask = >> + (1 << ND_CMD_ARS_CAP) | >> + (1 << ND_CMD_ARS_START) | >> + (1 << ND_CMD_ARS_STATUS) | >> + (1 << ND_CMD_CLEAR_ERROR) | >> + (1 << NFIT_CMD_TRANSLATE_SPA) | >> + (1 << NFIT_CMD_ARS_INJECT_SET) | >> + (1 << NFIT_CMD_ARS_INJECT_CLEAR) | >> + (1 << NFIT_CMD_ARS_INJECT_GET); >> >> This drops function number 0 which userspace has no need to call. > > Actually I like to call function 0. Its an excellent test when > modifying the code path as its a no side effects function whose output > is known in advance and instantly recognizable. I also use it when > testing new firmware. > > What is the downside to allowing it? What bad things happen? It allows implementations to bypass the standardization process and ship new root DSMs. It's always possible to patch the kernel locally for development, so I see no reason to ship this capability globally. > Also, I do have to ask why you allow function zero for NVDIMM_FAMILY_MSFT? Yeah, that's an oversight / mistake, but it's also benign since it can't be used to add support for new function numbers to the family since all 32 numbers are already taken. We also allow override for leaf devices since there's quite a bit more per vendor differentiation that might take a while to standardize.
next prev parent reply other threads:[~2017-07-01 20:07 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-30 16:09 [PATCH v3 0/7] Enable DSM pass thru for root functions Jerry Hoemann 2017-06-30 16:09 ` Jerry Hoemann 2017-06-30 16:09 ` [PATCH v4 1/6] libnvdimm: passthru functions clear to send Jerry Hoemann 2017-06-30 16:09 ` Jerry Hoemann 2017-06-30 16:09 ` [PATCH v4 2/6] acpi, nfit: Enable DSM pass thru for root functions Jerry Hoemann 2017-06-30 16:09 ` Jerry Hoemann 2017-06-30 16:09 ` [PATCH v4 3/6] libnvdimm, acpi, nfit: Add bus level dsm mask for pass thru Jerry Hoemann 2017-06-30 16:09 ` Jerry Hoemann 2017-07-01 3:55 ` Dan Williams 2017-07-01 3:55 ` Dan Williams 2017-07-01 19:58 ` Jerry Hoemann 2017-07-01 19:58 ` Jerry Hoemann 2017-07-01 20:08 ` Dan Williams [this message] 2017-07-01 20:08 ` Dan Williams 2017-07-01 20:10 ` Dan Williams 2017-07-01 20:10 ` Dan Williams 2017-07-01 20:38 ` Jerry Hoemann 2017-07-01 20:38 ` Jerry Hoemann 2017-07-01 20:46 ` Dan Williams 2017-07-01 20:46 ` Dan Williams 2017-07-04 20:08 ` Jerry Hoemann 2017-07-04 20:08 ` Jerry Hoemann 2017-07-04 20:37 ` Dan Williams 2017-07-04 20:37 ` Dan Williams 2017-07-05 15:26 ` Linda Knippers 2017-07-05 15:26 ` Linda Knippers 2017-07-05 16:24 ` Jerry Hoemann 2017-07-05 16:24 ` Jerry Hoemann 2017-07-05 16:35 ` Dan Williams 2017-07-05 16:35 ` Dan Williams 2017-07-05 23:14 ` Jerry Hoemann 2017-07-05 23:14 ` Jerry Hoemann 2017-07-06 5:25 ` Dan Williams 2017-07-06 5:25 ` Dan Williams 2017-06-30 16:09 ` [PATCH v4 4/6] acpi, nfit: Show bus_dsm_mask in sysfs Jerry Hoemann 2017-06-30 16:09 ` Jerry Hoemann 2017-06-30 16:09 ` [PATCH v4 5/6] libnvdimm: New ACPI 6.2 DSM functions Jerry Hoemann 2017-06-30 16:09 ` Jerry Hoemann 2017-06-30 16:09 ` [PATCH v4 6/6] nfit allow override of root dsm mask Jerry Hoemann 2017-06-30 16:09 ` Jerry Hoemann 2017-07-01 3:49 ` Dan Williams 2017-07-01 3:49 ` Dan Williams
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAPcyv4hAMHzi7OK-z-B9LpT83Q1evH2-B-NXUMB2ebtVbjOdsg@mail.gmail.com \ --to=dan.j.williams@intel.com \ --cc=Jerry.Hoemann@hpe.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvdimm@lists.01.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.