[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 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?

+
+/*
+ * 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




Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.