Message ID | 1433547989-7238-3-git-send-email-lersek@redhat.com |
---|---|
State | New |
Headers | show |
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 <marcel@redhat.com> > Cc: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > 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 <marcel@redhat.com> Thanks, Marcel
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,
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. The correct negative of the condition seen at the top is (range_base && range_base <= range_limit) Fix the expressions. Cc: Marcel Apfelbaum <marcel@redhat.com> Cc: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- hw/i386/acpi-build.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)