[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH V5 15/29] pci: typedef pcibus_t as uint64_t inst
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH V5 15/29] pci: typedef pcibus_t as uint64_t instead of uint32_t. |
Date: |
Wed, 14 Oct 2009 10:55:46 +0200 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Wed, Oct 14, 2009 at 01:35:49PM +0900, Isaku Yamahata wrote:
> On Tue, Oct 13, 2009 at 04:39:15PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Oct 13, 2009 at 10:31:33PM +0900, Isaku Yamahata wrote:
> > > On Sun, Oct 11, 2009 at 12:43:12PM +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 09, 2009 at 03:28:48PM +0900, Isaku Yamahata wrote:
> > > > > This patch is preliminary for 64bit bar.
> > > > > For 64bit bar support, change pcibus_t which represents
> > > > > pci bus addr/size from uint32_t to uint64_t.
> > > > > And also change FMT_pcibus for printf.
> > > > >
> > > > > In pci_update_mapping() checks 32bit overflow.
> > > > > So the check must be updated too.
> > > > >
> > > > > Signed-off-by: Isaku Yamahata <address@hidden>
> > > >
> > > > That's all fine, but if you look at users implementing
> > > > map io, they do: cpu_register_physical_memory()
> > > > on the address they are given. And if target_phys_addr_t is 32 bit,
> > > > this will silently truncate the address.
> > > >
> > > > So I would like to understand how this will all
> > > > work on 32 bit systems.
> > >
> > > The case is
> > > . BAR is memory 64bit and
> > > . target_phys_addr_t is 32bit and
> > > . bar is set to >4G.
> > > Hmm, the case isn't checked.
> > >
> > > It would be checked by
> > > - last_addr <= new_addr
> > > + (target_phys_addr_t)last_addr <= new_addr
> >
> > That's pretty tricky. Can we just convert everything into
> > 64 bit unconditionally and just do simple range checks?
> >
> > >
> > > I'll fix it with comments added. Nice catch.
> >
> > Is this the right thing to do though?
> > I think 32 bit CPU might address something like 64G
> > memory of highmem support, so a 64 bit value might
> > actually be valid.
> >
> > Let's step back and understand what the motivation is?
> > Maybe declaring all bars as 32 bit for 32 bit targets is enough?
>
> It is independent of guest OS for PCI device to have 64 bit BAR.
> It is valid to use PCI card with 64bit bar on 32 bit OS.
Yes, but qemu does not support this yet.
> In that case
> the OS will set the 64 bit bar within addressable region.
> And it is allowed for 32 bit OS to set 64bit BAR to >4GB.
> (which doesn't make sense, though.)
Yes, it does. E.g. with high memory, a 32 bit OS can address
more than 4G RAM.
>
> How about adding the following check?
> last_addr >= TARGET_PHYS_ADDR_MAX
And then what?
> --
> yamahata
- [Qemu-devel] Re: [PATCH V5 28/29] pci: initialize pci config headers depending it pci header type., (continued)
[Qemu-devel] [PATCH V5 17/29] pci: make pci configuration transaction more accurate., Isaku Yamahata, 2009/10/09
[Qemu-devel] [PATCH V5 15/29] pci: typedef pcibus_t as uint64_t instead of uint32_t., Isaku Yamahata, 2009/10/09
[Qemu-devel] [PATCH V5 11/29] pci_host.h: move functions in pci_host.h into .c file., Isaku Yamahata, 2009/10/09
[Qemu-devel] [PATCH V5 27/29] pci/bridge: don't update bar mapping when bar2-5 is changed., Isaku Yamahata, 2009/10/09
[Qemu-devel] [PATCH V5 18/29] pci: factor out the conversion logic from io port address into pci device., Isaku Yamahata, 2009/10/09
[Qemu-devel] [PATCH V5 07/29] pci/bridge: clean up of pci_bridge_initfn(), Isaku Yamahata, 2009/10/09