diff mbox

[20/39] virtio-pci: convert to memory API

Message ID 1312135082-31985-21-git-send-email-avi@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Avi Kivity July 31, 2011, 5:57 p.m. UTC
except msix.

[jan: fix build]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/virtio-pci.c |   74 ++++++++++++++++++++++--------------------------------
 hw/virtio-pci.h |    2 +-
 2 files changed, 31 insertions(+), 45 deletions(-)

Comments

Michael S. Tsirkin Aug. 1, 2011, 8:26 a.m. UTC | #1
On Sun, Jul 31, 2011 at 08:57:43PM +0300, Avi Kivity wrote:
> @@ -491,30 +473,26 @@ static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val)
>      virtio_config_writel(proxy->vdev, addr, val);
>  }
>  
> -static void virtio_map(PCIDevice *pci_dev, int region_num,
> -                       pcibus_t addr, pcibus_t size, int type)
> -{
> -    VirtIOPCIProxy *proxy = container_of(pci_dev, VirtIOPCIProxy, pci_dev);
> -    VirtIODevice *vdev = proxy->vdev;
> -    unsigned config_len = VIRTIO_PCI_REGION_SIZE(pci_dev) + vdev->config_len;
> -
> -    proxy->addr = addr;
> -
> -    register_ioport_write(addr, config_len, 1, virtio_pci_config_writeb, proxy);
> -    register_ioport_write(addr, config_len, 2, virtio_pci_config_writew, proxy);
> -    register_ioport_write(addr, config_len, 4, virtio_pci_config_writel, proxy);
> -    register_ioport_read(addr, config_len, 1, virtio_pci_config_readb, proxy);
> -    register_ioport_read(addr, config_len, 2, virtio_pci_config_readw, proxy);
> -    register_ioport_read(addr, config_len, 4, virtio_pci_config_readl, proxy);
> +const MemoryRegionPortio virtio_portio[] = {
> +    { 0, 0x10000, 1, .write = virtio_pci_config_writeb, },
> +    { 0, 0x10000, 2, .write = virtio_pci_config_writew, },
> +    { 0, 0x10000, 4, .write = virtio_pci_config_writel, },
> +    { 0, 0x10000, 1, .read = virtio_pci_config_readb, },
> +    { 0, 0x10000, 2, .read = virtio_pci_config_readw, },
> +    { 0, 0x10000, 4, .read = virtio_pci_config_readl, },
> +    PORTIO_END
> +};
>  
> -    if (vdev->config_len)
> -        vdev->get_config(vdev, vdev->config);
> -}
> +static const MemoryRegionOps virtio_pci_config_ops = {
> +    .old_portio = virtio_portio,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
>  
>  static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
>                                  uint32_t val, int len)
>  {
>      VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> +    VirtIODevice *vdev = proxy->vdev;
>  
>      if (PCI_COMMAND == address) {
>          if (!(val & PCI_COMMAND_MASTER)) {
> @@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
>              }
>          }
>      }
> +    if (address == PCI_BASE_ADDRESS_0 && vdev->config_len) {
> +        vdev->get_config(vdev, vdev->config);
> +    }
>  
>      pci_default_write_config(pci_dev, address, val, len);
>      msix_write_config(pci_dev, address, val, len);

I'm not really sure why did we get the config on map,
specifically - Anthony, do you know?
But if we want to do that, memory space enable might
be a better place. Or maybe we just want a callback on
map.
Avi Kivity Aug. 1, 2011, 9:35 a.m. UTC | #2
On 08/01/2011 11:26 AM, Michael S. Tsirkin wrote:
> >
> >   static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> >                                   uint32_t val, int len)
> >   {
> >       VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> >  +    VirtIODevice *vdev = proxy->vdev;
> >
> >       if (PCI_COMMAND == address) {
> >           if (!(val&  PCI_COMMAND_MASTER)) {
> >  @@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> >               }
> >           }
> >       }
> >  +    if (address == PCI_BASE_ADDRESS_0&&  vdev->config_len) {
> >  +        vdev->get_config(vdev, vdev->config);
> >  +    }
> >
> >       pci_default_write_config(pci_dev, address, val, len);
> >       msix_write_config(pci_dev, address, val, len);
>
> I'm not really sure why did we get the config on map,
> specifically - Anthony, do you know?
> But if we want to do that, memory space enable might
> be a better place. Or maybe we just want a callback on
> map.


