Messages in this thread | | | Date | Tue, 16 Nov 2010 14:26:44 -0800 | From | Yinghai Lu <> | Subject | Re: [PATCH, v2] x86-64: fix and clean up AMD Fam10 MMCONF enabling |
| |
On 11/16/2010 12:25 AM, Jan Beulich wrote: > Candidate memory ranges were not calculated properly (start addresses > got needlessly rounded down, and end addresses didn't get rounded up > at all), address comparison for secondary CPUs was done on only part > of the address, and disabled status wasn't tracked properly. > > Signed-off-by: Jan Beulich <jbeulich@novell.com> > Cc: Yinghai Lu <yinghai@kernel.org> > > --- > arch/x86/include/asm/msr-index.h | 2 - > arch/x86/kernel/mmconf-fam10h_64.c | 64 +++++++++++++++++-------------------- > 2 files changed, 31 insertions(+), 35 deletions(-) > > --- linux-2.6.37-rc2/arch/x86/include/asm/msr-index.h > +++ 2.6.37-rc2-x86_64-mmconf-fam10h/arch/x86/include/asm/msr-index.h > @@ -128,7 +128,7 @@ > #define FAM10H_MMIO_CONF_ENABLE (1<<0) > #define FAM10H_MMIO_CONF_BUSRANGE_MASK 0xf > #define FAM10H_MMIO_CONF_BUSRANGE_SHIFT 2 > -#define FAM10H_MMIO_CONF_BASE_MASK 0xfffffff > +#define FAM10H_MMIO_CONF_BASE_MASK 0xfffffffULL > #define FAM10H_MMIO_CONF_BASE_SHIFT 20 > #define MSR_FAM10H_NODE_ID 0xc001100c > > --- linux-2.6.37-rc2/arch/x86/kernel/mmconf-fam10h_64.c > +++ 2.6.37-rc2-x86_64-mmconf-fam10h/arch/x86/kernel/mmconf-fam10h_64.c > @@ -25,7 +25,6 @@ struct pci_hostbridge_probe { > }; > > static u64 __cpuinitdata fam10h_pci_mmconf_base; > -static int __cpuinitdata fam10h_pci_mmconf_base_status; > > static struct pci_hostbridge_probe pci_probes[] __cpuinitdata = { > { 0, 0x18, PCI_VENDOR_ID_AMD, 0x1200 }, > @@ -44,10 +43,12 @@ static int __cpuinit cmp_range(const voi > return start1 - start2; > } > > -/*[47:0] */ > -/* need to avoid (0xfd<<32) and (0xfe<<32), ht used space */ > +#define MMCONF_UNIT (1ULL << FAM10H_MMIO_CONF_BASE_SHIFT) > +#define MMCONF_MASK (~(MMCONF_UNIT - 1)) > +#define MMCONF_SIZE (MMCONF_UNIT << 8) > +/* need to avoid (0xfd<<32), (0xfe<<32), and (0xff<<32), ht used space */ > #define FAM10H_PCI_MMCONF_BASE (0xfcULL<<32) > -#define BASE_VALID(b) ((b != (0xfdULL << 32)) && (b != (0xfeULL << 32))) > +#define BASE_VALID(b) ((b) + MMCONF_SIZE <= (0xfdULL<<32) || (b) >= (1ULL<<40)) > static void __cpuinit get_fam10h_pci_mmconf_base(void) > { > int i; > @@ -64,12 +65,11 @@ static void __cpuinit get_fam10h_pci_mmc > struct range range[8]; > > /* only try to get setting from BSP */ > - /* -1 or 1 */ > - if (fam10h_pci_mmconf_base_status) > + if (fam10h_pci_mmconf_base) > return; > > if (!early_pci_allowed()) > - goto fail; > + return; > > found = 0; > for (i = 0; i < ARRAY_SIZE(pci_probes); i++) { > @@ -91,7 +91,7 @@ static void __cpuinit get_fam10h_pci_mmc > } > > if (!found) > - goto fail; > + return; > > /* SYS_CFG */ > address = MSR_K8_SYSCFG; > @@ -99,16 +99,16 @@ static void __cpuinit get_fam10h_pci_mmc > > /* TOP_MEM2 is not enabled? */ > if (!(val & (1<<21))) { > - tom2 = 0; > + tom2 = 1ULL << 32; > } else { > /* TOP_MEM2 */ > address = MSR_K8_TOP_MEM2; > rdmsrl(address, val); > - tom2 = val & (0xffffULL<<32); > + tom2 = max(val & 0xffffff800000ULL, 1ULL << 32); > } > > if (base <= tom2) > - base = tom2 + (1ULL<<32); > + base = (tom2 + 2 * MMCONF_UNIT - 1) & MMCONF_MASK; > > /* > * need to check if the range is in the high mmio range that is > @@ -123,11 +123,11 @@ static void __cpuinit get_fam10h_pci_mmc > if (!(reg & 3)) > continue; > > - start = (((u64)reg) << 8) & (0xffULL << 32); /* 39:16 on 31:8*/ > + start = (u64)(reg & 0xffffff00) << 8; /* 39:16 on 31:8*/ > reg = read_pci_config(bus, slot, 1, 0x84 + (i << 3)); > - end = (((u64)reg) << 8) & (0xffULL << 32); /* 39:16 on 31:8*/ > + end = ((u64)(reg & 0xffffff00) << 8) | 0xffff; /* 39:16 on 31:8*/ > > - if (!end) > + if (end < tom2) > continue; > > range[hi_mmio_num].start = start; > @@ -143,32 +143,27 @@ static void __cpuinit get_fam10h_pci_mmc > > if (range[hi_mmio_num - 1].end < base) > goto out; > - if (range[0].start > base) > + if (range[0].start > base + MMCONF_SIZE) > goto out; > > /* need to find one window */ > - base = range[0].start - (1ULL << 32); > + base = (range[0].start & MMCONF_MASK) - MMCONF_UNIT; > if ((base > tom2) && BASE_VALID(base)) > goto out; > - base = range[hi_mmio_num - 1].end + (1ULL << 32); > - if ((base > tom2) && BASE_VALID(base)) > + base = (range[hi_mmio_num - 1].end + MMCONF_UNIT) & MMCONF_MASK; > + if (BASE_VALID(base)) > goto out; > /* need to find window between ranges */ > - if (hi_mmio_num > 1) > - for (i = 0; i < hi_mmio_num - 1; i++) { > - if (range[i + 1].start > (range[i].end + (1ULL << 32))) { > - base = range[i].end + (1ULL << 32); > - if ((base > tom2) && BASE_VALID(base)) > - goto out; > - } > + for (i = 1; i < hi_mmio_num; i++) { > + base = (range[i - 1].end + MMCONF_UNIT) & MMCONF_MASK; > + val = range[i].start & MMCONF_MASK; > + if (val >= base + MMCONF_SIZE && BASE_VALID(base)) > + goto out; > } > - > -fail: > - fam10h_pci_mmconf_base_status = -1; > return; > + > out: > fam10h_pci_mmconf_base = base; > - fam10h_pci_mmconf_base_status = 1; > } > > void __cpuinit fam10h_check_enable_mmcfg(void) > @@ -190,11 +185,10 @@ void __cpuinit fam10h_check_enable_mmcfg > > /* only trust the one handle 256 buses, if acpi=off */ > if (!acpi_pci_disabled || busnbits >= 8) { > - u64 base; > - base = val & (0xffffULL << 32); > - if (fam10h_pci_mmconf_base_status <= 0) { > + u64 base = val & MMCONF_MASK; > + > + if (!fam10h_pci_mmconf_base) { > fam10h_pci_mmconf_base = base; > - fam10h_pci_mmconf_base_status = 1; > return; > } else if (fam10h_pci_mmconf_base == base) > return; > @@ -206,8 +200,10 @@ void __cpuinit fam10h_check_enable_mmcfg > * with 256 buses > */ > get_fam10h_pci_mmconf_base(); > - if (fam10h_pci_mmconf_base_status <= 0) > + if (!fam10h_pci_mmconf_base) { > + pci_probe &= ~PCI_CHECK_ENABLE_AMD_MMCONF;
case: if no cpu mmconf reg are set by BIOS and later if BSP can not get new base,
this pci_probe &= line will be called for every AP...
> return; > + } > > printk(KERN_INFO "Enable MMCONFIG on AMD Family 10h\n"); > val &= ~((FAM10H_MMIO_CONF_BASE_MASK<<FAM10H_MMIO_CONF_BASE_SHIFT) | > >
| |