PCI IO regions must be power of two

PCI IO regions must be power of two

[Marcelo Tosatti]

Anthony,

Both virtio-net and virtio-block currently register PCI IO space regions
that are not power of two in size.

The decoding process to discover the size of a PCI resource expects it
to be a power of two. The PCI controller masks the size out of what is
written into

config_space + 0x10 + (4 * region_num)

The result is that the size is calculated and registered erroneously 
by the OS:

[root@localhost ~]# cat /proc/ioports  | grep virtio
  c200-c203 : virtio-pci

This is a virtio-block device whose BAR0 has length (16+20)-1, not 4. 

      BAR0: I/O at 0xc200 [0xc223].

I suggest forcing the size to be power of two as follows:


Index: kvm-userspace.hotplug2/qemu/hw/pci.c
===================================================================
--- kvm-userspace.hotplug2.orig/qemu/hw/pci.c
+++ kvm-userspace.hotplug2/qemu/hw/pci.c
@@ -236,6 +236,13 @@ void pci_register_io_region(PCIDevice *p

     if ((unsigned int)region_num >= PCI_NUM_REGIONS)
         return;
+
+    /* IO region size must be power of two */
+    if (type == PCI_ADDRESS_SPACE_IO && (size & (size-1))) {
+        size = size << 1;
+        size &= size-1;
+    }
+
     r = &pci_dev->io_regions[region_num];
     r->addr = -1;
     r->size = size;


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: PCI IO regions must be power of two

[Anthony Liguori]

Marcelo Tosatti wrote:
> Anthony,
>
> Both virtio-net and virtio-block currently register PCI IO space regions
> that are not power of two in size.
>
> The decoding process to discover the size of a PCI resource expects it
> to be a power of two. The PCI controller masks the size out of what is
> written into
>
> config_space + 0x10 + (4 * region_num)
>
> The result is that the size is calculated and registered erroneously 
> by the OS:
>
> [root@localhost ~]# cat /proc/ioports  | grep virtio
>   c200-c203 : virtio-pci
>
> This is a virtio-block device whose BAR0 has length (16+20)-1, not 4. 
>
>       BAR0: I/O at 0xc200 [0xc223].
>
> I suggest forcing the size to be power of two as follows:
>
>   

A quick grep of the source show that virtio seems to be the only device 
that isn't behaving here.  I suggest modifying the virtio.c to always 
use a power of two and then perhaps adding a check in hw/pci.c to 
validate that the registered region is a power of two in size.

Regards,

Anthony Liguori

> Index: kvm-userspace.hotplug2/qemu/hw/pci.c
> ===================================================================
> --- kvm-userspace.hotplug2.orig/qemu/hw/pci.c
> +++ kvm-userspace.hotplug2/qemu/hw/pci.c
> @@ -236,6 +236,13 @@ void pci_register_io_region(PCIDevice *p
>
>      if ((unsigned int)region_num >= PCI_NUM_REGIONS)
>          return;
> +
> +    /* IO region size must be power of two */
> +    if (type == PCI_ADDRESS_SPACE_IO && (size & (size-1))) {
> +        size = size << 1;
> +        size &= size-1;
> +    }
> +
>      r = &pci_dev->io_regions[region_num];
>      r->addr = -1;
>      r->size = size;
>
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: PCI IO regions must be power of two

[Uri Lublin]

[-- Attachment #1.1: Type: text/plain, Size: 737 bytes --]

From: Marcelo Tosatti
>
>Index: kvm-userspace.hotplug2/qemu/hw/pci.c
>===================================================================
>--- kvm-userspace.hotplug2.orig/qemu/hw/pci.c
>+++ kvm-userspace.hotplug2/qemu/hw/pci.c
>@@ -236,6 +236,13 @@ void pci_register_io_region(PCIDevice *p
>
>     if ((unsigned int)region_num >= PCI_NUM_REGIONS)
>         return;
>+
>+    /* IO region size must be power of two */
>+    if (type == PCI_ADDRESS_SPACE_IO && (size & (size-1))) {

Why only for PCI IO regions ? Don't PCI memory regions have the same restriction ?

>+        size = size << 1;
>+        size &= size-1;

That would not make size a power of 2 (e.g. size=7 --> size=12).

>+    }
>+

Regards,
Uri.

[-- Attachment #1.2: Type: text/html, Size: 1486 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #3: Type: text/plain, Size: 158 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: PCI IO regions must be power of two

[Marcelo Tosatti]

On Wed, Mar 12, 2008 at 08:39:11AM -0700, Uri Lublin wrote:
> From: Marcelo Tosatti
> >
> >Index: kvm-userspace.hotplug2/qemu/hw/pci.c
> >===================================================================
> >--- kvm-userspace.hotplug2.orig/qemu/hw/pci.c
> >+++ kvm-userspace.hotplug2/qemu/hw/pci.c
> >@@ -236,6 +236,13 @@ void pci_register_io_region(PCIDevice *p
> >
> >     if ((unsigned int)region_num >= PCI_NUM_REGIONS)
> >         return;
> >+
> >+    /* IO region size must be power of two */
> >+    if (type == PCI_ADDRESS_SPACE_IO && (size & (size-1))) {
> 
> Why only for PCI IO regions ? Don't PCI memory regions have the same restriction ?

Yes, they do.

> >+        size = size << 1;
> >+        size &= size-1;
> 
> That would not make size a power of 2 (e.g. size=7 --> size=12).

Right, sorry.

I'll resend a patch to have virtio allocate IO regions with proper sizes
and warn if the IO _or_ memory region are not power of two, as Anthony
suggested.

Since this problem is only visible with device hotplug (QEMU will
allocate the region offsets properly during initialization) I'll wait
for that to hit the tree.

Thanks.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/