Just because a memory region becomes visible to the cpu is no reason to 
have a callback.  From the device perspective, it can't tell that it 
happened.
Michael S. Tsirkin Aug. 1, 2011, 10:23 a.m. UTC | #3
On Mon, Aug 01, 2011 at 12:35:44PM +0300, Avi Kivity wrote:
> On 08/01/2011 11:26 AM, Michael S. Tsirkin wrote:
> >>
> >>   static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> >>                                   uint32_t val, int len)
> >>   {
> >>       VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> >>  +    VirtIODevice *vdev = proxy->vdev;
> >>
> >>       if (PCI_COMMAND == address) {
> >>           if (!(val&  PCI_COMMAND_MASTER)) {
> >>  @@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> >>               }
> >>           }
> >>       }
> >>  +    if (address == PCI_BASE_ADDRESS_0&&  vdev->config_len) {
> >>  +        vdev->get_config(vdev, vdev->config);
> >>  +    }
> >>
> >>       pci_default_write_config(pci_dev, address, val, len);
> >>       msix_write_config(pci_dev, address, val, len);
> >
> >I'm not really sure why did we get the config on map,
> >specifically - Anthony, do you know?
> >But if we want to do that, memory space enable might
> >be a better place. Or maybe we just want a callback on
> >map.
> 
> 
> Just because a memory region becomes visible to the cpu is no reason
> to have a callback.  From the device perspective, it can't tell that
> it happened.

Well, the reason we have this logic here, I think, is
to make sure it runs before the guest accesses
the configuration with a write access.

I'm not sure why we don't do this in the init
callback - Anthony?


> -- 
> error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin Aug. 1, 2011, 7:53 p.m. UTC | #4
On Mon, Aug 01, 2011 at 12:35:44PM +0300, Avi Kivity wrote:
> On 08/01/2011 11:26 AM, Michael S. Tsirkin wrote:
> >>
> >>   static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> >>                                   uint32_t val, int len)
> >>   {
> >>       VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> >>  +    VirtIODevice *vdev = proxy->vdev;
> >>
> >>       if (PCI_COMMAND == address) {
> >>           if (!(val&  PCI_COMMAND_MASTER)) {
> >>  @@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> >>               }
> >>           }
> >>       }
> >>  +    if (address == PCI_BASE_ADDRESS_0&&  vdev->config_len) {
> >>  +        vdev->get_config(vdev, vdev->config);
> >>  +    }
> >>
> >>       pci_default_write_config(pci_dev, address, val, len);
> >>       msix_write_config(pci_dev, address, val, len);
> >
> >I'm not really sure why did we get the config on map,
> >specifically - Anthony, do you know?
> >But if we want to do that, memory space enable might
> >be a better place. Or maybe we just want a callback on
> >map.
> 
> 
> Just because a memory region becomes visible to the cpu is no reason
> to have a callback.  From the device perspective, it can't tell that
> it happened.

BTW this is what qxl does, too, conceptually: on config writes, it peeks
at the bar to detect whether that got unmapped.

> -- 
> error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori Aug. 1, 2011, 8:23 p.m. UTC | #5
On 08/01/2011 03:26 AM, Michael S. Tsirkin wrote:
> On Sun, Jul 31, 2011 at 08:57:43PM +0300, Avi Kivity wrote:
>> @@ -491,30 +473,26 @@ static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val)
>>       virtio_config_writel(proxy->vdev, addr, val);
>>   }
>>
>> -static void virtio_map(PCIDevice *pci_dev, int region_num,
>> -                       pcibus_t addr, pcibus_t size, int type)
>> -{
>> -    VirtIOPCIProxy *proxy = container_of(pci_dev, VirtIOPCIProxy, pci_dev);
>> -    VirtIODevice *vdev = proxy->vdev;
>> -    unsigned config_len = VIRTIO_PCI_REGION_SIZE(pci_dev) + vdev->config_len;
>> -
>> -    proxy->addr = addr;
>> -
>> -    register_ioport_write(addr, config_len, 1, virtio_pci_config_writeb, proxy);
>> -    register_ioport_write(addr, config_len, 2, virtio_pci_config_writew, proxy);
>> -    register_ioport_write(addr, config_len, 4, virtio_pci_config_writel, proxy);
>> -    register_ioport_read(addr, config_len, 1, virtio_pci_config_readb, proxy);
>> -    register_ioport_read(addr, config_len, 2, virtio_pci_config_readw, proxy);
>> -    register_ioport_read(addr, config_len, 4, virtio_pci_config_readl, proxy);
>> +const MemoryRegionPortio virtio_portio[] = {
>> +    { 0, 0x10000, 1, .write = virtio_pci_config_writeb, },
>> +    { 0, 0x10000, 2, .write = virtio_pci_config_writew, },
>> +    { 0, 0x10000, 4, .write = virtio_pci_config_writel, },
>> +    { 0, 0x10000, 1, .read = virtio_pci_config_readb, },
>> +    { 0, 0x10000, 2, .read = virtio_pci_config_readw, },
>> +    { 0, 0x10000, 4, .read = virtio_pci_config_readl, },
>> +    PORTIO_END
>> +};
>>
>> -    if (vdev->config_len)
>> -        vdev->get_config(vdev, vdev->config);
>> -}
>> +static const MemoryRegionOps virtio_pci_config_ops = {
>> +    .old_portio = virtio_portio,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>>
>>   static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
>>                                   uint32_t val, int len)
>>   {
>>       VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
>> +    VirtIODevice *vdev = proxy->vdev;
>>
>>       if (PCI_COMMAND == address) {
>>           if (!(val&  PCI_COMMAND_MASTER)) {
>> @@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
>>               }
>>           }
>>       }
>> +    if (address == PCI_BASE_ADDRESS_0&&  vdev->config_len) {
>> +        vdev->get_config(vdev, vdev->config);
>> +    }
>>
>>       pci_default_write_config(pci_dev, address, val, len);
>>       msix_write_config(pci_dev, address, val, len);
>
> I'm not really sure why did we get the config on map,
> specifically - Anthony, do you know?

