[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
On Wed, Nov 07, 2012 at 01:14:25PM -0500, Matthew Fioravante wrote: > On 11/07/2012 09:46 AM, Kent Yoder wrote: > >Hi Matthew, > > > >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. > >> > >>Signed-off-by: Matthew Fioravante <matthew.fioravante@xxxxxxxxxx> > >>--- > >> drivers/char/tpm/Kconfig | 9 + > >> drivers/char/tpm/Makefile | 2 + > >> drivers/char/tpm/tpm.h | 15 + > >> drivers/char/tpm/tpm_vtpm.c | 543 ++++++++++++++++++++++++++++++ > >> drivers/char/tpm/tpm_vtpm.h | 55 +++ > >> drivers/char/tpm/tpm_xen.c | 690 > >> ++++++++++++++++++++++++++++++++++++++ > >> include/xen/interface/io/tpmif.h | 77 +++++ > >> 7 files changed, 1391 insertions(+) > >> create mode 100644 drivers/char/tpm/tpm_vtpm.c > >> create mode 100644 drivers/char/tpm/tpm_vtpm.h > >> create mode 100644 drivers/char/tpm/tpm_xen.c > >> create mode 100644 include/xen/interface/io/tpmif.h > >> > >>diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig > >>index 915875e..08c1010 100644 > >>--- a/drivers/char/tpm/Kconfig > >>+++ b/drivers/char/tpm/Kconfig > >>@@ -81,4 +81,13 @@ config TCG_IBMVTPM > >> will be accessible from within Linux. To compile this driver > >> as a module, choose M here; the module will be called tpm_ibmvtpm. > >> > >>+config TCG_XEN > >>+ tristate "XEN TPM Interface" > >>+ depends on TCG_TPM && XEN > >>+ ---help--- > >>+ If you want to make TPM support available to a Xen user domain, > >>+ say Yes and it will be accessible from within Linux. > >>+ To compile this driver as a module, choose M here; the module > >>+ will be called tpm_xenu. > >>+ > >> endif # TCG_TPM > >>diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile > >>index 5b3fc8b..16911c5 100644 > >>--- a/drivers/char/tpm/Makefile > >>+++ b/drivers/char/tpm/Makefile > >>@@ -17,3 +17,5 @@ obj-$(CONFIG_TCG_NSC) += tpm_nsc.o > >> obj-$(CONFIG_TCG_ATMEL) += tpm_atmel.o > >> obj-$(CONFIG_TCG_INFINEON) += tpm_infineon.o > >> obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o > >>+obj-$(CONFIG_TCG_XEN) += tpm_xenu.o > >>+tpm_xenu-y = tpm_xen.o tpm_vtpm.o > > Let's match the naming convention of the other drivers if we can, so this > >would be something like tpm_xenvtpm.c. tpm_vtpm is too generic... > Makes sense, fixed. > > > >>diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > >>index 8ef7649..2e5a47a 100644 > >>--- a/drivers/char/tpm/tpm.h > >>+++ b/drivers/char/tpm/tpm.h > >>@@ -130,6 +130,9 @@ struct tpm_chip { > >> > >> struct list_head list; > >> void (*release) (struct device *); > >>+#if CONFIG_XEN > >>+ void *priv; > >>+#endif > > Can you use the chip->vendor.data pointer here instead? tpm_ibmvtpm is > >already using that as a priv pointer. I should probably change that name > >to make it more obvious what that's used for. > That makes more sense. I'm guessing your data pointer didn't exist > during the 2.6.18 kernel which is why they added their own priv > pointer. > > > >> }; > >> > >> #define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor) > >>@@ -310,6 +313,18 @@ struct tpm_cmd_t { > >> > >> ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *); > >> > >>+#ifdef CONFIG_XEN > >>+static inline void *chip_get_private(const struct tpm_chip *chip) > >>+{ > >>+ return chip->priv; > >>+} > >>+ > >>+static inline void chip_set_private(struct tpm_chip *chip, void *priv) > >>+{ > >>+ chip->priv = priv; > >>+} > >>+#endif > > Can you put these in tpm_vtpm.c please? One less #define. :-) > Agreed, I'd rather not have to modify your shared tpm.h interface at all. > > > >[cut] > >>+#ifndef __XEN_PUBLIC_IO_TPMIF_H__ > >>+#define __XEN_PUBLIC_IO_TPMIF_H__ > >>+ > >>+#include "../grant_table.h" > >>+ > >>+struct tpmif_tx_request { > >>+ unsigned long addr; /* Machine address of packet. */ > >>+ grant_ref_t ref; /* grant table access reference */ > >>+ uint16_t unused; > >>+ uint16_t size; /* Packet size in bytes. */ > >>+}; > >>+typedef struct tpmif_tx_request tpmif_tx_request_t; > > checkpatch warned on this new typedef - please run through checkpatch > >and fix up that stuff. > tpmif.h has a couple of typedefs which do trigger checkpatch > warnings. However it looks like the paradigm for xen is to have > these interface/io/<dev>if.h files and all of them have typedefs. I > think in this case the typedef should probably stay. > > Konrad your thoughts here? Rip them out plea > > > >>+ > >>+/* > >>+ * The TPMIF_TX_RING_SIZE defines the number of pages the > >>+ * front-end and backend can exchange (= size of array). > >>+ */ > >>+typedef uint32_t TPMIF_RING_IDX; > >>+ > >>+#define TPMIF_TX_RING_SIZE 1 > >>+ > >>+/* This structure must fit in a memory page. */ > >>+ > >>+struct tpmif_ring { > >>+ struct tpmif_tx_request req; > >>+}; > >>+typedef struct tpmif_ring tpmif_ring_t; > >>+ > >>+struct tpmif_tx_interface { > >>+ struct tpmif_ring ring[TPMIF_TX_RING_SIZE]; > >>+}; > >>+typedef struct tpmif_tx_interface tpmif_tx_interface_t; > >>+ > >>+#endif > >>+ > >>+/* > >>+ * Local variables: > >>+ * mode: C > >>+ * c-set-style: "BSD" > >>+ * c-basic-offset: 4 > >>+ * tab-width: 4 > >>+ * indent-tabs-mode: nil > >>+ * End: > >>+ */ > > Please take this comment out, see Documentation/CodingStyle. > Fixed > > > >Thanks, > >Kent > > > >>-- > >>1.7.10.4 > >> > >> > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |