On Fri, Apr 01, 2016 at 02:50:43PM -0400, Stefan Berger wrote:
> On 03/31/2016 10:07 AM, Igor Mammedov wrote:
> >On Thu, 31 Mar 2016 00:03:57 -0400
> >Stefan Berger <stef...@linux.vnet.ibm.com> wrote:
> >
> >>On 03/30/2016 09:33 AM, Igor Mammedov wrote:
> >>>On Mon, 21 Mar 2016 10:21:11 -0400
> >>>Stefan Berger <stef...@us.ibm.com> wrote:
> >>>
> >>>>This patch addresses BZ 1281413.
> >>>>
> >>>>Fix the APCI description to make it work on Windows again. The ACPI
> >>>>description was broken in commit 9e47226.
> >>>above commit just added missing ASL description for TMP device
> >>>and you also posted exactly similar patch back then.
> >>Sorry, I referenced the wrong commit. Here's the bad one:
> >>
> >>commit 72d97b3a543a9c2c820bd463ba24751ae4247ac3
> >>
> >>>Current commit message is pretty useless, Pls describe in commit
> >>>message what/how is broken and how/why patch fixes it.
> >>I am not sure what exactly broke it. All I know is that the scope was
> >>changed and the device name were change (ISA.TPM versus TPM). This
> >>was the working ACPI description from QEMU v2.3.1:
> >>
> >> Scope(\_SB) {
> >> /* TPM with emulated TPM TIS interface */
> >> Device (TPM) {
> >> Name (_HID, EisaID ("PNP0C31"))
> >> Name (_CRS, ResourceTemplate ()
> >> {
> >> Memory32Fixed (ReadWrite, TPM_TIS_ADDR_BASE,
> >>TPM_TIS_ADDR_SIZE)
> >> // older Linux tpm_tis drivers do not work with IRQ
> >> //IRQNoFlags () {TPM_TIS_IRQ}
> >> })
> >The first committed variant has IRQ uncommented, so it was always
> >present in _CRS (commit 9e47226)
> >
> >
> >[...]
> >>>>+ //aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
> >>>silent change,
> >>>why IRQ descriptor is commented out, it seems the device
> >>>uses/initializes it?
> >>>I'd split IRQ change in a separate patch that explains why it
> >>>shouldn't be in TPM._CRS.
> >>We can leave it there if it works. The bug report is related to
> >>Windows, which I don't have running in a VM. So the safest was to
> >>roll back to 2.3.1 equivalent, which had the IRQ disabled.
> >I've looked through code some more and Windows seems to be right in not
> >starting device as IRQ 5 might be used by PNP0C0F devices, see
>
> On Linux I can use it with IRQ 5. I have tried with 10, it also works. But
> Linux != Windows.
>
>
> >build_link_dev(). So potential conflict is there and moving TPM device
> >out of PCI.ISA scope just hides problem as resource conflict
> >checking doesn't work anymore.
> >
> >Current TPM code have 2 issues:
> > 1: it uses wrong IRQ, (I've tried with unused COM2's IRQ3 and warning
> > goes away)
> > 2: IRQ is hardcoded i.e. _CRS always has TPM_TIS_IRQ value, regardless
> > of what user specified at command line, for example:
> > -device tpm-tis,irq=3
> > has no effect on ACPI part
>
> So if we disable interrupt, as done with this patch here, it works. This was
> the configuration we had in QEMU 2.3.1 and was reported as working here:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1281413
>
> So I would repost the patch with a better description of what we are rolling
> back. Let me know whether you agree. We can then enable the interrupt mode
> in separate patches.
I'm fine with a revert as a first step. But really we should avoid
asserting the interrupt for no-interrupt mode and obey what user
specified.
>
>
> >So to fix it correctly on top of my patch:
> > 1: default IRQ shouldn't be 5 (CCing Marcel, may be he has an idea
> > what it should be)
> > 2: ACPI should pick up IRQ from tpm-tis device property, i.e. it
> > shouldn't be hardcoded in ACPI part.
> >
