| 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
| ||
|
Date: Wed, 03 Jul 2024 02:52:02 +0300
From: Jarkko Sakkinen <jarkko@...nel.org>
To: "Rob Herring (Arm)" <robh@...nel.org>, Peter Huewe <peterhuewe@....de>,
Jason Gunthorpe <jgg@...pe.ca>, Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>, Christophe Leroy
<christophe.leroy@...roup.eu>, "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>
Cc: linux-kernel@...r.kernel.org, linux-integrity@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH] tpm: atmel: Drop PPC64 specific MMIO setup
On Tue, 2024-07-02 at 10:10 -0600, Rob Herring (Arm) wrote:
> The PPC64 specific MMIO setup open codes DT address functions rather
> than using standard address parsing functions. The open-coded version
> fails to handle any address translation and is not endian safe.
>
> I haven't found any evidence of what platform used this. The only thing
> that turned up was a PPC405 platform, but that is 32-bit and PPC405
> support is being removed as well. CONFIG_TCG_ATMEL is not enabled for
> any powerpc config and never was. The support was added in 2005 and
> hasn't been touched since.
>
> Rather than try to modernize and fix this code, just remove it.
>
> Signed-off-by: Rob Herring (Arm) <robh@...nel.org>
> ---
> drivers/char/tpm/Kconfig | 2 +-
> drivers/char/tpm/tpm_atmel.c | 64 +++++++++++++++-
> drivers/char/tpm/tpm_atmel.h | 140 -----------------------------------
> 3 files changed, 62 insertions(+), 144 deletions(-)
> delete mode 100644 drivers/char/tpm/tpm_atmel.h
>
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index e63a6a17793c..9b655e9fc7ab 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -162,7 +162,7 @@ config TCG_NSC
>
> config TCG_ATMEL
> tristate "Atmel TPM Interface"
> - depends on PPC64 || HAS_IOPORT_MAP
> + depends on HAS_IOPORT_MAP
> depends on HAS_IOPORT
> help
> If you have a TPM security chip from Atmel say Yes and it
> diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
> index 9fb2defa9dc4..622c4abe8cb3 100644
> --- a/drivers/char/tpm/tpm_atmel.c
> +++ b/drivers/char/tpm/tpm_atmel.c
> @@ -15,7 +15,67 @@
> */
>
> #include "tpm.h"
> -#include "tpm_atmel.h"
> +
> +struct tpm_atmel_priv {
> + int region_size;
> + int have_region;
> + unsigned long base;
> + void __iomem *iobase;
> +};
> +
> +#define atmel_getb(chip, offset) inb(atmel_get_priv(chip)->base + offset)
> +#define atmel_putb(val, chip, offset) \
> + outb(val, atmel_get_priv(chip)->base + offset)
> +#define atmel_request_region request_region
> +#define atmel_release_region release_region
> +/* Atmel definitions */
> +enum tpm_atmel_addr {
> + TPM_ATMEL_BASE_ADDR_LO = 0x08,
> + TPM_ATMEL_BASE_ADDR_HI = 0x09
> +};
> +
> +static inline int tpm_read_index(int base, int index)
> +{
> + outb(index, base);
> + return inb(base+1) & 0xFF;
> +}
> +
> +/* Verify this is a 1.1 Atmel TPM */
> +static int atmel_verify_tpm11(void)
> +{
> +
> + /* verify that it is an Atmel part */
> + if (tpm_read_index(TPM_ADDR, 4) != 'A' ||
> + tpm_read_index(TPM_ADDR, 5) != 'T' ||
> + tpm_read_index(TPM_ADDR, 6) != 'M' ||
> + tpm_read_index(TPM_ADDR, 7) != 'L')
> + return 1;
> +
> + /* query chip for its version number */
> + if (tpm_read_index(TPM_ADDR, 0x00) != 1 ||
> + tpm_read_index(TPM_ADDR, 0x01) != 1)
> + return 1;
> +
> + /* This is an atmel supported part */
> + return 0;
> +}
> +
> +/* Determine where to talk to device */
> +static void __iomem * atmel_get_base_addr(unsigned long *base, int *region_size)
> +{
> + int lo, hi;
> +
> + if (atmel_verify_tpm11() != 0)
> + return NULL;
> +
> + lo = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_LO);
> + hi = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_HI);
> +
> + *base = (hi << 8) | lo;
> + *region_size = 2;
> +
> + return ioport_map(*base, *region_size);
> +}
>
> /* write status bits */
> enum tpm_atmel_write_status {
> @@ -142,7 +202,6 @@ static void atml_plat_remove(void)
> tpm_chip_unregister(chip);
> if (priv->have_region)
> atmel_release_region(priv->base, priv->region_size);
> - atmel_put_base_addr(priv->iobase);
> platform_device_unregister(pdev);
> }
>
> @@ -211,7 +270,6 @@ static int __init init_atmel(void)
> err_unreg_dev:
> platform_device_unregister(pdev);
> err_rel_reg:
> - atmel_put_base_addr(iobase);
> if (have_region)
> atmel_release_region(base,
> region_size);
> diff --git a/drivers/char/tpm/tpm_atmel.h b/drivers/char/tpm/tpm_atmel.h
> deleted file mode 100644
> index 7ac3f69dcf0f..000000000000
> --- a/drivers/char/tpm/tpm_atmel.h
> +++ /dev/null
> @@ -1,140 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Copyright (C) 2005 IBM Corporation
> - *
> - * Authors:
> - * Kylene Hall <kjhall@...ibm.com>
> - *
> - * Maintained by: <tpmdd-devel@...ts.sourceforge.net>
> - *
> - * Device driver for TCG/TCPA TPM (trusted platform module).
> - * Specifications at www.trustedcomputinggroup.org
> - *
> - * These difference are required on power because the device must be
> - * discovered through the device tree and iomap must be used to get
> - * around the need for holes in the io_page_mask. This does not happen
> - * automatically because the tpm is not a normal pci device and lives
> - * under the root node.
> - */
> -
> -struct tpm_atmel_priv {
> - int region_size;
> - int have_region;
> - unsigned long base;
> - void __iomem *iobase;
> -};
> -
> -#ifdef CONFIG_PPC64
> -
> -#include <linux/of.h>
> -
> -#define atmel_getb(priv, offset) readb(priv->iobase + offset)
> -#define atmel_putb(val, priv, offset) writeb(val, priv->iobase + offset)
> -#define atmel_request_region request_mem_region
> -#define atmel_release_region release_mem_region
> -
> -static inline void atmel_put_base_addr(void __iomem *iobase)
> -{
> - iounmap(iobase);
> -}
> -
> -static void __iomem * atmel_get_base_addr(unsigned long *base, int *region_size)
> -{
> - struct device_node *dn;
> - unsigned long address, size;
> - const unsigned int *reg;
> - int reglen;
> - int naddrc;
> - int nsizec;
> -
> - dn = of_find_node_by_name(NULL, "tpm");
> -
> - if (!dn)
> - return NULL;
> -
> - if (!of_device_is_compatible(dn, "AT97SC3201")) {
> - of_node_put(dn);
> - return NULL;
> - }
> -
> - reg = of_get_property(dn, "reg", ®len);
> - naddrc = of_n_addr_cells(dn);
> - nsizec = of_n_size_cells(dn);
> -
> - of_node_put(dn);
> -
> -
> - if (naddrc == 2)
> - address = ((unsigned long) reg[0] << 32) | reg[1];
> - else
> - address = reg[0];
> -
> - if (nsizec == 2)
> - size =
> - ((unsigned long) reg[naddrc] << 32) | reg[naddrc + 1];
> - else
> - size = reg[naddrc];
> -
> - *base = address;
> - *region_size = size;
> - return ioremap(*base, *region_size);
> -}
> -#else
> -#define atmel_getb(chip, offset) inb(atmel_get_priv(chip)->base + offset)
> -#define atmel_putb(val, chip, offset) \
> - outb(val, atmel_get_priv(chip)->base + offset)
> -#define atmel_request_region request_region
> -#define atmel_release_region release_region
> -/* Atmel definitions */
> -enum tpm_atmel_addr {
> - TPM_ATMEL_BASE_ADDR_LO = 0x08,
> - TPM_ATMEL_BASE_ADDR_HI = 0x09
> -};
> -
> -static inline int tpm_read_index(int base, int index)
> -{
> - outb(index, base);
> - return inb(base+1) & 0xFF;
> -}
> -
> -/* Verify this is a 1.1 Atmel TPM */
> -static int atmel_verify_tpm11(void)
> -{
> -
> - /* verify that it is an Atmel part */
> - if (tpm_read_index(TPM_ADDR, 4) != 'A' ||
> - tpm_read_index(TPM_ADDR, 5) != 'T' ||
> - tpm_read_index(TPM_ADDR, 6) != 'M' ||
> - tpm_read_index(TPM_ADDR, 7) != 'L')
> - return 1;
> -
> - /* query chip for its version number */
> - if (tpm_read_index(TPM_ADDR, 0x00) != 1 ||
> - tpm_read_index(TPM_ADDR, 0x01) != 1)
> - return 1;
> -
> - /* This is an atmel supported part */
> - return 0;
> -}
> -
> -static inline void atmel_put_base_addr(void __iomem *iobase)
> -{
> -}
> -
> -/* Determine where to talk to device */
> -static void __iomem * atmel_get_base_addr(unsigned long *base, int *region_size)
> -{
> - int lo, hi;
> -
> - if (atmel_verify_tpm11() != 0)
> - return NULL;
> -
> - lo = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_LO);
> - hi = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_HI);
> -
> - *base = (hi << 8) | lo;
> - *region_size = 2;
> -
> - return ioport_map(*base, *region_size);
> -}
> -#endif
Acknowledging that I noticed this but won't test
before week 31, when I'm back from holiday.
BR, Jarkko
Powered by blists - more mailing lists