Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751600AbbKJTty (ORCPT ); Tue, 10 Nov 2015 14:49:54 -0500 Received: from g9t5009.houston.hp.com ([15.240.92.67]:54927 "EHLO g9t5009.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751169AbbKJTtt (ORCPT ); Tue, 10 Nov 2015 14:49:49 -0500 Date: Tue, 10 Nov 2015 12:49:38 -0700 From: Jerry Hoemann To: Jeff Moyer Cc: ross.zwisler@linux.intel.com, rjw@rjwysocki.net, lenb@kernel.org, dan.j.williams@intel.com, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-nvdimm@ml01.01.org Subject: Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru. Message-ID: <20151110194938.GA47666@tevye.fc.hp.com> Reply-To: Jerry.Hoemann@hpe.com References: <5411049831df211c8280e3239aaf344af033fa25.1446684001.git.jerry.hoemann@hpe.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 Content-Length: 3134 Lines: 79 On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote: > Jerry Hoemann writes: > > > Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU. > > Can't you just make passthrough a separate command? If you actually add There are multiple conflicting NVDIMM _DSM running around, they are "device specific". So, we should plan in general and not just for the example DSM that Intel added support for. These DSM have over lapping and incompatible function ids. The Intel example is an example, not standard. They are free to change it at will. So, we can't be certain there won't be a conflict some time in the future if we try to use their number space. I'm trying to create a generic pass thru that any vendors can use. Putting this in the Intel function number space doesn't make a lot of sense to me. > the ioctl definition for passthrough (which you didn't do for some > reason?), it looks odd: The definition for the IOCTLs are in a user space application. These aren't required in the kernel as the kernel is only a pass thru. As the DSM I'm working with isn't yet finalized, I've been told that i can't share the user space portion yet. > > #define ND_IOCTL_PASSTHRU _IOWR(NVDIMM_TYPE_PASSTHRU,, ND_CMD_PASSTHRU, \ > struct ndn_package) > > Care to comment on why you chose a different type instead of specifying > a new command? > > > +struct ndn_pkg { > > + struct { > > + __u8 dsm_uuid[16]; > > + __u32 dsm_in; /* size of _DSM input */ > > + __u32 dsm_out; /* size of user buffer */ > > + __u32 dsm_rev; /* revision of dsm call */ > > + __u32 res[8]; /* reserved must be zero */ > > + __u32 dsm_size; /* size _DSM would write */ > > + } h; > > + unsigned char buf[]; > > Please change that to: > __u8 *buf; > since acpi_object.buffer.pointer is a u8 *. buf isn't being passed to acpi_evaluate_dsm. its just being used for pointer offset in acpi_nfit_ctl_passthru. The "payload" that will be passed to acpi_evaluate_dsm follows. > > Note that the size of this structure will be different for 32 vs. 64 > bit, but I don't think it matters since offsets won't change (the > pointer is at the end of the structure). I assume you mean size of struct changes if I use the pointer as substitute for the zero sized array? or are you saying that the packed attribute doesn't affect the layout of the anonymous struct? -- ----------------------------------------------------------------------------- Jerry Hoemann Software Engineer Hewlett-Packard Enterprise 3404 E Harmony Rd. MS 36 phone: (970) 898-1022 Ft. Collins, CO 80528 FAX: (970) 898-0707 email: jerry.hoemann@hpe.com ----------------------------------------------------------------------------- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/