Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753248AbdF2Vz6 (ORCPT ); Thu, 29 Jun 2017 17:55:58 -0400 Received: from mail-yw0-f176.google.com ([209.85.161.176]:36196 "EHLO mail-yw0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751626AbdF2Vz5 (ORCPT ); Thu, 29 Jun 2017 17:55:57 -0400 MIME-Version: 1.0 In-Reply-To: <20170629214735.GC6065@anatevka.americas.hpqcorp.net> References: <20170629214735.GC6065@anatevka.americas.hpqcorp.net> From: Dan Williams Date: Thu, 29 Jun 2017 14:55:55 -0700 Message-ID: Subject: Re: [PATCH v3 4/7] acpi, nfit: Use bus_dsm_mask for passthru To: Jerry Hoemann Cc: "linux-nvdimm@lists.01.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2900 Lines: 67 On Thu, Jun 29, 2017 at 2:47 PM, Jerry Hoemann wrote: > On Thu, Jun 29, 2017 at 02:35:14PM -0700, Dan Williams wrote: >> On Thu, Jun 29, 2017 at 9:56 AM, Jerry Hoemann wrote: >> > Populate bus_dsm_mask and use it to filter dsm calls that user can >> > make through the pass thru interface. >> > >> > Signed-off-by: Jerry Hoemann >> > --- >> > drivers/acpi/nfit/core.c | 5 +++++ >> > 1 file changed, 5 insertions(+) >> > >> > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c >> > index b46fca2..971002b 100644 >> > --- a/drivers/acpi/nfit/core.c >> > +++ b/drivers/acpi/nfit/core.c >> > @@ -253,6 +253,8 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, >> > cmd_name = nvdimm_bus_cmd_name(cmd); >> > cmd_mask = nd_desc->cmd_mask; >> > dsm_mask = cmd_mask; >> > + 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; >> > @@ -1624,6 +1626,9 @@ 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); >> > + for (i = 0; i < ND_CMD_CALL; i++) >> > + if (acpi_check_dsm(adev->handle, uuid, 1, 1ULL << i)) >> > + set_bit(i, &nd_desc->bus_dsm_mask); >> > } >> >> This loop checks for function 6 which is specified as reserved. Lets >> explicitly test for the known good function numbers with something >> like this: >> >> /* this should be private in drivers/acpi/nfit/nfit.h */ >> enum nfit_aux_cmds { >> NFIT_CMD_TRANSLATE_SPA = 5, >> NFIT_CMD_ARS_INJECT_SET = 7, >> NFIT_CMD_ARS_INJECT_CLEAR = 8, >> NFIT_CMD_ARS_INJECT_GET = 9, >> }; >> >> bus_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); >> >> for_each_set_bit(i, &bus_dsm_mask... > > > I added the for_each_set_bit check in patch 7 of the series. True, but in a patch series we shouldn't introduce a bug in one patch and fix it later in the same series. Also, if patch7 goes away we would need to fold that enabling in here. Part of trying to parse what 0x3bf meant lead me to this, and I'm wondering if we should do the same for the other magic vendor dsm_mask constants, but that's a patch for another time.