From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34171 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OYljo-0004Xb-Gg for qemu-devel@nongnu.org; Tue, 13 Jul 2010 16:05:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OYljm-0007bg-6m for qemu-devel@nongnu.org; Tue, 13 Jul 2010 16:05:56 -0400 Received: from mail-vw0-f45.google.com ([209.85.212.45]:47297) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OYljm-0007bU-2V for qemu-devel@nongnu.org; Tue, 13 Jul 2010 16:05:54 -0400 Received: by vws13 with SMTP id 13so2852784vws.4 for ; Tue, 13 Jul 2010 13:05:52 -0700 (PDT) MIME-Version: 1.0 Sender: camm@ualberta.ca In-Reply-To: <20100630032901.GA19142@valinux.co.jp> References: <4C175E30.2030605@redhat.com> <4C270E25.7070409@redhat.com> <4C2997C5.1020302@redhat.com> <20100630032901.GA19142@valinux.co.jp> Date: Tue, 13 Jul 2010 14:05:51 -0600 Message-ID: Subject: Re: [Qemu-devel] Re: Unusual physical address when using 64-bit BAR From: Cam Macdonell Content-Type: text/plain; charset=ISO-8859-1 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: "Michael S. Tsirkin" , Avi Kivity , "qemu-devel@nongnu.org Developers" On Tue, Jun 29, 2010 at 9:29 PM, Isaku Yamahata 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 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 > >