From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60731) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2c8G-0006wj-9n for qemu-devel@nongnu.org; Wed, 10 Jun 2015 05:17:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2c8F-0000d6-9R for qemu-devel@nongnu.org; Wed, 10 Jun 2015 05:17:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33851) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2c8F-0000d0-1b for qemu-devel@nongnu.org; Wed, 10 Jun 2015 05:17:11 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id BF03D2CD845 for ; Wed, 10 Jun 2015 09:17:10 +0000 (UTC) Message-ID: <55780093.9060409@redhat.com> Date: Wed, 10 Jun 2015 12:17:07 +0300 From: Marcel Apfelbaum MIME-Version: 1.0 References: <5572347E.1030105@redhat.com> <1433547989-7238-1-git-send-email-lersek@redhat.com> <1433547989-7238-3-git-send-email-lersek@redhat.com> In-Reply-To: <1433547989-7238-3-git-send-email-lersek@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/4] i386/acpi-build: fix PXB workarounds for unsupported BIOSes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek , qemu-devel@nongnu.org Cc: "Michael S. Tsirkin" On 06/06/2015 02:46 AM, Laszlo Ersek wrote: > The patch > > apci: fix PXB behaviour if used with unsupported BIOS > > uses the following condition to see if a "PXB mem/IO chunk" has *not* been > configured by the BIOS: > > (!range_base || range_base > range_limit) > > When this condition evaluates to true, said patch *omits* the > corresponding entry from the _CRS. > > Later on the patch checks for the opposite condition (with the intent of > *adding* entries to the _CRS if the "PXB mem/IO chunks" *have* been > configured). Unfortunately, the condition was negated incorrectly: only > the first ! operator was removed, which led to the nonsensical expression > > (range_base || range_base > range_limit) > > leading to bogus entries in the _CRS, and causing BSOD in Windows Server > 2012 R2 when it runs on OVMF. Thanks for catching this! > > The correct negative of the condition seen at the top is > > (range_base && range_base <= range_limit) > > Fix the expressions. > > Cc: Marcel Apfelbaum > Cc: Michael S. Tsirkin > Signed-off-by: Laszlo Ersek > --- > hw/i386/acpi-build.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 52c2591..b71e942 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -833,7 +833,7 @@ static Aml *build_crs(PCIHostState *host, > * Work-around for old bioses > * that do not support multiple root buses > */ > - if (range_base || range_base > range_limit) { > + if (range_base && range_base <= range_limit) { > aml_append(crs, > aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED, > AML_POS_DECODE, AML_ENTIRE_RANGE, > @@ -854,7 +854,7 @@ static Aml *build_crs(PCIHostState *host, > * Work-around for old bioses > * that do not support multiple root buses > */ > - if (range_base || range_base > range_limit) { > + if (range_base && range_base <= range_limit) { > aml_append(crs, > aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, > AML_MAX_FIXED, AML_NON_CACHEABLE, > @@ -865,7 +865,7 @@ static Aml *build_crs(PCIHostState *host, > 0, > range_limit - range_base + 1)); > crs_range_insert(mem_ranges, range_base, range_limit); > - } > + } > > range_base = > pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH); > @@ -876,7 +876,7 @@ static Aml *build_crs(PCIHostState *host, > * Work-around for old bioses > * that do not support multiple root buses > */ > - if (range_base || range_base > range_limit) { > + if (range_base && range_base <= range_limit) { > aml_append(crs, > aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, > AML_MAX_FIXED, AML_NON_CACHEABLE, > Reviewed-by: Marcel Apfelbaum Thanks, Marcel