All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cam Macdonell <cam@cs.ualberta.ca>
To: Isaku Yamahata <yamahata@valinux.co.jp>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	Avi Kivity <avi@redhat.com>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] Re: Unusual physical address when using 64-bit BAR
Date: Tue, 13 Jul 2010 14:05:51 -0600	[thread overview]
Message-ID: <AANLkTik54pAZL8oGhWnc6pVotDcLebCjZ4tPvVSItcKz@mail.gmail.com> (raw)
In-Reply-To: <20100630032901.GA19142@valinux.co.jp>

On Tue, Jun 29, 2010 at 9:29 PM, Isaku Yamahata <yamahata@valinux.co.jp> 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 <avi@redhat.com> 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
>
>

  reply	other threads:[~2010-07-13 20:05 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-19 16:41 [Qemu-devel] Unusual physical address when using 64-bit BAR Cam Macdonell
2010-06-11 17:31 ` [Qemu-devel] " Cam Macdonell
2010-06-15 11:04   ` Avi Kivity
2010-06-24 21:51     ` Cam Macdonell
2010-06-27  8:39       ` Avi Kivity
2010-06-28 20:38         ` Cam Macdonell
2010-06-29  6:50           ` Avi Kivity
2010-06-29 17:48             ` Cam Macdonell
2010-06-30  3:29               ` Isaku Yamahata
2010-07-13 20:05                 ` Cam Macdonell [this message]
2010-07-13 20:41                   ` Isaku Yamahata
2010-07-13 22:48                     ` Cam Macdonell
2010-07-14  2:52                       ` Isaku Yamahata
2010-07-14 15:10                         ` Cam Macdonell
2010-07-20  9:52                           ` Isaku Yamahata
2010-07-21  3:31                             ` Michael S. Tsirkin
2010-07-21  3:49                               ` Isaku Yamahata
2010-08-24 16:52                                 ` Cam Macdonell
2010-08-25  2:21                                   ` Isaku Yamahata
2010-08-27 19:35                                     ` Cam Macdonell
2010-08-30  2:36                                       ` Isaku Yamahata

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTik54pAZL8oGhWnc6pVotDcLebCjZ4tPvVSItcKz@mail.gmail.com \
    --to=cam@cs.ualberta.ca \
    --cc=avi@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yamahata@valinux.co.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.