linux-kernel - Re: Enabling interrupts in QEMU TPM TIS

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200626131505.GA25301@ziepe.ca>
Date:   Fri, 26 Jun 2020 10:15:05 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Stefan Berger <stefanb@...ux.ibm.com>
Cc:     Jerry Snitselaar <jsnitsel@...hat.com>,
        linux-integrity <linux-integrity@...r.kernel.org>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        LSM List <linux-security-module@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: Enabling interrupts in QEMU TPM TIS

On Fri, Jun 26, 2020 at 08:25:57AM -0400, Stefan Berger wrote:

> > I don't think the tpm driver was ever designed for edge, so most
> > likely the structure and order of the hard irq is not correct.
> 
> Right. For edge support I think we would need to avoid causing another
> interrupt (like locality change interrupt) before the interrupt handler
> hasn't finished dealing with an existing interrupt. Considering that Windows
> works on IRQ 13 (egde) and Linux driver cannot, I guess this is a good
> reason not to move QEMU TIS to IRQ 13 and try to support interrupts via ACPI
> table declaration.

Generaly clearing the IRQ needs to be done before testing for pending
IRQs - ie as the first thing

Move the write to status up higher:

	rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
	rc = tpm_tis_write32(priv, TPM_INT_STATUS(priv->locality), interrupt);
	
        [handle 'interrupt']

Then if new events set a status bit they will generate an edge and
re-enter here.

I don't know why there is an extra read at the end of the handler
either, seems sketchy.

Jason