From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57682) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1an2Jq-0007FK-Bu for qemu-devel@nongnu.org; Mon, 04 Apr 2016 07:05:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1an2Jl-0000yS-CN for qemu-devel@nongnu.org; Mon, 04 Apr 2016 07:05:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50585) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1an2Jl-0000yM-4R for qemu-devel@nongnu.org; Mon, 04 Apr 2016 07:05:13 -0400 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: Marcel Apfelbaum Message-ID: <57024A61.9080909@redhat.com> Date: Mon, 4 Apr 2016 14:05:05 +0300 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 , Stefan Berger Cc: crobinso@redhat.com, Stefan Berger , qemu-devel@nongnu.org, mst@redhat.com On 03/31/2016 05:07 PM, 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 > 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 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) Sorry, I have no idea. Thanks, Marcel > 2: ACPI should pick up IRQ from tpm-tis device property, i.e. it > shouldn't be hardcoded in ACPI part. >