Re: [Qemu-devel] [PATCH] pci_register_bar: cleanup

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, May 18, 2016 at 02:17:20PM +0200, Paolo Bonzini wrote:
> On 18/05/2016 14:16, Cao jin wrote:
> > I guess maybe this one is more suitable for trivial.
> 
> No, it's not trivial.  I guess it missed soft freeze.  Michael Tsirkin
> will pick it up.
> 
> Thanks,
> 
> Paolo

Yes - please repost whatever was posted before freeze.
I'll review and pick this one up.
Thanks!

> >>>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >>>>> index e67664d..f0f41dc 100644
> >>>>> --- a/hw/pci/pci.c
> >>>>> +++ b/hw/pci/pci.c
> >>>>> @@ -974,7 +974,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> >>>>>                         uint8_t type, MemoryRegion *memory)
> >>>>>   {
> >>>>>       PCIIORegion *r;
> >>>>> -    uint32_t addr;
> >>>>> +    uint32_t addr; /* offset in pci config space */
> >>>>>       uint64_t wmask;
> >>>>>       pcibus_t size = memory_region_size(memory);
> >>>>>
> >>>>> @@ -990,15 +990,20 @@ void pci_register_bar(PCIDevice *pci_dev, int
> >>>>> region_num,
> >>>>>       r->addr = PCI_BAR_UNMAPPED;
> >>>>>       r->size = size;
> >>>>>       r->type = type;
> >>>>> -    r->memory = NULL;
> >>>>> +    r->memory = memory;
> >>>>> +    r->address_space = type & PCI_BASE_ADDRESS_SPACE_IO
> >>>>> +                        ? pci_dev->bus->address_space_io
> >>>>> +                        : pci_dev->bus->address_space_mem;
> >>>>>
> >>>>>       wmask = ~(size - 1);
> >>>>> -    addr = pci_bar(pci_dev, region_num);
> >>>>>       if (region_num == PCI_ROM_SLOT) {
> >>>>>           /* ROM enable bit is writable */
> >>>>>           wmask |= PCI_ROM_ADDRESS_ENABLE;
> >>>>>       }
> >>>>> +
> >>>>> +    addr = pci_bar(pci_dev, region_num);
> >>>>>       pci_set_long(pci_dev->config + addr, type);
> >>>>> +
> >>>>>       if (!(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> >>>>>           r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> >>>>>           pci_set_quad(pci_dev->wmask + addr, wmask);
> >>>>> @@ -1007,11 +1012,6 @@ void pci_register_bar(PCIDevice *pci_dev, int
> >>>>> region_num,
> >>>>>           pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
> >>>>>           pci_set_long(pci_dev->cmask + addr, 0xffffffff);
> >>>>>       }
> >>>>> -    pci_dev->io_regions[region_num].memory = memory;
> >>>>> -    pci_dev->io_regions[region_num].address_space
> >>>>> -        = type & PCI_BASE_ADDRESS_SPACE_IO
> >>>>> -        ? pci_dev->bus->address_space_io
> >>>>> -        : pci_dev->bus->address_space_mem;
> >>>>>   }
> >>>>>
> >>>>>   static void pci_update_vga(PCIDevice *pci_dev)
> >>>>>
> >>>>
> >>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>