Hi This would need Tested-by's. I've tested it both PTT/fTPM based system and dTPM (2.0) based system. Thank you.
/Jarkko On Wed, Sep 30, 2015 at 06:19:28PM +0300, Jarkko Sakkinen wrote: > Both for FIFO and CRB interface TCG has decided to use the same HID > MSFT0101. They can be differentiated by looking at the start method from > TPM2 ACPI table. This patches makes necessary fixes to tpm_tis and > tpm_crb modules in order to correctly detect, which module should be > used. > > For MSFT0101 we must use struct acpi_driver because struct pnp_driver > has a 7 character limitation. > > It turned out that the root cause in b371616b8 was not correct for > https://bugzilla.kernel.org/show_bug.cgi?id=98181. > > v2: > > * One fixup was missing from v1: is_tpm2_fifo -> is_fifo > > v3: > > * Use pnp_driver for existing HIDs > > Reported-by: Michael Saunders <mick.saund...@gmail.com> > Reported-by: Michael Marley <mich...@michaelmarley.com> > Reported-by: Jethro Beekman <ker...@jbeekman.nl> > Reported-by: Matthew Garrett <mj...@srcf.ucam.org> > Signed-off-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> > --- > drivers/char/tpm/tpm.h | 7 ++ > drivers/char/tpm/tpm_crb.c | 20 ++--- > drivers/char/tpm/tpm_tis.c | 190 > ++++++++++++++++++++++++++++++++++++++------- > 3 files changed, 174 insertions(+), 43 deletions(-) > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index f8319a0..39be5ac 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -115,6 +115,13 @@ enum tpm2_startup_types { > TPM2_SU_STATE = 0x0001, > }; > > +enum tpm2_start_method { > + TPM2_START_ACPI = 2, > + TPM2_START_FIFO = 6, > + TPM2_START_CRB = 7, > + TPM2_START_CRB_WITH_ACPI = 8, > +}; > + > struct tpm_chip; > > struct tpm_vendor_specific { > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c > index 1267322..b4564b6 100644 > --- a/drivers/char/tpm/tpm_crb.c > +++ b/drivers/char/tpm/tpm_crb.c > @@ -34,12 +34,6 @@ enum crb_defaults { > CRB_ACPI_START_INDEX = 1, > }; > > -enum crb_start_method { > - CRB_SM_ACPI_START = 2, > - CRB_SM_CRB = 7, > - CRB_SM_CRB_WITH_ACPI_START = 8, > -}; > - > struct acpi_tpm2 { > struct acpi_table_header hdr; > u16 platform_class; > @@ -233,13 +227,9 @@ static int crb_acpi_add(struct acpi_device *device) > return -ENODEV; > } > > - /* At least some versions of AMI BIOS have a bug that TPM2 table has > - * zero address for the control area and therefore we must fail. > - */ > - if (!buf->control_area_pa) { > - dev_err(dev, "TPM2 ACPI table has a zero address for the > control area\n"); > - return -EINVAL; > - } > + /* Should the FIFO driver handle this? */ > + if (buf->start_method == TPM2_START_FIFO) > + return -ENODEV; > > if (buf->hdr.length < sizeof(struct acpi_tpm2)) { > dev_err(dev, "TPM2 ACPI table has wrong size"); > @@ -259,11 +249,11 @@ static int crb_acpi_add(struct acpi_device *device) > * report only ACPI start but in practice seems to require both > * ACPI start and CRB start. > */ > - if (sm == CRB_SM_CRB || sm == CRB_SM_CRB_WITH_ACPI_START || > + if (sm == TPM2_START_CRB || sm == TPM2_START_FIFO || > !strcmp(acpi_device_hid(device), "MSFT0101")) > priv->flags |= CRB_FL_CRB_START; > > - if (sm == CRB_SM_ACPI_START || sm == CRB_SM_CRB_WITH_ACPI_START) > + if (sm == TPM2_START_ACPI || sm == TPM2_START_CRB_WITH_ACPI) > priv->flags |= CRB_FL_ACPI_START; > > priv->cca = (struct crb_control_area __iomem *) > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index f2dffa7..6efdda1 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -1,6 +1,6 @@ > /* > * Copyright (C) 2005, 2006 IBM Corporation > - * Copyright (C) 2014 Intel Corporation > + * Copyright (C) 2014, 2015 Intel Corporation > * > * Authors: > * Leendert van Doorn <leend...@watson.ibm.com> > @@ -28,6 +28,7 @@ > #include <linux/wait.h> > #include <linux/acpi.h> > #include <linux/freezer.h> > +#include <acpi/actbl2.h> > #include "tpm.h" > > enum tis_access { > @@ -65,6 +66,17 @@ enum tis_defaults { > TIS_LONG_TIMEOUT = 2000, /* 2 sec */ > }; > > +struct tpm_info { > + unsigned long start; > + unsigned long len; > + unsigned int irq; > +}; > + > +static struct tpm_info tis_default_info = { > + .start = TIS_MEM_BASE, > + .len = TIS_MEM_LEN, > + .irq = 0, > +}; > > /* Some timeout values are needed before it is known whether the chip is > * TPM 1.0 or TPM 2.0. > @@ -91,26 +103,54 @@ struct priv_data { > }; > > #if defined(CONFIG_PNP) && defined(CONFIG_ACPI) > -static int is_itpm(struct pnp_dev *dev) > +static int has_hid(struct acpi_device *dev, const char *hid) > { > - struct acpi_device *acpi = pnp_acpi_device(dev); > struct acpi_hardware_id *id; > > - if (!acpi) > - return 0; > - > - list_for_each_entry(id, &acpi->pnp.ids, list) { > - if (!strcmp("INTC0102", id->id)) > + list_for_each_entry(id, &dev->pnp.ids, list) > + if (!strcmp(hid, id->id)) > return 1; > - } > > return 0; > } > + > +static inline int is_itpm(struct acpi_device *dev) > +{ > + return has_hid(dev, "INTC0102"); > +} > + > +static inline int is_fifo(struct acpi_device *dev) > +{ > + struct acpi_table_tpm2 *tbl; > + acpi_status st; > + > + /* TPM 1.2 FIFO */ > + if (!has_hid(dev, "MSFT0101")) > + return 1; > + > + st = acpi_get_table(ACPI_SIG_TPM2, 1, > + (struct acpi_table_header **) &tbl); > + if (ACPI_FAILURE(st)) { > + dev_err(&dev->dev, "failed to get TPM2 ACPI table\n"); > + return 0; > + } > + > + if (le32_to_cpu(tbl->start_method) != TPM2_START_FIFO) > + return 0; > + > + /* TPM 2.0 FIFO */ > + return 1; > +} > #else > -static inline int is_itpm(struct pnp_dev *dev) > +static inline int is_itpm(struct acpi_device *dev) > { > return 0; > } > + > +static inline int is_fifo(struct acpi_device *dev) > +{ > + return 1; > +} > #endif > > /* Before we attempt to access the TPM we must see that the valid bit is set. > @@ -600,9 +640,8 @@ static void tpm_tis_remove(struct tpm_chip *chip) > release_locality(chip, chip->vendor.locality, 1); > } > > -static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle, > - resource_size_t start, resource_size_t len, > - unsigned int irq) > +static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > + acpi_handle acpi_dev_handle) > { > u32 vendor, intfcaps, intmask; > int rc, i, irq_s, irq_e, probe; > @@ -622,7 +661,7 @@ static int tpm_tis_init(struct device *dev, acpi_handle > acpi_dev_handle, > chip->acpi_dev_handle = acpi_dev_handle; > #endif > > - chip->vendor.iobase = devm_ioremap(dev, start, len); > + chip->vendor.iobase = devm_ioremap(dev, tpm_info->start, tpm_info->len); > if (!chip->vendor.iobase) > return -EIO; > > @@ -707,7 +746,7 @@ static int tpm_tis_init(struct device *dev, acpi_handle > acpi_dev_handle, > chip->vendor.iobase + > TPM_INT_ENABLE(chip->vendor.locality)); > if (interrupts) > - chip->vendor.irq = irq; > + chip->vendor.irq = tpm_info->irq; > if (interrupts && !chip->vendor.irq) { > irq_s = > ioread8(chip->vendor.iobase + > @@ -890,27 +929,27 @@ static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, > tpm_tis_resume); > static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev, > const struct pnp_device_id *pnp_id) > { > - resource_size_t start, len; > - unsigned int irq = 0; > + struct tpm_info tpm_info = tis_default_info; > acpi_handle acpi_dev_handle = NULL; > > - start = pnp_mem_start(pnp_dev, 0); > - len = pnp_mem_len(pnp_dev, 0); > + tpm_info.start = pnp_mem_start(pnp_dev, 0); > + tpm_info.len = pnp_mem_len(pnp_dev, 0); > > if (pnp_irq_valid(pnp_dev, 0)) > - irq = pnp_irq(pnp_dev, 0); > + tpm_info.irq = pnp_irq(pnp_dev, 0); > else > interrupts = false; > > - if (is_itpm(pnp_dev)) > - itpm = true; > - > #ifdef CONFIG_ACPI > - if (pnp_acpi_device(pnp_dev)) > + if (pnp_acpi_device(pnp_dev)) { > + if (is_itpm(pnp_acpi_device(pnp_dev))) > + itpm = true; > + > acpi_dev_handle = pnp_acpi_device(pnp_dev)->handle; > + } > #endif > > - return tpm_tis_init(&pnp_dev->dev, acpi_dev_handle, start, len, irq); > + return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle); > } > > static struct pnp_device_id tpm_pnp_tbl[] = { > @@ -950,6 +989,79 @@ module_param_string(hid, tpm_pnp_tbl[TIS_HID_USR_IDX].id, > MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to > probe"); > #endif > > +#ifdef CONFIG_ACPI > +static int tpm_check_resource(struct acpi_resource *ares, void *data) > +{ > + struct tpm_info *tpm_info = (struct tpm_info *) data; > + struct resource res; > + > + if (acpi_dev_resource_interrupt(ares, 0, &res)) { > + tpm_info->irq = res.start; > + } else if (acpi_dev_resource_memory(ares, &res)) { > + tpm_info->start = res.start; > + tpm_info->len = resource_size(&res); > + } > + > + return 1; > +} > + > +static int tpm_tis_acpi_init(struct acpi_device *acpi_dev) > +{ > + struct list_head resources; > + struct tpm_info tpm_info = tis_default_info; > + int ret; > + > + if (!is_fifo(acpi_dev)) > + return -ENODEV; > + > + INIT_LIST_HEAD(&resources); > + ret = acpi_dev_get_resources(acpi_dev, &resources, tpm_check_resource, > + &tpm_info); > + if (ret < 0) > + return ret; > + > + acpi_dev_free_resource_list(&resources); > + > + if (!tpm_info.irq) > + interrupts = false; > + > + if (is_itpm(acpi_dev)) > + itpm = true; > + > + return tpm_tis_init(&acpi_dev->dev, &tpm_info, acpi_dev->handle); > +} > + > +static int tpm_tis_acpi_remove(struct acpi_device *dev) > +{ > + struct tpm_chip *chip = dev_get_drvdata(&dev->dev); > + > + tpm_chip_unregister(chip); > + tpm_tis_remove(chip); > + > + return 0; > +} > + > +static struct acpi_device_id tpm_acpi_tbl[] = { > + {"MSFT0101", 0}, /* TPM 2.0 */ > + /* Add new here */ > + {"", 0}, /* User Specified */ > + {"", 0} /* Terminator */ > +}; > +MODULE_DEVICE_TABLE(acpi, tpm_acpi_tbl); > + > +static struct acpi_driver tis_acpi_driver = { > + .name = "tpm_tis", > + .ids = tpm_acpi_tbl, > + .ops = { > + .add = tpm_tis_acpi_init, > + .remove = tpm_tis_acpi_remove, > + }, > + .drv = { > + .pm = &tpm_tis_pm, > + }, > +}; > +#endif > + > static struct platform_driver tis_drv = { > .driver = { > .name = "tpm_tis", > @@ -966,9 +1078,25 @@ static int __init init_tis(void) > { > int rc; > #ifdef CONFIG_PNP > - if (!force) > - return pnp_register_driver(&tis_pnp_driver); > + if (!force) { > + rc = pnp_register_driver(&tis_pnp_driver); > + if (rc) > + return rc; > + } > +#endif > +#ifdef CONFIG_ACPI > + if (!force) { > + rc = acpi_bus_register_driver(&tis_acpi_driver); > + if (rc) { > +#ifdef CONFIG_PNP > + pnp_unregister_driver(&tis_pnp_driver); > #endif > + return rc; > + } > + } > +#endif > + if (!force) > + return 0; > > rc = platform_driver_register(&tis_drv); > if (rc < 0) > @@ -978,7 +1106,7 @@ static int __init init_tis(void) > rc = PTR_ERR(pdev); > goto err_dev; > } > - rc = tpm_tis_init(&pdev->dev, NULL, TIS_MEM_BASE, TIS_MEM_LEN, 0); > + rc = tpm_tis_init(&pdev->dev, &tis_default_info, NULL); > if (rc) > goto err_init; > return 0; > @@ -992,6 +1120,12 @@ err_dev: > static void __exit cleanup_tis(void) > { > struct tpm_chip *chip; > +#ifdef CONFIG_ACPI > + if (!force) { > + acpi_bus_unregister_driver(&tis_acpi_driver); > + return; > + } > +#endif > #ifdef CONFIG_PNP > if (!force) { > pnp_unregister_driver(&tis_pnp_driver); > -- > 2.5.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/