From fa5b8a30cf03520737e9a0ee2ee03a61b2eccf05 Mon Sep 17 00:00:00 2001 From: Pavel Machek Date: Mon, 26 May 2008 20:40:47 +0200 Subject: [PATCH] aperture_64.c: duplicated code, buggy? Hi! void __init early_gart_iommu_check(void) contains for (num = 24; num < 32; num++) { if (!early_is_k8_nb(read_pci_config(0, num, 3, 0x00))) continue; loop, with very similar loop duplicated in void __init gart_iommu_hole_init(void) . First copy of a loop seems to be buggy, too. It uses 0 as a "nothing set" value, which may actually bite us in last_aper_enabled case (because it may be often zero). (Beware, it is hard to test this patch, because this code has about 2^8 different code paths, depending on hardware and cmdline settings). Plus, the second loop does not check for consistency of aper_enabled. Should it? Signed-off-by: Thomas Gleixner --- arch/x86/kernel/aperture_64.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c index 6eea42eb287..e5b17f910f8 100644 --- a/arch/x86/kernel/aperture_64.c +++ b/arch/x86/kernel/aperture_64.c @@ -271,16 +271,16 @@ void __init early_gart_iommu_check(void) * or BIOS forget to put that in reserved. * try to update e820 to make that region as reserved. */ - int fix, slot; + int i, fix, slot; u32 ctl; u32 aper_size = 0, aper_order = 0, last_aper_order = 0; u64 aper_base = 0, last_aper_base = 0; - int aper_enabled = 0, last_aper_enabled = 0; - int i; + int aper_enabled = 0, last_aper_enabled = 0, last_valid = 0; if (!early_pci_allowed()) return; + /* This is mostly duplicate of iommu_hole_init */ fix = 0; for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) { int bus; @@ -301,19 +301,22 @@ void __init early_gart_iommu_check(void) aper_base = read_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE) & 0x7fff; aper_base <<= 25; - if ((last_aper_order && aper_order != last_aper_order) || - (last_aper_base && aper_base != last_aper_base) || - (last_aper_enabled && aper_enabled != last_aper_enabled)) { - fix = 1; - goto out; + if (last_valid) { + if ((aper_order != last_aper_order) || + (aper_base != last_aper_base) || + (aper_enabled != last_aper_enabled)) { + fix = 1; + break; + } } + last_aper_order = aper_order; last_aper_base = aper_base; last_aper_enabled = aper_enabled; + last_valid = 1; } } -out: if (!fix && !aper_enabled) return; -- 2.41.0