qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH 16/20] pci: kill goto in pci_update_mappings()


From: Michael S. Tsirkin
Subject: [Qemu-devel] Re: [PATCH 16/20] pci: kill goto in pci_update_mappings()
Date: Thu, 12 Nov 2009 15:29:39 +0200
User-agent: Mutt/1.5.19 (2009-01-05)

On Thu, Nov 12, 2009 at 10:12:07PM +0900, Isaku Yamahata wrote:
> On Thu, Nov 12, 2009 at 02:06:22PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Nov 12, 2009 at 02:58:44PM +0900, Isaku Yamahata wrote:
> > > This patch kills nasty goto in pci_update_mappings().
> > > 
> > > Signed-off-by: Isaku Yamahata <address@hidden>
> > > ---
> > >  hw/pci.c |   54 ++++++++++++++++++++++++++++--------------------------
> > >  1 files changed, 28 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/hw/pci.c b/hw/pci.c
> > > index cae3d53..2eff7fe 100644
> > > --- a/hw/pci.c
> > > +++ b/hw/pci.c
> > > @@ -756,35 +756,37 @@ static void pci_update_mappings(PCIDevice *d)
> > >                      new_addr = pci_get_long(d->config + pci_bar(d, i));
> > >                  }
> > >                  /* the ROM slot has a specific enable bit */
> > > -                if (i == PCI_ROM_SLOT && !(new_addr & 
> > > PCI_ROM_ADDRESS_ENABLE))
> > > -                    goto no_mem_map;
> > > -                new_addr = new_addr & ~(r->size - 1);
> > > -                last_addr = new_addr + r->size - 1;
> > > -                /* NOTE: we do not support wrapping */
> > > -                /* XXX: as we cannot support really dynamic
> > > -                   mappings, we handle specific values as invalid
> > > -                   mappings. */
> > > -                if (last_addr <= new_addr || new_addr == 0 ||
> > 
> > By the way, any idea why do we need new_addr == 0
> > and last_addr == PCI_BAR_UNMAPPED?
> 
> As for new_addr == 0, since default BAR value is zero, plural BARs can
> overlap with each other. I think qemu can't handle BAR overlap anyway.
> 
> As for last_addr == PCI_BAR_UNMAPPED, it avoid to map
> around 4GB as discussed before.

So it should be ~0x0ull and not PCI_BAR_UNMAPPED,
PCI_BAR_UNMAPPED could be any value e.g. 0x1 would do
as well.

> I observed that guest doesn't boot
> without the check. However I didn't track it down further.
> Now it's 64bit though.

I really think we need to move mapping regions out of devices
and into pci.c. Then we can finally support overlapping BARs
(by being careful not to map common regions).


-- 
MST




reply via email to

[Prev in Thread] Current Thread [Next in Thread]