| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4CD454D1.2060503@kernel.org>
Date: Fri, 05 Nov 2010 12:02:41 -0700
From: Yinghai Lu <yinghai@...nel.org>
To: Jan Beulich <JBeulich@...ell.com>
CC: mingo@...e.hu, tglx@...utronix.de, hpa@...or.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86-64: fix and clean up AMD Fam10 MMCONF enabling
On 11/04/2010 08:22 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@...ell.com>
> Cc: Yinghai Lu <yinghai@...nel.org>
>
> ---
> arch/x86/kernel/mmconf-fam10h_64.c | 54 +++++++++++++++++--------------------
> 1 file changed, 25 insertions(+), 29 deletions(-)
>
> --- linux-2.6.37-rc1/arch/x86/kernel/mmconf-fam10h_64.c
> +++ 2.6.37-rc1-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,7 +43,9 @@ static int __cpuinit cmp_range(const voi
> return start1 - start2;
> }
>
> -/*[47:0] */
> +#define UNIT (1ULL << (5 + 3 + 12))
> +#define MASK (~(UNIT - 1))
> +#define SIZE (UNIT << 8)
these MACRO should be more meaningful
> /* need to avoid (0xfd<<32) and (0xfe<<32), ht used space */
> #define FAM10H_PCI_MMCONF_BASE (0xfcULL<<32)
> #define BASE_VALID(b) ((b != (0xfdULL << 32)) && (b != (0xfeULL << 32)))
> @@ -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;
> @@ -104,11 +104,11 @@ static void __cpuinit get_fam10h_pci_mmc
> /* TOP_MEM2 */
> address = MSR_K8_TOP_MEM2;
> rdmsrl(address, val);
> - tom2 = val & (0xffffULL<<32);
> + tom2 = val & 0xffffff800000ULL;
> }
>
> if (base <= tom2)
> - base = tom2 + (1ULL<<32);
> + base = (tom2 + 2 * UNIT - 1) & MASK;
>
> /*
> * need to check if the range is in the high mmio range that is
> @@ -123,9 +123,9 @@ 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)
> continue;
those section could be changed to:
start = reg & 0xffffff00; /* 39:16 on 31:8*/
start <<= 8;
reg = read_pci_config(bus, slot, 1, 0x84 + (i << 3));
end = (reg & 0xffffff00);
end <<= 8;
if (!end)
continue;
end |= 0xffff;
hi_mmio_num = add_range(range, 8, hi_mmio_num, start, end + 1);
1. append 0xffff after !end checking..
2. add_range should take end + 1;
> @@ -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 + SIZE)
> goto out;
>
> /* need to find one window */
> - base = range[0].start - (1ULL << 32);
> + base = (range[0].start & MASK) - UNIT;
> if ((base > tom2) && BASE_VALID(base))
> goto out;
> - base = range[hi_mmio_num - 1].end + (1ULL << 32);
> + base = (range[hi_mmio_num - 1].end + UNIT) & MASK;
> if ((base > tom2) && 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 + UNIT) & MASK;
> + val = range[i].start & MASK;
> + if (val >= base + SIZE && base > tom2 && 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 & 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;
> return;
> + }
>
> printk(KERN_INFO "Enable MMCONFIG on AMD Family 10h\n");
> val &= ~((FAM10H_MMIO_CONF_BASE_MASK<<FAM10H_MMIO_CONF_BASE_SHIFT) |
>
>
looks like fam10h_pci_mmconf_base_status should NOT be removed.
-1: mean we can not find right new base on BSP, so should not let AP to mess it again.
Thanks
Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists