From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45712) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1am49m-0005pu-6q for qemu-devel@nongnu.org; Fri, 01 Apr 2016 14:50:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1am49h-0001tY-6H for qemu-devel@nongnu.org; Fri, 01 Apr 2016 14:50:54 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:38021) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1am49g-0001tL-VR for qemu-devel@nongnu.org; Fri, 01 Apr 2016 14:50:49 -0400 Received: from localhost by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 1 Apr 2016 12:50:47 -0600 Received: from b03cxnp07029.gho.boulder.ibm.com (b03cxnp07029.gho.boulder.ibm.com [9.17.130.16]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id C93F53E40044 for ; Fri, 1 Apr 2016 12:50:44 -0600 (MDT) Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u31Ioitk36962414 for ; Fri, 1 Apr 2016 11:50:44 -0700 Received: from d03av03.boulder.ibm.com (localhost [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u31Ioi91003216 for ; Fri, 1 Apr 2016 12:50:44 -0600 References: <1458570071-1935-1-git-send-email-stefanb@us.ibm.com> <20160330153347.302b7d0b@igors-macbook-pro.local> <56FCA1AD.9000507@linux.vnet.ibm.com> <20160331160736.2576b590@igors-macbook-pro.local> From: Stefan Berger Message-ID: <56FEC303.4090504@linux.vnet.ibm.com> Date: Fri, 1 Apr 2016 14:50:43 -0400 MIME-Version: 1.0 In-Reply-To: <20160331160736.2576b590@igors-macbook-pro.local> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] acpi: Fix TPM ACPI description to make TPM usable on Windows List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: marcel@redhat.com, mst@redhat.com, Stefan Berger , qemu-devel@nongnu.org, crobinso@redhat.com On 03/31/2016 10:07 AM, Igor Mammedov wrote: > On Thu, 31 Mar 2016 00:03:57 -0400 > Stefan Berger wrote: > >> On 03/30/2016 09:33 AM, Igor Mammedov wrote: >>> On Mon, 21 Mar 2016 10:21:11 -0400 >>> Stefan Berger 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. > 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. >