On 6/25/20 7:19 PM, Jason Gunthorpe wrote:

On Thu, Jun 25, 2020 at 06:48:09PM -0400, Stefan Berger wrote:

quoted

On 6/25/20 5:26 PM, Stefan Berger wrote:

quoted

On 6/25/20 1:28 PM, Jason Gunthorpe wrote:

quoted

On Thu, Jun 25, 2020 at 10:56:43AM -0400, Stefan Berger wrote:

quoted

Hello!

   I want to enable IRQs now in QEMU's TPM TIS device model and I
need to work
with the following patch to Linux TIS. I am wondering whether
the changes
there look reasonable to you? Windows works with the QEMU modifications
as-is, so maybe it's a bug in the TIS code (which I had not run into
before).


The point of the loop I need to introduce in the interrupt
handler is that
while the interrupt handler is running another interrupt may
occur/be posted
that then does NOT cause the interrupt handler to be invoked again but
causes a stall, unless the loop is there.

That seems like a qemu bug, TPM interrupts are supposed to be level
interrupts, not edge.

Following this document here the hardware may choose to support
different types of interrutps:

https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p04_r0p37_pub-1.pdf


Table 23. Edge falling or rising, level low or level high.

So with different steps in the driver causing different types of
interrupts, we may get into such situations where we process some
interrupt 'reasons' but then another one gets posted, I guess due to
parallel processing.

Another data point: I had the TIS driver working on IRQ 5 (festeoi) without
the introduction of this loop. There are additional bits being set while the
interrupt handler is running, but the handler deals with them in the next
invocation. On IRQ 13 (edge), it does need the loop since the next interrupt
handler invocation is not happening.

A loop like that is never the correct way to handle edge interrupts.

Right, we can just miss the update of the interrupt flags and miss the 
loop and then afterwards the new flag gets set and we'd stall.

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.


     Stefan

Jason