Patchwork Re: Unusual physical address when using 64-bit BAR

login
register
mail settings
Submitter Isaku Yamahata
Date July 14, 2010, 2:52 a.m.
Message ID <20100714025216.GK31689@valinux.co.jp>
Download mbox | patch
Permalink /patch/58833/
State New
Headers show

Comments

Isaku Yamahata - July 14, 2010, 2:52 a.m.
On Tue, Jul 13, 2010 at 04:48:19PM -0600, Cam Macdonell wrote:
> On Tue, Jul 13, 2010 at 2:41 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> > On Tue, Jul 13, 2010 at 02:05:51PM -0600, Cam Macdonell wrote:
> >> >> > Seabios completely ignore the 64-bitness of the BAR. ?Looks like it also
> >> >> > thinks the second half of the BAR is an I/O region instead of memory (hence
> >> >> > the c200, that's part of the pci portio region.
> >> >
> >> > I've sent the patches to address it. But they haven't been merged yet.
> >> > seabios doesn't map BARs beyond 4GB.
> >> > If bar is mapped beyond 4GB, guest BIOS does it.
> >>
> >> Have those patches been merged yet?
> >
> > They have been merged into seabios upstream now.
> > qemu seabios fork hasn't pulled for a while, though.
> >
> >
> >> > To see how seabios works, it would help to increase CONFIG_DEBUG_LEVEL
> >> > in config.h of seabios
> >>
> >> Where does the output from seabios end up? ?Inside dmesg?
> >
> > It outputs them to the serial console which qemu emulates.
> > seabios is out of kernel control, so dmesg doesn't show it.
> >
> >
> >> >> pci_read_config: (val) 0x0 <- 0x1c (addr)
> >> >> pci_write_config: (val) 0x0 -> 0x1c (addr)
> >> >> pci_read_config: (val) 0xffffffff <- 0x1c (addr)
> >> >> pci_write_config: (val) 0x0 -> 0x1c (addr)
> >> >> pci_read_config: (val) 0x0 <- 0x1c (addr)
> >> >> pci_write_config: (val) 0x0 -> 0x1c (addr)
> >> >
> >> > seabios BAR3. Not sure how it is mapped from this
> >> > message.
> >>
> >> Isn't the BAR3 from the fact that a 64-bit BAR would use both BAR2 and
> >> BAR3 to store all 64-bits?
> >
> > Yes. Seabios misbehaves. 64bit bar is(was) a missing feature.
> > --
> > yamahata
> >
> >
> 
> With the latest seabios git passed via -bios, I no longer see the
> 48-bit address, but instead a 32-bit address and then
> ffffffff00000000.  This guest has 1gb of RAM so the address isn't be
> mapped beyond 4g.

Can I see the debug log like before?
(hopefully seabios with CONFIG_DEBUG_LEVEL enabled.)

Do you know who sets the BAR to ffffffff00000000?
If it's seabios, does the following patch help?




> IVSHMEM: guest pci addr = e0000000, guest h/w addr = 1090912256, size = 20000000
> IVSHMEM: guest pci addr = ffffffff00000000, guest h/w addr =
> 1090912256, size = 20000000
> 
> However, booting fails when I use a 64-bit BAR.  Booting is fine with
> a 32-bit BAR.
> 
> HPET: 1 timers in total, 0 timers will be used for per-cpu timer
> divide error: 0000 [#1] SMP last sysfs file:
> CPU 0
> Modules linked in:
> 
> Pid: 1, comm: swapper Not tainted 2.6.35-rc1 #280 /Bochs
> RIP: 0010:[<ffffffff812a801b>]  [<ffffffff812a801b>] hpet_alloc+0x12c/0x35b
> RSP: 0018:ffff88003e61fd80  EFLAGS: 00010246
> RAX: 00038d7ea4c68000 RBX: ffff88003e6efd80 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff817b8520
> RBP: ffff88003e61fdc0 R08: 00000000000080d0 R09: ffffc90000000000
> R10: ffff88003f9ac600 R11: 0000000000000000 R12: ffff88003e61fdd0
> R13: ffffc90000000000 R14: 0000000000000000 R15: ffffffff817a00a9
> FS:  0000000000000000(0000) GS:ffff880002000000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000000 CR3: 0000000001a42000 CR4: 00000000000006f0DR0:
> 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process swapper (pid: 1, threadinfo ffff88003e61e000, task ffff88003e620000)
> Stack:
>  ffff88003f43ab90 ffff88003f43ab90 ffffffff81ca3174 ffffffff81b1d596
> <0> 0000000000000000 0000000000000100 0000000000000100 0000000000000000
> <0> ffff88003e61fe80 ffffffff8102945d 00000000fed00000 ffffc90000000000
> Call Trace:
>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>  [<ffffffff8102945d>] hpet_reserve_platform_timers+0x10b/0x115
>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>  [<ffffffff81b1d601>] hpet_late_init+0x6b/0xea
>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>  [<ffffffff81002069>] do_one_initcall+0x5e/0x159
>  [<ffffffff81b0b71e>] kernel_init+0x18e/0x21c
>  [<ffffffff8100aa24>] kernel_thread_helper+0x4/0x10
>  [<ffffffff81b0b590>] ? kernel_init+0x0/0x21c
>  [<ffffffff8100aa20>] ? kernel_thread_helper+0x0/0x10
> Code: 89 1d 8a 92 b3 00 48 c1 ea 21 8b 73 34 49 c7 c7 a9 00 7a 81 48
> 8d 04 02 4c 89 f2 48 c7 c7 20 85 7b 81 48 c1 ea 20 48 89 d1 31 d2 <48>
> f7 f1 83 7b 30 01 48
> c7 c1 cd fa 7c 81 49 0f 46 cf 48 89 43
> RIP  [<ffffffff812a801b>] hpet_alloc+0x12c/0x35b
>  RSP <ffff88003e61fd80>
> ---[ end trace a7919e7f17c0a725 ]---
> Kernel panic - not syncing: Attempted to kill init!
> Pid: 1, comm: swapper Tainted: G      D     2.6.35-rc1 #280
> Call Trace:
>  [<ffffffff8145b870>] panic+0x8b/0x10b
>  [<ffffffff81056a93>] ? exit_ptrace+0x38/0x121
>  [<ffffffff8104f9e8>] do_exit+0x7a/0x722
>  [<ffffffff8104c3bd>] ? spin_unlock_irqrestore+0xe/0x10
>  [<ffffffff8104cfd8>] ? kmsg_dump+0x12b/0x145
>  [<ffffffff8145eaa9>] oops_end+0xbf/0xc7
>  [<ffffffff8100d299>] die+0x5a/0x63
>  [<ffffffff8145e4b3>] do_trap+0x121/0x130
>  [<ffffffff8100b560>] do_divide_error+0x96/0x9f
>  [<ffffffff812a801b>] ? hpet_alloc+0x12c/0x35b
>  [<ffffffff8100a83b>] divide_error+0x1b/0x20
>  [<ffffffff812a801b>] ? hpet_alloc+0x12c/0x35b
>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>  [<ffffffff8102945d>] hpet_reserve_platform_timers+0x10b/0x115
>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>  [<ffffffff81b1d601>] hpet_late_init+0x6b/0xea
>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>  [<ffffffff81002069>] do_one_initcall+0x5e/0x159
>  [<ffffffff81b0b71e>] kernel_init+0x18e/0x21c
>  [<ffffffff8100aa24>] kernel_thread_helper+0x4/0x10
>  [<ffffffff81b0b590>] ? kernel_init+0x0/0x21c
>  [<ffffffff8100aa20>] ? kernel_thread_helper+0x0/0x10
>
Cam Macdonell - July 14, 2010, 3:10 p.m.
On Tue, Jul 13, 2010 at 8:52 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> On Tue, Jul 13, 2010 at 04:48:19PM -0600, Cam Macdonell wrote:
>> On Tue, Jul 13, 2010 at 2:41 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
>> > On Tue, Jul 13, 2010 at 02:05:51PM -0600, Cam Macdonell wrote:
>> >> >> > Seabios completely ignore the 64-bitness of the BAR. ?Looks like it also
>> >> >> > thinks the second half of the BAR is an I/O region instead of memory (hence
>> >> >> > the c200, that's part of the pci portio region.
>> >> >
>> >> > I've sent the patches to address it. But they haven't been merged yet.
>> >> > seabios doesn't map BARs beyond 4GB.
>> >> > If bar is mapped beyond 4GB, guest BIOS does it.
>> >>
>> >> Have those patches been merged yet?
>> >
>> > They have been merged into seabios upstream now.
>> > qemu seabios fork hasn't pulled for a while, though.
>> >
>> >
>> >> > To see how seabios works, it would help to increase CONFIG_DEBUG_LEVEL
>> >> > in config.h of seabios
>> >>
>> >> Where does the output from seabios end up? ?Inside dmesg?
>> >
>> > It outputs them to the serial console which qemu emulates.
>> > seabios is out of kernel control, so dmesg doesn't show it.
>> >
>> >
>> >> >> pci_read_config: (val) 0x0 <- 0x1c (addr)
>> >> >> pci_write_config: (val) 0x0 -> 0x1c (addr)
>> >> >> pci_read_config: (val) 0xffffffff <- 0x1c (addr)
>> >> >> pci_write_config: (val) 0x0 -> 0x1c (addr)
>> >> >> pci_read_config: (val) 0x0 <- 0x1c (addr)
>> >> >> pci_write_config: (val) 0x0 -> 0x1c (addr)
>> >> >
>> >> > seabios BAR3. Not sure how it is mapped from this
>> >> > message.
>> >>
>> >> Isn't the BAR3 from the fact that a 64-bit BAR would use both BAR2 and
>> >> BAR3 to store all 64-bits?
>> >
>> > Yes. Seabios misbehaves. 64bit bar is(was) a missing feature.
>> > --
>> > yamahata
>> >
>> >
>>
>> With the latest seabios git passed via -bios, I no longer see the
>> 48-bit address, but instead a 32-bit address and then
>> ffffffff00000000.  This guest has 1gb of RAM so the address isn't be
>> mapped beyond 4g.
>
> Can I see the debug log like before?
> (hopefully seabios with CONFIG_DEBUG_LEVEL enabled.)

Here's the dump from SeaBIOS in the region related to the PCI devices.
 The SeaBIOS output is identical whether the BAR is 32-bit or 64-bit.

PCI: bus=0 devfn=0x10: vendor_id=0x1013 device_id=0x00b8
region 0: 0xf0000000
region 1: 0xf2000000
region 6: 0xf2010000
PCI: bus=0 devfn=0x18: vendor_id=0x1af4 device_id=0x1000
region 0: 0x0000c020
region 1: 0xf2020000
region 6: 0xf2030000
PCI: bus=0 devfn=0x20: vendor_id=0x1af4 device_id=0x1110
region 0: 0xf2040000
region 1: 0xf2041000
region 2: 0x00000000

>
> Do you know who sets the BAR to ffffffff00000000?

Here are the config reads/writes related to the 0x18/1c, the 'IVSHMEM'
lines are from the map function passed to pci_register_bar().  It
looks like SeaBIOS sets the address to 0 and then the potentially
useful e0000000 address gets mangled into ffffffff000000.

IVSHMEM: guest pci addr = 0, guest h/w addr = 1090912256, size = 536870912

...snip...

pci_read_config: (val) 0x4 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
IVSHMEM: guest pci addr = e0000000, guest h/w addr = 1090912256, size = 20000000
pci_read_config: (val) 0xe0000004 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0x0 <- 0x1c (addr)
pci_write_config: (val) 0x0 -> 0x1c (addr)
IVSHMEM: guest pci addr = ffffffff00000000, guest h/w addr =
1090912256, size = 20000000
pci_read_config: (val) 0xffffffff <- 0x1c (addr)
pci_write_config: (val) 0x0 -> 0x1c (addr)

and with the 64-bit guest I get this error as well (recall the guest
fails to boot on 64-bit)

BUG: kvm_dirty_pages_log_change: invalid parameters
00000000f0000000-00000000f0ffffff

> If it's seabios, does the following patch help?

The patch doesn't make a difference.  Perhaps it's Qemu then?

>
> diff --git a/src/pciinit.c b/src/pciinit.c
> index b110531..ce07709 100644
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -117,11 +117,7 @@ static int pci_bios_allocate_region(u16 bdf, int region_num)
>     int is_64bit = !(val & PCI_BASE_ADDRESS_SPACE_IO) &&
>         (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64;
>     if (is_64bit) {
> -        if (size > 0) {
> -            pci_config_writel(bdf, ofs + 4, 0);
> -        } else {
> -            pci_config_writel(bdf, ofs + 4, ~0);
> -        }
> +        pci_config_writel(bdf, ofs + 4, 0);
>     }
>     return is_64bit;
>  }
>
>
>
>> IVSHMEM: guest pci addr = e0000000, guest h/w addr = 1090912256, size = 20000000
>> IVSHMEM: guest pci addr = ffffffff00000000, guest h/w addr =
>> 1090912256, size = 20000000
>>
>> However, booting fails when I use a 64-bit BAR.  Booting is fine with
>> a 32-bit BAR.
>>
>> HPET: 1 timers in total, 0 timers will be used for per-cpu timer
>> divide error: 0000 [#1] SMP last sysfs file:
>> CPU 0
>> Modules linked in:
>>
>> Pid: 1, comm: swapper Not tainted 2.6.35-rc1 #280 /Bochs
>> RIP: 0010:[<ffffffff812a801b>]  [<ffffffff812a801b>] hpet_alloc+0x12c/0x35b
>> RSP: 0018:ffff88003e61fd80  EFLAGS: 00010246
>> RAX: 00038d7ea4c68000 RBX: ffff88003e6efd80 RCX: 0000000000000000
>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff817b8520
>> RBP: ffff88003e61fdc0 R08: 00000000000080d0 R09: ffffc90000000000
>> R10: ffff88003f9ac600 R11: 0000000000000000 R12: ffff88003e61fdd0
>> R13: ffffc90000000000 R14: 0000000000000000 R15: ffffffff817a00a9
>> FS:  0000000000000000(0000) GS:ffff880002000000(0000) knlGS:0000000000000000
>> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> CR2: 0000000000000000 CR3: 0000000001a42000 CR4: 00000000000006f0DR0:
>> 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>> Process swapper (pid: 1, threadinfo ffff88003e61e000, task ffff88003e620000)
>> Stack:
>>  ffff88003f43ab90 ffff88003f43ab90 ffffffff81ca3174 ffffffff81b1d596
>> <0> 0000000000000000 0000000000000100 0000000000000100 0000000000000000
>> <0> ffff88003e61fe80 ffffffff8102945d 00000000fed00000 ffffc90000000000
>> Call Trace:
>>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>>  [<ffffffff8102945d>] hpet_reserve_platform_timers+0x10b/0x115
>>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>>  [<ffffffff81b1d601>] hpet_late_init+0x6b/0xea
>>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>>  [<ffffffff81002069>] do_one_initcall+0x5e/0x159
>>  [<ffffffff81b0b71e>] kernel_init+0x18e/0x21c
>>  [<ffffffff8100aa24>] kernel_thread_helper+0x4/0x10
>>  [<ffffffff81b0b590>] ? kernel_init+0x0/0x21c
>>  [<ffffffff8100aa20>] ? kernel_thread_helper+0x0/0x10
>> Code: 89 1d 8a 92 b3 00 48 c1 ea 21 8b 73 34 49 c7 c7 a9 00 7a 81 48
>> 8d 04 02 4c 89 f2 48 c7 c7 20 85 7b 81 48 c1 ea 20 48 89 d1 31 d2 <48>
>> f7 f1 83 7b 30 01 48
>> c7 c1 cd fa 7c 81 49 0f 46 cf 48 89 43
>> RIP  [<ffffffff812a801b>] hpet_alloc+0x12c/0x35b
>>  RSP <ffff88003e61fd80>
>> ---[ end trace a7919e7f17c0a725 ]---
>> Kernel panic - not syncing: Attempted to kill init!
>> Pid: 1, comm: swapper Tainted: G      D     2.6.35-rc1 #280
>> Call Trace:
>>  [<ffffffff8145b870>] panic+0x8b/0x10b
>>  [<ffffffff81056a93>] ? exit_ptrace+0x38/0x121
>>  [<ffffffff8104f9e8>] do_exit+0x7a/0x722
>>  [<ffffffff8104c3bd>] ? spin_unlock_irqrestore+0xe/0x10
>>  [<ffffffff8104cfd8>] ? kmsg_dump+0x12b/0x145
>>  [<ffffffff8145eaa9>] oops_end+0xbf/0xc7
>>  [<ffffffff8100d299>] die+0x5a/0x63
>>  [<ffffffff8145e4b3>] do_trap+0x121/0x130
>>  [<ffffffff8100b560>] do_divide_error+0x96/0x9f
>>  [<ffffffff812a801b>] ? hpet_alloc+0x12c/0x35b
>>  [<ffffffff8100a83b>] divide_error+0x1b/0x20
>>  [<ffffffff812a801b>] ? hpet_alloc+0x12c/0x35b
>>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>>  [<ffffffff8102945d>] hpet_reserve_platform_timers+0x10b/0x115
>>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>>  [<ffffffff81b1d601>] hpet_late_init+0x6b/0xea
>>  [<ffffffff81b1d596>] ? hpet_late_init+0x0/0xea
>>  [<ffffffff81002069>] do_one_initcall+0x5e/0x159
>>  [<ffffffff81b0b71e>] kernel_init+0x18e/0x21c
>>  [<ffffffff8100aa24>] kernel_thread_helper+0x4/0x10
>>  [<ffffffff81b0b590>] ? kernel_init+0x0/0x21c
>>  [<ffffffff8100aa20>] ? kernel_thread_helper+0x0/0x10
>>
>
> --
> yamahata
>
>

Patch

diff --git a/src/pciinit.c b/src/pciinit.c
index b110531..ce07709 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -117,11 +117,7 @@  static int pci_bios_allocate_region(u16 bdf, int region_num)
     int is_64bit = !(val & PCI_BASE_ADDRESS_SPACE_IO) &&
         (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64;
     if (is_64bit) {
-        if (size > 0) {
-            pci_config_writel(bdf, ofs + 4, 0);
-        } else {
-            pci_config_writel(bdf, ofs + 4, ~0);
-        }
+        pci_config_writel(bdf, ofs + 4, 0);
     }
     return is_64bit;
 }