| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20121108011006.GC21302@phenom.dumpdata.com>
Date: Wed, 7 Nov 2012 20:10:06 -0500
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: Matthew Fioravante <matthew.fioravante@...apl.edu>
Cc: Kent Yoder <key@...ux.vnet.ibm.com>,
"mail@...jiv.net" <mail@...jiv.net>,
"jeremy@...p.org" <jeremy@...p.org>,
"tpmdd-devel@...ts.sourceforge.net"
<tpmdd-devel@...ts.sourceforge.net>,
"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [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@...apl.edu>
> >>---
> >> 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
> >>
> >>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists