From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 11 Dec 2015 11:09:50 -0700 From: Jerry Hoemann Subject: Re: [PATCH v3 3/3] nvdimm: Add IOCTL pass thru functions Message-ID: <20151211180950.GA57513@tevye.fc.hp.com> Reply-To: Jerry.Hoemann@hpe.com References: <20151210002442.GA104756@tevye.fc.hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Dan Williams Cc: Ross Zwisler , "Rafael J. Wysocki" , Len Brown , "Elliott, Robert (Persistent Memory)" , jmoyer , Dmitry Krivenok , Linda Knippers , "linux-nvdimm@lists.01.org" , Linux ACPI , "linux-kernel@vger.kernel.org" List-ID: On Wed, Dec 09, 2015 at 04:48:55PM -0800, Dan Williams wrote: > > Yes, still very much interested in this path. But the code will need > to be around for quite awhile now that it has appeared in a released > kernel and a released version of ndctl. So anything that makes the > code more maintainable in the interim is beneficial. > > > If yes, the work we need to do to integrate these two different approaches > > will just need to be undone. Further if we deprecate the current IOCTLs, > > then the nd_cmd_desc tables and related nd_cmd_in_size and nd_cmd_out_size > > could then be removed. > > I'm not seeing this as a large amount of work. Do you want to hand > off this task to me? I have implemented this and will send as part of next version. > > > ACPI defines arguments to a _DSM as 64 bit quantities. We want the interface > > exported to the user to follow the ACPI spec. The variables above collect > > the value of rev or func from the two different sources (wrapper or legacy) > > and then passes to acpi_evaluate_dsm which defines the parameters as simple > > ints. > > > > So, going from user interface to call of acpi_evaluate_dsm there will be > > a truncation somewhere. > > > > Looking at acpi_evaluate_dsm(), it uses union acpi_object and fills in > > .integer.value for both rev and func. These are defined as u64. > > > > So patching acpi_evaluate_dsm to make the rev and func parameters u64 might > > be do'able, but we'd still have potential sign issues with other callers > > to acpi_evaluate_dsm which look to be using simple ints in the call. > > > > Do you want me to look at patching acpi_evaluate_dsm (and possibly > > its callers) as part of this patch set? > > Yes, updating the acpi_evaluate_dsm() definition seems the best choice. > I have a patch for this. While not big (6 files), these files are outside of nvdimm and will have a two line of over a dozen reviewers/lists not previously reviewing this series. Do you want me to send this patch as one of this series (w/ the extra reviewers?) Or as separate stand alone to just those reviewers (and yourself.) (and if this option, before or after this series.) ----------------------------------------------------------------------------- Jerry Hoemann Software Engineer Hewlett Packard Enterprise ----------------------------------------------------------------------------- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751517AbbLKSJ4 (ORCPT ); Fri, 11 Dec 2015 13:09:56 -0500 Received: from g4t3426.houston.hp.com ([15.201.208.54]:33489 "EHLO g4t3426.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750923AbbLKSJy (ORCPT ); Fri, 11 Dec 2015 13:09:54 -0500 Date: Fri, 11 Dec 2015 11:09:50 -0700 From: Jerry Hoemann To: Dan Williams Cc: Ross Zwisler , "Rafael J. Wysocki" , Len Brown , "Elliott, Robert (Persistent Memory)" , jmoyer , Dmitry Krivenok , Linda Knippers , "linux-nvdimm@lists.01.org" , Linux ACPI , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v3 3/3] nvdimm: Add IOCTL pass thru functions Message-ID: <20151211180950.GA57513@tevye.fc.hp.com> Reply-To: Jerry.Hoemann@hpe.com References: <20151210002442.GA104756@tevye.fc.hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 09, 2015 at 04:48:55PM -0800, Dan Williams wrote: > > Yes, still very much interested in this path. But the code will need > to be around for quite awhile now that it has appeared in a released > kernel and a released version of ndctl. So anything that makes the > code more maintainable in the interim is beneficial. > > > If yes, the work we need to do to integrate these two different approaches > > will just need to be undone. Further if we deprecate the current IOCTLs, > > then the nd_cmd_desc tables and related nd_cmd_in_size and nd_cmd_out_size > > could then be removed. > > I'm not seeing this as a large amount of work. Do you want to hand > off this task to me? I have implemented this and will send as part of next version. > > > ACPI defines arguments to a _DSM as 64 bit quantities. We want the interface > > exported to the user to follow the ACPI spec. The variables above collect > > the value of rev or func from the two different sources (wrapper or legacy) > > and then passes to acpi_evaluate_dsm which defines the parameters as simple > > ints. > > > > So, going from user interface to call of acpi_evaluate_dsm there will be > > a truncation somewhere. > > > > Looking at acpi_evaluate_dsm(), it uses union acpi_object and fills in > > .integer.value for both rev and func. These are defined as u64. > > > > So patching acpi_evaluate_dsm to make the rev and func parameters u64 might > > be do'able, but we'd still have potential sign issues with other callers > > to acpi_evaluate_dsm which look to be using simple ints in the call. > > > > Do you want me to look at patching acpi_evaluate_dsm (and possibly > > its callers) as part of this patch set? > > Yes, updating the acpi_evaluate_dsm() definition seems the best choice. > I have a patch for this. While not big (6 files), these files are outside of nvdimm and will have a two line of over a dozen reviewers/lists not previously reviewing this series. Do you want me to send this patch as one of this series (w/ the extra reviewers?) Or as separate stand alone to just those reviewers (and yourself.) (and if this option, before or after this series.) ----------------------------------------------------------------------------- Jerry Hoemann Software Engineer Hewlett Packard Enterprise -----------------------------------------------------------------------------