[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
[Qemu-devel] Re: [PATCH V5 15/29] pci: typedef pcibus_t as uint64_t instead of uint32_t.
Wed, 14 Oct 2009 13:35:49 +0900
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. 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.)
How about adding the following check?
last_addr >= TARGET_PHYS_ADDR_MAX
[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
- [Qemu-devel] Re: [PATCH V5 28/29] pci: initialize pci config headers depending it pci header type., (continued)