From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752358AbdI0S1T (ORCPT ); Wed, 27 Sep 2017 14:27:19 -0400 Received: from esa2.dell-outbound.iphmx.com ([68.232.149.220]:46779 "EHLO esa2.dell-outbound.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751754AbdI0S1R (ORCPT ); Wed, 27 Sep 2017 14:27:17 -0400 From: X-LoopCount0: from 10.166.132.172 X-IronPort-AV: E=Sophos;i="5.42,446,1500958800"; d="scan'208";a="990956257" X-DLP: DLP_GlobalPCIDSS To: CC: , , , , Subject: RE: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs interface for SMBIOS tokens Thread-Topic: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs interface for SMBIOS tokens Thread-Index: AQHTNhqxkSn0H9szfkuV8OzZLWWcOaLGKEWA//+w9KCAA2/wgP//w76g Date: Wed, 27 Sep 2017 18:27:15 +0000 Message-ID: References: <1342f21a4f2c9af86d3033a9af9d9a4d05d27106.1505999739.git.mario.limonciello@dell.com> <20170925162333.GL22190@pali> <20170927165051.GE23572@fury> In-Reply-To: <20170927165051.GE23572@fury> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.143.18.86] Content-Type: text/plain; charset="iso-8859-1" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v8RIRNrZ008425 > -----Original Message----- > From: Darren Hart [mailto:dvhart@infradead.org] > Sent: Wednesday, September 27, 2017 12:51 PM > To: Limonciello, Mario > Cc: andy.shevchenko@gmail.com; pali.rohar@gmail.com; linux- > kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; > quasisec@google.com > Subject: Re: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs interface > for SMBIOS tokens > > On Mon, Sep 25, 2017 at 05:31:05PM +0000, Mario.Limonciello@dell.com wrote: > > > -----Original Message----- > > > From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] > > > Sent: Monday, September 25, 2017 1:04 PM > > > To: Pali Rohár > > > Cc: Limonciello, Mario ; dvhart@infradead.org; > > > LKML ; Platform Driver > > x86@vger.kernel.org>; quasisec@google.com > > > Subject: Re: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs > interface > > > for SMBIOS tokens > > > > > > On Mon, Sep 25, 2017 at 7:23 PM, Pali Rohár wrote: > > > > On Thursday 21 September 2017 08:57:11 Mario Limonciello wrote: > > > >> Currently userspace tools can access system tokens via the dcdbas > > > >> kernel module and a SMI call that will cause the platform to execute > > > >> SMM code. > > > >> > > > >> With a goal in mind of deprecating the dcdbas kernel module a different > > > >> method for accessing these tokens from userspace needs to be created. > > > >> > > > >> This is intentionally marked to only be readable as root as it can > > > >> contain sensitive information about the platform's configuration. > > > > > > > > Darren, Andy, any comments? I'm not quite sure if such API is suitable > > > > for long term in kernel. > > > > > > I would try to avoid sysfs interfaces for some particular devices. > > > Besides we are creating a character device. Would it be suitable there? > > > > If the character device having 2 different ioctls for different needs is > > acceptable I'm happy to adjust the series to do this instead. > > One piece of feedback I had re the char device was to see if we could avoid the > need for the IOCTL altogether, I'd like to have that discussion before we add > another. My original design was sysfs files for everything but it was raised by several folks that you run into the potential of two userspace processes stomping on each other's data when they run the ACPI call. That's why I need to have a mutex to protect and make sure that userspace calls get the right results. > > > > > > > > > > Basically tokens are list of tuples with > > > > possibility to active them, right? > > > > > > > > I didn't add a way to activate them through this, it was only for > > reading purpose. Activating them should be possible through the > > SMBIOS calling interface though. > > > > These are read-only as I understood it, and only with the right privileges. > Sysfs seemed appropriate for this to me. Andy S was against having this data as another sysfs file. From a userspace perspective I think it's simpler to just parse a sysfs file with read only static data as root. With the current ioctl based solution it requires userspace to run an ioctl to determine how many tokens exist, then allocate a chunk of memory big enough to hold all the token data and then run another ioctl to get all the tokens. Andy S, given this change between v1 and v2 what do you feel is better?