Re: [RFC PATCH v6] tpm_tis: verify interrupt during init
On Tue, Sep 02, 2014 at 08:22:58PM +, Scot Doyle wrote:
> It's spending that time (now 3 seconds) in tpm_tis_send_data.
Due to request_locality?
> > Looks really good to me, I can try and test the next version here this
> > week.
>
> Thanks!
So, I forgot my TIS systems have no IRQ, can't really test it
properly, but it compiles and doesn't muck up the no irq specified
case at least.
> + if (test_irq) {
Should be
if (test_irq && !priv->irq_tested)
We don't need to msleep if we got an irq already.
Reviewed-By: Jason Gunthorpe
You should post a final version and try and get it tested on a normal
x86 system.
Jason
--
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/
Re: [RFC PATCH v6] tpm_tis: verify interrupt during init
On Tue, Sep 02, 2014 at 08:22:58PM +, Scot Doyle wrote:
It's spending that time (now 3 seconds) in tpm_tis_send_data.
Due to request_locality?
Looks really good to me, I can try and test the next version here this
week.
Thanks!
So, I forgot my TIS systems have no IRQ, can't really test it
properly, but it compiles and doesn't muck up the no irq specified
case at least.
+ if (test_irq) {
Should be
if (test_irq !priv-irq_tested)
We don't need to msleep if we got an irq already.
Reviewed-By: Jason Gunthorpe jguntho...@obsidianresearch.com
You should post a final version and try and get it tested on a normal
x86 system.
Jason
--
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/
[RFC PATCH v6] tpm_tis: verify interrupt during init
On Tue, 2 Sep 2014, Jason Gunthorpe wrote:
> On Sat, Aug 30, 2014 at 11:23:56PM +, Scot Doyle wrote:
>> The output is now
>> [1.526798] tpm_tis 00:08: 1.2 TPM (device-id 0xB, rev-id 16)
>> [5.914732] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working,
>> polling instead
>
> Cool, why did it take 4 seconds though?
It's spending that time (now 3 seconds) in tpm_tis_send_data.
Output is
[2.928481] tpm_tis 00:08: 1.2 TPM (device-id 0xB, rev-id 16)
[5.943468] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working,
polling instead
> Looks really good to me, I can try and test the next version here this
> week.
Thanks!
---
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2c46734..6e42d60 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -75,6 +75,10 @@ enum tis_defaults {
#defineTPM_DID_VID(l) (0x0F00 | ((l) << 12))
#defineTPM_RID(l) (0x0F04 | ((l) << 12))
+struct priv_data {
+ bool irq_tested;
+};
+
static LIST_HEAD(tis_chips);
static DEFINE_MUTEX(tis_lock);
@@ -338,6 +342,21 @@ out_err:
return rc;
}
+static void disable_interrupts(struct tpm_chip *chip)
+{
+ u32 intmask;
+ intmask =
+ ioread32(chip->vendor.iobase +
+TPM_INT_ENABLE(chip->vendor.locality));
+ intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
+ TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
+ iowrite32(intmask,
+ chip->vendor.iobase +
+ TPM_INT_ENABLE(chip->vendor.locality));
+ free_irq(chip->vendor.irq, chip);
+ chip->vendor.irq = 0;
+}
+
/*
* If interrupts are used (signaled by an irq set in the vendor structure)
* tpm.c can skip polling for the data to be available as the interrupt is
@@ -345,8 +364,10 @@ out_err:
*/
static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
{
- int rc;
+ int rc, irq;
u32 ordinal;
+ bool test_irq;
+ struct priv_data *priv = chip->vendor.priv;
rc = tpm_tis_send_data(chip, buf, len);
if (rc < 0)
@@ -358,13 +379,30 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf,
size_t len)
if (chip->vendor.irq) {
ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
- if (wait_for_tpm_stat
- (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
-tpm_calc_ordinal_duration(chip, ordinal),
->vendor.read_queue, false) < 0) {
+ test_irq = !priv->irq_tested;
+ if (test_irq) {
+ irq = chip->vendor.irq;
+ chip->vendor.irq = 0;
+ }
+ rc = wait_for_tpm_stat
+(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+ tpm_calc_ordinal_duration(chip, ordinal),
+ >vendor.read_queue, false);
+ if (test_irq)
+ chip->vendor.irq = irq;
+ if (rc < 0) {
rc = -ETIME;
goto out_err;
}
+ if (test_irq) {
+ msleep(1);
+ if (!priv->irq_tested) {
+ disable_interrupts(chip);
+ dev_err(chip->dev,
+ FW_BUG "TPM interrupt not working,
polling instead\n");
+ }
+ priv->irq_tested = true;
+ }
}
return len;
out_err:
@@ -505,6 +543,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
if (interrupt == 0)
return IRQ_NONE;
+ ((struct priv_data*)chip->vendor.priv)->irq_tested = true;
if (interrupt & TPM_INTF_DATA_AVAIL_INT)
wake_up_interruptible(>vendor.read_queue);
if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
@@ -534,10 +573,14 @@ static int tpm_tis_init(struct device *dev,
resource_size_t start,
u32 vendor, intfcaps, intmask;
int rc, i, irq_s, irq_e, probe;
struct tpm_chip *chip;
+ struct priv_data *priv;
if (!(chip = tpm_register_hardware(dev, _tis)))
return -ENODEV;
+ priv = devm_kzalloc(dev, sizeof(struct priv_data), GFP_KERNEL);
+ chip->vendor.priv = priv;
+
chip->vendor.iobase = ioremap(start, len);
if (!chip->vendor.iobase) {
rc = -EIO;
@@ -605,19 +648,6 @@ static int tpm_tis_init(struct device *dev,
resource_size_t start,
if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
dev_dbg(dev, "\tData Avail Int Support\n");
- /* get the timeouts before testing for irqs */
- if (tpm_get_timeouts(chip)) {
- dev_err(dev, "Could not get TPM timeouts and durations\n");
- rc = -ENODEV;
- goto out_err;
[RFC PATCH v6] tpm_tis: verify interrupt during init
On Tue, 2 Sep 2014, Jason Gunthorpe wrote:
On Sat, Aug 30, 2014 at 11:23:56PM +, Scot Doyle wrote:
The output is now
[1.526798] tpm_tis 00:08: 1.2 TPM (device-id 0xB, rev-id 16)
[5.914732] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working,
polling instead
Cool, why did it take 4 seconds though?
It's spending that time (now 3 seconds) in tpm_tis_send_data.
Output is
[2.928481] tpm_tis 00:08: 1.2 TPM (device-id 0xB, rev-id 16)
[5.943468] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working,
polling instead
Looks really good to me, I can try and test the next version here this
week.
Thanks!
---
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2c46734..6e42d60 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -75,6 +75,10 @@ enum tis_defaults {
#defineTPM_DID_VID(l) (0x0F00 | ((l) 12))
#defineTPM_RID(l) (0x0F04 | ((l) 12))
+struct priv_data {
+ bool irq_tested;
+};
+
static LIST_HEAD(tis_chips);
static DEFINE_MUTEX(tis_lock);
@@ -338,6 +342,21 @@ out_err:
return rc;
}
+static void disable_interrupts(struct tpm_chip *chip)
+{
+ u32 intmask;
+ intmask =
+ ioread32(chip-vendor.iobase +
+TPM_INT_ENABLE(chip-vendor.locality));
+ intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
+ TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
+ iowrite32(intmask,
+ chip-vendor.iobase +
+ TPM_INT_ENABLE(chip-vendor.locality));
+ free_irq(chip-vendor.irq, chip);
+ chip-vendor.irq = 0;
+}
+
/*
* If interrupts are used (signaled by an irq set in the vendor structure)
* tpm.c can skip polling for the data to be available as the interrupt is
@@ -345,8 +364,10 @@ out_err:
*/
static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
{
- int rc;
+ int rc, irq;
u32 ordinal;
+ bool test_irq;
+ struct priv_data *priv = chip-vendor.priv;
rc = tpm_tis_send_data(chip, buf, len);
if (rc 0)
@@ -358,13 +379,30 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf,
size_t len)
if (chip-vendor.irq) {
ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
- if (wait_for_tpm_stat
- (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
-tpm_calc_ordinal_duration(chip, ordinal),
-chip-vendor.read_queue, false) 0) {
+ test_irq = !priv-irq_tested;
+ if (test_irq) {
+ irq = chip-vendor.irq;
+ chip-vendor.irq = 0;
+ }
+ rc = wait_for_tpm_stat
+(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+ tpm_calc_ordinal_duration(chip, ordinal),
+ chip-vendor.read_queue, false);
+ if (test_irq)
+ chip-vendor.irq = irq;
+ if (rc 0) {
rc = -ETIME;
goto out_err;
}
+ if (test_irq) {
+ msleep(1);
+ if (!priv-irq_tested) {
+ disable_interrupts(chip);
+ dev_err(chip-dev,
+ FW_BUG TPM interrupt not working,
polling instead\n);
+ }
+ priv-irq_tested = true;
+ }
}
return len;
out_err:
@@ -505,6 +543,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
if (interrupt == 0)
return IRQ_NONE;
+ ((struct priv_data*)chip-vendor.priv)-irq_tested = true;
if (interrupt TPM_INTF_DATA_AVAIL_INT)
wake_up_interruptible(chip-vendor.read_queue);
if (interrupt TPM_INTF_LOCALITY_CHANGE_INT)
@@ -534,10 +573,14 @@ static int tpm_tis_init(struct device *dev,
resource_size_t start,
u32 vendor, intfcaps, intmask;
int rc, i, irq_s, irq_e, probe;
struct tpm_chip *chip;
+ struct priv_data *priv;
if (!(chip = tpm_register_hardware(dev, tpm_tis)))
return -ENODEV;
+ priv = devm_kzalloc(dev, sizeof(struct priv_data), GFP_KERNEL);
+ chip-vendor.priv = priv;
+
chip-vendor.iobase = ioremap(start, len);
if (!chip-vendor.iobase) {
rc = -EIO;
@@ -605,19 +648,6 @@ static int tpm_tis_init(struct device *dev,
resource_size_t start,
if (intfcaps TPM_INTF_DATA_AVAIL_INT)
dev_dbg(dev, \tData Avail Int Support\n);
- /* get the timeouts before testing for irqs */
- if (tpm_get_timeouts(chip)) {
- dev_err(dev, Could not get TPM timeouts and durations\n);
- rc = -ENODEV;
- goto out_err;
- }
-
- if
