Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756195Ab2KHPmA (ORCPT ); Thu, 8 Nov 2012 10:42:00 -0500 Received: from pilot.jhuapl.edu ([128.244.251.36]:62540 "EHLO jhuapl.edu" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752415Ab2KHPlv convert rfc822-to-8bit (ORCPT ); Thu, 8 Nov 2012 10:41:51 -0500 From: "Fioravante, Matthew E." To: Konrad Rzeszutek Wilk CC: "key@linux.vnet.ibm.com" , "mail@srajiv.net" , "jeremy@goop.org" , "tpmdd-devel@lists.sourceforge.net" , "xen-devel@lists.xensource.com" , "linux-kernel@vger.kernel.org" Date: Thu, 8 Nov 2012 10:40:34 -0500 Subject: RE: [PATCH] add tpm_xenu.ko: Xen Virtual TPM frontend driver Thread-Topic: [PATCH] add tpm_xenu.ko: Xen Virtual TPM frontend driver Thread-Index: Ac29A74MlKXxsP0MQBeRIfuPxfA7IAAw1yJg Message-ID: <068F06DC4D106941B297C0C5F9F446EA48A229E705@aplesstripe.dom1.jhuapl.edu> References: <1352128197-1539-1-git-send-email-matthew.fioravante@jhuapl.edu> <20121106193921.GC28473@phenom.dumpdata.com> <509A6AA6.2000208@jhuapl.edu> <20121107160659.GH18615@phenom.dumpdata.com> In-Reply-To: <20121107160659.GH18615@phenom.dumpdata.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4478 Lines: 118 -----Original Message----- From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com] Sent: Wednesday, November 07, 2012 11:07 AM To: Fioravante, Matthew E. Cc: key@linux.vnet.ibm.com; mail@srajiv.net; jeremy@goop.org; tpmdd-devel@lists.sourceforge.net; xen-devel@lists.xensource.com; linux-kernel@vger.kernel.org Subject: Re: [PATCH] add tpm_xenu.ko: Xen Virtual TPM frontend driver On Wed, Nov 07, 2012 at 09:05:26AM -0500, Matthew Fioravante wrote: > On 11/06/2012 02:39 PM, Konrad Rzeszutek Wilk wrote: > >On Mon, Nov 05, 2012 at 10:09:57AM -0500, Matthew Fioravante wrote: > >>This patch ports the xen vtpm frontend driver for linux from the > >>linux-2.6.18-xen.hg tree to linux-stable. > >So how does on test it ? Set it up? Use it? Is there some > >documentation about it - if so it should be in the patch description. > Thats actually a question I had. To use this driver now you have to > use my vtpm mini-os domains which are currently being evaluated in the > xen-devel mailing list. Once they are accepted I will submit a > documentation update to the Xen tree. > > Whats the best practice for documentation in this case? All in xen? > Some linux/some xen? If the latter, how much goes in linux and where? As much as possible. I would say stick both of them in Xen and in Linux. And you can designate one of them as primary (say the Xen one) and say in the Linux: "For up-to-date information, please refer to XXYYZZ" I grepped through the linux source code and didn't see any documentation for the other xen drivers. I could put a "please refer to XXX in xen" line in the Kconfig or as a comment in the source code or both. Am I missing something or is there some standard way the xen devs handle this documentation issue for linux drivers? > > > >I did a very very cursory look at it, see some of the comments. > > > >> > >>+ > >>+ > >>+static inline struct transmission *transmission_alloc(void) { > >>+ return kzalloc(sizeof(struct transmission), GFP_ATOMIC); } > >>+ > >>+ static unsigned char * > > > >That is very weird tabbing? Did you run this patch through > >scripts/checkpatch.pl ? > Wow thats ugly. I ran the check script and it looks like it didn't > pick this up. For some reason my editor wants to autoindent like that. > Fixed. > > > >>+ > >>+static const struct file_operations vtpm_ops = { > >>+ .owner = THIS_MODULE, > >>+ .llseek = no_llseek, > >>+ .open = tpm_open, > >>+ .read = tpm_read, > >>+ .write = tpm_write, > >>+ .release = tpm_release, > >>+}; > >>+ > >>+static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL); static > >>+DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL); static > >>+DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL); static > >>+DEVICE_ATTR(active, S_IRUGO, tpm_show_active, NULL); static > >>+DEVICE_ATTR(owned, S_IRUGO, tpm_show_owned, NULL); static > >>+DEVICE_ATTR(temp_deactivated, S_IRUGO, tpm_show_temp_deactivated, > >>+ NULL); > >>+static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL); static > >>+DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel); > >>+ > >>+static struct attribute *vtpm_attrs[] = { > >>+ &dev_attr_pubek.attr, > >>+ &dev_attr_pcrs.attr, > >>+ &dev_attr_enabled.attr, > >>+ &dev_attr_active.attr, > >>+ &dev_attr_owned.attr, > >>+ &dev_attr_temp_deactivated.attr, > >>+ &dev_attr_caps.attr, > >>+ &dev_attr_cancel.attr, > >>+ NULL, > >So are these going to show up in SysFS? If so, there should also be a > >corresponding file in Documentation/.../sysfs/something. > These are similar to the entries made by the other tpm drivers. I > don't see any documentation about those either. TPM maintainers, any > guidance there? > > > >>+#include "tpm.h" > >>+#include "tpm_vtpm.h" > >>+ > >>+#undef DEBUG > >>+ > >>+#define GRANT_INVALID_REF 0 > >Interesting. The 0 grant value is actually a valid one. I think you > >want (-1ULL). > Is it? > drivers/block/xen-blkfront.c and > drivers/net/xen-netfront.c > > do the exact same thing You are right. Just leave it as that then. > >>+ > >>+ init_tpm_xenbus(); > >>+ return 0; > >>+} > >>+ > >>+ > >>+module_init(tpmif_init); > >no module_exit? > Will fix > > -- 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/