2010-11-11 00:56:09

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 1/2] x86: Change sanitize_e820_map() definition


Tt will use e820 directly.
So We don't need to carry e820.map with it.

Signed-off-by: Yinghai Lu <[email protected]>

---
arch/x86/include/asm/e820.h | 3 +--
arch/x86/kernel/e820.c | 17 +++++++++++------
arch/x86/kernel/setup.c | 6 +++---
arch/x86/platform/efi/efi.c | 2 +-
arch/x86/xen/setup.c | 5 ++---
5 files changed, 18 insertions(+), 15 deletions(-)

Index: linux-2.6/arch/x86/kernel/e820.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820.c
+++ linux-2.6/arch/x86/kernel/e820.c
@@ -225,7 +225,7 @@ void __init e820_print_map(char *who)
* ______________________4_
*/

-int __init sanitize_e820_map(struct e820entry *biosmap, int max_nr_map,
+static int __init __sanitize_e820_map(struct e820entry *biosmap, int max_nr_map,
u32 *pnr_map)
{
struct change_member {
@@ -384,6 +384,11 @@ int __init sanitize_e820_map(struct e820
return 0;
}

+int __init sanitize_e820_map(void)
+{
+ return __sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+}
+
static int __init __append_e820_map(struct e820entry *biosmap, int nr_map)
{
while (nr_map) {
@@ -572,7 +577,7 @@ void __init update_e820(void)
u32 nr_map;

nr_map = e820.nr_map;
- if (sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &nr_map))
+ if (__sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &nr_map))
return;
e820.nr_map = nr_map;
printk(KERN_INFO "modified physical RAM map:\n");
@@ -583,7 +588,7 @@ static void __init update_e820_saved(voi
u32 nr_map;

nr_map = e820_saved.nr_map;
- if (sanitize_e820_map(e820_saved.map, ARRAY_SIZE(e820_saved.map), &nr_map))
+ if (__sanitize_e820_map(e820_saved.map, ARRAY_SIZE(e820_saved.map), &nr_map))
return;
e820_saved.nr_map = nr_map;
}
@@ -678,7 +683,7 @@ void __init parse_e820_ext(struct setup_
sdata = early_ioremap(pa_data, map_len);
extmap = (struct e820entry *)(sdata->data);
__append_e820_map(extmap, entries);
- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+ sanitize_e820_map();
if (map_len > PAGE_SIZE)
early_iounmap(sdata, map_len);
printk(KERN_INFO "extended physical RAM map:\n");
@@ -910,7 +915,7 @@ void __init finish_e820_parsing(void)
if (userdef) {
u32 nr = e820.nr_map;

- if (sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &nr) < 0)
+ if (__sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &nr) < 0)
early_panic("Invalid user supplied memory map");
e820.nr_map = nr;

@@ -1040,7 +1045,7 @@ char *__init default_machine_specific_me
* the next section from 1mb->appropriate_mem_k
*/
new_nr = boot_params.e820_entries;
- sanitize_e820_map(boot_params.e820_map,
+ __sanitize_e820_map(boot_params.e820_map,
ARRAY_SIZE(boot_params.e820_map),
&new_nr);
boot_params.e820_entries = new_nr;
Index: linux-2.6/arch/x86/platform/efi/efi.c
===================================================================
--- linux-2.6.orig/arch/x86/platform/efi/efi.c
+++ linux-2.6/arch/x86/platform/efi/efi.c
@@ -273,7 +273,7 @@ static void __init do_add_efi_memmap(voi
}
e820_add_region(start, size, e820_type);
}
- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+ sanitize_e820_map();
}

void __init efi_memblock_x86_reserve_range(void)
Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c
@@ -462,7 +462,7 @@ static void __init e820_reserve_setup_da
if (!found)
return;

- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+ sanitize_e820_map();
memcpy(&e820_saved, &e820, sizeof(struct e820map));
printk(KERN_INFO "extended physical RAM map:\n");
e820_print_map("reserve setup_data");
@@ -644,7 +644,7 @@ static void __init trim_bios_range(void)
* take them out.
*/
e820_remove_range(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RAM, 1);
- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+ sanitize_e820_map();
}

static int __init parse_reservelow(char *p)
@@ -861,7 +861,7 @@ void __init setup_arch(char **cmdline_p)
if (ppro_with_ram_bug()) {
e820_update_range(0x70000000ULL, 0x40000ULL, E820_RAM,
E820_RESERVED);
- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+ sanitize_e820_map();
printk(KERN_INFO "fixed physical RAM map:\n");
e820_print_map("bad_ppro");
}
Index: linux-2.6/arch/x86/xen/setup.c
===================================================================
--- linux-2.6.orig/arch/x86/xen/setup.c
+++ linux-2.6/arch/x86/xen/setup.c
@@ -60,7 +60,7 @@ static __init void xen_add_extra_mem(uns
return;

e820_add_region(extra_start, size, E820_RAM);
- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+ sanitize_e820_map();

memblock_x86_reserve_range(extra_start, extra_start + size, "XEN EXTRA");

@@ -174,7 +174,6 @@ char * __init xen_memory_setup(void)
}
BUG_ON(rc);

- e820.nr_map = 0;
xen_extra_mem_start = mem_end;
for (i = 0; i < memmap.nr_entries; i++) {
unsigned long long end = map[i].addr + map[i].size;
@@ -221,7 +220,7 @@ char * __init xen_memory_setup(void)
__pa(xen_start_info->pt_base),
"XEN START INFO");

- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+ sanitize_e820_map();

extra_pages += xen_return_unused_memory(xen_start_info->nr_pages, &e820);

Index: linux-2.6/arch/x86/include/asm/e820.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/e820.h
+++ linux-2.6/arch/x86/include/asm/e820.h
@@ -82,8 +82,7 @@ extern int e820_any_mapped(u64 start, u6
extern int e820_all_mapped(u64 start, u64 end, unsigned type);
extern void e820_add_region(u64 start, u64 size, int type);
extern void e820_print_map(char *who);
-extern int
-sanitize_e820_map(struct e820entry *biosmap, int max_nr_map, u32 *pnr_map);
+int sanitize_e820_map(void);
extern u64 e820_update_range(u64 start, u64 size, unsigned old_type,
unsigned new_type);
extern u64 e820_remove_range(u64 start, u64 size, unsigned old_type,


2010-11-11 00:56:16

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH -v2 2/2] x86/pci: Add mmconf range into e820 for when it is from MSR with amd faml0h


for AMD Fam10h, it we read mmconf from MSR early, we should just trust it
because we check it and correct it already.

so add it to e820

Also correct the base calulating to make it work below 4g case.

-v2: remove unrelated sort_range changes.
also make FAM10H_MMIO_CONF_BASE_MASK to be ULL pointed by Jan.

Signed-off-by: Yinghai Lu <[email protected]>
Cc: Jan Beulich <[email protected]>

---
arch/x86/include/asm/msr-index.h | 2 +-
arch/x86/kernel/mmconf-fam10h_64.c | 21 ++++++++++++++++++++-
2 files changed, 21 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/kernel/mmconf-fam10h_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/mmconf-fam10h_64.c
+++ linux-2.6/arch/x86/kernel/mmconf-fam10h_64.c
@@ -16,6 +16,7 @@
#include <asm/acpi.h>
#include <asm/mmconfig.h>
#include <asm/pci_x86.h>
+#include <asm/e820.h>

struct pci_hostbridge_probe {
u32 bus;
@@ -27,6 +28,21 @@ struct pci_hostbridge_probe {
static u64 __cpuinitdata fam10h_pci_mmconf_base;
static int __cpuinitdata fam10h_pci_mmconf_base_status;

+/* only on BSP */
+static void __init_refok e820_add_mmconf_range(int busnbits)
+{
+ u64 end;
+
+ end = fam10h_pci_mmconf_base + (1ULL<<(busnbits + 20)) - 1;
+ if (!e820_all_mapped(fam10h_pci_mmconf_base, end+1, E820_RESERVED)) {
+ printk(KERN_DEBUG "Fam 10h mmconf [%llx, %llx]\n",
+ fam10h_pci_mmconf_base, end);
+ e820_add_region(fam10h_pci_mmconf_base, 1ULL<<(busnbits + 20),
+ E820_RESERVED);
+ sanitize_e820_map();
+ }
+}
+
static struct pci_hostbridge_probe pci_probes[] __cpuinitdata = {
{ 0, 0x18, PCI_VENDOR_ID_AMD, 0x1200 },
{ 0xff, 0, PCI_VENDOR_ID_AMD, 0x1200 },
@@ -169,6 +185,7 @@ fail:
out:
fam10h_pci_mmconf_base = base;
fam10h_pci_mmconf_base_status = 1;
+ e820_add_mmconf_range(8);
}

void __cpuinit fam10h_check_enable_mmcfg(void)
@@ -191,10 +208,12 @@ 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);
+ base = val & (FAM10H_MMIO_CONF_BASE_MASK <<
+ FAM10H_MMIO_CONF_BASE_SHIFT);
if (fam10h_pci_mmconf_base_status <= 0) {
fam10h_pci_mmconf_base = base;
fam10h_pci_mmconf_base_status = 1;
+ e820_add_mmconf_range(busnbits);
return;
} else if (fam10h_pci_mmconf_base == base)
return;
Index: linux-2.6/arch/x86/include/asm/msr-index.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/msr-index.h
+++ linux-2.6/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

2010-11-11 08:50:59

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH -v2 2/2] x86/pci: Add mmconf range into e820 for when it is from MSR with amd faml0h

>>> On 11.11.10 at 01:54, Yinghai Lu <[email protected]> wrote:
> @@ -191,10 +208,12 @@ 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);
> + base = val & (FAM10H_MMIO_CONF_BASE_MASK <<
> + FAM10H_MMIO_CONF_BASE_SHIFT);
> if (fam10h_pci_mmconf_base_status <= 0) {
> fam10h_pci_mmconf_base = base;
> fam10h_pci_mmconf_base_status = 1;
> + e820_add_mmconf_range(busnbits);
> return;
> } else if (fam10h_pci_mmconf_base == base)
> return;

I don't think adding the range in this case is correct: Here, we
found that the BIOS enabled the feature, and we're doing
nothing to "correct" it, so we also should expect the range
to be reserved by the BIOS.

Jan

2010-11-11 20:32:01

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH -v2 2/2] x86/pci: Add mmconf range into e820 for when it is from MSR with amd faml0h

On 11/11/2010 12:50 AM, Jan Beulich wrote:
>>>> On 11.11.10 at 01:54, Yinghai Lu <[email protected]> wrote:
>> @@ -191,10 +208,12 @@ 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);
>> + base = val & (FAM10H_MMIO_CONF_BASE_MASK <<
>> + FAM10H_MMIO_CONF_BASE_SHIFT);
>> if (fam10h_pci_mmconf_base_status <= 0) {
>> fam10h_pci_mmconf_base = base;
>> fam10h_pci_mmconf_base_status = 1;
>> + e820_add_mmconf_range(busnbits);
>> return;
>> } else if (fam10h_pci_mmconf_base == base)
>> return;
>
> I don't think adding the range in this case is correct: Here, we
> found that the BIOS enabled the feature, and we're doing
> nothing to "correct" it, so we also should expect the range
> to be reserved by the BIOS.

this function is protected by PCI_CHECK_ENABLE_AMD_MMCONF

void __cpuinit fam10h_check_enable_mmcfg(void)
{
u64 val;
u32 address;

if (!(pci_probe & PCI_CHECK_ENABLE_AMD_MMCONF))
return;

and could have systems that setting in hw config is right, but BIOS doesn't put
them with e820 reserved entries.

Thanks

Yinghai