It's just a mechanism to lazily load the config.  We could just as 
easily read the config whenever the (virtio) config space was accessed.

Regards,

Anthony Liguori

> But if we want to do that, memory space enable might
> be a better place. Or maybe we just want a callback on
> map.
>

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori Aug. 1, 2011, 8:24 p.m. UTC | #6
On 08/01/2011 05:23 AM, Michael S. Tsirkin wrote:
> On Mon, Aug 01, 2011 at 12:35:44PM +0300, Avi Kivity wrote:
>> On 08/01/2011 11:26 AM, Michael S. Tsirkin wrote:
>>>>
>>>>    static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
>>>>                                    uint32_t val, int len)
>>>>    {
>>>>        VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
>>>>   +    VirtIODevice *vdev = proxy->vdev;
>>>>
>>>>        if (PCI_COMMAND == address) {
>>>>            if (!(val&   PCI_COMMAND_MASTER)) {
>>>>   @@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
>>>>                }
>>>>            }
>>>>        }
>>>>   +    if (address == PCI_BASE_ADDRESS_0&&   vdev->config_len) {
>>>>   +        vdev->get_config(vdev, vdev->config);
>>>>   +    }
>>>>
>>>>        pci_default_write_config(pci_dev, address, val, len);
>>>>        msix_write_config(pci_dev, address, val, len);
>>>
>>> I'm not really sure why did we get the config on map,
>>> specifically - Anthony, do you know?
>>> But if we want to do that, memory space enable might
>>> be a better place. Or maybe we just want a callback on
>>> map.
>>
>>
>> Just because a memory region becomes visible to the cpu is no reason
>> to have a callback.  From the device perspective, it can't tell that
>> it happened.
>
> Well, the reason we have this logic here, I think, is
> to make sure it runs before the guest accesses
> the configuration with a write access.
>
> I'm not sure why we don't do this in the init
> callback - Anthony?

It could be done in init I think.  I can't recall why it didn't start 
that way initially.

Regards,

Anthony Liguori

>
>
>> --
>> error compiling committee.c: too many arguments to function
>

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity Aug. 2, 2011, 9:17 a.m. UTC | #7
On 08/01/2011 10:53 PM, Michael S. Tsirkin wrote:
> >
> >
> >  Just because a memory region becomes visible to the cpu is no reason
> >  to have a callback.  From the device perspective, it can't tell that
> >  it happened.
>
> BTW this is what qxl does, too, conceptually: on config writes, it peeks
> at the bar to detect whether that got unmapped.

Can you elaborate?  why?  what does it do?
Michael S. Tsirkin Aug. 2, 2011, 9:34 a.m. UTC | #8
On Tue, Aug 02, 2011 at 12:17:06PM +0300, Avi Kivity wrote:
> On 08/01/2011 10:53 PM, Michael S. Tsirkin wrote:
> >>
> >>
> >>  Just because a memory region becomes visible to the cpu is no reason
> >>  to have a callback.  From the device perspective, it can't tell that
> >>  it happened.
> >
> >BTW this is what qxl does, too, conceptually: on config writes, it peeks
> >at the bar to detect whether that got unmapped.
> 
> Can you elaborate?  why?  what does it do?


