[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] add tpm_xenu.ko: Xen Virtual TPM frontend driver
-----Original Message----- From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx] Sent: Wednesday, November 07, 2012 11:07 AM To: Fioravante, Matthew E. Cc: key@xxxxxxxxxxxxxxxxxx; mail@xxxxxxxxxx; jeremy@xxxxxxxx; tpmdd-devel@xxxxxxxxxxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx 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 > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |