Re: [Qemu-devel] Re: Unusual physical address when using 64-bit BAR

[Cam Macdonell]

On Tue, Jun 29, 2010 at 9:29 PM, Isaku Yamahata <address@hidden> wrote:
> On Tue, Jun 29, 2010 at 11:48:13AM -0600, Cam Macdonell wrote:
>> On Tue, Jun 29, 2010 at 12:50 AM, Avi Kivity <address@hidden> wrote:
>> > On 06/28/2010 11:38 PM, Cam Macdonell wrote:
>> >>
>> >>>
>> >>>>> Is this really the address the guest programmed, or is qemu
>> >>>>> misinterpreting
>> >>>>> it?
>> >>>>>
>> >>>>>
>> >>>
>> >>> Well, what's the answer?
>> >>>
>> >>
>> >> You're going to have to give me a hint on how to determine that.
>> >>
>> >> lspci in the guest shows the following
>> >>
>> >> Memory at c20000000000 (64-bit, non-prefetchable) [size=1024M]
>> >>
>> >> does that demonstrate a guest generated address?
>> >>
>> >
>> > That's the result of a round trip: the guest programmed the address and 
>> > then
>> > read it back. ?It could have been screwed up in the first place, or perhaps
>> > qemu screwed it up.
>> >
>> > Add a printf() to the config space handlers in qemu (likely in your own
>> > code) on writes and reads, and show the relevant writes (and reads) for 
>> > this
>> > BAR.
>> >
>> > That's the theory of deductive debugging; however browsing the code shows
>> > the guest is at fault:
>> >
>> > ? ? ? ?for (i = 0; i < PCI_NUM_REGIONS; i++) {
>> > ? ? ? ? ? ?int ofs;
>> > ? ? ? ? ? ?if (i == PCI_ROM_SLOT)
>> > ? ? ? ? ? ? ? ?ofs = PCI_ROM_ADDRESS;
>> > ? ? ? ? ? ?else
>> > ? ? ? ? ? ? ? ?ofs = PCI_BASE_ADDRESS_0 + i * 4;
>> >
>> > ? ? ? ? ? ?u32 old = pci_config_readl(bdf, ofs);
>> > ? ? ? ? ? ?u32 mask;
>> > ? ? ? ? ? ?if (i == PCI_ROM_SLOT) {
>> > ? ? ? ? ? ? ? ?mask = PCI_ROM_ADDRESS_MASK;
>> > ? ? ? ? ? ? ? ?pci_config_writel(bdf, ofs, mask);
>> > ? ? ? ? ? ?} else {
>> > ? ? ? ? ? ? ? ?if (old & PCI_BASE_ADDRESS_SPACE_IO)
>> > ? ? ? ? ? ? ? ? ? ?mask = PCI_BASE_ADDRESS_IO_MASK;
>> > ? ? ? ? ? ? ? ?else
>> > ? ? ? ? ? ? ? ? ? ?mask = PCI_BASE_ADDRESS_MEM_MASK;
>> > ? ? ? ? ? ? ? ?pci_config_writel(bdf, ofs, ~0);
>> > ? ? ? ? ? ?}
>> > ? ? ? ? ? ?u32 val = pci_config_readl(bdf, ofs);
>> > ? ? ? ? ? ?pci_config_writel(bdf, ofs, old);
>> >
>> > ? ? ? ? ? ?if (val != 0) {
>> > ? ? ? ? ? ? ? ?u32 size = (~(val & mask)) + 1;
>> > ? ? ? ? ? ? ? ?if (val & PCI_BASE_ADDRESS_SPACE_IO)
>> > ? ? ? ? ? ? ? ? ? ?paddr = &pci_bios_io_addr;
>> > ? ? ? ? ? ? ? ?else
>> > ? ? ? ? ? ? ? ? ? ?paddr = &pci_bios_mem_addr;
>> > ? ? ? ? ? ? ? ?*paddr = ALIGN(*paddr, size);
>> > ? ? ? ? ? ? ? ?pci_set_io_region_addr(bdf, i, *paddr);
>> > ? ? ? ? ? ? ? ?*paddr += size;
>> > ? ? ? ? ? ?}
>> > ? ? ? ?}
>> > ? ? ? ?break;
>> > ? ?}
>> >
>> > 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?

>
>
> 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?

>
>
>> > Do post those reads and writes, I think there's more than one thing wrong
>> > here.
>>
>> Here it is, I added the debug statements to pci_read_config and
>> pci_default_write_config.
>>
>> here are the reads and writes to offsets 0x18 and 0x1c where a 64-bit
>> BAR2 config would be configured
>
> It seems that 0x1c is accessed as BAR3.
> The write value in the log is always 0.
>
> Some comment in the log.
>
>>
>> pci_read_config: (val) 0x4 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> pci_read_config: (val) 0xc0000004 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> pci_read_config: (val) 0x4 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> 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)
>> pci_read_config: (val) 0x4 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> pci_read_config: (val) 0xc0000004 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> pci_read_config: (val) 0xc040 <- 0x1c (addr)
>> pci_write_config: (val) 0x0 -> 0x1c (addr)
>> pci_read_config: (val) 0xffffffff <- 0x1c (addr)
>> pci_write_config: (val) 0x0 -> 0x1c (addr)
>>
>> the complete read/write profile is below along with debug statements
>> from the map functions for the BARs (prefixed with IVSHMEM)
>>
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x1110 <- 0x2 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x500 <- 0xa (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x1110 <- 0x2 (addr)
>> pci_read_config: (val) 0x0 <- 0x10 (addr)
>> pci_write_config: (val) 0x0 -> 0x10 (addr)
>> pci_read_config: (val) 0xffffff00 <- 0x10 (addr)
>> pci_write_config: (val) 0x0 -> 0x10 (addr)
>> pci_read_config: (val) 0x0 <- 0x10 (addr)
>> pci_write_config: (val) 0x0 -> 0x10 (addr)
>> pci_read_config: (val) 0x0 <- 0x14 (addr)
>> pci_write_config: (val) 0x0 -> 0x14 (addr)
>> pci_read_config: (val) 0xfffff000 <- 0x14 (addr)
>> pci_write_config: (val) 0x0 -> 0x14 (addr)
>> pci_read_config: (val) 0x0 <- 0x14 (addr)
>> pci_write_config: (val) 0x0 -> 0x14 (addr)
>
>> pci_read_config: (val) 0x4 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> pci_read_config: (val) 0xc0000004 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> pci_read_config: (val) 0x4 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>
> seabios BAR2.
>
>> 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?

>
>
>
>> pci_read_config: (val) 0x0 <- 0x20 (addr)
>> pci_write_config: (val) 0x0 -> 0x20 (addr)
>> pci_read_config: (val) 0x0 <- 0x20 (addr)
>> pci_write_config: (val) 0x0 -> 0x20 (addr)
>> pci_read_config: (val) 0x0 <- 0x24 (addr)
>> pci_write_config: (val) 0x0 -> 0x24 (addr)
>> pci_read_config: (val) 0x0 <- 0x24 (addr)
>> pci_write_config: (val) 0x0 -> 0x24 (addr)
>> pci_read_config: (val) 0x0 <- 0x30 (addr)
>> pci_write_config: (val) 0x0 -> 0x30 (addr)
>> pci_read_config: (val) 0x0 <- 0x30 (addr)
>> pci_write_config: (val) 0x0 -> 0x30 (addr)
>> pci_read_config: (val) 0x0 <- 0x4 (addr)
>> pci_write_config: (val) 0x0 -> 0x4 (addr)
>> IVSHMEM: guest pci addr = c04000000000, guest h/w addr = 1090912256,
>> size = 40000000
>> pci_read_config: (val) 0x1 <- 0x3d (addr)
>> pci_write_config: (val) 0x0 -> 0x3c (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1 <- 0x3d (addr)
>> pci_read_config: (val) 0xb <- 0x3c (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x5000000 <- 0x8 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x500 <- 0xa (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x11101af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x500 <- 0xa (addr)
>> pci_read_config: (val) 0x11101af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x0 <- 0x30 (addr)
>> pci_write_config: (val) 0x0 -> 0x30 (addr)
>> pci_read_config: (val) 0x0 <- 0x30 (addr)
>> pci_write_config: (val) 0x0 -> 0x30 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x5000000 <- 0x8 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x11101af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x500 <- 0xa (addr)
>> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x1110 <- 0x2 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x5000000 <- 0x8 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x11101af4 <- 0x0 (addr)
>> pci_read_config: (val) 0x0 <- 0xe (addr)
>> pci_read_config: (val) 0x10 <- 0x6 (addr)
>> pci_read_config: (val) 0x40 <- 0x34 (addr)
>> pci_read_config: (val) 0x11 <- 0x40 (addr)
>> pci_read_config: (val) 0x0 <- 0x41 (addr)
>> pci_read_config: (val) 0x5000000 <- 0x8 (addr)
>> pci_read_config: (val) 0x10 <- 0x6 (addr)
>> pci_read_config: (val) 0x40 <- 0x34 (addr)
>> pci_read_config: (val) 0x11 <- 0x40 (addr)
>> pci_read_config: (val) 0x0 <- 0x41 (addr)
>> pci_read_config: (val) 0x1 <- 0x3d (addr)
>> pci_read_config: (val) 0xb <- 0x3c (addr)
>> pci_read_config: (val) 0xf2040000 <- 0x10 (addr)
>> pci_write_config: (val) 0x0 -> 0x10 (addr)
>> pci_read_config: (val) 0xffffff00 <- 0x10 (addr)
>> pci_write_config: (val) 0x0 -> 0x10 (addr)
>> pci_read_config: (val) 0xf2041000 <- 0x14 (addr)
>> pci_write_config: (val) 0x0 -> 0x14 (addr)
>> pci_read_config: (val) 0xfffff000 <- 0x14 (addr)
>> pci_write_config: (val) 0x0 -> 0x14 (addr)
>
>
>> pci_read_config: (val) 0x4 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> IVSHMEM: guest pci addr = c040c0000000, guest h/w addr = 1090912256,
>> size = 40000000
>> pci_read_config: (val) 0xc0000004 <- 0x18 (addr)
>> pci_write_config: (val) 0x0 -> 0x18 (addr)
>> IVSHMEM: guest pci addr = c04000000000, guest h/w addr = 1090912256,
>> size = 40000000
>> pci_read_config: (val) 0xc040 <- 0x1c (addr)
>> pci_write_config: (val) 0x0 -> 0x1c (addr)
>> IVSHMEM: guest pci addr = ffffffff00000000, guest h/w addr =
>> 1090912256, size = 40000000
>> pci_read_config: (val) 0xffffffff <- 0x1c (addr)
>> pci_write_config: (val) 0x0 -> 0x1c (addr)
>> IVSHMEM: guest pci addr = c04000000000, guest h/w addr = 1090912256,
>> size = 40000000
>
> guest OS?
> It seems that guset OS understands 64bit BAR
> and remaps the BAR.

But c0400000000 is a 48-bit address and the guest only has 40-bit
physical addresses, so the remap fails in the guest.

Cam

>
>
>> pci_read_config: (val) 0x0 <- 0x20 (addr)
>> pci_write_config: (val) 0x0 -> 0x20 (addr)
>> pci_read_config: (val) 0x0 <- 0x20 (addr)
>> pci_write_config: (val) 0x0 -> 0x20 (addr)
>> pci_read_config: (val) 0x0 <- 0x24 (addr)
>> pci_write_config: (val) 0x0 -> 0x24 (addr)
>> pci_read_config: (val) 0x0 <- 0x24 (addr)
>> pci_write_config: (val) 0x0 -> 0x24 (addr)
>> pci_read_config: (val) 0x0 <- 0x30 (addr)
>> pci_write_config: (val) 0x0 -> 0x30 (addr)
>> pci_read_config: (val) 0x0 <- 0x30 (addr)
>> pci_write_config: (val) 0x0 -> 0x30 (addr)
>> pci_read_config: (val) 0x1af4 <- 0x2c (addr)
>> pci_read_config: (val) 0x1100 <- 0x2e (addr)
>> pci_read_config: (val) 0x10 <- 0x6 (addr)
>> pci_read_config: (val) 0x40 <- 0x34 (addr)
>> pci_read_config: (val) 0x11 <- 0x40 (addr)
>> pci_read_config: (val) 0x0 <- 0x41 (addr)
>> pci_read_config: (val) 0x10 <- 0x6 (addr)
>> pci_read_config: (val) 0x40 <- 0x34 (addr)
>> pci_read_config: (val) 0x11 <- 0x40 (addr)
>> pci_read_config: (val) 0x0 <- 0x41 (addr)
>> pci_read_config: (val) 0x10 <- 0x6 (addr)
>> pci_read_config: (val) 0x40 <- 0x34 (addr)
>> pci_read_config: (val) 0x11 <- 0x40 (addr)
>> pci_read_config: (val) 0x0 <- 0x41 (addr)
>> pci_read_config: (val) 0x10 <- 0x6 (addr)
>> pci_read_config: (val) 0x40 <- 0x34 (addr)
>> pci_read_config: (val) 0x11 <- 0x40 (addr)
>> pci_read_config: (val) 0x0 <- 0x41 (addr)
>> pci_read_config: (val) 0x3 <- 0x4 (addr)
>> pci_read_config: (val) 0x3 <- 0x4 (addr)
>> pci_read_config: (val) 0x0 <- 0xc (addr)
>> pci_read_config: (val) 0x3 <- 0x4 (addr)
>> pci_read_config: (val) 0x3 <- 0x4 (addr)
>> pci_read_config: (val) 0x10 <- 0x6 (addr)
>> pci_read_config: (val) 0x40 <- 0x34 (addr)
>> pci_read_config: (val) 0x11 <- 0x40 (addr)
>> pci_read_config: (val) 0x0 <- 0x41 (addr)
>>
>>
>> >
>> >
>> > --
>> > I have a truly marvellous patch that fixes the bug which this
>> > signature is too narrow to contain.
>> >
>> >
>>
>
> --
> yamahata
>
>