Disables vga mapping when it sees io has been disabled.
What it really should do is check the io enable bit I think ...

    vga_dirty_log_stop(vga);
    pci_default_write_config(d, address, val, len);
    if (vga->map_addr && qxl->pci.io_regions[0].addr == -1) {
        vga->map_addr = 0;
    }
    vga_dirty_log_start(vga);

> -- 
> error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity Aug. 2, 2011, 12:39 p.m. UTC | #9
On 08/02/2011 12:34 PM, Michael S. Tsirkin wrote:
> On Tue, Aug 02, 2011 at 12:17:06PM +0300, Avi Kivity wrote:
> >  On 08/01/2011 10:53 PM, Michael S. Tsirkin wrote:
> >  >>
> >  >>
> >  >>   Just because a memory region becomes visible to the cpu is no reason
> >  >>   to have a callback.  From the device perspective, it can't tell that
> >  >>   it happened.
> >  >
> >  >BTW this is what qxl does, too, conceptually: on config writes, it peeks
> >  >at the bar to detect whether that got unmapped.
> >
> >  Can you elaborate?  why?  what does it do?
>
>
> Disables vga mapping when it sees io has been disabled.
> What it really should do is check the io enable bit I think ...
>
>      vga_dirty_log_stop(vga);
>      pci_default_write_config(d, address, val, len);
>      if (vga->map_addr&&  qxl->pci.io_regions[0].addr == -1) {
>          vga->map_addr = 0;
>      }
>      vga_dirty_log_start(vga);

If the memory is not visible to the guest, logging has no effect.
Gerd Hoffmann Aug. 2, 2011, 4:28 p.m. UTC | #10
On 08/02/11 11:17, Avi Kivity wrote:
> On 08/01/2011 10:53 PM, Michael S. Tsirkin wrote:
>> >
>> >
>> > Just because a memory region becomes visible to the cpu is no reason
>> > to have a callback. From the device perspective, it can't tell that
>> > it happened.
>>
>> BTW this is what qxl does, too, conceptually: on config writes, it peeks
>> at the bar to detect whether that got unmapped.
>
> Can you elaborate? why? what does it do?

Nothing special, just keep track of the map state and toggle logging if 
needed, especially make sure to (re-)enable logging after the guest 
mapped the bar somewhere.

I'm pretty sure this can be killed during the conversion as the new 
memory slot manager will care about this so qxl doesn't has to.

cheers,
   Gerd
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index d685243..c114e1a 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -161,7 +161,8 @@  static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
 {
     VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
     EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
-    int r;
+    int r = 0;
+
     if (assign) {
         r = event_notifier_init(notifier, 1);
         if (r < 0) {
@@ -169,24 +170,11 @@  static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
                          __func__, r);
             return r;
         }
-        r = kvm_set_ioeventfd_pio_word(event_notifier_get_fd(notifier),
-                                       proxy->addr + VIRTIO_PCI_QUEUE_NOTIFY,
-                                       n, assign);
-        if (r < 0) {
-            error_report("%s: unable to map ioeventfd: %d",
-                         __func__, r);
-            event_notifier_cleanup(notifier);
-        }
+        memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
+                                  true, n, event_notifier_get_fd(notifier));
     } else {
-        r = kvm_set_ioeventfd_pio_word(event_notifier_get_fd(notifier),
-                                       proxy->addr + VIRTIO_PCI_QUEUE_NOTIFY,
-                                       n, assign);
-        if (r < 0) {
-            error_report("%s: unable to unmap ioeventfd: %d",
-                         __func__, r);
-            return r;
-        }
-
+        memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
+                                  true, n, event_notifier_get_fd(notifier));
         /* Handle the race condition where the guest kicked and we deassigned
          * before we got around to handling the kick.
          */
