Prev: [PATCH] sunrpc: Include missing smp_lock.h
Next: [PATCH -v3 3/3] pci: don't allocate from a BUSY bus resource
From: Bjorn Helgaas on 13 Apr 2010 17:10
On Mon, 2010-04-12 at 15:32 -0700, Yinghai wrote:
> Update e820 at first, and later put them resource tree.
>
> -V2: reserved that early, no PCI BAR can use it, force them to get new one
>
> Signed-off-by: Yinghai Lu <yinghai(a)kernel.org>
> Cc: Guenter Roeck <guenter.roeck(a)ericsson.com>
> Cc: Andy Isaacson <adi(a)hexapodia.org>
> Tested-by: Andy Isaacson <adi(a)hexapodia.org>
> Cc: Bjorn Helgaas <bjorn.helgaas(a)hp.com>
> Acked-by: H. Peter Anvin <hpa(a)zytor.com>
>
> ---
> arch/x86/include/asm/setup.h | 1 -
> arch/x86/kernel/head32.c | 1 -
> arch/x86/kernel/setup.c | 19 +------------------
> 3 files changed, 1 insertion(+), 20 deletions(-)
>
> Index: linux-2.6/arch/x86/include/asm/setup.h
> ===================================================================
> --- linux-2.6.orig/arch/x86/include/asm/setup.h
> +++ linux-2.6/arch/x86/include/asm/setup.h
> @@ -44,7 +44,6 @@ static inline void visws_early_detect(vo
> extern unsigned long saved_video_mode;
>
> extern void reserve_standard_io_resources(void);
> -extern void i386_reserve_resources(void);
> extern void setup_default_timer_irq(void);
>
> #ifdef CONFIG_X86_MRST
> Index: linux-2.6/arch/x86/kernel/head32.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/head32.c
> +++ linux-2.6/arch/x86/kernel/head32.c
> @@ -22,7 +22,6 @@ static void __init i386_default_early_se
> {
> /* Initilize 32bit specific setup functions */
> x86_init.resources.probe_roms = probe_roms;
> - x86_init.resources.reserve_resources = i386_reserve_resources;
> x86_init.mpparse.setup_ioapic_ids = setup_ioapic_ids_from_mpc;
>
> reserve_ebda_region();

I like the fact that this makes x86_64 and x86_32 handle the legacy VGA
framebuffer the same way.

What about arch/x86/kernel/probe_roms_32.c? That deals with expansion
ROMs in the 0xc0000-0xfffff range, including the VGA ROM. We only build
it for x86_32; is that correct, or should it be unified, too?

> 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
> @@ -693,7 +693,7 @@ static void __init trim_bios_range(void)
> * area (640->1Mb) as ram even though it is not.
> * take them out.
> */
> - e820_remove_range(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RAM, 1);
> + e820_add_region(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RESERVED);

Let me see if I understand this. On Andy's machine, the e820 map
doesn't mention the 0xa0000-0xf0000 range at all:

BIOS-e820: 0000000000000000 - 000000000009ec00 (usable)
BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)

e820_reserve_resources() inserts resources for some e820 entries (those
that start before 0x100000 or are not E820_RESERVED). Andy's machine
didn't supply any e820 entries that cover 0xa0000-0xf0000, so we didn't
insert any resources there, and PCI assumed that range was available.

This patch adds the [0xa0000-0x100000] range as E820_RESERVED. Since
that starts below 0x100000, e820_reserve_resources() will insert a
corresponding resource marked as BUSY.

Then the second patch prevents PCI from using that BUSY region to
allocate resources to devices.

Is my understanding correct?

I don't feel like I know enough about x86 architecture to ack this
patch, but I don't object to it.

Bjorn

> sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
> }
>
> @@ -1052,20 +1052,3 @@ void __init setup_arch(char **cmdline_p)
>
> mcheck_init();
> }
> -
> -#ifdef CONFIG_X86_32
> -
> -static struct resource video_ram_resource = {
> - .name = "Video RAM area",
> - .start = 0xa0000,
> - .end = 0xbffff,
> - .flags = IORESOURCE_BUSY | IORESOURCE_MEM
> -};
> -
> -void __init i386_reserve_resources(void)
> -{
> - request_resource(&iomem_resource, &video_ram_resource);
> - reserve_standard_io_resources();
> -}
> -
> -#endif /* CONFIG_X86_32 */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai on 13 Apr 2010 17:20
On 04/13/2010 02:09 PM, H. Peter Anvin wrote:
> On 04/13/2010 02:02 PM, Bjorn Helgaas wrote:
>>
>> I like the fact that this makes x86_64 and x86_32 handle the legacy VGA
>> framebuffer the same way.
>>
>> What about arch/x86/kernel/probe_roms_32.c? That deals with expansion
>> ROMs in the 0xc0000-0xfffff range, including the VGA ROM. We only build
>> it for x86_32; is that correct, or should it be unified, too?
>>
>
> It should be.
>
>>> 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
>>> @@ -693,7 +693,7 @@ static void __init trim_bios_range(void)
>>> * area (640->1Mb) as ram even though it is not.
>>> * take them out.
>>> */
>>> - e820_remove_range(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RAM, 1);
>>> + e820_add_region(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RESERVED);
>>
>> Let me see if I understand this. On Andy's machine, the e820 map
>> doesn't mention the 0xa0000-0xf0000 range at all:
>>
>> BIOS-e820: 0000000000000000 - 000000000009ec00 (usable)
>> BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)
>>
>> e820_reserve_resources() inserts resources for some e820 entries (those
>> that start before 0x100000 or are not E820_RESERVED). Andy's machine
>> didn't supply any e820 entries that cover 0xa0000-0xf0000, so we didn't
>> insert any resources there, and PCI assumed that range was available.
>
> [Note: it's worth noting that the e820 spec specifically says that e820
> will not necessarily mark legacy ranges reserved.]
>
>> This patch adds the [0xa0000-0x100000] range as E820_RESERVED. Since
>> that starts below 0x100000, e820_reserve_resources() will insert a
>> corresponding resource marked as BUSY.
>>
>> Then the second patch prevents PCI from using that BUSY region to
>> allocate resources to devices.
>>
>> Is my understanding correct?
>>
>> I don't feel like I know enough about x86 architecture to ack this
>> patch, but I don't object to it.
>
> I guess the real question (which I haven't looked at myself) is if the
> E820_RESERVED -> BUSY will cause an explicitly assigned BAR from being
> moved. That's bad, not so much for this particular range, but from BARs
> which may be assigned by SMM. Hacking that up in a simulator
> (Qemu/Bochs) and testing it is probably on the to do list...

no, if some device BAR fall in that range, it should still use that range, and will not be relocated.

will update the change log.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 13 Apr 2010 17:20
On 04/13/2010 02:02 PM, Bjorn Helgaas wrote:
>
> I like the fact that this makes x86_64 and x86_32 handle the legacy VGA
> framebuffer the same way.
>
> What about arch/x86/kernel/probe_roms_32.c? That deals with expansion
> ROMs in the 0xc0000-0xfffff range, including the VGA ROM. We only build
> it for x86_32; is that correct, or should it be unified, too?
>

It should be.

>> 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
>> @@ -693,7 +693,7 @@ static void __init trim_bios_range(void)
>> * area (640->1Mb) as ram even though it is not.
>> * take them out.
>> */
>> - e820_remove_range(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RAM, 1);
>> + e820_add_region(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RESERVED);
>
> Let me see if I understand this. On Andy's machine, the e820 map
> doesn't mention the 0xa0000-0xf0000 range at all:
>
> BIOS-e820: 0000000000000000 - 000000000009ec00 (usable)
> BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)
>
> e820_reserve_resources() inserts resources for some e820 entries (those
> that start before 0x100000 or are not E820_RESERVED). Andy's machine
> didn't supply any e820 entries that cover 0xa0000-0xf0000, so we didn't
> insert any resources there, and PCI assumed that range was available.

[Note: it's worth noting that the e820 spec specifically says that e820
will not necessarily mark legacy ranges reserved.]

> This patch adds the [0xa0000-0x100000] range as E820_RESERVED. Since
> that starts below 0x100000, e820_reserve_resources() will insert a
> corresponding resource marked as BUSY.
>
> Then the second patch prevents PCI from using that BUSY region to
> allocate resources to devices.
>
> Is my understanding correct?
>
> I don't feel like I know enough about x86 architecture to ack this
> patch, but I don't object to it.

I guess the real question (which I haven't looked at myself) is if the
E820_RESERVED -> BUSY will cause an explicitly assigned BAR from being
moved. That's bad, not so much for this particular range, but from BARs
which may be assigned by SMM. Hacking that up in a simulator
(Qemu/Bochs) and testing it is probably on the to do list...

-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 13 Apr 2010 17:20
On 04/13/2010 02:11 PM, Yinghai wrote:
>>
>> I guess the real question (which I haven't looked at myself) is if the
>> E820_RESERVED -> BUSY will cause an explicitly assigned BAR from being
>> moved. That's bad, not so much for this particular range, but from BARs
>> which may be assigned by SMM. Hacking that up in a simulator
>> (Qemu/Bochs) and testing it is probably on the to do list...
>
> no, if some device BAR fall in that range, it should still use that range, and will not be relocated.
>
> will update the change log.
>

Good, that's what we want.

ROM probing in this region should really be possible to skip, since the
only thing safe is to treat the whole region as special. This is not in
any way 32-bit-specific.

-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai on 13 Apr 2010 17:20
On 04/13/2010 02:02 PM, Bjorn Helgaas wrote:
> On Mon, 2010-04-12 at 15:32 -0700, Yinghai wrote:
>> Update e820 at first, and later put them resource tree.
>>
>> -V2: reserved that early, no PCI BAR can use it, force them to get new one
>>
>> Signed-off-by: Yinghai Lu <yinghai(a)kernel.org>
>> Cc: Guenter Roeck <guenter.roeck(a)ericsson.com>
>> Cc: Andy Isaacson <adi(a)hexapodia.org>
>> Tested-by: Andy Isaacson <adi(a)hexapodia.org>
>> Cc: Bjorn Helgaas <bjorn.helgaas(a)hp.com>
>> Acked-by: H. Peter Anvin <hpa(a)zytor.com>
>>
>> ---
>> arch/x86/include/asm/setup.h | 1 -
>> arch/x86/kernel/head32.c | 1 -
>> arch/x86/kernel/setup.c | 19 +------------------
>> 3 files changed, 1 insertion(+), 20 deletions(-)
>>
>> Index: linux-2.6/arch/x86/include/asm/setup.h
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/include/asm/setup.h
>> +++ linux-2.6/arch/x86/include/asm/setup.h
>> @@ -44,7 +44,6 @@ static inline void visws_early_detect(vo
>> extern unsigned long saved_video_mode;
>>
>> extern void reserve_standard_io_resources(void);
>> -extern void i386_reserve_resources(void);
>> extern void setup_default_timer_irq(void);
>>
>> #ifdef CONFIG_X86_MRST
>> Index: linux-2.6/arch/x86/kernel/head32.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/head32.c
>> +++ linux-2.6/arch/x86/kernel/head32.c
>> @@ -22,7 +22,6 @@ static void __init i386_default_early_se
>> {
>> /* Initilize 32bit specific setup functions */
>> x86_init.resources.probe_roms = probe_roms;
>> - x86_init.resources.reserve_resources = i386_reserve_resources;
>> x86_init.mpparse.setup_ioapic_ids = setup_ioapic_ids_from_mpc;
>>
>> reserve_ebda_region();
>
> I like the fact that this makes x86_64 and x86_32 handle the legacy VGA
> framebuffer the same way.
>
> What about arch/x86/kernel/probe_roms_32.c? That deals with expansion
> ROMs in the 0xc0000-0xfffff range, including the VGA ROM. We only build
> it for x86_32; is that correct, or should it be unified, too?

looks that file could be dropped.

We already put 0xa0000 - 0x100000 to reserved region.

>
>> 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
>> @@ -693,7 +693,7 @@ static void __init trim_bios_range(void)
>> * area (640->1Mb) as ram even though it is not.
>> * take them out.
>> */
>> - e820_remove_range(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RAM, 1);
>> + e820_add_region(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RESERVED);
>
> Let me see if I understand this. On Andy's machine, the e820 map
> doesn't mention the 0xa0000-0xf0000 range at all:
>
> BIOS-e820: 0000000000000000 - 000000000009ec00 (usable)
> BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)
>
> e820_reserve_resources() inserts resources for some e820 entries (those
> that start before 0x100000 or are not E820_RESERVED). Andy's machine
> didn't supply any e820 entries that cover 0xa0000-0xf0000, so we didn't
> insert any resources there, and PCI assumed that range was available.
>
> This patch adds the [0xa0000-0x100000] range as E820_RESERVED. Since
> that starts below 0x100000, e820_reserve_resources() will insert a
> corresponding resource marked as BUSY.
>
> Then the second patch prevents PCI from using that BUSY region to
> allocate resources to devices.
>
> Is my understanding correct?

yes.

>
> I don't feel like I know enough about x86 architecture to ack this
> patch, but I don't object to it.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2 3 4 5
Prev: [PATCH] sunrpc: Include missing smp_lock.h
Next: [PATCH -v3 3/3] pci: don't allocate from a BUSY bus resource