@@ -423,7 +411,6 @@  static uint32_t virtio_pci_config_readb(void *opaque, uint32_t addr)
 {
     VirtIOPCIProxy *proxy = opaque;
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
-    addr -= proxy->addr;
     if (addr < config)
         return virtio_ioport_read(proxy, addr);
     addr -= config;
@@ -434,7 +421,6 @@  static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr)
 {
     VirtIOPCIProxy *proxy = opaque;
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
-    addr -= proxy->addr;
     if (addr < config)
         return virtio_ioport_read(proxy, addr);
     addr -= config;
@@ -445,7 +431,6 @@  static uint32_t virtio_pci_config_readl(void *opaque, uint32_t addr)
 {
     VirtIOPCIProxy *proxy = opaque;
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
-    addr -= proxy->addr;
     if (addr < config)
         return virtio_ioport_read(proxy, addr);
     addr -= config;
@@ -456,7 +441,6 @@  static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
-    addr -= proxy->addr;
     if (addr < config) {
         virtio_ioport_write(proxy, addr, val);
         return;
@@ -469,7 +453,6 @@  static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
-    addr -= proxy->addr;
     if (addr < config) {
         virtio_ioport_write(proxy, addr, val);
         return;
@@ -482,7 +465,6 @@  static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
-    addr -= proxy->addr;
     if (addr < config) {
         virtio_ioport_write(proxy, addr, val);
         return;
@@ -491,30 +473,26 @@  static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val)
     virtio_config_writel(proxy->vdev, addr, val);
 }
 
-static void virtio_map(PCIDevice *pci_dev, int region_num,
-                       pcibus_t addr, pcibus_t size, int type)
-{
-    VirtIOPCIProxy *proxy = container_of(pci_dev, VirtIOPCIProxy, pci_dev);
-    VirtIODevice *vdev = proxy->vdev;
-    unsigned config_len = VIRTIO_PCI_REGION_SIZE(pci_dev) + vdev->config_len;
-
-    proxy->addr = addr;
-
-    register_ioport_write(addr, config_len, 1, virtio_pci_config_writeb, proxy);
-    register_ioport_write(addr, config_len, 2, virtio_pci_config_writew, proxy);
-    register_ioport_write(addr, config_len, 4, virtio_pci_config_writel, proxy);
-    register_ioport_read(addr, config_len, 1, virtio_pci_config_readb, proxy);
-    register_ioport_read(addr, config_len, 2, virtio_pci_config_readw, proxy);
-    register_ioport_read(addr, config_len, 4, virtio_pci_config_readl, proxy);
+const MemoryRegionPortio virtio_portio[] = {
+    { 0, 0x10000, 1, .write = virtio_pci_config_writeb, },
+    { 0, 0x10000, 2, .write = virtio_pci_config_writew, },
+    { 0, 0x10000, 4, .write = virtio_pci_config_writel, },
+    { 0, 0x10000, 1, .read = virtio_pci_config_readb, },
+    { 0, 0x10000, 2, .read = virtio_pci_config_readw, },
+    { 0, 0x10000, 4, .read = virtio_pci_config_readl, },
+    PORTIO_END
+};
 
-    if (vdev->config_len)
-        vdev->get_config(vdev, vdev->config);
-}
+static const MemoryRegionOps virtio_pci_config_ops = {
+    .old_portio = virtio_portio,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
 
 static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
                                 uint32_t val, int len)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+    VirtIODevice *vdev = proxy->vdev;
 
     if (PCI_COMMAND == address) {
         if (!(val & PCI_COMMAND_MASTER)) {
@@ -525,6 +503,9 @@  static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
             }
         }
     }
+    if (address == PCI_BASE_ADDRESS_0 && vdev->config_len) {
+        vdev->get_config(vdev, vdev->config);
+    }
 
     pci_default_write_config(pci_dev, address, val, len);
     msix_write_config(pci_dev, address, val, len);
@@ -678,8 +659,10 @@  void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev)
     if (size & (size-1))
         size = 1 << qemu_fls(size);
 
-    pci_register_bar(&proxy->pci_dev, 0, size, PCI_BASE_ADDRESS_SPACE_IO,
-                           virtio_map);
+    memory_region_init_io(&proxy->bar, &virtio_pci_config_ops, proxy,
+                          "virtio-pci", size);
+    pci_register_bar_region(&proxy->pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
+                            &proxy->bar);
 
     if (!kvm_has_many_ioeventfds()) {
         proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
@@ -714,6 +697,9 @@  static int virtio_blk_init_pci(PCIDevice *pci_dev)
 
 static int virtio_exit_pci(PCIDevice *pci_dev)
 {
+    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+
+    memory_region_destroy(&proxy->bar);
     return msix_uninit(pci_dev);
 }
 
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index 1f0de56..5af1c8c 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -21,8 +21,8 @@ 
 typedef struct {
     PCIDevice pci_dev;
     VirtIODevice *vdev;
+    MemoryRegion bar;
     uint32_t flags;
-    uint32_t addr;
     uint32_t class_code;
     uint32_t nvectors;
     BlockConf block;